Re: [PATCH v2 05/14] target/riscv: Calculate address according to XLEN

[Richard Henderson]

On 11/10/21 8:04 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/insn_trans/trans_rvd.c.inc | 23 ++---------------------
>   target/riscv/insn_trans/trans_rvf.c.inc | 23 ++---------------------
>   target/riscv/insn_trans/trans_rvi.c.inc | 18 ++----------------
>   target/riscv/translate.c                | 13 +++++++++++++
>   4 files changed, 19 insertions(+), 58 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc 
> b/target/riscv/insn_trans/trans_rvd.c.inc
> index 64fb0046f7..29066a8ef3 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -20,19 +20,10 @@
>   
>   static bool trans_fld(DisasContext *ctx, arg_fld *a)
>   {
> -    TCGv addr;
> -
> +    TCGv addr = get_address(ctx, a->rs1, a->imm);
>       REQUIRE_FPU;
>       REQUIRE_EXT(ctx, RVD);

It would be better to leave the address calculation after the REQUIRE checks.

> +static TCGv get_address(DisasContext *ctx, int rs1, int imm)
> +{
> +    TCGv addr = temp_new(ctx);
> +    TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
> +
> +    tcg_gen_addi_tl(addr, src1, imm);
> +    addr = gen_pm_adjust_address(ctx, addr);
> +    if (get_xl(ctx) == MXL_RV32) {
> +        tcg_gen_ext32u_tl(addr, addr);
> +    }
> +    return addr;
> +}

I suspect the extend should come before the pointer mask and not after, but this is is a 
weakness in the current RVJ spec that it does not specify how the extension interacts with 
UXL.  (The reverse ordering would allow a 64-bit os to place a 32-bit application at a 
base address above 4gb, which could allow address separation without paging enabled.)

I do think we should merge gen_pm_adjust_address into this function so that we only create 
one new temporary.


r~