Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-14 Thread Emilio G. Cota
On Wed, Feb 14, 2018 at 13:13:09 -0800, Richard Henderson wrote:
> On 02/14/2018 11:52 AM, Emilio G. Cota wrote:
> > Should I send those patches to the list, or let Michael squash their 
> > changes?
> 
> That's up to you, I guess.  I don't mind if it goes in before or after merge.

OK, will send to the list once the merge is complete.

Thanks,

Emilio



Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-14 Thread Richard Henderson
On 02/14/2018 11:52 AM, Emilio G. Cota wrote:
> Should I send those patches to the list, or let Michael squash their changes?

That's up to you, I guess.  I don't mind if it goes in before or after merge.


r~




Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-14 Thread Emilio G. Cota
On Wed, Feb 14, 2018 at 11:14:48 -0800, Richard Henderson wrote:
> On 02/13/2018 04:10 PM, Emilio G. Cota wrote:
> > On Tue, Feb 13, 2018 at 14:10:20 -0800, Richard Henderson wrote:
> >> On 02/13/2018 01:55 PM, Emilio G. Cota wrote:
> >>> Are we planning to use BS_STOP in the future? I see it has no setters,
> >>> although we check for it in gen_intermediate_code:
> >>
> >> No, but the whole port should be converted to exec/translator.h, which 
> >> defines
> >> DisasJumpType.  Not something I'm going to require on initial submission 
> >> until
> >> we've gotten most of the other targets cleaned up.
> > 
> > I see. I've just done the conversion for v5:
> >   https://github.com/cota/qemu/commits/riscv-v5-trloop
> > 
> > Can you please take a look?
> 
> Looks ok.  Watch your formatting, e.g { } on the same line.

Thanks, checkpatch didn't complain about that one though.

Should I send those patches to the list, or let Michael squash their changes?

E.



Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-14 Thread Richard Henderson
On 02/13/2018 04:10 PM, Emilio G. Cota wrote:
> On Tue, Feb 13, 2018 at 14:10:20 -0800, Richard Henderson wrote:
>> On 02/13/2018 01:55 PM, Emilio G. Cota wrote:
>>> Are we planning to use BS_STOP in the future? I see it has no setters,
>>> although we check for it in gen_intermediate_code:
>>
>> No, but the whole port should be converted to exec/translator.h, which 
>> defines
>> DisasJumpType.  Not something I'm going to require on initial submission 
>> until
>> we've gotten most of the other targets cleaned up.
> 
> I see. I've just done the conversion for v5:
>   https://github.com/cota/qemu/commits/riscv-v5-trloop
> 
> Can you please take a look?

Looks ok.  Watch your formatting, e.g { } on the same line.


r~



Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-13 Thread Emilio G. Cota
On Tue, Feb 13, 2018 at 14:10:20 -0800, Richard Henderson wrote:
> On 02/13/2018 01:55 PM, Emilio G. Cota wrote:
> > Are we planning to use BS_STOP in the future? I see it has no setters,
> > although we check for it in gen_intermediate_code:
> 
> No, but the whole port should be converted to exec/translator.h, which defines
> DisasJumpType.  Not something I'm going to require on initial submission until
> we've gotten most of the other targets cleaned up.

I see. I've just done the conversion for v5:
  https://github.com/cota/qemu/commits/riscv-v5-trloop

Can you please take a look?

Thanks,

Emilio



Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-13 Thread Richard Henderson
On 02/13/2018 01:55 PM, Emilio G. Cota wrote:
> On Thu, Feb 08, 2018 at 14:28:34 +1300, 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
>>
>> Reviewed-by: Richard Henderson 
>> Signed-off-by: Michael Clark 
>> ---
> (snip)
>> +++ b/target/riscv/translate.c
> (snip)
>> +enum {
>> +BS_NONE = 0, /* When seen outside of translation while loop, 
>> indicates
>> + need to exit tb due to end of page. */
>> +BS_STOP = 1, /* Need to exit tb for syscall, sret, etc. */
> 
> Are we planning to use BS_STOP in the future? I see it has no setters,
> although we check for it in gen_intermediate_code:

No, but the whole port should be converted to exec/translator.h, which defines
DisasJumpType.  Not something I'm going to require on initial submission until
we've gotten most of the other targets cleaned up.


r~




Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-13 Thread Emilio G. Cota
On Thu, Feb 08, 2018 at 14:28:34 +1300, 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
> 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Michael Clark 
> ---
(snip)
> +++ b/target/riscv/translate.c
(snip)
> +/* Address comparion failure.  However, we still need to
> +   provide the memory barrier implied by AQ/RL.  */

s/comparion/comparison/

E.



Re: [Qemu-devel] [PATCH v5 09/23] RISC-V TCG Code Generation

2018-02-13 Thread Emilio G. Cota
On Thu, Feb 08, 2018 at 14:28:34 +1300, 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
> 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Michael Clark 
> ---
(snip)
> +++ b/target/riscv/translate.c
(snip)
> +enum {
> +BS_NONE = 0, /* When seen outside of translation while loop, 
> indicates
> + need to exit tb due to end of page. */
> +BS_STOP = 1, /* Need to exit tb for syscall, sret, etc. */

Are we planning to use BS_STOP in the future? I see it has no setters,
although we check for it in gen_intermediate_code:

(snip)
> +switch (ctx.bstate) {
> +case BS_STOP:
> +gen_goto_tb(&ctx, 0, ctx.pc);
> +break;
> +case BS_NONE: /* handle end of page - DO NOT CHAIN. See gen_goto_tb. */

Should we get rid of it?

Emilio