[PATCH] testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]

[PATCH] testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]

[Peter Bergner]

testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]

Jeff's commit r15-831-g05daf617ea22e1 changed the instruction we expected
for this test case into an equivalent instruction.  Modify the test case
so it will accept any of three equivalent instructions we could get depending
on the options used.

I've verified this test case PASSes on all scenarios where the three possible
instructions are generated.  Ok for trunk?

Peter


2024-06-12  Peter Bergner  <bergner@linux.ibm.com>

gcc/testsuite/
	PR testsuite/115262
	* gcc.target/powerpc/pr66144-3.c: Add -fno-unroll-loops to options.
	(scan-assembler): Change from this...
	(scan-assembler-times): ...to this.  Tweak regex to accept multiple
	equivalent instructions.


diff --git a/gcc/testsuite/gcc.target/powerpc/pr66144-3.c b/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
index 4c93b2a7a3d..dbd746c5489 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
@@ -1,5 +1,5 @@
 /* { dg-do compile { target { powerpc64*-*-* } } } */
-/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2 -ftree-vectorize" } */
+/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2 -ftree-vectorize -fno-unroll-loops" } */
 /* { dg-require-effective-target powerpc_vsx } */
 
 /* Verify that we can optimize a vector conditional move, where one of the arms
@@ -20,7 +20,7 @@ test (void)
     a[i] = (b[i] == c[i]) ? -1 : a[i];
 }
 
-/* { dg-final { scan-assembler     {\mvcmpequw\M} } } */
-/* { dg-final { scan-assembler     {\mxxsel\M}    } } */
+/* { dg-final { scan-assembler-times {\mvcmpequw\M} 1 } } */
+/* { dg-final { scan-assembler-times {\m(?:xxsel|xxlor|vor)\M} 1 } } */
 /* { dg-final { scan-assembler-not {\mvspltisw\M} } } */
 /* { dg-final { scan-assembler-not {\mxxlorc\M}   } } */

Re: [PATCH] testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]

[Segher Boessenkool]

Hi!

On Wed, Jun 12, 2024 at 02:49:40PM -0500, Peter Bergner wrote:
> testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]

("rs6000:", not "testsuite")

> Jeff's commit r15-831-g05daf617ea22e1 changed the instruction we expected
> for this test case into an equivalent instruction.  Modify the test case
> so it will accept any of three equivalent instructions we could get depending
> on the options used.

They aren't equivalent insns, but they are all simple insns, and all
just as cheap :-)

> I've verified this test case PASSes on all scenarios where the three possible
> instructions are generated.  Ok for trunk?

> --- a/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile { target { powerpc64*-*-* } } } */

Probably should be an "lp64" instead?

> -/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2 -ftree-vectorize" } */
> +/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2 -ftree-vectorize -fno-unroll-loops" } */

The "-mvsx" is superfluous btw (implied by -mcpu=power8).

>  /* { dg-require-effective-target powerpc_vsx } */

This isn't needed either.

> -/* { dg-final { scan-assembler     {\mvcmpequw\M} } } */
> -/* { dg-final { scan-assembler     {\mxxsel\M}    } } */
> +/* { dg-final { scan-assembler-times {\mvcmpequw\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\m(?:xxsel|xxlor|vor)\M} 1 } } */
>  /* { dg-final { scan-assembler-not {\mvspltisw\M} } } */
>  /* { dg-final { scan-assembler-not {\mxxlorc\M}   } } */

Okido, thanks!  The three trivial tweaks I suggest here are okay as
well if you want, after testing of course ;-)


Segher

Re: [PATCH] testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]

[Peter Bergner]

On 6/12/24 3:00 PM, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Jun 12, 2024 at 02:49:40PM -0500, Peter Bergner wrote:
>> testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]
> 
> ("rs6000:", not "testsuite")

Done.



>>  /* { dg-do compile { target { powerpc64*-*-* } } } */
> 
> Probably should be an "lp64" instead?

Actually, there is nothing inherently 64-bit about the test case.
I removed the target test altogether and it executes just fine on
our BE system in both 32-bit and 64-bit modes, so I'll just drop
the target test as part of the patch.



