[PATCH][Binutils]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions

[PATCH][Binutils]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 5734 bytes --]

Hi All,

At the beginning of the port it was decided that enabling features should always
come before disabling features. i.e. +foo should always be before any +nofoo.

For years now this has been relaxed in GCC but binutils has remained rather
strict.  This removes the restriction from gas as well giving users less
friction.

build on native hardware and regtested on
  aarch64-none-elf, aarch64-none-elf (32 bit host),
  aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host)

Cross-compiled and regtested on
  aarch64-none-linux-gnu, aarch64_be-none-linux-gnu

and no issues.

Ok for master?

Thanks,
Tamar

gas/ChangeLog:

2023-02-01  Tamar Christina  <tamar.christina@arm.com>

	* config/tc-aarch64.c (aarch64_parse_features): Remove feature ordering
	checks.
	* testsuite/gas/aarch64/opt_order.l: New test.
	* testsuite/gas/aarch64/opt_order.s: New test.
	* testsuite/gas/aarch64/opt_order_1.d: New test.
	* testsuite/gas/aarch64/opt_order_2.d: New test.
	* testsuite/gas/aarch64/opt_order_3.d: New test.
	* testsuite/gas/aarch64/opt_order_4.d: New test.
	* testsuite/gas/aarch64/opt_order_5.d: New test.
	* testsuite/gas/aarch64/opt_order_6.d: New test.

