This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] aarch64: allow adding/removing just feature flags via .arch_extension
- From: Marcus Shawcroft <marcus dot shawcroft at arm dot com>
- To: Jan Beulich <JBeulich at suse dot com>
- Cc: Marcus Shawcroft <marcus dot shawcroft at gmail dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 04 Nov 2014 10:01:34 +0000
- Subject: Re: [PATCH] aarch64: allow adding/removing just feature flags via .arch_extension
- Authentication-results: sourceware.org; auth=none
- References: <544A64EC0200007800041F06 at mail dot emea dot novell dot com> <CAFqB+Pzm9=VAvC25LAMgP8zicU4kmq6rvWEdEdtOZeCnam5_kQ at mail dot gmail dot com> <5458928D0200007800044A3F at mail dot emea dot novell dot com>
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