Message ID | 20210805025312.15720-6-zhiwei_liu@c-sky.com |
---|---|
State | New |
Headers | show |
Series | Support UXL field in mstatus | expand |
On 8/4/21 4:53 PM, LIU Zhiwei wrote: > static bool trans_srli(DisasContext *ctx, arg_srli *a) > { > + if (ctx->uxl32) { > + return trans_srliw(ctx, a); > + } > return gen_shifti(ctx, a, tcg_gen_shr_tl); > } First, trans_srliw begins with REQUIRE_64BIT, which *should* fail when RV32 is in effect. This means there's a missing change to is_32bit(). Second, the current decode for srli allows 7 bits of shift, while srilw only allows 5. As a consequence, gen_shifti contains a check for TARGET_LONG_BITS and trans_srliw does not contain a check at all. We need to diagnose an out-of-range shift for RV32 somewhere. I recommend extending the gen_shift* family of helpers. static bool gen_shifti_imm(DisasContext *ctx, arg_shift *a, int width, void (*func)(TCGv, TCGv, target_long)) { TCGv dest, src1; if (a->shamt >= width) { return false; } dest = gpr_dst(ctx, a->rd); src1 = gpr_src(ctx, a->rs1); func(dest, src1, a->shamt); return true; } static bool gen_shifti(DisasContext *ctx, arg_shift *a, int width, void (*func)(TCGv, TCGv, TCGv)) {...} static void gen_srliw(TCGv dest, TCGv src1, target_long shamt) { tcg_gen_extract_tl(dest, src1, shamt, 32 - shamt); tcg_gen_ext32s_tl(dest, dest); } static bool trans_srliw(DisasContext *ctx, arg_shift *a) { REQUIRE_64BIT(ctx); return gen_shifti_imm(ctx, a, 32, gen_srliw); } static bool trans_srli(DisasContext *ctx, arg_shift *a) { int xlen = is_32bit(ctx) ? 32 : 64; return gen_shifti_imm(ctx, a, xlen, xlen == TARGET_LONG_BITS ? tcg_gen_shri_tl : gen_srliw); } etc. Perhaps that is_32bit() check above could be consolidated into some macro/inline. r~
On 2021/8/6 上午6:17, Richard Henderson wrote: > On 8/4/21 4:53 PM, LIU Zhiwei wrote: >> static bool trans_srli(DisasContext *ctx, arg_srli *a) >> { >> + if (ctx->uxl32) { >> + return trans_srliw(ctx, a); >> + } >> return gen_shifti(ctx, a, tcg_gen_shr_tl); >> } > > First, trans_srliw begins with REQUIRE_64BIT, which *should* fail when > RV32 is in effect. This means there's a missing change to is_32bit(). As I have replied in another patch, ctx->uxl32 already indicats 64 bit CPU. Anyway, I will think more about how to merge is_32bit() and uxl32 in next patch set. > > Second, the current decode for srli allows 7 bits of shift, while > srilw only allows 5. As a consequence, gen_shifti contains a check > for TARGET_LONG_BITS and trans_srliw does not contain a check at all. > We need to diagnose an out-of-range shift for RV32 somewhere. > Yes, it's not proper directly use *w here. Fix it in next patch set. Zhiwei > I recommend extending the gen_shift* family of helpers. > > static bool gen_shifti_imm(DisasContext *ctx, arg_shift *a, int width, > void (*func)(TCGv, TCGv, target_long)) > { > TCGv dest, src1; > > if (a->shamt >= width) { > return false; > } > dest = gpr_dst(ctx, a->rd); > src1 = gpr_src(ctx, a->rs1); > func(dest, src1, a->shamt); > return true; > } > > static bool gen_shifti(DisasContext *ctx, arg_shift *a, int width, > void (*func)(TCGv, TCGv, TCGv)) > {...} > > static void gen_srliw(TCGv dest, TCGv src1, target_long shamt) > { > tcg_gen_extract_tl(dest, src1, shamt, 32 - shamt); > tcg_gen_ext32s_tl(dest, dest); > } > > static bool trans_srliw(DisasContext *ctx, arg_shift *a) > { > REQUIRE_64BIT(ctx); > return gen_shifti_imm(ctx, a, 32, gen_srliw); > } > > static bool trans_srli(DisasContext *ctx, arg_shift *a) > { > int xlen = is_32bit(ctx) ? 32 : 64; > return gen_shifti_imm(ctx, a, xlen, > xlen == TARGET_LONG_BITS > ? tcg_gen_shri_tl : gen_srliw); > } > > etc. Perhaps that is_32bit() check above could be consolidated into > some macro/inline. > > > r~
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index 6201c07795..698a28731e 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -303,11 +303,17 @@ static bool trans_slli(DisasContext *ctx, arg_slli *a) static bool trans_srli(DisasContext *ctx, arg_srli *a) { + if (ctx->uxl32) { + return trans_srliw(ctx, a); + } return gen_shifti(ctx, a, tcg_gen_shr_tl); } static bool trans_srai(DisasContext *ctx, arg_srai *a) { + if (ctx->uxl32) { + return trans_sraiw(ctx, a); + } return gen_shifti(ctx, a, tcg_gen_sar_tl); } @@ -343,11 +349,17 @@ static bool trans_xor(DisasContext *ctx, arg_xor *a) static bool trans_srl(DisasContext *ctx, arg_srl *a) { + if (ctx->uxl32) { + return trans_srlw(ctx, a); + } return gen_shift(ctx, a, &tcg_gen_shr_tl); } static bool trans_sra(DisasContext *ctx, arg_sra *a) { + if (ctx->uxl32) { + return trans_sraw(ctx, a); + } return gen_shift(ctx, a, &tcg_gen_sar_tl); }
Reuse 32-bit right shift instructions. Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com> --- target/riscv/insn_trans/trans_rvi.c.inc | 12 ++++++++++++ 1 file changed, 12 insertions(+)