Re: [PATCH v3 3/6] tcg/riscv64: Extract elements for vector arithmetic immediates

[Aurelien Jarno <aurelien@aurel32.net>]

On 2026-10-28 15:33, Philippe Mathieu-Daudé wrote:
> The .vi emitters for add, reverse sub and saturating ops pass the
> whole replicated constant (e.g. 0x0505050505050505) to encode_vi()
> instead of a single element. This aborts debug builds on common
> vectorized guest code and is undefined behaviour in release builds.
> 
> Add vec_imm() to extract the VECE element, and use it.
> 
> Fixes: 006352d69f14 ("tcg/riscv64: Assert immediate in range ...")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>  tcg/riscv64/tcg-target.c.inc | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc
> index 0e749ae296b8..a3d995e290b7 100644
> --- a/tcg/riscv64/tcg-target.c.inc
> +++ b/tcg/riscv64/tcg-target.c.inc
> @@ -735,6 +735,12 @@ static void tcg_out_opc_vi_u(TCGContext *s, RISCVInsn opc,
>      tcg_out32(s, encode_vi_u(opc, vd, imm, vs2, true));
>  }
>  
> +/* Constants are replicated by VECE; extract one element for .vi forms. */
> +static int64_t vec_imm(TCGArg arg, unsigned vece)
> +{
> +    return sextract64(arg, 0, 8 << vece);
> +}
> +
>  static void tcg_out_opc_vv_vi(TCGContext *s, RISCVInsn o_vv, RISCVInsn o_vi,
>                                TCGReg vd, TCGReg vs2, TCGArg vi1, int c_vi1)
>  {
> @@ -2788,12 +2794,13 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>          break;
>      case INDEX_op_add_vec:
>          set_vtype_len_sew(s, type, vece);
> -        tcg_out_opc_vv_vi(s, OPC_VADD_VV, OPC_VADD_VI, a0, a1, a2, c2);
> +        tcg_out_opc_vv_vi(s, OPC_VADD_VV, OPC_VADD_VI, a0, a1,
> +                          c2 ? vec_imm(a2, vece) : a2, c2);
>          break;
>      case INDEX_op_sub_vec:
>          set_vtype_len_sew(s, type, vece);
>          if (const_args[1]) {
> -            tcg_out_opc_vi(s, OPC_VRSUB_VI, a0, a2, a1);
> +            tcg_out_opc_vi(s, OPC_VRSUB_VI, a0, a2, vec_imm(a1, vece));
>          } else {
>              tcg_out_opc_vv(s, OPC_VSUB_VV, a0, a1, a2);
>          }
> @@ -2824,19 +2831,23 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>          break;
>      case INDEX_op_ssadd_vec:
>          set_vtype_len_sew(s, type, vece);
> -        tcg_out_opc_vv_vi(s, OPC_VSADD_VV, OPC_VSADD_VI, a0, a1, a2, c2);
> +        tcg_out_opc_vv_vi(s, OPC_VSADD_VV, OPC_VSADD_VI, a0, a1,
> +                          c2 ? vec_imm(a2, vece) : a2, c2);
>          break;
>      case INDEX_op_sssub_vec:
>          set_vtype_len_sew(s, type, vece);
> -        tcg_out_opc_vv_vi(s, OPC_VSSUB_VV, OPC_VSSUB_VI, a0, a1, a2, c2);
> +        tcg_out_opc_vv_vi(s, OPC_VSSUB_VV, OPC_VSSUB_VI, a0, a1,
> +                          c2 ? vec_imm(a2, vece) : a2, c2);
>          break;
>      case INDEX_op_usadd_vec:
>          set_vtype_len_sew(s, type, vece);
> -        tcg_out_opc_vv_vi(s, OPC_VSADDU_VV, OPC_VSADDU_VI, a0, a1, a2, c2);
> +        tcg_out_opc_vv_vi(s, OPC_VSADDU_VV, OPC_VSADDU_VI, a0, a1,
> +                          c2 ? vec_imm(a2, vece) : a2, c2);
>          break;
>      case INDEX_op_ussub_vec:
>          set_vtype_len_sew(s, type, vece);
> -        tcg_out_opc_vv_vi(s, OPC_VSSUBU_VV, OPC_VSSUBU_VI, a0, a1, a2, c2);
> +        tcg_out_opc_vv_vi(s, OPC_VSSUBU_VV, OPC_VSSUBU_VI, a0, a1,
> +                          c2 ? vec_imm(a2, vece) : a2, c2);
>          break;
>      case INDEX_op_smax_vec:
>          set_vtype_len_sew(s, type, vece);

This is technically correct. That said, the goal of all this extra code 
is to satisfy the assert added in commit 006352d69f14 ("tcg/riscv64: 
Assert immediate in range for {i,s,u,vi}-type insns") not to fix the 
code generation which is correct as it only keeps the lower 5 bits of 
the immediate.

At this stage we already know that the immediate can be represented as a 
duplicated simm5 constant, so just dropping the assert could be a 
simpler way to fix the issue.

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