[PATCH] arm: ignore inapplicable .arch=no...

[Richard Earnshaw]

On 30/04/2025 15:20, Jan Beulich wrote:
> On 30.04.2025 16:10, Richard Earnshaw (lists) wrote:
>> On 30/04/2025 14:34, Jan Beulich wrote:
>>> Unlike for command line options, where a base architecture needs to be
>>> provided explicitly, the .arch directive doesn't have such a
>>> requirement. Therefore it is odd that disabling of an inapplicable
>>> extension isn't silently ignored; claiming "not allowed for the current
>>> base architecture" is at best misleading. Simply skip the error path,
>>> leveraging that in such a case ARM_CLEAR_FEATURE() is (supposed to be)
>>> a no-op.
>>>
>>> --- a/gas/config/tc-arm.c
>>> +++ b/gas/config/tc-arm.c
>>> @@ -32792,7 +32792,7 @@ s_arm_arch_extension (int ignored ATTRIB
>>>   	      break;
>>>   	  }
>>>   
>>> -	if (i == nb_allowed_archs)
>>> +	if (i == nb_allowed_archs && adding_value)
>>>   	  {
>>>   	    as_bad (_("architectural extension `%s' is not allowed for the "
>>>   		      "current base architecture"), name);
>>
>>
>> Sorry, I don't follow the logic you have in mind.  Trying to enable, for example, MVE on an a-profile architecture is completely meaningless and should be diagnosed as an error in the source file.
> 
> Of course; the change here is about disabling of extensions.
> 
>> Can you give an example of what you're trying to permit here?
> 
> The example I noticed this was with ".arch nocrypto" under -march=all. I
> simply don't think this needs diagnosing. And if there was a diagnostic,
> I think it shouldn't say "crypto", kind of suggesting that's being turned
> on, not off.
> 
> Jan

Sorry for dropping the ball on this.

I agree the message is misleading.  But you've also changed the 
subsequent logic here as well, and I'm not sure if that would be 100% safe.

Perhaps a compromise would be a tsktsk along the lines:

	disabling feature `%s' has no effect on the current base architecture.

so:

   if (i == nb_allowed_archs)
     {
       if (adding_value)
         as_bad (_("architectural extension `%s' is not allowed for the "
                   "current base architecture"), name);
       else
         as_tsktsk (_"(disabling feature `%s' has no effect on the "
                       "current base architecture"), name);
       break;
     }

R.