Re: [PATCH] Fix gcc.target/mips/extend-1.c

[Richard Sandiford <rdsandiford at googlemail dot com>]

> As it can been seen from Laurent's testing:
>
>   http://gcc.gnu.org/ml/gcc-testresults/2009-08/msg00020.html
>
> extend-1.c is currently failing with mips64-linux/-mabi=32.
>
> The problem is that the !cpu logic does not take the -mgp64 test option into
> consideration when picking the generic ISA.  (The test has dg-option "-mgp64
> isa=!octeon".)  Therefore it's happy to go with a < mips3 ISA, which causes a
> compilation error.  Logically, the test also fails on mips-elf.
>
> Looking at this once more, it's clear now that the !cpu code and the code
> dealing with a mismatch with the isa* directives should share the logic of how
> a generic ISA is picked from the isa* directives.  This is what the patch does
> below.

Hmm, as far as this goes:

Adam Nemet <anemet@caviumnetworks.com> writes:
> ! 	    # Canonicalize isa=!cpu to isa>=1!cpu or isa>=3!cpu depending on
> ! 	    # the -mgp* flags.
> ! 	    if { $value eq "" } {
> ! 		set prop "isa"
> ! 		set relation ">="
> ! 		set value \
> ! 		    [expr { [mips_have_option_p options "-mgp32"] ? 1 : 3 }]

I'd have expected:

       [expr { [mips_have_option_p options "-mgp64"] ? 3 : 1 }]

instead.  I.e. be as accomodating as possible.  But I don't think this
would work with things like "isa=!octeon -mbranch-likely", where we need
MIPS II.  (We don't need to worry about that for "isa(=|>=|<=)N
-mbranch-likely", because it's the test writer's responsibility
to make sure that the combinations are OK.)

How about the patch below?  Sorry for going ahead and writing it,
but I started trying to describe it in words, and soon realised it
wasn't going to make much sense.

> I've also removed the redundant isa level spec from the dmult-1.c testcase.

Agreed.

I've included the -b form too.  Not tested much.

Richard


2009-08-18  Adam Nemet  <anemet@caviumnetworks.com>
	    Richard Sandiford  <rdsandiford@googlemail.com>

	* gcc.target/mips/mips.exp: Replace isa(_rev)=...!... mechanism
	with "forbid_cpu".
	* gcc.target/mips/branch-1.c: Update accordingly.
	* gcc.target/mips/extend-1.c: Likewise.
	* gcc.target/mips/dmult-1.c: Likewise.  Remove redundant isa=64.

Index: gcc/testsuite/gcc.target/mips/mips.exp
===================================================================
--- gcc/testsuite/gcc.target/mips/mips.exp	(revision 150802)
+++ gcc/testsuite/gcc.target/mips/mips.exp	(working copy)
@@ -137,13 +137,6 @@
 # For example, "isa_rev>=1" selects a MIPS32 or MIPS64 processor,
 # "isa=4" selects a MIPS IV processor, and so on.
 #
-# If certain processor-specific extensions are not applicable to the
-# test you can list them as !CPU in the isa or isa_rev options.  For
-# example, isa=64!octeon enforces MIPS64 while avoiding octeon.  You
-# can also use ! without an ISA value.  For example
-# isa=!octeon!loongson2e disables octeon and loongson2e if otherwise
-# you would compile for one of them.
-#
 # There are also the following special pseudo-options:
 #
 #   isa=loongson
@@ -152,7 +145,11 @@
 #   addressing=absolute
 #	force absolute addresses to be used
 #
+#   forbid_cpu=REGEXP
+#	forbid processors that match the given regexp; choose a
+#	generic ISA instead.
 #
+#
 # In summary:
 #
 #   (1) Try to avoid { target ... } requirements wherever possible.
@@ -183,7 +180,7 @@
 #	"-mips32r2" or "-mips64r2".
 #
 #   (6) If you need to disable processor-specific extensions use
-#	isa=!CPU instead of forcing a generic ISA.
+#	forbid_cpu=CPU instead of forcing a generic ISA.
 
 # Exit immediately if this isn't a MIPS target.
 if ![istarget mips*-*-*] {
@@ -203,6 +200,7 @@
     dump_pattern "-dp"
     endianness "-E(L|B)|-me(l|b)"
     float "-m(hard|soft)-float"
+    forbid_cpu "forbid_cpu=.*"
     fp "-mfp(32|64)"
     gp "-mgp(32|64)"
     long "-mlong(32|64)"
@@ -827,60 +825,47 @@
 	}
     }
 
+    # See whether forbid_cpu forces us to choose a new architecture.
+    set arch [mips_option mips_base_options arch]
+    set force_generic_isa_p [expr {
+	[regexp "forbid_cpu=(.*)" [mips_option options forbid_cpu] dummy spec]
+	&& [regexp -- "^-march=$spec\$" $arch]
+    }]
+
     # Interpret the special "isa" and "isa_rev" options.  If we have
     # a choice of a 32-bit or a 64-bit architecture, prefer to keep
     # the -mgp setting the same.
     set spec [mips_option options arch]
     if { [regexp {^[^-]} $spec] } {
-	set arch [mips_option mips_base_options arch]
 	if { [string equal $spec "isa=loongson"] } {
 	    if { ![regexp {^-march=loongson} $arch] } {
 		set arch "-march=loongson2f"
 	    }
 	} else {
-	    # With ! and = the ISA value is optional.
-	    if { ![regexp {^(isa(?:|_rev))(=|<=|>=)([0-9]*)((?:![^!]+)*)$} \
-		       $spec dummy prop relation value nocpus]
-		 || ($value eq ""
-		     && ($relation ne "="
-			 || $nocpus eq ""))} {
+	    if { ![regexp {^(isa(?:|_rev))(=|<=|>=)([0-9]*)$} \
+		       $spec dummy prop relation value nocpus] } {
 		error "Unrecognized isa specification: $spec"
 	    }
-	    if { $value ne "" } {
-		set current [mips_arch_info $arch $prop]
-		if { ($current < $value && ![string equal $relation "<="])
-		     || ($current > $value && ![string equal $relation ">="])
-		     || ([mips_have_test_option_p options "-mgp64"]
-			 && [mips_32bit_arch_p $arch]) } {
-		    # The current setting is out of range; it cannot
-		    # possibly be used.  Find a replacement that can.
-		    if { [string equal $prop "isa"] } {
-			set arch "-mips$value"
-		    } elseif { $value == 0 } {
-			set arch "-mips4"
+	    set current [mips_arch_info $arch $prop]
+	    if { $force_generic_isa_p
+		 || ($current < $value && ![string equal $relation "<="])
+		 || ($current > $value && ![string equal $relation ">="])
+		 || ([mips_have_test_option_p options "-mgp64"]
+		     && [mips_32bit_arch_p $arch]) } {
+		# The current setting is out of range; it cannot
+		# possibly be used.  Find a replacement that can.
+		if { [string equal $prop "isa"] } {
+		    set arch "-mips$value"
+		} elseif { $value == 0 } {
+		    set arch "-mips4"
+		} else {
+		    if { [mips_have_option_p options "-mgp32"] } {
+			set arch "-mips32"
 		    } else {
-			if { [mips_have_option_p options "-mgp32"] } {
-			    set arch "-mips32"
-			} else {
-			    set arch "-mips64"
-			}
-			if { $value > 1 } {
-			    append arch "r$value"
-			}
+			set arch "-mips64"
 		    }
-		}
-	    }
-	    # If we haven't switched to a generic ISA based on the
-	    # isa* value, do it here if the processor-specific
-	    # extension is not allowed.
-	    if { $nocpus ne ""
-		 && $arch eq [mips_option mips_base_options arch] } {
-		set cpu [regsub -- {-march=} $arch ""]
-		if { [regexp "!$cpu!" "$nocpus!"] } {
-		    set isa_rev [mips_arch_info $arch isa_rev]
-		    set arch "-mips[mips_arch_info $arch isa]"		
-		    if { $isa_rev > 1 } {
-			append arch "r$isa_rev"
+		    if { $value > 1 } {
+			append arch "r$value"
 		    }
 		}
 	    }
@@ -965,6 +950,14 @@
 	    } else {
 		mips_make_test_option options "-mips64r$isa_rev"
 	    }
+	# Otherwise, if the current choice of architecture is unacceptable,
+	# choose the equivalent generic architecture.
+	} elseif { $force_generic_isa_p } {
+	    set arch "-mips[mips_arch_info $arch isa]"		
+	    if { $isa_rev > 1 } {
+		append arch "r$isa_rev"
+	    }
+	    mips_make_test_option options $arch
 	}
 	unset arch
 	unset isa
@@ -1111,6 +1104,7 @@
 
     # Add all options to the dg variable.
     set options(explicit_p,addressing) 0
+    set options(explicit_p,forbid_cpu) 0
     foreach { group regexp } $mips_option_groups {
 	if { $options(explicit_p,$group) } {
 	    append extra_tool_flags " " $options(option,$group)
Index: gcc/testsuite/gcc.target/mips/dmult-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/dmult-1.c	(revision 150802)
+++ gcc/testsuite/gcc.target/mips/dmult-1.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-options "isa=64!octeon -mgp64" } */
+/* { dg-options "forbid_cpu=octeon -mgp64" } */
 /* { dg-final { scan-assembler "\tdmult\t" } } */
 /* { dg-final { scan-assembler "\tmflo\t" } } */
 /* { dg-final { scan-assembler-not "\tdmul\t" } } */
Index: gcc/testsuite/gcc.target/mips/branch-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/branch-1.c	(revision 150802)
+++ gcc/testsuite/gcc.target/mips/branch-1.c	(working copy)
@@ -2,7 +2,7 @@
    but we test for "bbit" elsewhere.  On other targets, we should implement
    the "if" statements using an "andi" instruction followed by a branch
    on zero.  */
-/* { dg-options "-O2 isa=!octeon" } */
+/* { dg-options "-O2 forbid_cpu=octeon" } */
 
 void bar (void);
 NOMIPS16 void f1 (int x) { if (x & 4) bar (); }
Index: gcc/testsuite/gcc.target/mips/extend-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/extend-1.c	(revision 150802)
+++ gcc/testsuite/gcc.target/mips/extend-1.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-options "-O -mgp64 isa=!octeon" } */
+/* { dg-options "-O -mgp64 forbid_cpu=octeon" } */
 /* { dg-final { scan-assembler-times "\tdsll\t" 5 } } */
 /* { dg-final { scan-assembler-times "\tdsra\t" 5 } } */
 /* { dg-final { scan-assembler-not "\tsll\t" } } */

-------------------------

Index: gcc/testsuite/gcc.target/mips/mips.exp
===================================================================
--- gcc/testsuite/gcc.target/mips/mips.exp	(revision 150802)
+++ gcc/testsuite/gcc.target/mips/mips.exp	(working copy)
@@ -137,13 +137,6 @@
 # For example, "isa_rev>=1" selects a MIPS32 or MIPS64 processor,
 # "isa=4" selects a MIPS IV processor, and so on.
 #
-# If certain processor-specific extensions are not applicable to the
-# test you can list them as !CPU in the isa or isa_rev options.  For
-# example, isa=64!octeon enforces MIPS64 while avoiding octeon.  You
-# can also use ! without an ISA value.  For example
-# isa=!octeon!loongson2e disables octeon and loongson2e if otherwise
-# you would compile for one of them.
-#
 # There are also the following special pseudo-options:
 #
 #   isa=loongson
@@ -152,7 +145,11 @@
 #   addressing=absolute
 #	force absolute addresses to be used
 #
+#   forbid_cpu=REGEXP
+#	forbid processors that match the given regexp; choose a
+#	generic ISA instead.
 #
+#
 # In summary:
 #
 #   (1) Try to avoid { target ... } requirements wherever possible.
@@ -183,7 +180,7 @@
 #	"-mips32r2" or "-mips64r2".
 #
 #   (6) If you need to disable processor-specific extensions use
-#	isa=!CPU instead of forcing a generic ISA.
+#	forbid_cpu=CPU instead of forcing a generic ISA.
 
 # Exit immediately if this isn't a MIPS target.
 if ![istarget mips*-*-*] {
@@ -203,6 +200,7 @@
     dump_pattern "-dp"
     endianness "-E(L|B)|-me(l|b)"
     float "-m(hard|soft)-float"
+    forbid_cpu "forbid_cpu=.*"
     fp "-mfp(32|64)"
     gp "-mgp(32|64)"
     long "-mlong(32|64)"
@@ -827,28 +825,30 @@
 	}
     }
 
+    # See whether forbid_cpu forces us to choose a new architecture.
+    set arch [mips_option mips_base_options arch]
+    set force_generic_isa_p [expr {
+	[regexp "forbid_cpu=(.*)" [mips_option options forbid_cpu] dummy spec]
+	&& [regexp -- "^-march=$spec\$" $arch]
+    }]
+
     # Interpret the special "isa" and "isa_rev" options.  If we have
     # a choice of a 32-bit or a 64-bit architecture, prefer to keep
     # the -mgp setting the same.
     set spec [mips_option options arch]
     if { [regexp {^[^-]} $spec] } {
-	set arch [mips_option mips_base_options arch]
 	if { [string equal $spec "isa=loongson"] } {
 	    if { ![regexp {^-march=loongson} $arch] } {
 		set arch "-march=loongson2f"
 	    }
 	} else {
-	    # With ! and = the ISA value is optional.
-	    if { ![regexp {^(isa(?:|_rev))(=|<=|>=)([0-9]*)((?:![^!]+)*)$} \
-		       $spec dummy prop relation value nocpus]
-		 || ($value eq ""
-		     && ($relation ne "="
-			 || $nocpus eq ""))} {
+	    if { ![regexp {^(isa(?:|_rev))(=|<=|>=)([0-9]*)$} \
+		       $spec dummy prop relation value nocpus] } {
 		error "Unrecognized isa specification: $spec"
 	    }
-	    if { $value ne "" } {
 		set current [mips_arch_info $arch $prop]
-		if { ($current < $value && ![string equal $relation "<="])
+	    if { $force_generic_isa_p
+		 || ($current < $value && ![string equal $relation "<="])
 		     || ($current > $value && ![string equal $relation ">="])
 		     || ([mips_have_test_option_p options "-mgp64"]
 			 && [mips_32bit_arch_p $arch]) } {
@@ -870,21 +870,6 @@
 		    }
 		}
 	    }
-	    # If we haven't switched to a generic ISA based on the
-	    # isa* value, do it here if the processor-specific
-	    # extension is not allowed.
-	    if { $nocpus ne ""
-		 && $arch eq [mips_option mips_base_options arch] } {
-		set cpu [regsub -- {-march=} $arch ""]
-		if { [regexp "!$cpu!" "$nocpus!"] } {
-		    set isa_rev [mips_arch_info $arch isa_rev]
-		    set arch "-mips[mips_arch_info $arch isa]"		
-		    if { $isa_rev > 1 } {
-			append arch "r$isa_rev"
-		    }
-		}
-	    }
-	}
 	set options(option,arch) $arch
     }
 
@@ -965,7 +950,15 @@
 	    } else {
 		mips_make_test_option options "-mips64r$isa_rev"
 	    }
+	# Otherwise, if the current choice of architecture is unacceptable,
+	# choose the equivalent generic architecture.
+	} elseif { $force_generic_isa_p } {
+	    set arch "-mips[mips_arch_info $arch isa]"		
+	    if { $isa_rev > 1 } {
+		append arch "r$isa_rev"
 	}
+	    mips_make_test_option options $arch
+	}
 	unset arch
 	unset isa
 	unset isa_rev
@@ -1111,6 +1104,7 @@
 
     # Add all options to the dg variable.
     set options(explicit_p,addressing) 0
+    set options(explicit_p,forbid_cpu) 0
     foreach { group regexp } $mips_option_groups {
 	if { $options(explicit_p,$group) } {
 	    append extra_tool_flags " " $options(option,$group)