Re: [PATCH v3 2/6] tcg/riscv64: Use a uimm5 encoder for vector shift immediates

[Aurelien Jarno <aurelien@aurel32.net>]

On 2026-10-28 15:33, Philippe Mathieu-Daudé wrote:
> RVV shift .vi forms take an unsigned 5-bit immediate, but
> encode_vi() checks for simm5, so shift counts 16..31 trip
> the debug assertion (and are undefined behaviour in release
> builds).
> 
> Add encode_vi_u() and use it in tcg_out_vshifti().
> 
> Fixes: 006352d69f14 ("tcg/riscv64: Assert immediate in range ...")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>  tcg/riscv64/tcg-target.c.inc | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc
> index 712170c1203e..0e749ae296b8 100644
> --- a/tcg/riscv64/tcg-target.c.inc
> +++ b/tcg/riscv64/tcg-target.c.inc
> @@ -541,6 +541,7 @@ static tcg_insn_unit encode_vi_raw(RISCVInsn opc, TCGReg rd, unsigned imm5,
>             (vs2 & 0x1f) << 20 | (vm << 25);
>  }
>  
> +/* OPIVI with a signed immediate (simm5). */
>  static tcg_insn_unit encode_vi(RISCVInsn opc, TCGReg rd, tcg_target_long imm,
>                                 TCGReg vs2, bool vm)
>  {
> @@ -548,6 +549,14 @@ static tcg_insn_unit encode_vi(RISCVInsn opc, TCGReg rd, tcg_target_long imm,
>      return encode_vi_raw(opc, rd, imm, vs2, vm);
>  }
>  
> +/* OPIVI with an unsigned immediate (uimm5) */
> +static tcg_insn_unit encode_vi_u(RISCVInsn opc, TCGReg rd, unsigned imm,
> +                                 TCGReg vs2, bool vm)
> +{
> +    tcg_debug_assert(imm == extract32(imm, 0, 5));
> +    return encode_vi_raw(opc, rd, imm, vs2, vm);
> +}
> +
>  /* Type-OPIVV/OPMVV/OPIVX/OPMVX, Vector load and store */
>  
>  static tcg_insn_unit encode_v(RISCVInsn opc, TCGReg d, TCGReg s1,
> @@ -720,6 +729,12 @@ static void tcg_out_opc_vi(TCGContext *s, RISCVInsn opc,
>      tcg_out32(s, encode_vi(opc, vd, imm, vs2, true));
>  }
>  
> +static void tcg_out_opc_vi_u(TCGContext *s, RISCVInsn opc,
> +                             TCGReg vd, TCGReg vs2, unsigned imm)
> +{
> +    tcg_out32(s, encode_vi_u(opc, vd, imm, vs2, true));
> +}
> +
>  static void tcg_out_opc_vv_vi(TCGContext *s, RISCVInsn o_vv, RISCVInsn o_vi,
>                                TCGReg vd, TCGReg vs2, TCGArg vi1, int c_vi1)
>  {
> @@ -1578,7 +1593,7 @@ static void tcg_out_vshifti(TCGContext *s, RISCVInsn opc_vi, RISCVInsn opc_vx,
>                               TCGReg dst, TCGReg src, unsigned imm)
>  {
>      if (imm < 32) {
> -        tcg_out_opc_vi(s, opc_vi, dst, src, imm);
> +        tcg_out_opc_vi_u(s, opc_vi, dst, src, imm);
>      } else {
>          tcg_out_movi(s, TCG_TYPE_I32, TCG_REG_TMP0, imm);
>          tcg_out_opc_vx(s, opc_vx, dst, src, TCG_REG_TMP0);

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net