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 v2 1/3] [ARC] Add NPS400 instruction subclass and flag


Hi Graham,

  Just to check - this is version2 of patch 1 of *4* not 1 of *3*, right ?


  A couple of points:

  * Please could you update gas/config/tc-arc.c:md_show_usage() to include 
    the new command line option.  (Several other options are also missing
    from that function.  If you could include them, that would be swell).

  * Please could you update gas/doc/c-arc.texi to include the new option.


> +  if (!(arc_features & ARC_NPS400)
> +      && is_nps400_p (sc))
> +    return FALSE;

  * I know that you are copying the other code fragments in this function,
    but I find this test to completely backwards.  Much better, IMHO, would
    be:

       if (is_nps400_p (sc) && ! (arc_features & ARC_NPS400))
         return FALSE;

   I see no need to break the tests up over two lines, and testing the subclass
   before testing the feature enablement just makes more sense to me.

   You do not have to make this change if you do not want to.  But if you
   agree with me and want to change the other tests in this function too, then
   that would be appreciated.

Cheers
  Nick


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