On 2026-07-17 10:06, Molly Chen wrote:
> Signed-off-by: Molly Chen <[email protected]>
> ---
> target/riscv/helper.h | 41 ++
> target/riscv/insn32.decode | 63 +++
> target/riscv/tcg/insn_trans/trans_rvp.c.inc | 547 ++++++++++++++++++++
> target/riscv/tcg/psimd_helper.c | 114 ++++
> target/riscv/tcg/translate.c | 4 +
> 5 files changed, 769 insertions(+)
> create mode 100644 target/riscv/tcg/insn_trans/trans_rvp.c.inc
...
> diff --git a/target/riscv/tcg/insn_trans/trans_rvp.c.inc
> b/target/riscv/tcg/insn_trans/trans_rvp.c.inc
> new file mode 100644
> index 00000000000..056ccfb486e
> --- /dev/null
> +++ b/target/riscv/tcg/insn_trans/trans_rvp.c.inc
...
> +#if defined(TARGET_RISCV32)
> +#define GEN_SIMD_TRANS_REG_PAIR_LANE_SCALAR_OP(INSN, HELPER, VXSAT) \
> +static bool trans_##INSN(DisasContext *ctx, arg_##INSN * a) \
> +{ \
> + REQUIRE_32BIT(ctx); \
> + REQUIRE_RVP(ctx); \
> + if (VXSAT && !prepare_rvp_vxsat(ctx)) { \
> + return false; \
> + } \
> + TCGv src1_0 = get_gpr(ctx, (a->rs1) * 2, EXT_NONE); \
> + TCGv dest_0 = dest_gpr(ctx, (a->rd) * 2); \
> + TCGv src1_1 = get_gpr(ctx, (a->rs1) * 2 + 1, EXT_NONE); \
> + TCGv dest_1 = dest_gpr(ctx, (a->rd) * 2 + 1); \
> + TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE); \
> + gen_helper_##HELPER(dest_0, tcg_env, src1_0, src2); \
> + gen_helper_##HELPER(dest_1, tcg_env, src1_1, src2); \
> + gen_set_gpr(ctx, (a->rd) * 2, dest_0); \
> + gen_set_gpr(ctx, (a->rd) * 2 + 1, dest_1); \
> + return true; \
> +}
There is a read-after-write hazard on src2 when the destination pair
overlaps rs2.
get_gpr(*, EXT_NONE) returns cpu_gpr[n] itself (EXT_NONE takes no
snapshot). And dest_gpr(ctx, rd) also returns cpu_gpr[rd] itself.
So if rd*2 == a->rs2 (or rd*2 + 1 == a->rs2), dest_0/dest_1 and src2
are the same TCG global, and the first gen_helper call op writes it
before the second call op reads it as its source.
I suggest that we could fix it by snapshot rs2:
TCGv dest_1 = dest_gpr(ctx, (a->rd) * 2 + 1); \
- TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE); \
+ TCGv src2 = tcg_temp_new(); \
+ tcg_gen_mov_tl(src2, get_gpr(ctx, a->rs2, EXT_NONE)); \
gen_helper_##HELPER(dest_0, tcg_env, src1_0, src2); \
> +#if defined(TARGET_RISCV32)
> +#define GEN_SIMD_TRANS_PN_OP(NAME, SRC2, VXSAT) \
> +static bool trans_##NAME(DisasContext *ctx, arg_##NAME * a) \
> +{ \
> + REQUIRE_32BIT(ctx); \
> + REQUIRE_RVP(ctx); \
> + if (VXSAT && !prepare_rvp_vxsat(ctx)) { \
> + return false; \
> + } \
> + TCGv_i64 s1 = tcg_temp_new_i64(); \
> + if (a->rs1 == 0) { \
> + tcg_gen_mov_i64(s1, 0); \
I think that we should use tcg_gen_movi_i64 here.
rnax
> + } else { \
> + get_pair_regs(ctx, s1, a->rs1 * 2); \
> + } \
> + TCGv src2 = SRC2; \
> + TCGv dest = dest_gpr(ctx, a->rd); \
> + gen_helper_##NAME(dest, tcg_env, s1, src2); \
> + gen_set_gpr(ctx, a->rd, dest); \
> + return true; \
> +}
