[PATCH] aarch64: allow adding/removing just feature flags via .arch_extension

[Marcus Shawcroft]

Sorry, resending, previous attempt bounced by the list...

On 4 Nov 2014, at 07:47, Jan Beulich <JBeulich@suse.com> wrote:

>>>> On 03.11.14 at 18:44, <marcus.shawcroft@gmail.com> wrote:
>> On 24 October 2014 13:40, Jan Beulich <JBeulich@suse.com> wrote:
>>
>>> static int
>>> -aarch64_parse_features (char *str, const aarch64_feature_set **opt_p)
>>> +aarch64_parse_features (char *str, const aarch64_feature_set **opt_p,
>>> +                       char *ext)
>>
>> The new parameter here is being used as a boolean flag, represent it
>> with a bfd_boolean rather than a char *.
>
> While I was expecting a comment to that effect, I had the slight hope
> to get things accepted as is: This being an internal helper function,
> using the parameter as local variable at the same time seems more
> efficient to me. But if you insist, I'll have to convert it back.


The interface is confusing in the current form, I'd prefer it was 
written in the 'obvious' manner.

>>> @@ -7267,16 +7270,18 @@ aarch64_parse_features (char *str, const
>>>   while (str != NULL && *str != 0)
>>>     {
>>>       const struct aarch64_option_cpu_value_table *opt;
>>> -      char *ext;
>>>       int optlen;
>>>
>>> -      if (*str != '+')
>>> +      if (!ext)
>>>        {
>>> -         as_bad (_("invalid architectural extension"));
>>> -         return 0;
>>> -       }
>>> +         if (*str != '+')
>>> +           {
>>> +             as_bad (_("invalid architectural extension"));
>>> +             return 0;
>>> +           }
>>>
>>> -      str++;
>>> +         str++;
>>> +       }
>>>       ext = strchr (str, '+');
>>
>> The logic here is odd. I don;t think we want this in the case when ext
>> != NULL.  The effect is that:
>>
>> .arch_extension +crc
>>
>> ...will give a "missing extension" rather than an "unknown extension"
>> error message.
>
> Which is quite right: There is an extension name missing ahead of the
> + character. The other error message would be right too (on the
> grounds that no extension name starts with +).
>
> Jan
>

 From c-arm.texi:

.arch_extension NAME

there is no support for multiple + separated features.

Cheers
/Marcus