| Message ID | 20231116061551.1218932-1-philipp.tomsich@vrull.eu |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: costs: update for TARGET_CSSC | expand |
On 16/11/2023 06:15, Philipp Tomsich wrote: > With the addition of CSSC (Common Short Sequence Compression) > instructions, a number of idioms match to single instructions (e.g., > abs) that previously expanded to multi-instruction sequences. > > This recognizes (some of) those idioms that are now misclassified and > returns a cost of a single instruction. > > gcc/ChangeLog: > > * config/aarch64/aarch64.cc (aarch64_rtx_costs): Support > idioms matching to CSSC instructions, if target CSSC is > present > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > --- > > gcc/config/aarch64/aarch64.cc | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index 800a8b0e110..d89c94519e9 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -14431,10 +14431,17 @@ aarch64_rtx_costs (rtx x, machine_mode mode, int outer ATTRIBUTE_UNUSED, > return false; > > case CTZ: > - *cost = COSTS_N_INSNS (2); > + if (!TARGET_CSSC) > + { > + /* Will be split to a bit-reversal + clz */ > + *cost = COSTS_N_INSNS (2); > + > + if (speed) > + *cost += extra_cost->alu.clz + extra_cost->alu.rev; > + } > + else > + *cost = COSTS_N_INSNS (1); There should be some speed-related extra_cost to add here as well, so that target-specific costing can be taken into account. > > - if (speed) > - *cost += extra_cost->alu.clz + extra_cost->alu.rev; > return false; > > case COMPARE: > @@ -15373,12 +15380,17 @@ cost_plus: > } > else > { > - /* Integer ABS will either be split to > - two arithmetic instructions, or will be an ABS > - (scalar), which we don't model. */ > - *cost = COSTS_N_INSNS (2); > - if (speed) > - *cost += 2 * extra_cost->alu.arith; > + if (!TARGET_CSSC) > + { > + /* Integer ABS will either be split to > + two arithmetic instructions, or will be an ABS > + (scalar), which we don't model. */ > + *cost = COSTS_N_INSNS (2); > + if (speed) > + *cost += 2 * extra_cost->alu.arith; > + } > + else > + *cost = COSTS_N_INSNS (1); same here. > } > return false; > > @@ -15388,13 +15400,15 @@ cost_plus: > { > if (VECTOR_MODE_P (mode)) > *cost += extra_cost->vect.alu; > - else > + else if (GET_MODE_CLASS (mode) == MODE_FLOAT) > { > /* FMAXNM/FMINNM/FMAX/FMIN. > TODO: This may not be accurate for all implementations, but > we do not model this in the cost tables. */ > *cost += extra_cost->fp[mode == DFmode].addsub; > } > + else if (TARGET_CSSC) > + *cost = COSTS_N_INSNS (1); and here. > } > return false; > R.
> -----Original Message----- > From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> > Sent: Thursday, November 16, 2023 8:53 AM > To: Philipp Tomsich <philipp.tomsich@vrull.eu>; gcc-patches@gcc.gnu.org > Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> > Subject: Re: [PATCH] aarch64: costs: update for TARGET_CSSC > > > > On 16/11/2023 06:15, Philipp Tomsich wrote: > > With the addition of CSSC (Common Short Sequence Compression) > > instructions, a number of idioms match to single instructions (e.g., > > abs) that previously expanded to multi-instruction sequences. > > > > This recognizes (some of) those idioms that are now misclassified and > > returns a cost of a single instruction. > > > > gcc/ChangeLog: > > > > * config/aarch64/aarch64.cc (aarch64_rtx_costs): Support > > idioms matching to CSSC instructions, if target CSSC is > > present > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > --- > > > > gcc/config/aarch64/aarch64.cc | 34 ++++++++++++++++++++++++---------- > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > > index 800a8b0e110..d89c94519e9 100644 > > --- a/gcc/config/aarch64/aarch64.cc > > +++ b/gcc/config/aarch64/aarch64.cc > > @@ -14431,10 +14431,17 @@ aarch64_rtx_costs (rtx x, machine_mode > mode, int outer ATTRIBUTE_UNUSED, > > return false; > > > > case CTZ: > > - *cost = COSTS_N_INSNS (2); > > + if (!TARGET_CSSC) > > + { > > + /* Will be split to a bit-reversal + clz */ > > + *cost = COSTS_N_INSNS (2); > > + > > + if (speed) > > + *cost += extra_cost->alu.clz + extra_cost->alu.rev; > > + } > > + else > > + *cost = COSTS_N_INSNS (1); > > There should be some speed-related extra_cost to add here as well, so > that target-specific costing can be taken into account. And I'd rather have the conditions be not inverted i.e. If (TARGET_CSSC) ... else ... Thanks, Kyrill > > > > > - if (speed) > > - *cost += extra_cost->alu.clz + extra_cost->alu.rev; > > return false; > > > > case COMPARE: > > @@ -15373,12 +15380,17 @@ cost_plus: > > } > > else > > { > > - /* Integer ABS will either be split to > > - two arithmetic instructions, or will be an ABS > > - (scalar), which we don't model. */ > > - *cost = COSTS_N_INSNS (2); > > - if (speed) > > - *cost += 2 * extra_cost->alu.arith; > > + if (!TARGET_CSSC) > > + { > > + /* Integer ABS will either be split to > > + two arithmetic instructions, or will be an ABS > > + (scalar), which we don't model. */ > > + *cost = COSTS_N_INSNS (2); > > + if (speed) > > + *cost += 2 * extra_cost->alu.arith; > > + } > > + else > > + *cost = COSTS_N_INSNS (1); > > same here. > > > } > > return false; > > > > @@ -15388,13 +15400,15 @@ cost_plus: > > { > > if (VECTOR_MODE_P (mode)) > > *cost += extra_cost->vect.alu; > > - else > > + else if (GET_MODE_CLASS (mode) == MODE_FLOAT) > > { > > /* FMAXNM/FMINNM/FMAX/FMIN. > > TODO: This may not be accurate for all implementations, but > > we do not model this in the cost tables. */ > > *cost += extra_cost->fp[mode == DFmode].addsub; > > } > > + else if (TARGET_CSSC) > > + *cost = COSTS_N_INSNS (1); > > and here. > > > } > > return false; > > > > R.
Thanks for the quick turnaround on the review. I'll send a v2 after the mcpu=ampere1b change has landed, as the extra-costs change will have an interaction with that change (due to the extra fields in the structure). Philipp. On Thu, 16 Nov 2023 at 15:12, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote: > > > > > -----Original Message----- > > From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> > > Sent: Thursday, November 16, 2023 8:53 AM > > To: Philipp Tomsich <philipp.tomsich@vrull.eu>; gcc-patches@gcc.gnu.org > > Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> > > Subject: Re: [PATCH] aarch64: costs: update for TARGET_CSSC > > > > > > > > On 16/11/2023 06:15, Philipp Tomsich wrote: > > > With the addition of CSSC (Common Short Sequence Compression) > > > instructions, a number of idioms match to single instructions (e.g., > > > abs) that previously expanded to multi-instruction sequences. > > > > > > This recognizes (some of) those idioms that are now misclassified and > > > returns a cost of a single instruction. > > > > > > gcc/ChangeLog: > > > > > > * config/aarch64/aarch64.cc (aarch64_rtx_costs): Support > > > idioms matching to CSSC instructions, if target CSSC is > > > present > > > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > > --- > > > > > > gcc/config/aarch64/aarch64.cc | 34 ++++++++++++++++++++++++---------- > > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > > > index 800a8b0e110..d89c94519e9 100644 > > > --- a/gcc/config/aarch64/aarch64.cc > > > +++ b/gcc/config/aarch64/aarch64.cc > > > @@ -14431,10 +14431,17 @@ aarch64_rtx_costs (rtx x, machine_mode > > mode, int outer ATTRIBUTE_UNUSED, > > > return false; > > > > > > case CTZ: > > > - *cost = COSTS_N_INSNS (2); > > > + if (!TARGET_CSSC) > > > + { > > > + /* Will be split to a bit-reversal + clz */ > > > + *cost = COSTS_N_INSNS (2); > > > + > > > + if (speed) > > > + *cost += extra_cost->alu.clz + extra_cost->alu.rev; > > > + } > > > + else > > > + *cost = COSTS_N_INSNS (1); > > > > There should be some speed-related extra_cost to add here as well, so > > that target-specific costing can be taken into account. > > And I'd rather have the conditions be not inverted i.e. > If (TARGET_CSSC) > ... > else > ... > > Thanks, > Kyrill > > > > > > > > - if (speed) > > > - *cost += extra_cost->alu.clz + extra_cost->alu.rev; > > > return false; > > > > > > case COMPARE: > > > @@ -15373,12 +15380,17 @@ cost_plus: > > > } > > > else > > > { > > > - /* Integer ABS will either be split to > > > - two arithmetic instructions, or will be an ABS > > > - (scalar), which we don't model. */ > > > - *cost = COSTS_N_INSNS (2); > > > - if (speed) > > > - *cost += 2 * extra_cost->alu.arith; > > > + if (!TARGET_CSSC) > > > + { > > > + /* Integer ABS will either be split to > > > + two arithmetic instructions, or will be an ABS > > > + (scalar), which we don't model. */ > > > + *cost = COSTS_N_INSNS (2); > > > + if (speed) > > > + *cost += 2 * extra_cost->alu.arith; > > > + } > > > + else > > > + *cost = COSTS_N_INSNS (1); > > > > same here. > > > > > } > > > return false; > > > > > > @@ -15388,13 +15400,15 @@ cost_plus: > > > { > > > if (VECTOR_MODE_P (mode)) > > > *cost += extra_cost->vect.alu; > > > - else > > > + else if (GET_MODE_CLASS (mode) == MODE_FLOAT) > > > { > > > /* FMAXNM/FMINNM/FMAX/FMIN. > > > TODO: This may not be accurate for all implementations, but > > > we do not model this in the cost tables. */ > > > *cost += extra_cost->fp[mode == DFmode].addsub; > > > } > > > + else if (TARGET_CSSC) > > > + *cost = COSTS_N_INSNS (1); > > > > and here. > > > > > } > > > return false; > > > > > > > R.
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index 800a8b0e110..d89c94519e9 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -14431,10 +14431,17 @@ aarch64_rtx_costs (rtx x, machine_mode mode, int outer ATTRIBUTE_UNUSED, return false; case CTZ: - *cost = COSTS_N_INSNS (2); + if (!TARGET_CSSC) + { + /* Will be split to a bit-reversal + clz */ + *cost = COSTS_N_INSNS (2); + + if (speed) + *cost += extra_cost->alu.clz + extra_cost->alu.rev; + } + else + *cost = COSTS_N_INSNS (1); - if (speed) - *cost += extra_cost->alu.clz + extra_cost->alu.rev; return false; case COMPARE: @@ -15373,12 +15380,17 @@ cost_plus: } else { - /* Integer ABS will either be split to - two arithmetic instructions, or will be an ABS - (scalar), which we don't model. */ - *cost = COSTS_N_INSNS (2); - if (speed) - *cost += 2 * extra_cost->alu.arith; + if (!TARGET_CSSC) + { + /* Integer ABS will either be split to + two arithmetic instructions, or will be an ABS + (scalar), which we don't model. */ + *cost = COSTS_N_INSNS (2); + if (speed) + *cost += 2 * extra_cost->alu.arith; + } + else + *cost = COSTS_N_INSNS (1); } return false; @@ -15388,13 +15400,15 @@ cost_plus: { if (VECTOR_MODE_P (mode)) *cost += extra_cost->vect.alu; - else + else if (GET_MODE_CLASS (mode) == MODE_FLOAT) { /* FMAXNM/FMINNM/FMAX/FMIN. TODO: This may not be accurate for all implementations, but we do not model this in the cost tables. */ *cost += extra_cost->fp[mode == DFmode].addsub; } + else if (TARGET_CSSC) + *cost = COSTS_N_INSNS (1); } return false;
With the addition of CSSC (Common Short Sequence Compression) instructions, a number of idioms match to single instructions (e.g., abs) that previously expanded to multi-instruction sequences. This recognizes (some of) those idioms that are now misclassified and returns a cost of a single instruction. gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_rtx_costs): Support idioms matching to CSSC instructions, if target CSSC is present Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> --- gcc/config/aarch64/aarch64.cc | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-)