diff mbox series

[v2,1/7] Alpha: Always respect -mbwx, -mcix, -mfix, -mmax, and their inverse

Message ID alpine.DEB.2.21.2501050258180.49841@angie.orcam.me.uk
State Committed
Commit 19fdb9f3792d4c3c9ff3d18dc4566bb16e62de60
Headers
Series Fix data races with sub-longword accesses on Alpha |

Commit Message

Maciej W. Rozycki Jan. 6, 2025, 1:03 p.m. UTC 
  Contrary to user documentation the `-mbwx', `-mcix', `-mfix', `-mmax' 
feature options and their inverse forms are ignored whenever `-mcpu=' 
option is in effect, either by having been given explicitly or where 
configured as the default such as with the `alphaev56-linux-gnu' target.  
In the latter case there is no way to change the settings these options 
are supposed to tweak other than with `-mcpu=' and the settings cannot 
be individually controlled, making all the feature options permanently 
inactive.

It seems a regression from commit 7816bea0e23b ("config.gcc: Reorganize 
--with-cpu logic.") back in 2003, which replaced the setting of the 
default feature mask with the setting of the default CPU across a few 
targets, and the complementing logic in the Alpha backend wasn't updated 
accordingly.

Fix this by making the individual feature options take precedence over 
`-mcpu='.  Add test cases to verify this is the case, and to cover the 
defaults as well for the boundary cases.

This has a drawback where the order of the options is ignored between 
`-mcpu=' and these individual options, so e.g. `-mno-bwx -mcpu=ev6' will 
keep the BWX feature disabled even though `-mcpu=ev6' comes later in the 
command line.  This may affect some scenarios involving user overrides 
such as with CFLAGS passed to `configure' and `make' invocations.  I do 
believe it has been our practice anyway for more finegrained options to 
override group options regardless of their relative order on the command 
line and in any case using `-mcpu=ev6 -mbwx' as the override will do the 
right thing if required, canceling any previous `-mno-bwx'.

This has been spotted with `alphaev56-linux-gnu' target verification and 
a recently added test case:

FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\sldq_u\\s 2
FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\smskwh\\s 1
FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\smskwl\\s 1
FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\sstq_u\\s 2

(and similarly for the remaining optimization levels covered) which this 
fix has addressed.

	gcc/
	* config/alpha/alpha.cc (alpha_option_override): Ignore CPU 
	flags corresponding to features the enabling or disabling of 
	which has been requested with an individual feature option.

	gcc/testsuite/
	* gcc.target/alpha/target-bwx-1.c: New file.
	* gcc.target/alpha/target-bwx-2.c: New file.
	* gcc.target/alpha/target-bwx-3.c: New file.
	* gcc.target/alpha/target-bwx-4.c: New file.
	* gcc.target/alpha/target-cix-1.c: New file.
	* gcc.target/alpha/target-cix-2.c: New file.
	* gcc.target/alpha/target-cix-3.c: New file.
	* gcc.target/alpha/target-cix-4.c: New file.
	* gcc.target/alpha/target-fix-1.c: New file.
	* gcc.target/alpha/target-fix-2.c: New file.
	* gcc.target/alpha/target-fix-3.c: New file.
	* gcc.target/alpha/target-fix-4.c: New file.
	* gcc.target/alpha/target-max-1.c: New file.
	* gcc.target/alpha/target-max-2.c: New file.
	* gcc.target/alpha/target-max-3.c: New file.
	* gcc.target/alpha/target-max-4.c: New file.
---
Changes from v1:

- Original standalone submission: 
  <https://inbox.sourceware.org/gcc-patches/alpine.DEB.2.21.2412300410050.20821@angie.orcam.me.uk/>.

- Fold into this series to prevent failures with newly-added tests in 2/7, 
  6/7, and 7/7.

- Mention the likely cause of the regression in the description.
---
 gcc/config/alpha/alpha.cc                     |    5 +++--
 gcc/testsuite/gcc.target/alpha/target-bwx-1.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-bwx-2.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-bwx-3.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-bwx-4.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-cix-1.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-cix-2.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-cix-3.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-cix-4.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-fix-1.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-fix-2.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-fix-3.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-fix-4.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-max-1.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-max-2.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-max-3.c |    6 ++++++
 gcc/testsuite/gcc.target/alpha/target-max-4.c |    6 ++++++
 17 files changed, 99 insertions(+), 2 deletions(-)

gcc-alpha-flags-explicit.diff

Comments

Jeff Law Jan. 6, 2025, 7:34 p.m. UTC | #1 
On 1/6/25 6:03 AM, Maciej W. Rozycki wrote:
> Contrary to user documentation the `-mbwx', `-mcix', `-mfix', `-mmax'
> feature options and their inverse forms are ignored whenever `-mcpu='
> option is in effect, either by having been given explicitly or where
> configured as the default such as with the `alphaev56-linux-gnu' target.
> In the latter case there is no way to change the settings these options
> are supposed to tweak other than with `-mcpu=' and the settings cannot
> be individually controlled, making all the feature options permanently
> inactive.
> 
> It seems a regression from commit 7816bea0e23b ("config.gcc: Reorganize
> --with-cpu logic.") back in 2003, which replaced the setting of the
> default feature mask with the setting of the default CPU across a few
> targets, and the complementing logic in the Alpha backend wasn't updated
> accordingly.
> 
> Fix this by making the individual feature options take precedence over
> `-mcpu='.  Add test cases to verify this is the case, and to cover the
> defaults as well for the boundary cases.
> 
> This has a drawback where the order of the options is ignored between
> `-mcpu=' and these individual options, so e.g. `-mno-bwx -mcpu=ev6' will
> keep the BWX feature disabled even though `-mcpu=ev6' comes later in the
> command line.  This may affect some scenarios involving user overrides
> such as with CFLAGS passed to `configure' and `make' invocations.  I do
> believe it has been our practice anyway for more finegrained options to
> override group options regardless of their relative order on the command
> line and in any case using `-mcpu=ev6 -mbwx' as the override will do the
> right thing if required, canceling any previous `-mno-bwx'.
> 
> This has been spotted with `alphaev56-linux-gnu' target verification and
> a recently added test case:
> 
> FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\sldq_u\\s 2
> FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\smskwh\\s 1
> FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\smskwl\\s 1
> FAIL: gcc.target/alpha/stwx0.c   -O1   scan-assembler-times \\sstq_u\\s 2
> 
> (and similarly for the remaining optimization levels covered) which this
> fix has addressed.
> 
> 	gcc/
> 	* config/alpha/alpha.cc (alpha_option_override): Ignore CPU
> 	flags corresponding to features the enabling or disabling of
> 	which has been requested with an individual feature option.
> 
> 	gcc/testsuite/
> 	* gcc.target/alpha/target-bwx-1.c: New file.
> 	* gcc.target/alpha/target-bwx-2.c: New file.
> 	* gcc.target/alpha/target-bwx-3.c: New file.
> 	* gcc.target/alpha/target-bwx-4.c: New file.
> 	* gcc.target/alpha/target-cix-1.c: New file.
> 	* gcc.target/alpha/target-cix-2.c: New file.
> 	* gcc.target/alpha/target-cix-3.c: New file.
> 	* gcc.target/alpha/target-cix-4.c: New file.
> 	* gcc.target/alpha/target-fix-1.c: New file.
> 	* gcc.target/alpha/target-fix-2.c: New file.
> 	* gcc.target/alpha/target-fix-3.c: New file.
> 	* gcc.target/alpha/target-fix-4.c: New file.
> 	* gcc.target/alpha/target-max-1.c: New file.
> 	* gcc.target/alpha/target-max-2.c: New file.
> 	* gcc.target/alpha/target-max-3.c: New file.
> 	* gcc.target/alpha/target-max-4.c: New file.
OK
jeff
Maciej W. Rozycki Jan. 12, 2025, 4:51 p.m. UTC | #2 
On Mon, 6 Jan 2025, Jeff Law wrote:

> > 	gcc/
> > 	* config/alpha/alpha.cc (alpha_option_override): Ignore CPU
> > 	flags corresponding to features the enabling or disabling of
> > 	which has been requested with an individual feature option.
> > 
> > 	gcc/testsuite/
> > 	* gcc.target/alpha/target-bwx-1.c: New file.
> > 	* gcc.target/alpha/target-bwx-2.c: New file.
> > 	* gcc.target/alpha/target-bwx-3.c: New file.
> > 	* gcc.target/alpha/target-bwx-4.c: New file.
> > 	* gcc.target/alpha/target-cix-1.c: New file.
> > 	* gcc.target/alpha/target-cix-2.c: New file.
> > 	* gcc.target/alpha/target-cix-3.c: New file.
> > 	* gcc.target/alpha/target-cix-4.c: New file.
> > 	* gcc.target/alpha/target-fix-1.c: New file.
> > 	* gcc.target/alpha/target-fix-2.c: New file.
> > 	* gcc.target/alpha/target-fix-3.c: New file.
> > 	* gcc.target/alpha/target-fix-4.c: New file.
> > 	* gcc.target/alpha/target-max-1.c: New file.
> > 	* gcc.target/alpha/target-max-2.c: New file.
> > 	* gcc.target/alpha/target-max-3.c: New file.
> > 	* gcc.target/alpha/target-max-4.c: New file.
> OK

 Applied now, thanks for your review.

  Maciej
diff mbox series

Patch

Index: gcc/gcc/config/alpha/alpha.cc
===================================================================
--- gcc.orig/gcc/config/alpha/alpha.cc
+++ gcc/gcc/config/alpha/alpha.cc
@@ -460,8 +460,9 @@  alpha_option_override (void)
 	    line_size = cpu_table[i].line_size;
 	    l1_size = cpu_table[i].l1_size;
 	    l2_size = cpu_table[i].l2_size;
-	    target_flags &= ~ (MASK_BWX | MASK_MAX | MASK_FIX | MASK_CIX);
-	    target_flags |= cpu_table[i].flags;
+	    target_flags &= ~((MASK_BWX | MASK_MAX | MASK_FIX | MASK_CIX)
+			      & ~target_flags_explicit);
+	    target_flags |= cpu_table[i].flags & ~target_flags_explicit;
 	    break;
 	  }
       if (i == ct_size)
Index: gcc/gcc/testsuite/gcc.target/alpha/target-bwx-1.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-bwx-1.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev5" } */
+
+#ifdef __alpha_bwx__
+# error "BWX enabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-bwx-2.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-bwx-2.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev56" } */
+
+#ifndef __alpha_bwx__
+# error "BWX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-bwx-3.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-bwx-3.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev5 -mbwx" } */
+
+#ifndef __alpha_bwx__
+# error "BWX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-bwx-4.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-bwx-4.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev56 -mno-bwx" } */
+
+#ifdef __alpha_bwx__
+# error "BWX enabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-cix-1.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-cix-1.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev6" } */
+
+#ifdef __alpha_cix__
+# error "CIX enabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-cix-2.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-cix-2.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev67" } */
+
+#ifndef __alpha_cix__
+# error "CIX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-cix-3.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-cix-3.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev6 -mcix" } */
+
+#ifndef __alpha_cix__
+# error "CIX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-cix-4.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-cix-4.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev67 -mno-cix" } */
+
+#ifdef __alpha_cix__
+# error "CIX enabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-fix-1.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-fix-1.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=pca56" } */
+
+#ifdef __alpha_fix__
+# error "FIX enabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-fix-2.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-fix-2.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev6" } */
+
+#ifndef __alpha_fix__
+# error "FIX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-fix-3.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-fix-3.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=pca56 -mfix" } */
+
+#ifndef __alpha_fix__
+# error "FIX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-fix-4.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-fix-4.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev6 -mno-fix" } */
+
+#ifdef __alpha_fix__
+# error "FIX enabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-max-1.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-max-1.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev56" } */
+
+#ifdef __alpha_max__
+# error "MAX enabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-max-2.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-max-2.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=pca56" } */
+
+#ifndef __alpha_max__
+# error "MAX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-max-3.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-max-3.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ev56 -mmax" } */
+
+#ifndef __alpha_max__
+# error "MAX disabled"
+#endif
Index: gcc/gcc/testsuite/gcc.target/alpha/target-max-4.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.target/alpha/target-max-4.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=pca56 -mno-max" } */
+
+#ifdef __alpha_max__
+# error "MAX enabled"
+#endif