Re: [PATCH v6 4/6] target/riscv: Add support for PC-relative translation

[liweiwei]

On 2023/4/4 21:56, Richard Henderson wrote:

> On
>       4/3/23 19:06, Weiwei Li wrote:
>       
> >   static bool trans_auipc(DisasContext
> >         *ctx, arg_auipc *a)
> >         
> >           {
> >         
> >         -    gen_set_gpri(ctx, a->rd, a->imm +
> >         ctx->base.pc_next);
> >         
> >         +    TCGv target_pc = dest_gpr(ctx, a->rd);
> >         
> >         +    gen_pc_plus_diff(target_pc, ctx, a->imm +
> >         ctx->base.pc_next);
> >         
> >         +    gen_set_gpr(ctx, a->rd, target_pc);
> >         
> >               return true;
> >         
> >           }
> >         
> >       
>       
>       This is not how I expect a function called "pc plus diff" to work.
>       
>

Yeah, it's different from the similar function in ARM.

However, it's more in line with the original RISC-V logic.

Maybe we can change a name for the function, such as gen_pc_relative_address().

> It
>       should be simpler:
>       
>       
>       
>           TCGv rd = dest_gpr(ctx, a->rd);
>       
>       
>           gen_pc_plus_diff(ctx, rd, a->imm);
>       
>           gen_set_gpr(ctx, a->rd, rd);
>       
>       
>       All of the manipulation of cpu_pc, pc_save, and pc_next are all
>       hidden inside the function.  All that "add upper immediate to pc"
>       should do is supply the immediate.
>       
>

If we want to hide all of them in gen_pc_plus_diff,  then we need calculate the diff for pc_succ_insn or introduce a new API for it, since we need get the successor pc in many instructions.

And the logic for gen_goto_tb or gen_set_pc_imm also need update.

Regards,

Weiwei Li

>       
>       
>       r~
>       
>