* [PATCH] aarch64: costs: update for TARGET_CSSC
@ 2023-11-16 6:15 Philipp Tomsich
2023-11-16 8:53 ` Richard Earnshaw
0 siblings, 1 reply; 4+ messages in thread
From: Philipp Tomsich @ 2023-11-16 6:15 UTC (permalink / raw)
To: gcc-patches; +Cc: Kyrylo Tkachov, Philipp Tomsich
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);
- 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;
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] aarch64: costs: update for TARGET_CSSC
2023-11-16 6:15 [PATCH] aarch64: costs: update for TARGET_CSSC Philipp Tomsich
@ 2023-11-16 8:53 ` Richard Earnshaw
2023-11-16 9:11 ` Kyrylo Tkachov
0 siblings, 1 reply; 4+ messages in thread
From: Richard Earnshaw @ 2023-11-16 8:53 UTC (permalink / raw)
To: Philipp Tomsich, gcc-patches; +Cc: Kyrylo Tkachov
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] aarch64: costs: update for TARGET_CSSC
2023-11-16 8:53 ` Richard Earnshaw
@ 2023-11-16 9:11 ` Kyrylo Tkachov
2023-11-16 9:26 ` Philipp Tomsich
0 siblings, 1 reply; 4+ messages in thread
From: Kyrylo Tkachov @ 2023-11-16 9:11 UTC (permalink / raw)
To: Richard Earnshaw, Philipp Tomsich, gcc-patches
> -----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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] aarch64: costs: update for TARGET_CSSC
2023-11-16 9:11 ` Kyrylo Tkachov
@ 2023-11-16 9:26 ` Philipp Tomsich
0 siblings, 0 replies; 4+ messages in thread
From: Philipp Tomsich @ 2023-11-16 9:26 UTC (permalink / raw)
To: Kyrylo Tkachov; +Cc: Richard Earnshaw, gcc-patches
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-16 9:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-16 6:15 [PATCH] aarch64: costs: update for TARGET_CSSC Philipp Tomsich
2023-11-16 8:53 ` Richard Earnshaw
2023-11-16 9:11 ` Kyrylo Tkachov
2023-11-16 9:26 ` Philipp Tomsich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).