Re: [PATCH] [ARC] Add new ARC EM opcodes.

[Nick Clifton <nickc at redhat dot com>]

Hi Claudiu,

> Ping. The patch is strait forward, if no one complains, I will push it in the next days.

No, don't do that.  You currently only have write-after-approval privileges so you must
wait for someone to review the patch.  Even if you have to prod them several times first...

The only exception is if the patch can be considered to be "obvious", in which case you 
can check it in without prior approval, but you must still post the patch to the list,
and tell people that you are committing an obvious fix.  The exact definition of obvious
in this context is a bit nebulous, but I consider it to mean not "legally significant"[1],
not adding a new feature, and one to which any seasoned programmer would say "oh yes,
that is obvious".

[1] http://www.gnu.org/prep/maintain/maintain.html#Legally-Significant


Anyway, on to the patch review...

>> +++ b/include/opcode/arc.h
>> @@ -66,7 +66,8 @@ typedef enum
>>       SHFT1,
>>       SHFT2,
>>       SWAP,
>> -    SP
>> +    SP,
>> +    QUARKSE
>>     } insn_subclass_t;

This enum was alpha- sorted before this change.  Is there any particular reason for 
removing that property ?


>> +/* QuarkSE specific instructions.  */
>> +{"dsp_fp_div", 0x382A0000, 0xF8FF0000, ARC_OPCODE_ARCv2EM, FLOAT,
>> QUARKSE, { RA, RB, RC }, { C_F }},

Given that there is so much repetition in these new entries, wouldn't it make sense
to use a couple of macros to automate most of the fields ?  There is less chance for
typographical errors that way too.

Cheers
  Nick