Re: [Qemu-devel] [PATCH v3 08/21] RISC-V TCG Code Generation

[Michael Clark]

On Fri, Jan 12, 2018 at 7:15 AM, Michael Clark <address@hidden> wrote:

>
>
> On Fri, Jan 12, 2018 at 4:47 AM, Richard Henderson <
> address@hidden> wrote:
>
>> On 01/10/2018 06:21 PM, Michael Clark wrote:
>> > TCG code generation for the RV32IMAFDC and RV64IMAFDC. The QEMU
>> > RISC-V code generator has complete coverage for the Base ISA v2.2,
>> > Privileged ISA v1.9.1 and Privileged ISA v1.10:
>> >
>> > - RISC-V Instruction Set Manual Volume I: User-Level ISA Version 2.2
>> > - RISC-V Instruction Set Manual Volume II: Privileged ISA Version 1.9.1
>> > - RISC-V Instruction Set Manual Volume II: Privileged ISA Version 1.10
>> >
>> > Signed-off-by: Michael Clark <address@hidden>
>> > ---
>> >  target/riscv/instmap.h   |  377 +++++++++
>> >  target/riscv/translate.c | 1982 ++++++++++++++++++++++++++++++
>> ++++++++++++++++
>> >  2 files changed, 2359 insertions(+)
>> >  create mode 100644 target/riscv/instmap.h
>> >  create mode 100644 target/riscv/translate.c
>>
>> While there is forward progress within translate.c, it would seem that
>> quite a
>> lot of my v1 comments are not yet addressed.
>>
>> Also,
>>
>> > +        if (!riscv_has_ext(env, RVC)) {
>>
>> You may not access env in this way.  Any code generation differences must
>> be
>> computable from either unchanging values on the cpu or tb->flags.  MISA
>> is not
>> unchanging -- you allow runtime modification.
>>
>> Again, the partial patch I saw from Stefan would fix this.
>
>
> We'll definately address this in the next spin...
>
> Could you pleease forward me the patch or send me a pointer to where i
> might find it?
>

Nevermind. Stefan has raised an issue with pointers to the commits in an
outstanding PR. It likely has conflicts due to all of the recent changes so
i'll forward port it for the next spin.

Thanks again for you review. I'm working through the patch series
numerically which is why I had not got up to your FPU comments. I'm adding
Reviwed-by tags to the next series. I dropped them from the disasembler and
Maintainers in v2/v3 because we had made some changes.

It seems like we might get through most if not all of the review comments
by sometime in February...