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

[Jan Beulich <jbeulich@suse.com>]

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