Re: [PATCH v3 4/6] tcg/riscv64: Fix constant handling in vector comparisons and selects

[Aurelien Jarno <aurelien@aurel32.net>]

On 2026-10-28 15:33, Philippe Mathieu-Daudé wrote:
> tcg_out_cmpsel() takes constants as TCGReg and passes them to the
> encoders unextracted. In a release build, "cmlt v0.8h, #0" emitted
> the illegal instruction 0xffffb057.
> 
> Use TCGArg for these operands and extract the element with
> vec_imm() before encoding, including before the LT/GE adjustment.
> 
> Fixes: 006352d69f14 ("tcg/riscv64: Assert immediate in range ...")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>  tcg/riscv64/tcg-target.c.inc | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc
> index a3d995e290b7..168932711796 100644
> --- a/tcg/riscv64/tcg-target.c.inc
> +++ b/tcg/riscv64/tcg-target.c.inc
> @@ -1555,9 +1555,9 @@ static void tcg_out_cltz(TCGContext *s, TCGType type, RISCVInsn insn,
>  
>  static void tcg_out_cmpsel(TCGContext *s, TCGType type, unsigned vece,
>                             TCGCond cond, TCGReg ret,
> -                           TCGReg cmp1, TCGReg cmp2, bool c_cmp2,
> -                           TCGReg val1, bool c_val1,
> -                           TCGReg val2, bool c_val2)
> +                           TCGReg cmp1, TCGArg cmp2, bool c_cmp2,
> +                           TCGArg val1, bool c_val1,
> +                           TCGArg val2, bool c_val2)
>  {
>      set_vtype_len_sew(s, type, vece);
>  
> @@ -1574,7 +1574,7 @@ static void tcg_out_cmpsel(TCGContext *s, TCGType type, unsigned vece,
>      /* Perform the comparison into V0 mask. */
>      if (c_cmp2) {
>          tcg_out_opc_vi(s, tcg_cmpcond_to_rvv_vi[cond].op, TCG_REG_V0, cmp1,
> -                       cmp2 - tcg_cmpcond_to_rvv_vi[cond].adjust);
> +                       vec_imm(cmp2, vece) - tcg_cmpcond_to_rvv_vi[cond].adjust);
>      } else if (tcg_cmpcond_to_rvv_vv[cond].swap) {
>          tcg_out_opc_vv(s, tcg_cmpcond_to_rvv_vv[cond].op,
>                         TCG_REG_V0, cmp2, cmp1);
> @@ -1584,11 +1584,12 @@ static void tcg_out_cmpsel(TCGContext *s, TCGType type, unsigned vece,
>      }
>      if (c_val1) {
>          if (c_val2) {
> -            tcg_out_opc_vi(s, OPC_VMV_V_I, ret, 0, val2);
> +            tcg_out_opc_vi(s, OPC_VMV_V_I, ret, 0, vec_imm(val2, vece));
>              val2 = ret;
>          }
>          /* vd[i] == v0.mask[i] ? imm : vs2[i] */
> -        tcg_out_opc_vim_mask(s, OPC_VMERGE_VIM, ret, val2, val1);
> +        tcg_out_opc_vim_mask(s, OPC_VMERGE_VIM, ret, val2,
> +                             vec_imm(val1, vece));
>      } else {
>          /* vd[i] == v0.mask[i] ? vs1[i] : vs2[i] */
>          tcg_out_opc_vvm_mask(s, OPC_VMERGE_VVM, ret, val2, val1);

Similar to my previous comment. This is technically correct, but if such 
an instruction is generated, it also means that a field is not properly 
masked when generating the opcode.

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