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

2021-01-15 Thread Richard Henderson
On 1/15/21 7:11 AM, Cupertino Miranda wrote:
> 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 ?

Yes, certainly.


r~


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


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

2021-01-15 Thread 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, cupertinomira...@gmail.com wrote:
>> From: Claudiu Zissulescu 
>>
>> Signed-off-by: Claudiu Zissulescu 
>> ---
>>   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 00100bbb0010FBBBCC001001.  */
>> +{ "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
> (00100bbb0010FBBBCC001001) 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 ?


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


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

2020-12-01 Thread Richard Henderson
On 11/11/20 10:17 AM, cupertinomira...@gmail.com wrote:
> From: Claudiu Zissulescu 
> 
> Signed-off-by: Claudiu Zissulescu 
> ---
>  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 00100bbb0010FBBBCC001001.  */
> +{ "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
(00100bbb0010FBBBCC001001) 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.


r~

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc