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.tkac...@arm.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Richard Earnshaw <richard.earns...@foss.arm.com>
> > Sent: Thursday, November 16, 2023 8:53 AM
> > To: Philipp Tomsich <philipp.toms...@vrull.eu>; gcc-patches@gcc.gnu.org
> > Cc: Kyrylo Tkachov <kyrylo.tkac...@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.toms...@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.
