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

[Jan Beulich]

On 03.06.2025 17:20, Richard Earnshaw wrote:
> 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;
>      }

I'll do as you say, noting that I dislike as_tsktsk(), not the least because
of there not being a way to suppress it (without changing the source). Since
you suggested this alternative, I'll take it as implicit approval to commit
in that shape. Please shout (soon) in case that's not appropriate.

Jan