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

[Tamar Christina <Tamar.Christina@arm.com>]

> -----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