Re: [PATCH 08/17] target/riscv: Use gpr_{src, dst} for word shift operations

[Alistair Francis]

On Fri, Jul 9, 2021 at 2:35 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For trans_sllw, we can just use gen_shiftw.  The others use
> various tricks to reduce the tcg operation count.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/insn_trans/trans_rvi.c.inc | 82 ++++++++++---------------
>  1 file changed, 31 insertions(+), 51 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index a422dc9ef4..840187a4d6 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -352,24 +352,23 @@ static bool trans_slliw(DisasContext *ctx, arg_slliw *a)
>  static bool trans_srliw(DisasContext *ctx, arg_srliw *a)
>  {
>      REQUIRE_64BIT(ctx);
> -    TCGv t = tcg_temp_new();
> -    gen_get_gpr(t, a->rs1);
> -    tcg_gen_extract_tl(t, t, a->shamt, 32 - a->shamt);
> -    /* sign-extend for W instructions */
> -    tcg_gen_ext32s_tl(t, t);
> -    gen_set_gpr(a->rd, t);
> -    tcg_temp_free(t);
> +
> +    TCGv dest = gpr_dst(ctx, a->rd);
> +    TCGv src1 = gpr_src(ctx, a->rs1);
> +
> +    tcg_gen_extract_tl(dest, src1, a->shamt, 32 - a->shamt);
> +    tcg_gen_ext32s_tl(dest, dest);
>      return true;
>  }
>
>  static bool trans_sraiw(DisasContext *ctx, arg_sraiw *a)
>  {
>      REQUIRE_64BIT(ctx);
> -    TCGv t = tcg_temp_new();
> -    gen_get_gpr(t, a->rs1);
> -    tcg_gen_sextract_tl(t, t, a->shamt, 32 - a->shamt);
> -    gen_set_gpr(a->rd, t);
> -    tcg_temp_free(t);
> +
> +    TCGv dest = gpr_dst(ctx, a->rd);
> +    TCGv src1 = gpr_src(ctx, a->rs1);
> +
> +    tcg_gen_sextract_tl(dest, src1, a->shamt, 32 - a->shamt);
>      return true;
>  }
>
> @@ -388,64 +387,45 @@ static bool trans_subw(DisasContext *ctx, arg_subw *a)
>  static bool trans_sllw(DisasContext *ctx, arg_sllw *a)
>  {
>      REQUIRE_64BIT(ctx);
> -    TCGv source1 = tcg_temp_new();
> -    TCGv source2 = tcg_temp_new();
> -
> -    gen_get_gpr(source1, a->rs1);
> -    gen_get_gpr(source2, a->rs2);
> -
> -    tcg_gen_andi_tl(source2, source2, 0x1F);
> -    tcg_gen_shl_tl(source1, source1, source2);
> -
> -    tcg_gen_ext32s_tl(source1, source1);
> -    gen_set_gpr(a->rd, source1);
> -    tcg_temp_free(source1);
> -    tcg_temp_free(source2);
> -    return true;
> +    return gen_shiftw(ctx, a, tcg_gen_shl_tl);
>  }
>
>  static bool trans_srlw(DisasContext *ctx, arg_srlw *a)
>  {
>      REQUIRE_64BIT(ctx);
> -    TCGv source1 = tcg_temp_new();
> -    TCGv source2 = tcg_temp_new();
>
> -    gen_get_gpr(source1, a->rs1);
> -    gen_get_gpr(source2, a->rs2);
> +    TCGv dest = gpr_dst(ctx, a->rd);
> +    TCGv src1 = gpr_src(ctx, a->rs1);
> +    TCGv src2 = gpr_src(ctx, a->rs2);
> +    TCGv ext2 = tcg_temp_new();
>
> -    /* clear upper 32 */
> -    tcg_gen_ext32u_tl(source1, source1);
> -    tcg_gen_andi_tl(source2, source2, 0x1F);
> -    tcg_gen_shr_tl(source1, source1, source2);
> +    tcg_gen_andi_tl(ext2, src2, 31);
> +    tcg_gen_ext32u_tl(dest, src1);
> +    tcg_gen_shr_tl(dest, dest, ext2);
> +    tcg_gen_ext32s_tl(dest, dest);
>
> -    tcg_gen_ext32s_tl(source1, source1);
> -    gen_set_gpr(a->rd, source1);
> -    tcg_temp_free(source1);
> -    tcg_temp_free(source2);
> +    tcg_temp_free(ext2);
>      return true;
>  }
>
>  static bool trans_sraw(DisasContext *ctx, arg_sraw *a)
>  {
>      REQUIRE_64BIT(ctx);
> -    TCGv source1 = tcg_temp_new();
> -    TCGv source2 = tcg_temp_new();
>
> -    gen_get_gpr(source1, a->rs1);
> -    gen_get_gpr(source2, a->rs2);
> +    TCGv dest = gpr_dst(ctx, a->rd);
> +    TCGv src1 = gpr_src(ctx, a->rs1);
> +    TCGv src2 = gpr_src(ctx, a->rs2);
> +    TCGv ext2 = tcg_temp_new();
>
> +    tcg_gen_andi_tl(ext2, src2, 31);
>      /*
> -     * first, trick to get it to act like working on 32 bits (get rid of
> -     * upper 32, sign extend to fill space)
> +     * First, trick to get it to act like working on 32 bits
> +     * (get rid of upper 32, sign extend to fill space)
>       */
> -    tcg_gen_ext32s_tl(source1, source1);
> -    tcg_gen_andi_tl(source2, source2, 0x1F);
> -    tcg_gen_sar_tl(source1, source1, source2);
> -
> -    gen_set_gpr(a->rd, source1);
> -    tcg_temp_free(source1);
> -    tcg_temp_free(source2);
> +    tcg_gen_ext32s_tl(dest, src1);
> +    tcg_gen_sar_tl(dest, dest, ext2);
>
> +    tcg_temp_free(ext2);
>      return true;
>  }
>
> --
> 2.25.1
>
>