Re: [PATCH v2 06/18] target/riscv: Add packed SIMD shift instructions

On 2026-07-17 10:06, Molly Chen wrote:
> Signed-off-by: Molly Chen <[email protected]>
> ---
>  target/riscv/helper.h                       |  43 ++++++
>  target/riscv/insn32.decode                  |  57 ++++++++
>  target/riscv/tcg/insn_trans/trans_rvp.c.inc |  43 ++++++
>  target/riscv/tcg/psimd_helper.c             | 149 ++++++++++++++++++++
>  4 files changed, 292 insertions(+)
...
> diff --git a/target/riscv/tcg/psimd_helper.c b/target/riscv/tcg/psimd_helper.c
> index 034afe5a054..8b106a336f5 100644
> --- a/target/riscv/tcg/psimd_helper.c
> +++ b/target/riscv/tcg/psimd_helper.c
> @@ -1903,3 +1903,152 @@ GEN_PSIMD_BINOP(mslt, uint32_t, int32_t, uint32_t,
>  GEN_PSIMD_BINOP(msltu, uint32_t, uint32_t, uint32_t,
>                  EXTRACT32, INSERT32, ELEMS_W, PSIMD_DO_LT_MASK)
>  
> +/* Shift operations (immediate and register) */
> +
> +GEN_PSIMD_SHIFTOP(pslli_b, target_ulong, uint8_t, uint8_t,
> +                  EXTRACT8, INSERT8, ELEMS_B, 0x07, PSIMD_DO_SLL)
> +GEN_PSIMD_SHIFTOP(psll_bs, target_ulong, uint8_t, uint8_t,
> +                  EXTRACT8, INSERT8, ELEMS_B, 0x07, PSIMD_DO_SLL)

The spec instead specifies a uniform 5-bit mask (shamt = X[rs2][4:0]),
regardless of element width.
So the SHMASK should be 0x1f for PSLL/PSRL/PSRA.[BS|HS] instructions.

...

> +
> +/**
> + * SSHAR - 32-bit scalar variable shift with rounding and saturation
> + */
> +uint32_t HELPER(sshar)(CPURISCVState *env, uint32_t rs1, uint32_t rs2)
> +{
> +    int32_t a = (int32_t)rs1;
> +    int8_t shamt = (int8_t)(rs2 & 0xFF);
> +    int sat = 0;
> +    int32_t res;
> +
> +    if (shamt >= 0) {
> +        int64_t shifted = (int64_t)a << shamt;
> +        res = signed_saturate_w(shifted, &sat);
> +    } else {
> +        int right = -shamt;
> +        if (right >= 32) {
> +            res = (a < 0) ? -1 : 0;

According to the p ext isa spec, SSHAR here extracts a sign-filled
33-bit value and then applies RNU rounding. The result here should be
0 for both positive and negative inputs.

> +        } else {
> +            int64_t rounded = ((a >> (right - 1)) + 1) >> 1;

Here may has undefined behavior for negative sources at in-range
amounts.

I think that we could extract the SSHAR implementation from
GEN_PSIMD_VAR_SSHAR and share it between here and
GEN_PSIMD_VAR_SSHAR.

rnax