Message ID | cf1e3f0ca3613ef7938b9b5dec88bc7cc76ea110.camel@vnet.ibm.com |
---|---|
State | New |
Headers | show |
Series | [rs6000] Tests of ARCH_PWR8 and -mno-vsx option. (1/2) | expand |
Hi Will, Some comments are inline. on 2022/9/20 00:05, will schmidt wrote: > [PATCH, rs6000] Tests of ARCH_PWR8 and -mno-vsx option. > > Hi, > > This adds an assortment of tests to exercise the -mno-vsx option and > confirm the impacts on the ARCH_PWR8 define. > > These are based on and inspired by PR 101865, which > reports that _ARCH_PWR8 is disabled when -mno-vsx > is passed on the commandline. > > There are a small number of failures introduced by these tests, > those are resolved with the changes in part 2. > > OK for trunk? > Thanks, > -Will > > > gcc/testsuite: > * gcc.target/powerpc/predefine_p7-novsx.c: New test. > * gcc.target/powerpc/predefine_p8-noaltivec-novsx.c: New test. > * gcc.target/powerpc/predefine_p8-novsx.c: New test. > * gcc.target/powerpc/predefine_p9-novsx.c: New test. > * gcc.target/powerpc/predefine_pragma_vsx.c: New test. > > > diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p7-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p7-novsx.c > new file mode 100644 > index 000000000000..e842025b4d3c > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p7-novsx.c > @@ -0,0 +1,9 @@ > +/* { dg-do preprocess } */ > +/* Test whether the ARCH_PWR7 and ARCH_PWR8 defines gets set Nit: s/gets/get. > + * when we specify power7, plus options. > +/* This is a variation of the test at issue in GCC PR 101865 */ > +/* { dg-options "-dM -E -mdejagnu-cpu=power7 -mno-vsx" } */ > +/* { dg-final { scan-file predefine_p7-novsx.i "(^|\\n)#define _ARCH_PWR7 1($|\\n)" } } */ > +/* { dg-final { scan-file-not predefine_p7-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ > +/* { dg-final { scan-file-not predefine_p7-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ > +/* { dg-final { scan-file predefine_p7-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ > diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p8-noaltivec-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p8-noaltivec-novsx.c > new file mode 100644 > index 000000000000..c3b705ca3d48 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p8-noaltivec-novsx.c > @@ -0,0 +1,7 @@ > +/* { dg-do preprocess } */ > +/* Test whether the ARCH_PWR8 define remains set after disabling both altivec and vsx. */ > +/* { dg-options "-dM -E -mdejagnu-cpu=power8 -mno-altivec -mno-vsx" } */ > +/* { dg-final { scan-file predefine_p8-noaltivec-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ > +/* { dg-final { scan-file-not predefine_p8-noaltivec-novsx.i "(^|\\n)#define _ARCH_PWR9 1($|\\n)" } } */ > +/* { dg-final { scan-file-not predefine_p8-noaltivec-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ > +/* { dg-final { scan-file-not predefine_p8-noaltivec-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ > diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p8-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p8-novsx.c > new file mode 100644 > index 000000000000..8b6c69b20104 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p8-novsx.c > @@ -0,0 +1,8 @@ > +/* { dg-do preprocess } */ > +/* Test whether the ARCH_PWR8 define remains set after disabling vsx. > + This also confirms __ALTIVEC__ remains set when VSX is disabled. */ > +/* This is the primary test at issue in GCC PR 101865 */ Nit: the last comment missing a period. > +/* { dg-options "-dM -E -mdejagnu-cpu=power8 -mno-vsx" } */ > +/* { dg-final { scan-file predefine_p8-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ > +/* { dg-final { scan-file-not predefine_p8-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ > +/* { dg-final { scan-file predefine_p8-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ > diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p9-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p9-novsx.c > new file mode 100644 > index 000000000000..eef42c111663 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p9-novsx.c > @@ -0,0 +1,10 @@ > +/* { dg-do preprocess } */ > +/* Test whether the ARCH_PWR8 define remains set after disabling vsx. > + This also confirms __ALTIVEC__ remains set when VSX is disabled. */ > +/* This is the primary test at issue in GCC PR 101865 */ Nit: it seems this part of comments were copied from the above case? better with "s/ARCH_PWR8 define/ARCH_PWR8 and ARCH_PWR9 defines/" and and removing the last sentence since power9 isn't the primary test? > +/* { dg-options "-dM -E -mdejagnu-cpu=power9 -mno-vsx" } */ > +/* {xfail *-*-*} */ > +/* { dg-final { scan-file predefine_p9-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ > +/* { dg-final { scan-file predefine_p9-novsx.i "(^|\\n)#define _ARCH_PWR9 1($|\\n)" } } */ > +/* { dg-final { scan-file-not predefine_p9-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ > +/* { dg-final { scan-file predefine_p9-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ > diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_pragma_vsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_pragma_vsx.c > new file mode 100644 > index 000000000000..b300600af999 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/predefine_pragma_vsx.c > @@ -0,0 +1,83 @@ > +/* { dg-do run } */ > +/* { dg-require-effective-target powerpc_p9vector_ok } */ I guess you meant to use "p8vector_ok"? Ideally we need a power8_hw here since we force to use -mdejagnu-cpu=power8 below, there could be some actual p8 insns generated, but as the functions are very very simple, I don't think it's possible. :) > +/* { dg-require-effective-target lp64 } */ Is there a particular reason to exclude 32 bit here? > +/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2" } */ > + > +/* Ensure that if we set a pragma gcc target for an > + older processor, we do not compile builtins that > + the older target does not support. */ > + Nit: The comments here seem inconsistent to the contents below, I guessed it's stale (you used bifs but switched to check variable values later.)? BR, Kewen > +#include <altivec.h> > +#include <stdio.h> > +#include <stdlib.h> > + > +volatile int power8_set; > +volatile int vsx_set; > + > +void reset_values() { > + vsx_set=0; > + power8_set=0; > +} > + > +void test_default() { > + reset_values(); > +#ifdef _ARCH_PWR8 > + power8_set=1; > +#endif > +#ifdef __VSX__ > + vsx_set=1; > +#endif > +} > + > +#pragma GCC target "no-vsx" > +void test_no_vsx() { > + reset_values(); > +#ifdef _ARCH_PWR8 > + power8_set=1; > +#endif > +#ifdef __VSX__ > + vsx_set=1; > +#endif > +} > + > +#pragma GCC reset_options > +void test_reset_options() { > + reset_values(); > +#ifdef _ARCH_PWR8 > + power8_set=1; > +#endif > +#ifdef __VSX__ > + vsx_set=1; > +#endif > +} > + > +int main (int argc, char *argv []) { > + test_default(); > + if (!power8_set) { > + printf("_ARCH_PWR8 is not set.\n"); > + abort(); > + } > + if (!vsx_set) { > + printf("__VSX__ is not set.\n"); > + abort(); > + } > + test_no_vsx(); > + if (!power8_set) { > + printf("_ARCH_PWR8 is not set.\n"); > + abort(); > + } > + if (vsx_set) { > + printf("__VSX__ is unexpectedly set.\n"); > + abort(); > + } > + test_reset_options(); > + if (!power8_set) { > + printf("_ARCH_PWR8 is not set.\n"); > + abort(); > + } > + if (!vsx_set) { > + printf("__VSX__ is not set.\n"); > + abort(); > + } > +} > + >
Hi! Everything Ke Wen said. Some more commments / hints: On Mon, Sep 19, 2022 at 11:05:17AM -0500, will schmidt wrote: > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p7-novsx.c > @@ -0,0 +1,9 @@ > +/* { dg-do preprocess } */ > +/* Test whether the ARCH_PWR7 and ARCH_PWR8 defines gets set > + * when we specify power7, plus options. > +/* This is a variation of the test at issue in GCC PR 101865 */ Please don't start comment lines with stars. And don't start a comment inside of another comment :-) > +/* { dg-options "-dM -E -mdejagnu-cpu=power7 -mno-vsx" } */ > +/* { dg-final { scan-file predefine_p7-novsx.i "(^|\\n)#define _ARCH_PWR7 1($|\\n)" } } */ REs are easier to read and write if you write them inside {} instead of inside "". Whenever you see (^|\n) it should hint you to use newline-sensitive matching? Like {(?n)^#define _ARCH_PWR7 1$} (it makes ^ and $ match the begin/end of lines instead of of the string, and makes . and similar not match newlines). > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p9-novsx.c > @@ -0,0 +1,10 @@ > +/* { dg-do preprocess } */ > +/* Test whether the ARCH_PWR8 define remains set after disabling vsx. > + This also confirms __ALTIVEC__ remains set when VSX is disabled. */ > +/* This is the primary test at issue in GCC PR 101865 */ > +/* { dg-options "-dM -E -mdejagnu-cpu=power9 -mno-vsx" } */ > +/* {xfail *-*-*} */ An xfail always needs a comment :-) Segher
On Mon, 2022-10-17 at 10:32 -0500, Segher Boessenkool wrote: > Hi! > > Everything Ke Wen said. Some more commments / hints: Thanks for the reviews. :-) I'll rework things and repost 'soon'. Thanks -WIll
diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p7-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p7-novsx.c new file mode 100644 index 000000000000..e842025b4d3c --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p7-novsx.c @@ -0,0 +1,9 @@ +/* { dg-do preprocess } */ +/* Test whether the ARCH_PWR7 and ARCH_PWR8 defines gets set + * when we specify power7, plus options. +/* This is a variation of the test at issue in GCC PR 101865 */ +/* { dg-options "-dM -E -mdejagnu-cpu=power7 -mno-vsx" } */ +/* { dg-final { scan-file predefine_p7-novsx.i "(^|\\n)#define _ARCH_PWR7 1($|\\n)" } } */ +/* { dg-final { scan-file-not predefine_p7-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ +/* { dg-final { scan-file-not predefine_p7-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ +/* { dg-final { scan-file predefine_p7-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p8-noaltivec-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p8-noaltivec-novsx.c new file mode 100644 index 000000000000..c3b705ca3d48 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p8-noaltivec-novsx.c @@ -0,0 +1,7 @@ +/* { dg-do preprocess } */ +/* Test whether the ARCH_PWR8 define remains set after disabling both altivec and vsx. */ +/* { dg-options "-dM -E -mdejagnu-cpu=power8 -mno-altivec -mno-vsx" } */ +/* { dg-final { scan-file predefine_p8-noaltivec-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ +/* { dg-final { scan-file-not predefine_p8-noaltivec-novsx.i "(^|\\n)#define _ARCH_PWR9 1($|\\n)" } } */ +/* { dg-final { scan-file-not predefine_p8-noaltivec-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ +/* { dg-final { scan-file-not predefine_p8-noaltivec-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p8-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p8-novsx.c new file mode 100644 index 000000000000..8b6c69b20104 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p8-novsx.c @@ -0,0 +1,8 @@ +/* { dg-do preprocess } */ +/* Test whether the ARCH_PWR8 define remains set after disabling vsx. + This also confirms __ALTIVEC__ remains set when VSX is disabled. */ +/* This is the primary test at issue in GCC PR 101865 */ +/* { dg-options "-dM -E -mdejagnu-cpu=power8 -mno-vsx" } */ +/* { dg-final { scan-file predefine_p8-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ +/* { dg-final { scan-file-not predefine_p8-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ +/* { dg-final { scan-file predefine_p8-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_p9-novsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_p9-novsx.c new file mode 100644 index 000000000000..eef42c111663 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/predefine_p9-novsx.c @@ -0,0 +1,10 @@ +/* { dg-do preprocess } */ +/* Test whether the ARCH_PWR8 define remains set after disabling vsx. + This also confirms __ALTIVEC__ remains set when VSX is disabled. */ +/* This is the primary test at issue in GCC PR 101865 */ +/* { dg-options "-dM -E -mdejagnu-cpu=power9 -mno-vsx" } */ +/* {xfail *-*-*} */ +/* { dg-final { scan-file predefine_p9-novsx.i "(^|\\n)#define _ARCH_PWR8 1($|\\n)" } } */ +/* { dg-final { scan-file predefine_p9-novsx.i "(^|\\n)#define _ARCH_PWR9 1($|\\n)" } } */ +/* { dg-final { scan-file-not predefine_p9-novsx.i "(^|\\n)#define __VSX__ 1($|\\n)" } } */ +/* { dg-final { scan-file predefine_p9-novsx.i "(^|\\n)#define __ALTIVEC__ 1($|\\n)" } } */ diff --git a/gcc/testsuite/gcc.target/powerpc/predefine_pragma_vsx.c b/gcc/testsuite/gcc.target/powerpc/predefine_pragma_vsx.c new file mode 100644 index 000000000000..b300600af999 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/predefine_pragma_vsx.c @@ -0,0 +1,83 @@ +/* { dg-do run } */ +/* { dg-require-effective-target powerpc_p9vector_ok } */ +/* { dg-require-effective-target lp64 } */ +/* { dg-options "-mdejagnu-cpu=power8 -mvsx -O2" } */ + +/* Ensure that if we set a pragma gcc target for an + older processor, we do not compile builtins that + the older target does not support. */ + +#include <altivec.h> +#include <stdio.h> +#include <stdlib.h> + +volatile int power8_set; +volatile int vsx_set; + +void reset_values() { + vsx_set=0; + power8_set=0; +} + +void test_default() { + reset_values(); +#ifdef _ARCH_PWR8 + power8_set=1; +#endif +#ifdef __VSX__ + vsx_set=1; +#endif +} + +#pragma GCC target "no-vsx" +void test_no_vsx() { + reset_values(); +#ifdef _ARCH_PWR8 + power8_set=1; +#endif +#ifdef __VSX__ + vsx_set=1; +#endif +} + +#pragma GCC reset_options +void test_reset_options() { + reset_values(); +#ifdef _ARCH_PWR8 + power8_set=1; +#endif +#ifdef __VSX__ + vsx_set=1; +#endif +} + +int main (int argc, char *argv []) { + test_default(); + if (!power8_set) { + printf("_ARCH_PWR8 is not set.\n"); + abort(); + } + if (!vsx_set) { + printf("__VSX__ is not set.\n"); + abort(); + } + test_no_vsx(); + if (!power8_set) { + printf("_ARCH_PWR8 is not set.\n"); + abort(); + } + if (vsx_set) { + printf("__VSX__ is unexpectedly set.\n"); + abort(); + } + test_reset_options(); + if (!power8_set) { + printf("_ARCH_PWR8 is not set.\n"); + abort(); + } + if (!vsx_set) { + printf("__VSX__ is not set.\n"); + abort(); + } +} +