| Message ID | bc2a018b-e69e-4961-b8e1-0e0b05b37624@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | testsuite: Fix pr66144-3.c test to accept multiple equivalent insns. [PR115262] | expand |
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
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} } } */
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
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} } } */