>> -/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2 -ftree-vectorize" } */
>> +/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2 -ftree-vectorize -fno-unroll-loops" } */
> 
> The "-mvsx" is superfluous btw (implied by -mcpu=power8).

Ok, I removed -mvsx since I'm touching the line already.


>>  /* { dg-require-effective-target powerpc_vsx } */
> 
> This isn't needed either.

Maybe not strictly needed, but it shields us from users who force
some options to be used via RUNTESTFLAGS env var that can cause the
test case to FAIL.  I'm going to leave this for someone else to
clean up.



>> -/* { dg-final { scan-assembler     {\mvcmpequw\M} } } */
>> -/* { dg-final { scan-assembler     {\mxxsel\M}    } } */
>> +/* { dg-final { scan-assembler-times {\mvcmpequw\M} 1 } } */
>> +/* { dg-final { scan-assembler-times {\m(?:xxsel|xxlor|vor)\M} 1 } } */
>>  /* { dg-final { scan-assembler-not {\mvspltisw\M} } } */
>>  /* { dg-final { scan-assembler-not {\mxxlorc\M}   } } */
> 
> Okido, thanks!  The three trivial tweaks I suggest here are okay as
> well if you want, after testing of course ;-)

Thanks.  For completeness, the final patch looks like the following.

Peter



gcc/testsuite/
        PR testsuite/115262
        * gcc.target/powerpc/pr66144-3.c (dg-do): Compile for all targets.
        (dg-options): Add -fno-unroll-loops and remove -mvsx.
        (scan-assembler): Change from this...
        (scan-assembler-times): ...to this.  Tweak regex to accept multiple
        allowable instructions.
---
 gcc/testsuite/gcc.target/powerpc/pr66144-3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/pr66144-3.c b/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
index 4c93b2a7a3d..14ecb809edc 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr66144-3.c
@@ -1,5 +1,5 @@
-/* { dg-do compile { target { powerpc64*-*-* } } } */
-/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2 -ftree-vectorize" } */
+/* { dg-do compile } */
+/* { dg-options "-mdejagnu-cpu=power8 -O2 -ftree-vectorize -fno-unroll-loops" } */
 /* { dg-require-effective-target powerpc_vsx } */
 
 /* Verify that we can optimize a vector conditional move, where one of the arms
@@ -20,7 +20,7 @@ test (void)
     a[i] = (b[i] == c[i]) ? -1 : a[i];
 }
 
-/* { dg-final { scan-assembler     {\mvcmpequw\M} } } */
-/* { dg-final { scan-assembler     {\mxxsel\M}    } } */
+/* { dg-final { scan-assembler-times {\mvcmpequw\M} 1 } } */
+/* { dg-final { scan-assembler-times {\m(?:xxsel|xxlor|vor)\M} 1 } } */
 /* { dg-final { scan-assembler-not {\mvspltisw\M} } } */
 /* { dg-final { scan-assembler-not {\mxxlorc\M}   } } */

Re: [PATCH] testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262]

[Segher Boessenkool]

On Wed, Jun 12, 2024 at 07:02:31PM -0500, Peter Bergner wrote:
> On 6/12/24 3:00 PM, Segher Boessenkool wrote:
> >>  /* { dg-do compile { target { powerpc64*-*-* } } } */
> > 
> > Probably should be an "lp64" instead?
> 
> Actually, there is nothing inherently 64-bit about the test case.
> I removed the target test altogether and it executes just fine on
> our BE system in both 32-bit and 64-bit modes, so I'll just drop
> the target test as part of the patch.

Ha, even better!

> >>  /* { dg-require-effective-target powerpc_vsx } */
> > 
> > This isn't needed either.
> 
> Maybe not strictly needed, but it shields us from users who force
> some options to be used via RUNTESTFLAGS env var that can cause the
> test case to FAIL.  I'm going to leave this for someone else to
> clean up.

Users can make most tests fail in interesting and exciting ways like
that, heh.

In general, only realistic settings are supported: things for which
hardware exists, an OS exists for, etc.  With any other settings many
things can fail, and that is Just Fine.

Thanks again,


Segher