Re: [RFC PATCH 02/13] target/riscv: Support UXL32 for branch instructions

[LIU Zhiwei]

On 2021/8/6 上午3:06, Richard Henderson wrote:
> On 8/4/21 4:53 PM, LIU Zhiwei wrote:
> > +static TCGv gpr_src_u(DisasContext *ctx, int reg_num)
> > +{
> > +    if (reg_num == 0) {
> > +        return ctx->zero;
> > +    }
> > +    if (ctx->uxl32) {
> > +        tcg_gen_ext32u_tl(cpu_gpr[reg_num], cpu_gpr[reg_num]);
> > +    }
> > +    return cpu_gpr[reg_num];
> > +}
> > +
> > +static TCGv gpr_src_s(DisasContext *ctx, int reg_num)
> > +{
> > +    if (reg_num == 0) {
> > +        return ctx->zero;
> > +    }
> > +    if (ctx->uxl32) {
> > +        tcg_gen_ext32s_tl(cpu_gpr[reg_num], cpu_gpr[reg_num]);
> > +    }
> > +    return cpu_gpr[reg_num];
> > +}
>
> This is bad: you cannot modify the source registers like this.

In my opinion, when uxl32, the only meaningful part is the low 32 bits, 
and it doesn't matter to modify the high parts.

> These incorrect modifications will be visible to the kernel on 
> transition back to S-mode.

When transition back to S-mode, I think the kernel will save the U-mode 
registers to memory.

Thanks,
Zhiwei

> > +static bool gen_branch_u(DisasContext *ctx, arg_b *a, TCGCond cond)
> > +{
> > +    TCGv src1 = gpr_src_u(ctx, a->rs1);
> > +    TCGv src2 = gpr_src_u(ctx, a->rs2);
> > +
> > +    return gen_branch_internal(ctx, a, cond, src1, src2);
> > +}
>
> This is unnecessary.  Unsigned comparisons work just fine with 
> sign-extended values.  It will be simpler to keep all values 
> sign-extended.
>
>
> r~