diff mbox series

[V4] Do not allow -mvsx to boost the cpu to power7

Message ID ZzjYrw3LdkkMbZ5y@cowardly-lion.the-meissners.org
State New
Headers show
Series [V4] Do not allow -mvsx to boost the cpu to power7 | expand

Commit Message

Michael Meissner Nov. 16, 2024, 5:38 p.m. UTC
Peter Bergner asked me to reorganize my V3 patch that separates the
architecture bits (set via -mcpu=<xxx>) compared to the ISA bits that are based
on options.

Here is the beginning of the V3 patch for reference:

https://gcc.gnu.org/pipermail/gcc-patches/2024-November/668643.html

As Peter has suggested, I reworked the patch so the the bits where I rename the
target flags comes before the bit about separating the architecture bits.  I
also moved the -mcpu=future bits before the architecture bits as well.

There are 4 separate patch sets (that might have separate patches within the
patch sets).  Each of the patch sets is a logical entity.

The 4 patch sets are:

    1:	Rename the TARGET_<xxx> options so that they say TARGET_POWER5 instead
	of TARGET_POPCNTB.  In this patch set, TARGET_POWER5 is a macro that
	references TARGET_POPCNTB.  The 5 patches in this patch set rename each
	option in turn.

    2:	Add support for -mcpu=future.  Because the architecture mask support is
	in the 4th patch set, this patch set adds a dummy switch -mfuture.

    3:	Make -mvsx not internally set -mcpu=power7.  This has come up in
	several bugs.  This is independent of the other patches, and can be
	omitted if desired.

    4:	The last patch set now provides the separation between the architecture
	bits and the ISA bits.  It removes several of the dummy switches
	(-mpower10, -mpower11, -mfuture) that were added to support those
	processors, but users aren't supposed to use those options.

This patch restructures the code so that -mvsx for example will not silently
convert the processor to power7.  The user must now use -mcpu=power7 or higher.
This means if the user does -mvsx and the default processor does not have VSX
support, it will be an error.

I have built both big endian and little endian bootstrap compilers and there
were no regressions.

I updated the 2 tests that used -mvsx to raise the cpu to power7, and the test
case that checks if -mno-vsx produces the expected warning.

Note, Peter had some questions about one of the tests in the previous version of
the patch.  The test is still the same in this patch.  But the code for
preventing -mvsx is different from the previous patch, and I wanted to get that
patch for review before stage1 closes.

Can I install this patch on the GCC 15 trunk?

2024-11-16  Michael Meissner  <meissner@linux.ibm.com>

gcc/

	* config/rs6000/rs6000.cc (rs6000_option_override_internal): Check if
	the user asked for VSX instructions whether the cpu was at least power7.

gcc/testsuite/

	* gcc.target/powerpc/ppc-target-4.c: Rewrite the test to add cpu=power7
	when we need to add VSX support.  Add test for adding cpu=power7 no-vsx
	to generate only Altivec instructions.
	* gcc.target/powerpc/pr115688.c: Add cpu=power7 in target __attribute__
	when requesting VSX instructions.
	* gcc.target/powerpc/pr87496-1.c: Update options to use
	-mdejagnu-cpu=power6 to get the appropriate error message.
---
 gcc/config/rs6000/rs6000.cc                   |  7 ++++
 .../gcc.target/powerpc/ppc-target-4.c         | 38 ++++++++++++++-----
 gcc/testsuite/gcc.target/powerpc/pr115688.c   |  3 +-
 gcc/testsuite/gcc.target/powerpc/pr87496-1.c  |  2 +-
 4 files changed, 39 insertions(+), 11 deletions(-)

Comments

Michael Meissner Dec. 4, 2024, 7:46 a.m. UTC | #1
Ping patch to not allow -mvsx to boost the cpu to power7

Message-ID <ZzjYrw3LdkkMbZ5y@cowardly-lion.the-meissners.org>

https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669106.html
Michael Meissner Jan. 9, 2025, 6:02 p.m. UTC | #2
Ping patch to not allow -mvsx to boost the cpu to power7

Message-ID <ZzjYrw3LdkkMbZ5y@cowardly-lion.the-meissners.org>

https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669106.html
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 183940f07bd..214c8f326e3 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -3860,6 +3860,13 @@  rs6000_option_override_internal (bool global_init_p)
 	  rs6000_isa_flags &= ~OPTION_MASK_VSX;
 	  rs6000_isa_flags_explicit |= OPTION_MASK_VSX;
 	}
