Re: [Qemu-devel] [sw-dev] [RFC] RISC-V Decoder generator

[Richard Henderson]

On 10/22/2017 03:22 PM, Bastian Koppelmann wrote:
> Hi Richard,
> 
> On 10/21/2017 08:44 AM, Richard W.M. Jones wrote:
>> On Fri, Oct 20, 2017 at 03:46:54PM +0200, Bastian Koppelmann wrote:
>>> I asked you for feedback some while ago regarding a modular RISC-V QEMU
>>> target (see discussion [1]). I tried getting it to work with the good
>>> old C preprocessor and quickly realized that it is too limiting. Instead
>>> I created a data-driven decoder generator written in python (see the
>>> code on github [2]) using YAML as a description language.
>>>
>>> I'd like to get some feedback whether this is acceptable to be
>>> upstreamed to QEMU or if you have any suggestions for improvements.
>>> Right now only RV32I instruction are implemented in this scheme.
>>
>> My suggestion would be to reimplement (part of) the s390x decoder
>> using this scheme.  That would give us a direct comparison of how your
>> scheme is better or worse than the existing macros.
> 
> Yeah that would be a great test. However I'm not sure if it's worth the
> effort. AFAIK s390x will not be extended with new instructions, so there
> is no need for a new scheme unless it helps making the code better
> maintainable. But that's up to the s390x maintainers.

s390x has been getting regular updates and new instructions.

However, s390x is much more regular in its opcode placement wrt the various
insn formats, so it's slightly easier to decode than riscv.

In addition, the s390x translator has more sophisticated handling of operands
and writeback than you have for riscv at present.  That is, where we indicate
that particular register or memory arguments should be loaded into TCG
temporaries; storing the result; computing the PSW flags; etc.  This is because
there are so many CISC instruction variants where arguments come from whereever.

So, really, I think it would be a large amount of work to extend this riscv
translator to handle s390x, and would largely be a waste.

It would perhaps more interesting to apply it to other risc targets.  But that
is surely out of scope for one patch set.


r~