Re: [PATCH v2 02/18] target/riscv: Add packed SIMD helper framework

On 2026-07-17 10:06, Molly Chen wrote:
> Signed-off-by: Molly Chen <[email protected]>
> ---
>  target/riscv/tcg/meson.build    |    3 +-
>  target/riscv/tcg/psimd_helper.c | 1656 +++++++++++++++++++++++++++++++
>  2 files changed, 1658 insertions(+), 1 deletion(-)
>  create mode 100644 target/riscv/tcg/psimd_helper.c
> 
...
>  riscv_system_ss.add(files(
> diff --git a/target/riscv/tcg/psimd_helper.c b/target/riscv/tcg/psimd_helper.c
> new file mode 100644
> index 00000000000..2948bbb2a86
> --- /dev/null
> +++ b/target/riscv/tcg/psimd_helper.c
...
> +#define GEN_PSIMD_VAR_USHLR(NAME, RTYPE, ETYPE, WTYPE, EXTRACT, INSERT,   \
> +                            ELEMS, BITS, SAT_FN)                          \
> +RTYPE HELPER(NAME)(CPURISCVState *env, RTYPE rs1, RTYPE rs2)              \
> +{                                                                         \
> +    RTYPE rd = 0;                                                         \
> +    int elems = ELEMS(rd);                                                \
> +    int sat = 0;                                                          \
> +    int8_t shamt = (int8_t)(rs2 & 0xff);                                  \
> +                                                                          \
> +    for (int i = 0; i < elems; i++) {                                     \
> +        ETYPE e1 = (ETYPE)EXTRACT(rs1, i);                                \
> +        ETYPE res;                                                        \
> +                                                                          \
> +        if (shamt >= 0) {                                                 \
> +            WTYPE shifted = (shamt >= (BITS)) ?                           \
> +                            ((WTYPE)e1 << (BITS)) :                       \
> +                            ((WTYPE)e1 << shamt);                         \
> +            res = SAT_FN(shifted, &sat);                                  \
> +        } else {                                                          \
> +            int right = -shamt;                                           \
> +            if (right > (BITS)) {                                         \
> +                res = 0;                                                  \

Here may has an issue that a rounding right shift past the element
width does not degenerate to zero -- it saturates to the rounding of
the element's MSB.

According to the P ext isa spec:

  "The SSHLR instruction performs an unsigned variable shift of `rs1`
   using the signed shift amount in `rs2[7:0]`. Right shifts are
   rounded, and left shifts saturate to the signed 32-bit range."

    if sshamt < 0:
        // arithmetic right shift with rounding
        x  = zero_extend(64, s1) @ 0b0            // 65-bit
        y  = (sshamt <= -32) ? x[64:32]
                             : (x >> (0 - shamt)[4:0])[32:0]
        X[rd] = (y + 1)[32:1]

The operation Sail code shows that every right shift at or beyond
the element width collapses to the same x[64:32], which is the
element's MSB, and then still gets the (y + 1) >> 1 rounding.
PSSHLR.HS and PSSHLR.WS have the identical construct at the 16- and
32-bit boundaries.

Maybe we could fix it liked:

           } else {                                                          \
               int right = MIN(-shamt, (BITS));                              \
               WTYPE rounded = ((WTYPE)e1 >> (right - 1)) + 1;               \
               res = (ETYPE)(rounded >> 1);                                  \
           }                                                                 \


rnax

> +            } else {                                                      \
> +                WTYPE rounded = ((WTYPE)e1 >> (right - 1)) + 1;           \
> +                res = (ETYPE)(rounded >> 1);                              \
> +            }                                                             \
> +        }                                                                 \