Re: [PATCH, rs6000] Tests of ARCH_PWR8 and -mno-vsx option. (1/2)

2022-10-17 Thread will schmidt via Gcc-patches
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



Re: [PATCH, rs6000] Tests of ARCH_PWR8 and -mno-vsx option. (1/2)

2022-10-17 Thread Segher Boessenkool
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


Re: [PATCH, rs6000] Tests of ARCH_PWR8 and -mno-vsx option. (1/2)

2022-10-17 Thread Kewen.Lin via Gcc-patches
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 ..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 ..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 ..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 ..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/p

[PATCH, rs6000] Tests of ARCH_PWR8 and -mno-vsx option. (1/2)

2022-09-19 Thread will schmidt via Gcc-patches
[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 ..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 ..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 ..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 ..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 ..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 supp