--- inline copy of patch -- 
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 2aeab6f958ac6435c528b4394412d6395932a6d5..0c2ffd7bb85dbf892f265b6b394de7709de2f60e 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -10234,11 +10234,7 @@ static int
 aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 			bool ext_only)
 {
-  /* We insist on extensions being added before being removed.  We achieve
-     this by using the ADDING_VALUE variable to indicate whether we are
-     adding an extension (1) or removing it (0) and only allowing it to
-     change in the order -1 -> 1 -> 0.  */
-  int adding_value = -1;
+  bool adding_value = false;
   aarch64_feature_set *ext_set = XNEW (aarch64_feature_set);
 
   /* Copy the feature set, so that we can modify it.  */
@@ -10269,31 +10265,20 @@ aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 
       if (optlen >= 2 && startswith (str, "no"))
 	{
-	  if (adding_value != 0)
-	    adding_value = 0;
+	  adding_value = 0;
 	  optlen -= 2;
 	  str += 2;
 	}
       else if (optlen > 0)
 	{
-	  if (adding_value == -1)
-	    adding_value = 1;
-	  else if (adding_value != 1)
-	    {
-	      as_bad (_("must specify extensions to add before specifying "
-			"those to remove"));
-	      return false;
-	    }
+	  adding_value = 1;
 	}
-
-      if (optlen == 0)
+      else if (optlen == 0)
 	{
 	  as_bad (_("missing architectural extension"));
 	  return 0;
 	}
 
-      gas_assert (adding_value != -1);
-
       for (opt = aarch64_features; opt->name != NULL; opt++)
 	if (strncmp (opt->name, str, optlen) == 0)
 	  {
diff --git a/gas/testsuite/gas/aarch64/opt_order.l b/gas/testsuite/gas/aarch64/opt_order.l
new file mode 100644
index 0000000000000000000000000000000000000000..94820513356d2d3c2a02c1f348e2c5423e43f1cd
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.l
@@ -0,0 +1 @@
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order.s b/gas/testsuite/gas/aarch64/opt_order.s
new file mode 100644
index 0000000000000000000000000000000000000000..e0642b3a811ad7d944418f36e237e6fd87771051
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.s
@@ -0,0 +1 @@
+UDOT    V0.2S, V0.8B, V0.8B
diff --git a/gas/testsuite/gas/aarch64/opt_order_1.d b/gas/testsuite/gas/aarch64/opt_order_1.d
new file mode 100644
index 0000000000000000000000000000000000000000..862f7be85f462bfc470a466e9652471d0d268652
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_1.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_2.d b/gas/testsuite/gas/aarch64/opt_order_2.d
new file mode 100644
index 0000000000000000000000000000000000000000..77bd6c57ec9be2e24c3b673f4eccb2c0f7bfc16c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_2.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+nodotprod+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_3.d b/gas/testsuite/gas/aarch64/opt_order_3.d
new file mode 100644
index 0000000000000000000000000000000000000000..5b22090331d11965097a35bad181693c75d51f3c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_3.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_4.d b/gas/testsuite/gas/aarch64/opt_order_4.d
new file mode 100644
index 0000000000000000000000000000000000000000..7ffe2a3011632fdcf25aa32eeb1bedbcc1f408ba
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_4.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_5.d b/gas/testsuite/gas/aarch64/opt_order_5.d
new file mode 100644
index 0000000000000000000000000000000000000000..e7d057c77e7efa14caf55c96cba321e99c36ca21
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_5.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_6.d b/gas/testsuite/gas/aarch64/opt_order_6.d
new file mode 100644
index 0000000000000000000000000000000000000000..b39247de3b3c7261a700b96be8cfbf6b781cbb6b
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_6.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+dotprod+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...




-- 

[-- Attachment #2: rb16852.patch --]
[-- Type: text/plain, Size: 4530 bytes --]

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 2aeab6f958ac6435c528b4394412d6395932a6d5..0c2ffd7bb85dbf892f265b6b394de7709de2f60e 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -10234,11 +10234,7 @@ static int
 aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 			bool ext_only)
 {
-  /* We insist on extensions being added before being removed.  We achieve
-     this by using the ADDING_VALUE variable to indicate whether we are
-     adding an extension (1) or removing it (0) and only allowing it to
-     change in the order -1 -> 1 -> 0.  */
-  int adding_value = -1;
+  bool adding_value = false;
   aarch64_feature_set *ext_set = XNEW (aarch64_feature_set);
 
   /* Copy the feature set, so that we can modify it.  */
@@ -10269,31 +10265,20 @@ aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 
       if (optlen >= 2 && startswith (str, "no"))
 	{
-	  if (adding_value != 0)
-	    adding_value = 0;
+	  adding_value = 0;
 	  optlen -= 2;
 	  str += 2;
 	}
       else if (optlen > 0)
 	{
-	  if (adding_value == -1)
-	    adding_value = 1;
-	  else if (adding_value != 1)
-	    {
-	      as_bad (_("must specify extensions to add before specifying "
-			"those to remove"));
-	      return false;
-	    }
+	  adding_value = 1;
 	}
-
-      if (optlen == 0)
+      else if (optlen == 0)
 	{
 	  as_bad (_("missing architectural extension"));
 	  return 0;
 	}
 
-      gas_assert (adding_value != -1);
-
       for (opt = aarch64_features; opt->name != NULL; opt++)
 	if (strncmp (opt->name, str, optlen) == 0)
 	  {
diff --git a/gas/testsuite/gas/aarch64/opt_order.l b/gas/testsuite/gas/aarch64/opt_order.l
new file mode 100644
index 0000000000000000000000000000000000000000..94820513356d2d3c2a02c1f348e2c5423e43f1cd
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.l
@@ -0,0 +1 @@
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order.s b/gas/testsuite/gas/aarch64/opt_order.s
new file mode 100644
index 0000000000000000000000000000000000000000..e0642b3a811ad7d944418f36e237e6fd87771051
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.s
@@ -0,0 +1 @@
+UDOT    V0.2S, V0.8B, V0.8B
diff --git a/gas/testsuite/gas/aarch64/opt_order_1.d b/gas/testsuite/gas/aarch64/opt_order_1.d
new file mode 100644
index 0000000000000000000000000000000000000000..862f7be85f462bfc470a466e9652471d0d268652
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_1.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_2.d b/gas/testsuite/gas/aarch64/opt_order_2.d
new file mode 100644
index 0000000000000000000000000000000000000000..77bd6c57ec9be2e24c3b673f4eccb2c0f7bfc16c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_2.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+nodotprod+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_3.d b/gas/testsuite/gas/aarch64/opt_order_3.d
new file mode 100644
index 0000000000000000000000000000000000000000..5b22090331d11965097a35bad181693c75d51f3c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_3.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_4.d b/gas/testsuite/gas/aarch64/opt_order_4.d
new file mode 100644
index 0000000000000000000000000000000000000000..7ffe2a3011632fdcf25aa32eeb1bedbcc1f408ba
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_4.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_5.d b/gas/testsuite/gas/aarch64/opt_order_5.d
new file mode 100644
index 0000000000000000000000000000000000000000..e7d057c77e7efa14caf55c96cba321e99c36ca21
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_5.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_6.d b/gas/testsuite/gas/aarch64/opt_order_6.d
new file mode 100644
index 0000000000000000000000000000000000000000..b39247de3b3c7261a700b96be8cfbf6b781cbb6b
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_6.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+dotprod+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...

Re: [PATCH][Binutils]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions

[Jan Beulich]

On 01.02.2023 20:25, Tamar Christina via Binutils wrote:
> At the beginning of the port it was decided that enabling features should always
> come before disabling features. i.e. +foo should always be before any +nofoo.
> 
> For years now this has been relaxed in GCC but binutils has remained rather
> strict.  This removes the restriction from gas as well giving users less
> friction.

Hmm, specifying negative before positive settings may mean the negative ones
don't take effect at all (because of feature dependencies). While the same
of course is true the other way around as well, silently accepting
supposedly disabled insns is imo quite a bit more risky than complaining
about supposedly enabled ones: The programmer may unknowingly produce a bad
binary. This is even more so that the dependencies can't be considered set
in stone - bugs may be found which require adjustments to them.

There actually is an example of something which may want adjusting: Both
F32MM and F64MM take SVE as prereq. While SVE can't easily be a prereq to
I8MM (because there are also SIMD insn forms), it's unclear why SIMD
- itself being a prereq to SVE - isn't a prereq to I8MM.

> @@ -10269,31 +10265,20 @@ aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
>  
>        if (optlen >= 2 && startswith (str, "no"))
>  	{
> -	  if (adding_value != 0)
> -	    adding_value = 0;
> +	  adding_value = 0;
>  	  optlen -= 2;
>  	  str += 2;
>  	}
>        else if (optlen > 0)
>  	{
> -	  if (adding_value == -1)
> -	    adding_value = 1;
> -	  else if (adding_value != 1)
> -	    {
> -	      as_bad (_("must specify extensions to add before specifying "
> -			"those to remove"));
> -	      return false;
> -	    }
> +	  adding_value = 1;
>  	}
> -
> -      if (optlen == 0)
> +      else if (optlen == 0)
>  	{
>  	  as_bad (_("missing architectural extension"));
>  	  return 0;
>  	}


This "if" -> "else if" transformation looks not only unrelated to me, but
actually slightly wrong: The message now won't trigger anymore when there
just "+no" was specified.

Jan

RE: [PATCH][Binutils]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions

[Tamar Christina]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Thursday, February 2, 2023 11:30 AM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: nd <nd@arm.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>;
> Marcus Shawcroft <Marcus.Shawcroft@arm.com>; binutils@sourceware.org
> Subject: Re: [PATCH][Binutils]AArch64 gas: relax ordering constriants on
> enabling and disabling feature extensions
> 
> On 01.02.2023 20:25, Tamar Christina via Binutils wrote:
> > At the beginning of the port it was decided that enabling features
> > should always come before disabling features. i.e. +foo should always be
> before any +nofoo.
> >
> > For years now this has been relaxed in GCC but binutils has remained
> > rather strict.  This removes the restriction from gas as well giving
> > users less friction.
> 
> Hmm, specifying negative before positive settings may mean the negative
> ones don't take effect at all (because of feature dependencies). While the
> same of course is true the other way around as well, silently accepting
> supposedly disabled insns is imo quite a bit more risky than complaining
> about supposedly enabled ones: The programmer may unknowingly produce
> a bad binary. This is even more so that the dependencies can't be considered
> set in stone - bugs may be found which require adjustments to them.

Perhaps, but this change was to address some user feedback in which determining
the order means they have to parse what definition for an -mcpu value the compiler
has and understand the options and re-order them to add their own extensions on top.

People do this because binutils doesn't understand -mcpu.

The compiler already doesn't care about the order,  even though for a compiler the no
Makes more sense as it turns off code generation for a particular instruction,
particular auto-vec and intrinsics.  For an assembler, your assembly file doesn't magically
include instructions for things you want to disable, you have to actively do so. Also today we
only have this limit on instructions, registers don't get gated like this.  Those already ignore
all extension flags. So I think the risk is much lower in reality.

But.. If it's really a concern you have I can make it a warning instead of a hard error.

> 
> There actually is an example of something which may want adjusting: Both
> F32MM and F64MM take SVE as prereq. While SVE can't easily be a prereq to
> I8MM (because there are also SIMD insn forms), it's unclear why SIMD
> - itself being a prereq to SVE - isn't a prereq to I8MM.

> 
> > @@ -10269,31 +10265,20 @@ aarch64_parse_features (const char *str,
> > const aarch64_feature_set **opt_p,
> >
> >        if (optlen >= 2 && startswith (str, "no"))
> >  	{
> > -	  if (adding_value != 0)
> > -	    adding_value = 0;
> > +	  adding_value = 0;
> >  	  optlen -= 2;
> >  	  str += 2;
> >  	}
> >        else if (optlen > 0)
> >  	{
> > -	  if (adding_value == -1)
> > -	    adding_value = 1;
> > -	  else if (adding_value != 1)
> > -	    {
> > -	      as_bad (_("must specify extensions to add before specifying "
> > -			"those to remove"));
> > -	      return false;
> > -	    }
> > +	  adding_value = 1;
> >  	}
> > -
> > -      if (optlen == 0)
> > +      else if (optlen == 0)
> >  	{
> >  	  as_bad (_("missing architectural extension"));
> >  	  return 0;
> >  	}
> 
> 
> This "if" -> "else if" transformation looks not only unrelated to me, but
> actually slightly wrong: The message now won't trigger anymore when there
> just "+no" was specified.

That's true, I'll fix that, thanks.

Tamar

> 
> Jan

Re: [PATCH][Binutils]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions

[Jan Beulich]

On 02.02.2023 13:13, Tamar Christina wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Thursday, February 2, 2023 11:30 AM
>> To: Tamar Christina <Tamar.Christina@arm.com>
>> Cc: nd <nd@arm.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>;
>> Marcus Shawcroft <Marcus.Shawcroft@arm.com>; binutils@sourceware.org
>> Subject: Re: [PATCH][Binutils]AArch64 gas: relax ordering constriants on
>> enabling and disabling feature extensions
>>
>> On 01.02.2023 20:25, Tamar Christina via Binutils wrote:
>>> At the beginning of the port it was decided that enabling features
>>> should always come before disabling features. i.e. +foo should always be
>> before any +nofoo.
>>>
>>> For years now this has been relaxed in GCC but binutils has remained
>>> rather strict.  This removes the restriction from gas as well giving
>>> users less friction.
>>
>> Hmm, specifying negative before positive settings may mean the negative
>> ones don't take effect at all (because of feature dependencies). While the
>> same of course is true the other way around as well, silently accepting
>> supposedly disabled insns is imo quite a bit more risky than complaining
>> about supposedly enabled ones: The programmer may unknowingly produce
>> a bad binary. This is even more so that the dependencies can't be considered
>> set in stone - bugs may be found which require adjustments to them.
> 
> Perhaps, but this change was to address some user feedback in which determining
> the order means they have to parse what definition for an -mcpu value the compiler
> has and understand the options and re-order them to add their own extensions on top.
> 
> People do this because binutils doesn't understand -mcpu.
> 
> The compiler already doesn't care about the order,  even though for a compiler the no
> Makes more sense as it turns off code generation for a particular instruction,
> particular auto-vec and intrinsics.  For an assembler, your assembly file doesn't magically
> include instructions for things you want to disable, you have to actively do so.

Well, an assembler file can grow over time, and it may contain multiple variants
of code targeting hardware with different capabilities. There may be many
instances of directives controlling the enabled/disabled extensions. Someone
adding new code there may not pay close enough attention to feature prereqs and
mix up insn flavors (like the SIMD vs SVE variants of the I8MM example I gave),
resulting in wrong code to be produced instead of a build time error.

> Also today we
> only have this limit on instructions, registers don't get gated like this.  Those already ignore
> all extension flags. So I think the risk is much lower in reality.
> 
> But.. If it's really a concern you have I can make it a warning instead of a hard error.

Would that help the people who have asked for the change you're proposing here?
I guess if it at all then only if additionally there is a command line option
to silence that warning?

Jan

Re: [PATCH]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions

[Richard Sandiford]

Jan Beulich via Binutils <binutils@sourceware.org> writes:
> On 01.02.2023 20:25, Tamar Christina via Binutils wrote:
>> At the beginning of the port it was decided that enabling features should always
>> come before disabling features. i.e. +foo should always be before any +nofoo.
>> 
>> For years now this has been relaxed in GCC but binutils has remained rather
>> strict.  This removes the restriction from gas as well giving users less
>> friction.
>
> Hmm, specifying negative before positive settings may mean the negative ones
> don't take effect at all (because of feature dependencies). While the same
> of course is true the other way around as well, silently accepting
> supposedly disabled insns is imo quite a bit more risky than complaining
> about supposedly enabled ones: The programmer may unknowingly produce a bad
> binary. This is even more so that the dependencies can't be considered set
> in stone - bugs may be found which require adjustments to them.

Like you say, if we allow positive and negative options to be specified
at the same time, there's no structural way of avoiding the risk that
one option will fully override the other.  But that's true of many other
aspects of command-line option handling too, especially in GCC.

I don't think there's any push to prevent positive and negative options
from being used together in all circumstances.  It's just a question
of degree.  But when deciding that degree, there is (IMO) no good reason
for GCC to be more lenient than GAS, or GAS to be more strict than GCC.
Whatever risks there are are broadly the same for both.  For example,
any mistake that a programmer can make directly in assembly, they can
also make in inline asm.  And as things stand, GCC will accept
negative-before-positive features and emit a conforming .arch, which
already allows inline asm programmers to fall into whatever traps the
patch would open up for direct use of GAS.  I haven't heard of any
instances of that causing problems in practice.

There's also the question of whether people writing directly in assembly
need to be protected more than people writing in intrinsics.  Personally
I think it's the opposite: the assembler is the lowest-level coding tool
we provide, and it should trust the user as much as possible.

> There actually is an example of something which may want adjusting: Both
> F32MM and F64MM take SVE as prereq. While SVE can't easily be a prereq to
> I8MM (because there are also SIMD insn forms), it's unclear why SIMD
> - itself being a prereq to SVE - isn't a prereq to I8MM.

Good spot!  That looks like a bug.  In the corresponding GCC logic,
+i8mm does imply +simd, which like you say seems like the correct
behaviour.

Thanks,
Richard

Re: [PATCH]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions

[Jan Beulich]

On 02.02.2023 14:21, Richard Sandiford wrote:
> Jan Beulich via Binutils <binutils@sourceware.org> writes:
>> On 01.02.2023 20:25, Tamar Christina via Binutils wrote:
>>> At the beginning of the port it was decided that enabling features should always
>>> come before disabling features. i.e. +foo should always be before any +nofoo.
>>>
>>> For years now this has been relaxed in GCC but binutils has remained rather
>>> strict.  This removes the restriction from gas as well giving users less
>>> friction.
>>
>> Hmm, specifying negative before positive settings may mean the negative ones
>> don't take effect at all (because of feature dependencies). While the same
>> of course is true the other way around as well, silently accepting
>> supposedly disabled insns is imo quite a bit more risky than complaining
>> about supposedly enabled ones: The programmer may unknowingly produce a bad
>> binary. This is even more so that the dependencies can't be considered set
>> in stone - bugs may be found which require adjustments to them.
> 
> Like you say, if we allow positive and negative options to be specified
> at the same time, there's no structural way of avoiding the risk that
> one option will fully override the other.  But that's true of many other
> aspects of command-line option handling too, especially in GCC.
> 
> I don't think there's any push to prevent positive and negative options
> from being used together in all circumstances.  It's just a question
> of degree.  But when deciding that degree, there is (IMO) no good reason
> for GCC to be more lenient than GAS, or GAS to be more strict than GCC.
> Whatever risks there are are broadly the same for both.  For example,
> any mistake that a programmer can make directly in assembly, they can
> also make in inline asm.  And as things stand, GCC will accept
> negative-before-positive features and emit a conforming .arch, which
> already allows inline asm programmers to fall into whatever traps the
> patch would open up for direct use of GAS.  I haven't heard of any
> instances of that causing problems in practice.
> 
> There's also the question of whether people writing directly in assembly
> need to be protected more than people writing in intrinsics.  Personally
> I think it's the opposite: the assembler is the lowest-level coding tool
> we provide, and it should trust the user as much as possible.

I don't agree here (if anything, I'd expect the compiler to be more strict),
but I realize that via a sequence of .arch_extension directives the same
situation could arise. Hence I guess there's no real reason to not also be
more relaxed in the parsing here.

>> There actually is an example of something which may want adjusting: Both
>> F32MM and F64MM take SVE as prereq. While SVE can't easily be a prereq to
>> I8MM (because there are also SIMD insn forms), it's unclear why SIMD
>> - itself being a prereq to SVE - isn't a prereq to I8MM.
> 
> Good spot!  That looks like a bug.  In the corresponding GCC logic,
> +i8mm does imply +simd, which like you say seems like the correct
> behaviour.

I'll make a patch then.

Jan