Re: [PATCH 03/15] arc: Opcode definitions table

[Cupertino Miranda]

Hi Richard,

Sorry to take so long to get through the changes after your review.
I am still going through the improving process and waiting for some 
internal company approval to publish the generator of the TCG 
instruction definitions, as we have discussed.

Nevertheless, there are some questions. I will address them near the 
proper places and through the different patches.

Once again, thank you very much for your reviews. ;-)

On 12/1/20 8:22 PM, Richard Henderson wrote:
> On 11/11/20 10:17 AM, cupertinomiranda@gmail.com wrote:
>> From: Claudiu Zissulescu <claziss@synopsys.com>
>>
>> Signed-off-by: Claudiu Zissulescu <claziss@synopsys.com>
>> ---
>>   target/arc/opcodes.def | 19976 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 19976 insertions(+)
>>   create mode 100644 target/arc/opcodes.def
> 
> OMG.  20k lines.
> 
> I assume this is gnu binutils opcodes/arc-tbl.h?
> 
> You are the contributor there, so a re-license is fine.  It would be good to
> document the upstream location and revision, against some future re-sync.
> 
> That said, this format is less than ideal:
> 
>> +/* abs<.f> b,c 00100bbb00101111FBBBCCCCCC001001.  */
>> +{ "abs", 0x202F0009, 0xF8FF003F, ARC_OPCODE_ARC600 | ARC_OPCODE_ARC700 | 
>> ARC_OPCODE_ARCv2EM | ARC_OPCODE_ARCv2HS, ARITH, NONE, { OPERAND_RB, 
>> OPERAND_RC }, { C_F }},
> 
> You've got the same information in two places
> (00100bbb00101111FBBBCCCCCC001001) vs (0x202F0009, 0xF8FF003F, OPERAND_*).
> Moreover, "abs" as a string is not especially useful, and means that you have
> to deal with strings in the translator instead of C symbols or enumerators.
> 
> It would be relatively easy to generate a decodetree file from this input,
> which would simplify the translator.
> 
> At a bare minimum strip the quotes and wrap in a macro so that you can (1)
> define an enumerator and (2) put the entries into an array indexed by the
> enumerator.
> 

As you know, we reused the code from binutils to implement the decoder.
In that sense, we kindly request to allow us to do it through binutils 
development flow later on. We will change the tables in binutils
and those changes will also be mirrored to QEMU.
Is this Ok ?