| Message ID | 34fce945-c226-49e7-a948-b73870f30480@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000, altivec-2-runnable.c should be a runnable test | expand |
Hi! On Thu, Jun 13, 2024 at 11:32:58AM -0700, Carl Love wrote: > The test gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c is supposed to be a runnable test > to verify the execution of the vec_unpackl and vec_unpackh built-ins. The dg-do command is set to > compile not run. This patch fixes the dg-do command argument. > > The patch has been verified on a P10. The test runs without errors. > rs6000, altivec-2-runnable.c should be a runnable test > > The test case has "dg-do compile" set not "dg-do run" for a runnable > test. This patch changes the dg-do command argument to run. > > gcc/testsuite/ChangeLog:gcc/testsuite/ChangeLog: > * gcc.target/powerpc/altivec-2-runnable.c: Change dg-do > argument to run. > --- a/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c > +++ b/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c > @@ -1,4 +1,4 @@ > -/* { dg-do compile { target powerpc*-*-* } } */ > +/* { dg-do run { target powerpc*-*-* } } */ > /* { dg-options "-mvsx" } */ > /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ > /* { dg-require-effective-target powerpc_vsx } */ Everything in gcc.target/powerpc/ is tested for "target powerpc*-*-*" already, so you could remove that target clause even (after testing of course :-) ) Okay for trunk with or without that extra tweak. Thank you! Segher
Segher: On 6/13/24 12:51, Segher Boessenkool wrote: <snip> > >> --- a/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c >> +++ b/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c >> @@ -1,4 +1,4 @@ >> -/* { dg-do compile { target powerpc*-*-* } } */ >> +/* { dg-do run { target powerpc*-*-* } } */ >> /* { dg-options "-mvsx" } */ >> /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ >> /* { dg-require-effective-target powerpc_vsx } */ > > Everything in gcc.target/powerpc/ is tested for "target powerpc*-*-*" > already, so you could remove that target clause even (after testing of > course :-) ) > > Okay for trunk with or without that extra tweak. Thank you! I updated the patch by removing the target clause as suggested: -/* { dg-do compile { target powerpc*-*-* } } */ +/* { dg-do run } */ /* { dg-options "-mvsx" } */ /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ /* { dg-require-effective-target powerpc_vsx } */ Retested on Power 10. Reports 2 passes and no failures. I will go ahead and commit. Thanks. Carl
Hi! on 2024/6/14 05:16, Carl Love wrote: > Segher: > > On 6/13/24 12:51, Segher Boessenkool wrote: > > <snip> > >> >>> --- a/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c >>> +++ b/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c >>> @@ -1,4 +1,4 @@ >>> -/* { dg-do compile { target powerpc*-*-* } } */ >>> +/* { dg-do run { target powerpc*-*-* } } */ >>> /* { dg-options "-mvsx" } */ >>> /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ >>> /* { dg-require-effective-target powerpc_vsx } */ >> >> Everything in gcc.target/powerpc/ is tested for "target powerpc*-*-*" >> already, so you could remove that target clause even (after testing of >> course :-) ) >> >> Okay for trunk with or without that extra tweak. Thank you! > > I updated the patch by removing the target clause as suggested: > > -/* { dg-do compile { target powerpc*-*-* } } */ > +/* { dg-do run } */ > /* { dg-options "-mvsx" } */ > /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ > /* { dg-require-effective-target powerpc_vsx } */ Since you changed this for "run", I think you also want s/powerpc_vsx/vsx_hw/ . BR, Kewen > > Retested on Power 10. Reports 2 passes and no failures. I will go ahead and commit. > > Thanks. > > Carl
On 6/13/24 9:34 PM, Kewen.Lin wrote: > on 2024/6/14 05:16, Carl Love wrote: >> /* { dg-options "-mvsx" } */ >> /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ With the above, we're going to compile and run this test case with -mcpu=power8 or higher, which means we could have P8, P9 or even P10 instructions emitted. >> /* { dg-require-effective-target powerpc_vsx } */ > > Since you changed this for "run", I think you also want s/powerpc_vsx/vsx_hw/ . ...which means we'd need p8vector_hw, p9vector_hw or ... here. Should we just always compile with -mcpu=power8 and then check for p8vector_hw to make our lives easier? Ala... /* { dg-options "-mdejagnu-cpu=power8" } */ ... /* { dg-require-effective-target p8vector_hw } */ Note I've removed -mvsx, since that is implied by -mcpu=power8 and no need for dg-additional-options. Maybe we want to add -O2 as well? Thoughts? Peter
Hi, on 2024/6/14 11:58, Peter Bergner wrote: > On 6/13/24 9:34 PM, Kewen.Lin wrote: >> on 2024/6/14 05:16, Carl Love wrote: > >>> /* { dg-options "-mvsx" } */ >>> /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ > > With the above, we're going to compile and run this test case with -mcpu=power8 > or higher, which means we could have P8, P9 or even P10 instructions emitted. > > > >>> /* { dg-require-effective-target powerpc_vsx } */ >> >> Since you changed this for "run", I think you also want s/powerpc_vsx/vsx_hw/ . > > ...which means we'd need p8vector_hw, p9vector_hw or ... here. Ah, good catch! Yes, it would require some harder guard. > > > Should we just always compile with -mcpu=power8 and then check for p8vector_hw > to make our lives easier? Ala... > > > /* { dg-options "-mdejagnu-cpu=power8" } */ > ... > /* { dg-require-effective-target p8vector_hw } */ > > > Note I've removed -mvsx, since that is implied by -mcpu=power8 and no > need for dg-additional-options. Maybe we want to add -O2 as well? > Thoughts? Both sounds reasonable to me, it looks useless to distinguish p8 or p8-up for this test case. BR, Kewen
diff --git a/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c b/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c index 6975ea57e65..3e66435d0d2 100644 --- a/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c +++ b/gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c @@ -1,4 +1,4 @@ -/* { dg-do compile { target powerpc*-*-* } } */ +/* { dg-do run { target powerpc*-*-* } } */ /* { dg-options "-mvsx" } */ /* { dg-additional-options "-mdejagnu-cpu=power8" { target { ! has_arch_pwr8 } } } */ /* { dg-require-effective-target powerpc_vsx } */
GCC maintainers: The test gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c is supposed to be a runnable test to verify the execution of the vec_unpackl and vec_unpackh built-ins. The dg-do command is set to compile not run. This patch fixes the dg-do command argument. The patch has been verified on a P10. The test runs without errors. Please let me know if the patch is acceptable. Thanks. Carl ------------------------------------------------- rs6000, altivec-2-runnable.c should be a runnable test The test case has "dg-do compile" set not "dg-do run" for a runnable test. This patch changes the dg-do command argument to run. gcc/testsuite/ChangeLog:gcc/testsuite/ChangeLog: * gcc.target/powerpc/altivec-2-runnable.c: Change dg-do argument to run. --- gcc/testsuite/gcc.target/powerpc/altivec-2-runnable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)