qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 23/24] target/riscv: Tidy trans_rvh.c.inc


From: Alistair Francis
Subject: Re: [PATCH v5 23/24] target/riscv: Tidy trans_rvh.c.inc
Date: Mon, 30 Aug 2021 14:54:28 +1000

On Tue, Aug 24, 2021 at 6:07 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Exit early if check_access fails.
> Split out do_hlv, do_hsv, do_hlvx subroutines.
> Use dest_gpr, get_gpr in the new subroutines.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn32.decode              |   1 +
>  target/riscv/insn_trans/trans_rvh.c.inc | 266 +++++-------------------
>  2 files changed, 57 insertions(+), 210 deletions(-)
>
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index f09f8d5faf..2cd921d51c 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -42,6 +42,7 @@
>  &j    imm rd
>  &r    rd rs1 rs2
>  &r2   rd rs1
> +&r2_s rs1 rs2
>  &s    imm rs1 rs2
>  &u    imm rd
>  &shift     shamt rs1 rd
> diff --git a/target/riscv/insn_trans/trans_rvh.c.inc 
> b/target/riscv/insn_trans/trans_rvh.c.inc
> index 585eb1d87e..ecbf77ff9c 100644
> --- a/target/riscv/insn_trans/trans_rvh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvh.c.inc
> @@ -17,281 +17,139 @@
>   */
>
>  #ifndef CONFIG_USER_ONLY
> -static void check_access(DisasContext *ctx) {
> +static bool check_access(DisasContext *ctx)
> +{
>      if (!ctx->hlsx) {
>          if (ctx->virt_enabled) {
>              generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT);
>          } else {
>              generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
>          }
> +        return false;
>      }
> +    return true;
>  }
>  #endif
>
> +static bool do_hlv(DisasContext *ctx, arg_r2 *a, MemOp mop)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return false;
> +#else
> +    if (check_access(ctx)) {
> +        TCGv dest = dest_gpr(ctx, a->rd);
> +        TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
> +        int mem_idx = ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
> +        tcg_gen_qemu_ld_tl(dest, addr, mem_idx, mop);
> +        gen_set_gpr(ctx, a->rd, dest);
> +    }
> +    return true;
> +#endif
> +}
> +
>  static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, 
> MO_SB);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hlv(ctx, a, MO_SB);
>  }
>
>  static bool trans_hlv_h(DisasContext *ctx, arg_hlv_h *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, 
> MO_TESW);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hlv(ctx, a, MO_TESW);
>  }
>
>  static bool trans_hlv_w(DisasContext *ctx, arg_hlv_w *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, 
> MO_TESL);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hlv(ctx, a, MO_TESL);
>  }
>
>  static bool trans_hlv_bu(DisasContext *ctx, arg_hlv_bu *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, 
> MO_UB);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hlv(ctx, a, MO_UB);
>  }
>
>  static bool trans_hlv_hu(DisasContext *ctx, arg_hlv_hu *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> +    return do_hlv(ctx, a, MO_TEUW);
> +}
>
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, 
> MO_TEUW);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> -#else
> +static bool do_hsv(DisasContext *ctx, arg_r2_s *a, MemOp mop)
> +{
> +#ifdef CONFIG_USER_ONLY
>      return false;
> +#else
> +    if (check_access(ctx)) {
> +        TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
> +        TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
> +        int mem_idx = ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
> +        tcg_gen_qemu_st_tl(data, addr, mem_idx, mop);
> +    }
> +    return true;
>  #endif
>  }
>
>  static bool trans_hsv_b(DisasContext *ctx, arg_hsv_b *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv dat = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -    gen_get_gpr(ctx, dat, a->rs2);
> -
> -    tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | 
> TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_SB);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(dat);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hsv(ctx, a, MO_SB);
>  }
>
>  static bool trans_hsv_h(DisasContext *ctx, arg_hsv_h *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv dat = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -    gen_get_gpr(ctx, dat, a->rs2);
> -
> -    tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | 
> TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TESW);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(dat);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hsv(ctx, a, MO_TESW);
>  }
>
>  static bool trans_hsv_w(DisasContext *ctx, arg_hsv_w *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv dat = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -    gen_get_gpr(ctx, dat, a->rs2);
> -
> -    tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | 
> TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TESL);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(dat);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hsv(ctx, a, MO_TESL);
>  }
>
>  static bool trans_hlv_wu(DisasContext *ctx, arg_hlv_wu *a)
>  {
>      REQUIRE_64BIT(ctx);
>      REQUIRE_EXT(ctx, RVH);
> -
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, 
> MO_TEUL);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hlv(ctx, a, MO_TEUL);
>  }
>
>  static bool trans_hlv_d(DisasContext *ctx, arg_hlv_d *a)
>  {
>      REQUIRE_64BIT(ctx);
>      REQUIRE_EXT(ctx, RVH);
> -
> -#ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK, 
> MO_TEQ);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> -#else
> -    return false;
> -#endif
> +    return do_hlv(ctx, a, MO_TEQ);
>  }
>
>  static bool trans_hsv_d(DisasContext *ctx, arg_hsv_d *a)
>  {
>      REQUIRE_64BIT(ctx);
>      REQUIRE_EXT(ctx, RVH);
> +    return do_hsv(ctx, a, MO_TEQ);
> +}
>
>  #ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv dat = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -    gen_get_gpr(ctx, dat, a->rs2);
> -
> -    tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | 
> TB_FLAGS_PRIV_HYP_ACCESS_MASK, MO_TEQ);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(dat);
> +static bool do_hlvx(DisasContext *ctx, arg_r2 *a,
> +                    void (*func)(TCGv, TCGv_env, TCGv))
> +{
> +    if (check_access(ctx)) {
> +        TCGv dest = dest_gpr(ctx, a->rd);
> +        TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
> +        func(dest, cpu_env, addr);
> +        gen_set_gpr(ctx, a->rd, dest);
> +    }
>      return true;
> -#else
> -    return false;
> -#endif
>  }
> +#endif
>
>  static bool trans_hlvx_hu(DisasContext *ctx, arg_hlvx_hu *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
>  #ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    gen_helper_hyp_hlvx_hu(t1, cpu_env, t0);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> +    return do_hlvx(ctx, a, gen_helper_hyp_hlvx_hu);
>  #else
>      return false;
>  #endif
> @@ -301,19 +159,7 @@ static bool trans_hlvx_wu(DisasContext *ctx, arg_hlvx_wu 
> *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
>  #ifndef CONFIG_USER_ONLY
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    check_access(ctx);
> -
> -    gen_get_gpr(ctx, t0, a->rs1);
> -
> -    gen_helper_hyp_hlvx_wu(t1, cpu_env, t0);
> -    gen_set_gpr(ctx, a->rd, t1);
> -
> -    tcg_temp_free(t0);
> -    tcg_temp_free(t1);
> -    return true;
> +    return do_hlvx(ctx, a, gen_helper_hyp_hlvx_wu);
>  #else
>      return false;
>  #endif
> --
> 2.25.1
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]