diff mbox series

s390: testsuite: Fix vcond-shift.c

Message ID 20240719052921.1950410-2-stefansf@gcc.gnu.org
State New
Headers show
Series s390: testsuite: Fix vcond-shift.c | expand

Commit Message

Stefan Schulze Frielinghaus July 19, 2024, 5:29 a.m. UTC
Previously we optimized expressions of the form a < 0 ? -1 : 0 to
(signed)a >> 31 during vcond expanding.  Since r15-1741-g2ccdd0f22312a1
this is done in match.pd.  The implementation in the back end as well as
in match.pd are basically the same but still distinct.  For the tests in
vcond-shift.c the back end emitted

  (xx - (xx >> 31)) >> 1

whereas now via match.pd

  ((int) ((unsigned int) xx >> 31) + xx) >> 1

which is basically the same.  We just have to adapt the scan-assembler
directives w.r.t. signed/unsigned shifts which is done by this patch.

gcc/testsuite/ChangeLog:

	* gcc.target/s390/vector/vcond-shift.c: Adapt to new match.pd
	rule and change scan-assembler-times for shifts.
---
 Regtested on s390.  Ok for mainline?

 gcc/testsuite/gcc.target/s390/vector/vcond-shift.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Andrew Pinski July 19, 2024, 6:58 a.m. UTC | #1
On Thu, Jul 18, 2024 at 10:31 PM Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org> wrote:
>
> Previously we optimized expressions of the form a < 0 ? -1 : 0 to
> (signed)a >> 31 during vcond expanding.  Since r15-1741-g2ccdd0f22312a1
> this is done in match.pd.  The implementation in the back end as well as
> in match.pd are basically the same but still distinct.  For the tests in
> vcond-shift.c the back end emitted
>
>   (xx - (xx >> 31)) >> 1
>
> whereas now via match.pd
>
>   ((int) ((unsigned int) xx >> 31) + xx) >> 1
>
> which is basically the same.  We just have to adapt the scan-assembler
> directives w.r.t. signed/unsigned shifts which is done by this patch.

Note I filed https://gcc.gnu.org/PR115999 because I noticed those 2
form produce slightly different code generation for scalars (I assume
it will produce similar issues for vectors too).

Thanks,
Andrew Pinski

>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/s390/vector/vcond-shift.c: Adapt to new match.pd
>         rule and change scan-assembler-times for shifts.
> ---
>  Regtested on s390.  Ok for mainline?
>
>  gcc/testsuite/gcc.target/s390/vector/vcond-shift.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> index a6b4e97aa50..b942f44039d 100644
> --- a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> +++ b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> @@ -3,13 +3,13 @@
>  /* { dg-do compile { target { s390*-*-* } } } */
>  /* { dg-options "-O3 -march=z13 -mzarch" } */
>
> -/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 6 } } */
> -/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 6 } } */
> -/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 6 } } */
> +/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 4 } } */
> +/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 4 } } */
> +/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 4 } } */
>  /* { dg-final { scan-assembler-not "vzero\t*" } } */
> -/* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 4 } } */
> -/* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 4 } } */
> -/* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 4 } } */
> +/* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 6 } } */
> +/* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 6 } } */
> +/* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 6 } } */
>
>  /* Make it expand to two vector operations.  */
>  #define ITER(X) (2 * (16 / sizeof (X[1])))
> --
> 2.45.2
>
Stefan Schulze Frielinghaus July 19, 2024, 7:54 a.m. UTC | #2
On Thu, Jul 18, 2024 at 11:58:10PM -0700, Andrew Pinski wrote:
> On Thu, Jul 18, 2024 at 10:31 PM Stefan Schulze Frielinghaus
> <stefansf@gcc.gnu.org> wrote:
> >
> > Previously we optimized expressions of the form a < 0 ? -1 : 0 to
> > (signed)a >> 31 during vcond expanding.  Since r15-1741-g2ccdd0f22312a1
> > this is done in match.pd.  The implementation in the back end as well as
> > in match.pd are basically the same but still distinct.  For the tests in
> > vcond-shift.c the back end emitted
> >
> >   (xx - (xx >> 31)) >> 1
> >
> > whereas now via match.pd
> >
> >   ((int) ((unsigned int) xx >> 31) + xx) >> 1
> >
> > which is basically the same.  We just have to adapt the scan-assembler
> > directives w.r.t. signed/unsigned shifts which is done by this patch.
> 
> Note I filed https://gcc.gnu.org/PR115999 because I noticed those 2
> form produce slightly different code generation for scalars (I assume
> it will produce similar issues for vectors too).

Thanks for the heads up.  In that case we should probably wait a bit
once a normal form or whatever has settled.

Cheers,
Stefan

> 
> Thanks,
> Andrew Pinski
> 
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/s390/vector/vcond-shift.c: Adapt to new match.pd
> >         rule and change scan-assembler-times for shifts.
> > ---
> >  Regtested on s390.  Ok for mainline?
> >
> >  gcc/testsuite/gcc.target/s390/vector/vcond-shift.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > index a6b4e97aa50..b942f44039d 100644
> > --- a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > +++ b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > @@ -3,13 +3,13 @@
> >  /* { dg-do compile { target { s390*-*-* } } } */
> >  /* { dg-options "-O3 -march=z13 -mzarch" } */
> >
> > -/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 6 } } */
> > -/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 6 } } */
> > -/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 4 } } */
> > +/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 4 } } */
> > +/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 4 } } */
> >  /* { dg-final { scan-assembler-not "vzero\t*" } } */
> > -/* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 4 } } */
> > -/* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 4 } } */
> > -/* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 4 } } */
> > +/* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 6 } } */
> >
> >  /* Make it expand to two vector operations.  */
> >  #define ITER(X) (2 * (16 / sizeof (X[1])))
> > --
> > 2.45.2
> >
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
index a6b4e97aa50..b942f44039d 100644
--- a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
+++ b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
@@ -3,13 +3,13 @@ 
 /* { dg-do compile { target { s390*-*-* } } } */
 /* { dg-options "-O3 -march=z13 -mzarch" } */
 
-/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 6 } } */
-/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 6 } } */
-/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 6 } } */
+/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 4 } } */
+/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 4 } } */
+/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 4 } } */
 /* { dg-final { scan-assembler-not "vzero\t*" } } */
-/* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 4 } } */
-/* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 4 } } */
-/* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 4 } } */
+/* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 6 } } */
+/* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 6 } } */
+/* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 6 } } */
 
 /* Make it expand to two vector operations.  */
 #define ITER(X) (2 * (16 / sizeof (X[1])))