This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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








Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]