+      else if (!TARGET_POWER7)
+	{
+	  if (explicit_vsx_p)
+	    error ("%<-mvsx%> requires at least %<-mcpu=power%>");
+	  rs6000_isa_flags &= ~OPTION_MASK_VSX;
+	  rs6000_isa_flags_explicit |= OPTION_MASK_VSX;
+	}
     }
 
   /* If hard-float/altivec/vsx were explicitly turned off then don't allow
diff --git a/gcc/testsuite/gcc.target/powerpc/ppc-target-4.c b/gcc/testsuite/gcc.target/powerpc/ppc-target-4.c
index feef76db461..5e2ecf34f24 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc-target-4.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-target-4.c
@@ -2,7 +2,7 @@ 
 /* { dg-skip-if "" { powerpc*-*-darwin* } } */
 /* { dg-require-effective-target powerpc_fprs } */
 /* { dg-options "-O2 -ffast-math -mdejagnu-cpu=power5 -mno-altivec -mabi=altivec -fno-unroll-loops" } */
-/* { dg-final { scan-assembler-times "vaddfp" 1 } } */
+/* { dg-final { scan-assembler-times "vaddfp" 2 } } */
 /* { dg-final { scan-assembler-times "xvaddsp" 1 } } */
 /* { dg-final { scan-assembler-times "fadds" 1 } } */
 
@@ -18,10 +18,6 @@ 
 #error "__VSX__ should not be defined."
 #endif
 
-#pragma GCC target("altivec,vsx")
-#include <altivec.h>
-#pragma GCC reset_options
-
 #pragma GCC push_options
 #pragma GCC target("altivec,no-vsx")
 
@@ -33,6 +29,7 @@ 
 #error "__VSX__ should not be defined."
 #endif
 
+/* Altivec build, generate vaddfp.  */
 void
 av_add (vector float *a, vector float *b, vector float *c)
 {
@@ -40,10 +37,11 @@  av_add (vector float *a, vector float *b, vector float *c)
   unsigned long n = SIZE / 4;
 
   for (i = 0; i < n; i++)
-    a[i] = vec_add (b[i], c[i]);
+    a[i] = b[i] + c[i];
 }
 
-#pragma GCC target("vsx")
+/* cpu=power7 must be used to enable VSX.  */
+#pragma GCC target("cpu=power7,vsx")
 
 #ifndef __ALTIVEC__
 #error "__ALTIVEC__ should be defined."
@@ -53,6 +51,7 @@  av_add (vector float *a, vector float *b, vector float *c)
 #error "__VSX__ should be defined."
 #endif
 
+/* VSX build on power7, generate xsaddsp.  */
 void
 vsx_add (vector float *a, vector float *b, vector float *c)
 {
@@ -60,11 +59,31 @@  vsx_add (vector float *a, vector float *b, vector float *c)
   unsigned long n = SIZE / 4;
 
   for (i = 0; i < n; i++)
-    a[i] = vec_add (b[i], c[i]);
+    a[i] = b[i] + c[i];
+}
+
+#pragma GCC target("cpu=power7,no-vsx")
+
+#ifndef __ALTIVEC__
+#error "__ALTIVEC__ should be defined."
+#endif
+
+#ifdef __VSX__
+#error "__VSX__ should not be defined."
+#endif
+
+/* Altivec build on power7 with no VSX, generate vaddfp.  */
+void
+av2_add (vector float *a, vector float *b, vector float *c)
+{
+  unsigned long i;
+  unsigned long n = SIZE / 4;
+
+  for (i = 0; i < n; i++)
+    a[i] = b[i] + c[i];
 }
 
 #pragma GCC pop_options
-#pragma GCC target("no-vsx,no-altivec")
 
 #ifdef __ALTIVEC__
 #error "__ALTIVEC__ should not be defined."
@@ -74,6 +93,7 @@  vsx_add (vector float *a, vector float *b, vector float *c)
 #error "__VSX__ should not be defined."
 #endif
 
+/* Default power5 build, generate scalar fadds.  */
 void
 norm_add (float *a, float *b, float *c)
 {
diff --git a/gcc/testsuite/gcc.target/powerpc/pr115688.c b/gcc/testsuite/gcc.target/powerpc/pr115688.c
index 5222e66ef17..00c7c301436 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr115688.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr115688.c
@@ -7,7 +7,8 @@ 
 
 /* Verify there is no ICE under 32 bit env.  */
 
-__attribute__((target("vsx")))
+/* cpu=power7 must be used to enable VSX.  */
+__attribute__((target("cpu=power7,vsx")))
 int test (void)
 {
   return 0;
diff --git a/gcc/testsuite/gcc.target/powerpc/pr87496-1.c b/gcc/testsuite/gcc.target/powerpc/pr87496-1.c
index b8d00286256..2029ecacaf9 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr87496-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr87496-1.c
@@ -2,7 +2,7 @@ 
 /* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
 /* { dg-skip-if "" { powerpc*-*-darwin* } } */
 /* { dg-require-effective-target longdouble128 } */
-/* { dg-options "-O2 -mdejagnu-cpu=power7 -mabi=ieeelongdouble -mno-popcntd -Wno-psabi" } */
+/* { dg-options "-O2 -mdejagnu-cpu=power6 -mabi=ieeelongdouble -Wno-psabi" } */
 
 int i;