* [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64
@ 2025-01-24 7:33 Max Chou
2025-09-23 9:21 ` Max Chou
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Max Chou @ 2025-01-24 7:33 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Max Chou, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Richard Henderson,
Philippe Mathieu-Daudé
When XLEN is 32 and SEW is 64, the original implementation of
vslide1up.vx and vslide1down.vx helper functions fills the 32-bit value
of rs1 into the first element of the destination vector register (rd),
which is a 64-bit element.
This commit attempted to resolve the issue by extending the rs1 value
to 64 bits during the TCG translation phase to ensure that the helper
functions won't lost the higer 32 bits.
Signed-off-by: Max Chou <max.chou@sifive.com>
---
target/riscv/helper.h | 16 ++++----
target/riscv/insn_trans/trans_rvv.c.inc | 50 ++++++++++++++++++++++++-
target/riscv/vector_helper.c | 20 +++++-----
3 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 16ea240d26d..0578d153bdf 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1099,14 +1099,14 @@ DEF_HELPER_6(vslidedown_vx_b, void, ptr, ptr, tl, ptr, env, i32)
DEF_HELPER_6(vslidedown_vx_h, void, ptr, ptr, tl, ptr, env, i32)
DEF_HELPER_6(vslidedown_vx_w, void, ptr, ptr, tl, ptr, env, i32)
DEF_HELPER_6(vslidedown_vx_d, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, tl, ptr, env, i32)
-DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, i64, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, i64, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, i64, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, i64, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, i64, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, i64, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, i64, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, i64, ptr, env, i32)
DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32)
DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index b9883a5d323..775fe1baae7 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3391,7 +3391,6 @@ static bool slideup_check(DisasContext *s, arg_rmrr *a)
}
GEN_OPIVX_TRANS(vslideup_vx, slideup_check)
-GEN_OPIVX_TRANS(vslide1up_vx, slideup_check)
GEN_OPIVI_TRANS(vslideup_vi, IMM_ZX, vslideup_vx, slideup_check)
static bool slidedown_check(DisasContext *s, arg_rmrr *a)
@@ -3402,9 +3401,56 @@ static bool slidedown_check(DisasContext *s, arg_rmrr *a)
}
GEN_OPIVX_TRANS(vslidedown_vx, slidedown_check)
-GEN_OPIVX_TRANS(vslide1down_vx, slidedown_check)
GEN_OPIVI_TRANS(vslidedown_vi, IMM_ZX, vslidedown_vx, slidedown_check)
+typedef void gen_helper_vslide1_vx(TCGv_ptr, TCGv_ptr, TCGv_i64, TCGv_ptr,
+ TCGv_env, TCGv_i32);
+
+#define GEN_OPIVX_VSLIDE1_TRANS(NAME, CHECK) \
+static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \
+{ \
+ if (CHECK(s, a)) { \
+ static gen_helper_vslide1_vx * const fns[4] = { \
+ gen_helper_##NAME##_b, gen_helper_##NAME##_h, \
+ gen_helper_##NAME##_w, gen_helper_##NAME##_d, \
+ }; \
+ \
+ TCGv_ptr dest, src2, mask; \
+ TCGv_i64 src1; \
+ TCGv_i32 desc; \
+ uint32_t data = 0; \
+ \
+ dest = tcg_temp_new_ptr(); \
+ mask = tcg_temp_new_ptr(); \
+ src2 = tcg_temp_new_ptr(); \
+ src1 = tcg_temp_new_i64(); \
+ \
+ data = FIELD_DP32(data, VDATA, VM, a->vm); \
+ data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
+ data = FIELD_DP32(data, VDATA, VTA, s->vta); \
+ data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s); \
+ data = FIELD_DP32(data, VDATA, VMA, s->vma); \
+ desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlenb, \
+ s->cfg_ptr->vlenb, data)); \
+ \
+ tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd)); \
+ tcg_gen_addi_ptr(src2, tcg_env, vreg_ofs(s, a->rs2)); \
+ tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); \
+ tcg_gen_ext_tl_i64(src1, get_gpr(s, a->rs1, EXT_SIGN)); \
+ \
+ fns[s->sew](dest, mask, src1, src2, tcg_env, desc); \
+ \
+ tcg_gen_movi_tl(cpu_vstart, 0); \
+ finalize_rvv_inst(s); \
+ \
+ return true; \
+ } \
+ return false; \
+}
+
+GEN_OPIVX_VSLIDE1_TRANS(vslide1up_vx, slideup_check)
+GEN_OPIVX_VSLIDE1_TRANS(vslide1down_vx, slidedown_check)
+
/* Vector Floating-Point Slide Instructions */
static bool fslideup_check(DisasContext *s, arg_rmrr *a)
{
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 5386e3b97c5..c7fe3424c47 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -5145,11 +5145,11 @@ GEN_VEXT_VSLIE1UP(16, H2)
GEN_VEXT_VSLIE1UP(32, H4)
GEN_VEXT_VSLIE1UP(64, H8)
-#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
-void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
- CPURISCVState *env, uint32_t desc) \
-{ \
- vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
+#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
+void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
+ CPURISCVState *env, uint32_t desc) \
+{ \
+ vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
}
/* vslide1up.vx vd, vs2, rs1, vm # vd[0]=x[rs1], vd[i+1] = vs2[i] */
@@ -5196,11 +5196,11 @@ GEN_VEXT_VSLIDE1DOWN(16, H2)
GEN_VEXT_VSLIDE1DOWN(32, H4)
GEN_VEXT_VSLIDE1DOWN(64, H8)
-#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
-void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
- CPURISCVState *env, uint32_t desc) \
-{ \
- vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
+#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
+void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
+ CPURISCVState *env, uint32_t desc) \
+{ \
+ vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
}
/* vslide1down.vx vd, vs2, rs1, vm # vd[i] = vs2[i+1], vd[vl-1]=x[rs1] */
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64
2025-01-24 7:33 [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64 Max Chou
@ 2025-09-23 9:21 ` Max Chou
2025-09-29 1:30 ` Alistair Francis
2025-09-29 1:35 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Max Chou @ 2025-09-23 9:21 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Richard Henderson,
Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 8952 bytes --]
ping
On Fri, Jan 24, 2025 at 3:33 PM Max Chou <max.chou@sifive.com> wrote:
> When XLEN is 32 and SEW is 64, the original implementation of
> vslide1up.vx and vslide1down.vx helper functions fills the 32-bit value
> of rs1 into the first element of the destination vector register (rd),
> which is a 64-bit element.
>
> This commit attempted to resolve the issue by extending the rs1 value
> to 64 bits during the TCG translation phase to ensure that the helper
> functions won't lost the higer 32 bits.
>
> Signed-off-by: Max Chou <max.chou@sifive.com>
> ---
> target/riscv/helper.h | 16 ++++----
> target/riscv/insn_trans/trans_rvv.c.inc | 50 ++++++++++++++++++++++++-
> target/riscv/vector_helper.c | 20 +++++-----
> 3 files changed, 66 insertions(+), 20 deletions(-)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 16ea240d26d..0578d153bdf 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -1099,14 +1099,14 @@ DEF_HELPER_6(vslidedown_vx_b, void, ptr, ptr, tl,
> ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, i64, ptr, env, i32)
>
> DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32)
> DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32)
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index b9883a5d323..775fe1baae7 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3391,7 +3391,6 @@ static bool slideup_check(DisasContext *s, arg_rmrr
> *a)
> }
>
> GEN_OPIVX_TRANS(vslideup_vx, slideup_check)
> -GEN_OPIVX_TRANS(vslide1up_vx, slideup_check)
> GEN_OPIVI_TRANS(vslideup_vi, IMM_ZX, vslideup_vx, slideup_check)
>
> static bool slidedown_check(DisasContext *s, arg_rmrr *a)
> @@ -3402,9 +3401,56 @@ static bool slidedown_check(DisasContext *s,
> arg_rmrr *a)
> }
>
> GEN_OPIVX_TRANS(vslidedown_vx, slidedown_check)
> -GEN_OPIVX_TRANS(vslide1down_vx, slidedown_check)
> GEN_OPIVI_TRANS(vslidedown_vi, IMM_ZX, vslidedown_vx, slidedown_check)
>
> +typedef void gen_helper_vslide1_vx(TCGv_ptr, TCGv_ptr, TCGv_i64, TCGv_ptr,
> + TCGv_env, TCGv_i32);
> +
> +#define GEN_OPIVX_VSLIDE1_TRANS(NAME, CHECK) \
> +static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \
> +{ \
> + if (CHECK(s, a)) { \
> + static gen_helper_vslide1_vx * const fns[4] = { \
> + gen_helper_##NAME##_b, gen_helper_##NAME##_h, \
> + gen_helper_##NAME##_w, gen_helper_##NAME##_d, \
> + }; \
> + \
> + TCGv_ptr dest, src2, mask; \
> + TCGv_i64 src1; \
> + TCGv_i32 desc; \
> + uint32_t data = 0; \
> + \
> + dest = tcg_temp_new_ptr(); \
> + mask = tcg_temp_new_ptr(); \
> + src2 = tcg_temp_new_ptr(); \
> + src1 = tcg_temp_new_i64(); \
> + \
> + data = FIELD_DP32(data, VDATA, VM, a->vm); \
> + data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
> + data = FIELD_DP32(data, VDATA, VTA, s->vta); \
> + data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s); \
> + data = FIELD_DP32(data, VDATA, VMA, s->vma); \
> + desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlenb, \
> + s->cfg_ptr->vlenb, data)); \
> + \
> + tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd)); \
> + tcg_gen_addi_ptr(src2, tcg_env, vreg_ofs(s, a->rs2)); \
> + tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); \
> + tcg_gen_ext_tl_i64(src1, get_gpr(s, a->rs1, EXT_SIGN)); \
> + \
> + fns[s->sew](dest, mask, src1, src2, tcg_env, desc); \
> + \
> + tcg_gen_movi_tl(cpu_vstart, 0); \
> + finalize_rvv_inst(s); \
> + \
> + return true; \
> + } \
> + return false; \
> +}
> +
> +GEN_OPIVX_VSLIDE1_TRANS(vslide1up_vx, slideup_check)
> +GEN_OPIVX_VSLIDE1_TRANS(vslide1down_vx, slidedown_check)
> +
> /* Vector Floating-Point Slide Instructions */
> static bool fslideup_check(DisasContext *s, arg_rmrr *a)
> {
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 5386e3b97c5..c7fe3424c47 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -5145,11 +5145,11 @@ GEN_VEXT_VSLIE1UP(16, H2)
> GEN_VEXT_VSLIE1UP(32, H4)
> GEN_VEXT_VSLIE1UP(64, H8)
>
> -#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
> -void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> - CPURISCVState *env, uint32_t desc) \
> -{ \
> - vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> +#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
> +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
> + CPURISCVState *env, uint32_t desc) \
> +{ \
> + vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> }
>
> /* vslide1up.vx vd, vs2, rs1, vm # vd[0]=x[rs1], vd[i+1] = vs2[i] */
> @@ -5196,11 +5196,11 @@ GEN_VEXT_VSLIDE1DOWN(16, H2)
> GEN_VEXT_VSLIDE1DOWN(32, H4)
> GEN_VEXT_VSLIDE1DOWN(64, H8)
>
> -#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
> -void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> - CPURISCVState *env, uint32_t desc) \
> -{ \
> - vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> +#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
> +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
> + CPURISCVState *env, uint32_t desc) \
> +{ \
> + vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> }
>
> /* vslide1down.vx vd, vs2, rs1, vm # vd[i] = vs2[i+1], vd[vl-1]=x[rs1] */
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 10945 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64
2025-01-24 7:33 [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64 Max Chou
2025-09-23 9:21 ` Max Chou
@ 2025-09-29 1:30 ` Alistair Francis
2025-09-29 1:35 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2025-09-29 1:30 UTC (permalink / raw)
To: Max Chou
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei,
Richard Henderson, Philippe Mathieu-Daudé
On Fri, Jan 24, 2025 at 5:34 PM Max Chou <max.chou@sifive.com> wrote:
>
> When XLEN is 32 and SEW is 64, the original implementation of
> vslide1up.vx and vslide1down.vx helper functions fills the 32-bit value
> of rs1 into the first element of the destination vector register (rd),
> which is a 64-bit element.
>
> This commit attempted to resolve the issue by extending the rs1 value
> to 64 bits during the TCG translation phase to ensure that the helper
> functions won't lost the higer 32 bits.
>
> Signed-off-by: Max Chou <max.chou@sifive.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/helper.h | 16 ++++----
> target/riscv/insn_trans/trans_rvv.c.inc | 50 ++++++++++++++++++++++++-
> target/riscv/vector_helper.c | 20 +++++-----
> 3 files changed, 66 insertions(+), 20 deletions(-)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 16ea240d26d..0578d153bdf 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -1099,14 +1099,14 @@ DEF_HELPER_6(vslidedown_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, i64, ptr, env, i32)
>
> DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32)
> DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32)
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index b9883a5d323..775fe1baae7 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3391,7 +3391,6 @@ static bool slideup_check(DisasContext *s, arg_rmrr *a)
> }
>
> GEN_OPIVX_TRANS(vslideup_vx, slideup_check)
> -GEN_OPIVX_TRANS(vslide1up_vx, slideup_check)
> GEN_OPIVI_TRANS(vslideup_vi, IMM_ZX, vslideup_vx, slideup_check)
>
> static bool slidedown_check(DisasContext *s, arg_rmrr *a)
> @@ -3402,9 +3401,56 @@ static bool slidedown_check(DisasContext *s, arg_rmrr *a)
> }
>
> GEN_OPIVX_TRANS(vslidedown_vx, slidedown_check)
> -GEN_OPIVX_TRANS(vslide1down_vx, slidedown_check)
> GEN_OPIVI_TRANS(vslidedown_vi, IMM_ZX, vslidedown_vx, slidedown_check)
>
> +typedef void gen_helper_vslide1_vx(TCGv_ptr, TCGv_ptr, TCGv_i64, TCGv_ptr,
> + TCGv_env, TCGv_i32);
> +
> +#define GEN_OPIVX_VSLIDE1_TRANS(NAME, CHECK) \
> +static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \
> +{ \
> + if (CHECK(s, a)) { \
> + static gen_helper_vslide1_vx * const fns[4] = { \
> + gen_helper_##NAME##_b, gen_helper_##NAME##_h, \
> + gen_helper_##NAME##_w, gen_helper_##NAME##_d, \
> + }; \
> + \
> + TCGv_ptr dest, src2, mask; \
> + TCGv_i64 src1; \
> + TCGv_i32 desc; \
> + uint32_t data = 0; \
> + \
> + dest = tcg_temp_new_ptr(); \
> + mask = tcg_temp_new_ptr(); \
> + src2 = tcg_temp_new_ptr(); \
> + src1 = tcg_temp_new_i64(); \
> + \
> + data = FIELD_DP32(data, VDATA, VM, a->vm); \
> + data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
> + data = FIELD_DP32(data, VDATA, VTA, s->vta); \
> + data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s); \
> + data = FIELD_DP32(data, VDATA, VMA, s->vma); \
> + desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlenb, \
> + s->cfg_ptr->vlenb, data)); \
> + \
> + tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd)); \
> + tcg_gen_addi_ptr(src2, tcg_env, vreg_ofs(s, a->rs2)); \
> + tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); \
> + tcg_gen_ext_tl_i64(src1, get_gpr(s, a->rs1, EXT_SIGN)); \
> + \
> + fns[s->sew](dest, mask, src1, src2, tcg_env, desc); \
> + \
> + tcg_gen_movi_tl(cpu_vstart, 0); \
> + finalize_rvv_inst(s); \
> + \
> + return true; \
> + } \
> + return false; \
> +}
> +
> +GEN_OPIVX_VSLIDE1_TRANS(vslide1up_vx, slideup_check)
> +GEN_OPIVX_VSLIDE1_TRANS(vslide1down_vx, slidedown_check)
> +
> /* Vector Floating-Point Slide Instructions */
> static bool fslideup_check(DisasContext *s, arg_rmrr *a)
> {
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 5386e3b97c5..c7fe3424c47 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -5145,11 +5145,11 @@ GEN_VEXT_VSLIE1UP(16, H2)
> GEN_VEXT_VSLIE1UP(32, H4)
> GEN_VEXT_VSLIE1UP(64, H8)
>
> -#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
> -void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> - CPURISCVState *env, uint32_t desc) \
> -{ \
> - vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> +#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
> +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
> + CPURISCVState *env, uint32_t desc) \
> +{ \
> + vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> }
>
> /* vslide1up.vx vd, vs2, rs1, vm # vd[0]=x[rs1], vd[i+1] = vs2[i] */
> @@ -5196,11 +5196,11 @@ GEN_VEXT_VSLIDE1DOWN(16, H2)
> GEN_VEXT_VSLIDE1DOWN(32, H4)
> GEN_VEXT_VSLIDE1DOWN(64, H8)
>
> -#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
> -void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> - CPURISCVState *env, uint32_t desc) \
> -{ \
> - vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> +#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
> +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
> + CPURISCVState *env, uint32_t desc) \
> +{ \
> + vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> }
>
> /* vslide1down.vx vd, vs2, rs1, vm # vd[i] = vs2[i+1], vd[vl-1]=x[rs1] */
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64
2025-01-24 7:33 [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64 Max Chou
2025-09-23 9:21 ` Max Chou
2025-09-29 1:30 ` Alistair Francis
@ 2025-09-29 1:35 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2025-09-29 1:35 UTC (permalink / raw)
To: Max Chou
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei,
Richard Henderson, Philippe Mathieu-Daudé
On Fri, Jan 24, 2025 at 5:34 PM Max Chou <max.chou@sifive.com> wrote:
>
> When XLEN is 32 and SEW is 64, the original implementation of
> vslide1up.vx and vslide1down.vx helper functions fills the 32-bit value
> of rs1 into the first element of the destination vector register (rd),
> which is a 64-bit element.
>
> This commit attempted to resolve the issue by extending the rs1 value
> to 64 bits during the TCG translation phase to ensure that the helper
> functions won't lost the higer 32 bits.
>
> Signed-off-by: Max Chou <max.chou@sifive.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/helper.h | 16 ++++----
> target/riscv/insn_trans/trans_rvv.c.inc | 50 ++++++++++++++++++++++++-
> target/riscv/vector_helper.c | 20 +++++-----
> 3 files changed, 66 insertions(+), 20 deletions(-)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 16ea240d26d..0578d153bdf 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -1099,14 +1099,14 @@ DEF_HELPER_6(vslidedown_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> DEF_HELPER_6(vslidedown_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, tl, ptr, env, i32)
> -DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, tl, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, i64, ptr, env, i32)
>
> DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32)
> DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32)
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index b9883a5d323..775fe1baae7 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3391,7 +3391,6 @@ static bool slideup_check(DisasContext *s, arg_rmrr *a)
> }
>
> GEN_OPIVX_TRANS(vslideup_vx, slideup_check)
> -GEN_OPIVX_TRANS(vslide1up_vx, slideup_check)
> GEN_OPIVI_TRANS(vslideup_vi, IMM_ZX, vslideup_vx, slideup_check)
>
> static bool slidedown_check(DisasContext *s, arg_rmrr *a)
> @@ -3402,9 +3401,56 @@ static bool slidedown_check(DisasContext *s, arg_rmrr *a)
> }
>
> GEN_OPIVX_TRANS(vslidedown_vx, slidedown_check)
> -GEN_OPIVX_TRANS(vslide1down_vx, slidedown_check)
> GEN_OPIVI_TRANS(vslidedown_vi, IMM_ZX, vslidedown_vx, slidedown_check)
>
> +typedef void gen_helper_vslide1_vx(TCGv_ptr, TCGv_ptr, TCGv_i64, TCGv_ptr,
> + TCGv_env, TCGv_i32);
> +
> +#define GEN_OPIVX_VSLIDE1_TRANS(NAME, CHECK) \
> +static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \
> +{ \
> + if (CHECK(s, a)) { \
> + static gen_helper_vslide1_vx * const fns[4] = { \
> + gen_helper_##NAME##_b, gen_helper_##NAME##_h, \
> + gen_helper_##NAME##_w, gen_helper_##NAME##_d, \
> + }; \
> + \
> + TCGv_ptr dest, src2, mask; \
> + TCGv_i64 src1; \
> + TCGv_i32 desc; \
> + uint32_t data = 0; \
> + \
> + dest = tcg_temp_new_ptr(); \
> + mask = tcg_temp_new_ptr(); \
> + src2 = tcg_temp_new_ptr(); \
> + src1 = tcg_temp_new_i64(); \
> + \
> + data = FIELD_DP32(data, VDATA, VM, a->vm); \
> + data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
> + data = FIELD_DP32(data, VDATA, VTA, s->vta); \
> + data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s); \
> + data = FIELD_DP32(data, VDATA, VMA, s->vma); \
> + desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlenb, \
> + s->cfg_ptr->vlenb, data)); \
> + \
> + tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd)); \
> + tcg_gen_addi_ptr(src2, tcg_env, vreg_ofs(s, a->rs2)); \
> + tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0)); \
> + tcg_gen_ext_tl_i64(src1, get_gpr(s, a->rs1, EXT_SIGN)); \
> + \
> + fns[s->sew](dest, mask, src1, src2, tcg_env, desc); \
> + \
> + tcg_gen_movi_tl(cpu_vstart, 0); \
> + finalize_rvv_inst(s); \
> + \
> + return true; \
> + } \
> + return false; \
> +}
> +
> +GEN_OPIVX_VSLIDE1_TRANS(vslide1up_vx, slideup_check)
> +GEN_OPIVX_VSLIDE1_TRANS(vslide1down_vx, slidedown_check)
> +
> /* Vector Floating-Point Slide Instructions */
> static bool fslideup_check(DisasContext *s, arg_rmrr *a)
> {
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 5386e3b97c5..c7fe3424c47 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -5145,11 +5145,11 @@ GEN_VEXT_VSLIE1UP(16, H2)
> GEN_VEXT_VSLIE1UP(32, H4)
> GEN_VEXT_VSLIE1UP(64, H8)
>
> -#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
> -void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> - CPURISCVState *env, uint32_t desc) \
> -{ \
> - vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> +#define GEN_VEXT_VSLIDE1UP_VX(NAME, BITWIDTH) \
> +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
> + CPURISCVState *env, uint32_t desc) \
> +{ \
> + vslide1up_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> }
>
> /* vslide1up.vx vd, vs2, rs1, vm # vd[0]=x[rs1], vd[i+1] = vs2[i] */
> @@ -5196,11 +5196,11 @@ GEN_VEXT_VSLIDE1DOWN(16, H2)
> GEN_VEXT_VSLIDE1DOWN(32, H4)
> GEN_VEXT_VSLIDE1DOWN(64, H8)
>
> -#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
> -void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> - CPURISCVState *env, uint32_t desc) \
> -{ \
> - vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> +#define GEN_VEXT_VSLIDE1DOWN_VX(NAME, BITWIDTH) \
> +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \
> + CPURISCVState *env, uint32_t desc) \
> +{ \
> + vslide1down_##BITWIDTH(vd, v0, s1, vs2, env, desc); \
> }
>
> /* vslide1down.vx vd, vs2, rs1, vm # vd[i] = vs2[i+1], vd[vl-1]=x[rs1] */
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-29 1:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-24 7:33 [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64 Max Chou
2025-09-23 9:21 ` Max Chou
2025-09-29 1:30 ` Alistair Francis
2025-09-29 1:35 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).