From: Alistair Francis <alistair23@gmail.com>
To: Max Chou <max.chou@sifive.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Alistair Francis" <alistair.francis@wdc.com>,
"Bin Meng" <bmeng.cn@gmail.com>,
"Weiwei Li" <liwei1518@gmail.com>,
"Daniel Henrique Barboza" <dbarboza@ventanamicro.com>,
"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: Re: [PATCH] target/riscv: rvv: Fix vslide1[up|down].vx unexpected result when XLEN=32 and SEW=64
Date: Mon, 29 Sep 2025 11:35:40 +1000 [thread overview]
Message-ID: <CAKmqyKPWbC2kOL5hU2hhKOiiQrsEuM-e9MB4yYB8Zp9CSZ0Nrg@mail.gmail.com> (raw)
In-Reply-To: <20250124073325.2467664-1-max.chou@sifive.com>
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
>
>
prev parent reply other threads:[~2025-09-29 1:38 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKmqyKPWbC2kOL5hU2hhKOiiQrsEuM-e9MB4yYB8Zp9CSZ0Nrg@mail.gmail.com \
--to=alistair23@gmail.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng.cn@gmail.com \
--cc=dbarboza@ventanamicro.com \
--cc=liwei1518@gmail.com \
--cc=max.chou@sifive.com \
--cc=palmer@dabbelt.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).