On 2026-10-28 15:33, Philippe Mathieu-Daudé wrote: > and/or/xor_vec always have VECE 0, yet their constant was matched > as a byte element and applied at the stale SEW of the previous op: > "bic v4.8h, #0xf0" became "vand.vi ..., 15". > > Add the 'N' constraint: use the smallest element size at which the > constant replicates, if that element fits simm5, and switch SEW to > it. A wider element can't fit when the smallest doesn't: its upper > half repeats the smaller element, which would then be 0 or -1. > > Cc: [email protected] > Fixes: 5a63f5998791 ("tcg/riscv: Add support for basic vector opcodes") > Reported-by: Aurelien Jarno <[email protected]> > Suggested-by: Richard Henderson <[email protected]> > Signed-off-by: Philippe Mathieu-Daudé <[email protected]> > --- > tcg/riscv64/tcg-target-con-set.h | 1 + > tcg/riscv64/tcg-target-con-str.h | 1 + > tcg/riscv64/tcg-target.c.inc | 53 +++++++++++++++++++++++++++----- > 3 files changed, 48 insertions(+), 7 deletions(-) > > diff --git a/tcg/riscv64/tcg-target-con-set.h > b/tcg/riscv64/tcg-target-con-set.h > index 0fc26d3f98b4..9b4e219fac16 100644 > --- a/tcg/riscv64/tcg-target-con-set.h > +++ b/tcg/riscv64/tcg-target-con-set.h > @@ -26,4 +26,5 @@ C_O1_I2(v, v, v) > C_O1_I2(v, vK, v) > C_O1_I2(v, v, vK) > C_O1_I2(v, v, vL) > +C_O1_I2(v, v, vN) > C_O1_I4(v, v, vL, vK, vK) > diff --git a/tcg/riscv64/tcg-target-con-str.h > b/tcg/riscv64/tcg-target-con-str.h > index c04e15ddfa3c..d7e8b2ce4f2f 100644 > --- a/tcg/riscv64/tcg-target-con-str.h > +++ b/tcg/riscv64/tcg-target-con-str.h > @@ -19,3 +19,4 @@ CONST('I', TCG_CT_CONST_S12) > CONST('K', TCG_CT_CONST_S5) > CONST('L', TCG_CT_CONST_CMP_VI) > CONST('M', TCG_CT_CONST_M12) > +CONST('N', TCG_CT_CONST_LOGICAL_VI) > diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc > index 168932711796..4faf7ff36e07 100644 > --- a/tcg/riscv64/tcg-target.c.inc > +++ b/tcg/riscv64/tcg-target.c.inc > @@ -116,6 +116,7 @@ static TCGReg tcg_target_call_oarg_reg(TCGCallReturnKind > kind, int slot) > #define TCG_CT_CONST_M12 0x200 > #define TCG_CT_CONST_S5 0x400 > #define TCG_CT_CONST_CMP_VI 0x800 > +#define TCG_CT_CONST_LOGICAL_VI 0x1000 > > #define ALL_GENERAL_REGS MAKE_64BIT_MASK(0, 32) > #define ALL_VECTOR_REGS MAKE_64BIT_MASK(32, 32) > @@ -384,6 +385,22 @@ static const struct { > [TCG_COND_GEU] = { OPC_VMSGTU_VI, 1, 16, true }, > }; > > +/* > + * Return the element size at which VAL is a replicated simm5, > + * or -1 if there is none. Used for vector logical ops, whose > + * VECE is not meaningful. > + */ > +static int vec_logical_immediate_vece(int64_t val) > +{ > + for (int e = MO_8; e <= MO_64; e++) { > + if (val == dup_const(e, val)) { > + int64_t x = sextract64(val, 0, 8 << e); > + return x >= -0x10 && x <= 0x0f ? e : -1; > + } > + } > + g_assert_not_reached(); > +} > + > /* test if a constant matches the constraint */ > static bool tcg_target_const_match(int64_t val, int ct, > TCGType type, TCGCond cond, int vece) > @@ -391,6 +408,10 @@ static bool tcg_target_const_match(int64_t val, int ct, > if (ct & TCG_CT_CONST) { > return 1; > } > + if ((ct & TCG_CT_CONST_LOGICAL_VI) && > + vec_logical_immediate_vece(val) >= 0) { > + return true; > + } > if (type >= TCG_TYPE_V64) { > /* Val is replicated by VECE; extract the highest element. */ > val >>= (-8 << vece) & 63; > @@ -2769,6 +2790,23 @@ static const TCGOutOpStore outop_st = { > }; > > > +static void tcg_out_opc_vv_vi_logical(TCGContext *s, TCGType type, > + RISCVInsn o_vv, RISCVInsn o_vi, > + TCGReg vd, TCGReg vs2, > + TCGArg vi1, int c_vi1) > +{ > + if (c_vi1) { > + int vece = vec_logical_immediate_vece(vi1); > + > + tcg_debug_assert(vece >= 0); > + set_vtype_len_sew(s, type, vece); > + tcg_out_opc_vi(s, o_vi, vd, vs2, vec_imm(vi1, vece)); > + } else { > + set_vtype_len(s, type); > + tcg_out_opc_vv(s, o_vv, vd, vs2, vi1); > + } > +} > + > static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, > unsigned vecl, unsigned vece, > const TCGArg args[TCG_MAX_OP_ARGS], > @@ -2807,16 +2845,16 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode > opc, > } > break; > case INDEX_op_and_vec: > - set_vtype_len(s, type); > - tcg_out_opc_vv_vi(s, OPC_VAND_VV, OPC_VAND_VI, a0, a1, a2, c2); > + tcg_out_opc_vv_vi_logical(s, type, OPC_VAND_VV, OPC_VAND_VI, > + a0, a1, a2, c2); > break; > case INDEX_op_or_vec: > - set_vtype_len(s, type); > - tcg_out_opc_vv_vi(s, OPC_VOR_VV, OPC_VOR_VI, a0, a1, a2, c2); > + tcg_out_opc_vv_vi_logical(s, type, OPC_VOR_VV, OPC_VOR_VI, > + a0, a1, a2, c2); > break; > case INDEX_op_xor_vec: > - set_vtype_len(s, type); > - tcg_out_opc_vv_vi(s, OPC_VXOR_VV, OPC_VXOR_VI, a0, a1, a2, c2); > + tcg_out_opc_vv_vi_logical(s, type, OPC_VXOR_VV, OPC_VXOR_VI, > + a0, a1, a2, c2); > break; > case INDEX_op_not_vec: > set_vtype_len(s, type); > @@ -3008,10 +3046,11 @@ tcg_target_op_def(TCGOpcode op, TCGType type, > unsigned flags) > case INDEX_op_sari_vec: > case INDEX_op_rotli_vec: > return C_O1_I1(v, v); > - case INDEX_op_add_vec: > case INDEX_op_and_vec: > case INDEX_op_or_vec: > case INDEX_op_xor_vec: > + return C_O1_I2(v, v, vN); > + case INDEX_op_add_vec: > case INDEX_op_ssadd_vec: > case INDEX_op_sssub_vec: > case INDEX_op_usadd_vec:
I am still wondering why not propagating VECE for logical ops at the TCG level, instead of recomputing it at the TCG target level. But this is how TCG is currently designed, so: Reviewed-by: Aurelien Jarno <[email protected]> -- Aurelien Jarno GPG: 4096R/1DDD8C9B [email protected] http://aurel32.net
