From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
1.Address immediate value constraints in RISC-V Vector Extension 1.0 for
comparison instructions.
2.Extend comparison results from mask registers to SEW-width elements,
following recommendations in The RISC-V SPEC Volume I (Version 20240411).
This aligns with TCG's cmp_vec behavior by expanding compare results to
full element width: all 1s for true, all 0s for false.
Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
---
tcg/riscv/tcg-target-con-set.h | 3 +
tcg/riscv/tcg-target-con-str.h | 2 +
tcg/riscv/tcg-target.c.inc | 252 +++++++++++++++++++++++++++------
tcg/riscv/tcg-target.h | 2 +-
tcg/tcg-internal.h | 2 +
tcg/tcg-op-vec.c | 2 +-
6 files changed, 214 insertions(+), 49 deletions(-)
diff --git a/tcg/riscv/tcg-target-con-set.h b/tcg/riscv/tcg-target-con-set.h
index d4504122a2..cc06102ccf 100644
--- a/tcg/riscv/tcg-target-con-set.h
+++ b/tcg/riscv/tcg-target-con-set.h
@@ -24,3 +24,6 @@ C_O2_I4(r, r, rZ, rZ, rM, rM)
C_O0_I2(v, r)
C_O1_I1(v, r)
C_O1_I1(v, v)
+C_O1_I2(v, v, v)
+C_O1_I2(v, v, vL)
+C_O1_I4(v, v, vL, vK, vK)
diff --git a/tcg/riscv/tcg-target-con-str.h b/tcg/riscv/tcg-target-con-str.h
index b2b3211bcb..089efe96ca 100644
--- a/tcg/riscv/tcg-target-con-str.h
+++ b/tcg/riscv/tcg-target-con-str.h
@@ -17,6 +17,8 @@ REGS('v', ALL_VECTOR_REGS)
*/
CONST('I', TCG_CT_CONST_S12)
CONST('J', TCG_CT_CONST_J12)
+CONST('K', TCG_CT_CONST_S5)
+CONST('L', TCG_CT_CONST_CMP_VI)
CONST('N', TCG_CT_CONST_N12)
CONST('M', TCG_CT_CONST_M12)
CONST('Z', TCG_CT_CONST_ZERO)
diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
index c89d1a5dc9..37909e56fb 100644
--- a/tcg/riscv/tcg-target.c.inc
+++ b/tcg/riscv/tcg-target.c.inc
@@ -106,11 +106,13 @@ static TCGReg tcg_target_call_oarg_reg(TCGCallReturnKind kind, int slot)
return TCG_REG_A0 + slot;
}
-#define TCG_CT_CONST_ZERO 0x100
-#define TCG_CT_CONST_S12 0x200
-#define TCG_CT_CONST_N12 0x400
-#define TCG_CT_CONST_M12 0x800
-#define TCG_CT_CONST_J12 0x1000
+#define TCG_CT_CONST_ZERO 0x100
+#define TCG_CT_CONST_S12 0x200
+#define TCG_CT_CONST_N12 0x400
+#define TCG_CT_CONST_M12 0x800
+#define TCG_CT_CONST_J12 0x1000
+#define TCG_CT_CONST_S5 0x2000
+#define TCG_CT_CONST_CMP_VI 0x4000
#define ALL_GENERAL_REGS MAKE_64BIT_MASK(0, 32)
#define ALL_VECTOR_REGS MAKE_64BIT_MASK(32, 32)
@@ -119,48 +121,6 @@ static TCGReg tcg_target_call_oarg_reg(TCGCallReturnKind kind, int slot)
#define sextreg sextract64
-/* test if a constant matches the constraint */
-static bool tcg_target_const_match(int64_t val, int ct,
- TCGType type, TCGCond cond, int vece)
-{
- if (ct & TCG_CT_CONST) {
- return 1;
- }
- if ((ct & TCG_CT_CONST_ZERO) && val == 0) {
- return 1;
- }
- /*
- * Sign extended from 12 bits: [-0x800, 0x7ff].
- * Used for most arithmetic, as this is the isa field.
- */
- if ((ct & TCG_CT_CONST_S12) && val >= -0x800 && val <= 0x7ff) {
- return 1;
- }
- /*
- * Sign extended from 12 bits, negated: [-0x7ff, 0x800].
- * Used for subtraction, where a constant must be handled by ADDI.
- */
- if ((ct & TCG_CT_CONST_N12) && val >= -0x7ff && val <= 0x800) {
- return 1;
- }
- /*
- * Sign extended from 12 bits, +/- matching: [-0x7ff, 0x7ff].
- * Used by addsub2 and movcond, which may need the negative value,
- * and requires the modified constant to be representable.
- */
- if ((ct & TCG_CT_CONST_M12) && val >= -0x7ff && val <= 0x7ff) {
- return 1;
- }
- /*
- * Inverse of sign extended from 12 bits: ~[-0x800, 0x7ff].
- * Used to map ANDN back to ANDI, etc.
- */
- if ((ct & TCG_CT_CONST_J12) && ~val >= -0x800 && ~val <= 0x7ff) {
- return 1;
- }
- return 0;
-}
-
/*
* RISC-V Base ISA opcodes (IM)
*/
@@ -310,6 +270,9 @@ typedef enum {
OPC_VS4R_V = 0x2000027 | V_UNIT_STRIDE_WHOLE_REG | V_NF(3),
OPC_VS8R_V = 0x2000027 | V_UNIT_STRIDE_WHOLE_REG | V_NF(7),
+ OPC_VMERGE_VIM = 0x5c000057 | V_OPIVI,
+ OPC_VMERGE_VVM = 0x5c000057 | V_OPIVV,
+
OPC_VADD_VV = 0x57 | V_OPIVV,
OPC_VSUB_VV = 0x8000057 | V_OPIVV,
OPC_VAND_VV = 0x24000057 | V_OPIVV,
@@ -317,6 +280,29 @@ typedef enum {
OPC_VXOR_VV = 0x2c000057 | V_OPIVV,
OPC_VXOR_VI = 0x2c000057 | V_OPIVI,
+ OPC_VMSEQ_VV = 0x60000057 | V_OPIVV,
+ OPC_VMSEQ_VI = 0x60000057 | V_OPIVI,
+ OPC_VMSEQ_VX = 0x60000057 | V_OPIVX,
+ OPC_VMSNE_VV = 0x64000057 | V_OPIVV,
+ OPC_VMSNE_VI = 0x64000057 | V_OPIVI,
+ OPC_VMSNE_VX = 0x64000057 | V_OPIVX,
+
+ OPC_VMSLTU_VV = 0x68000057 | V_OPIVV,
+ OPC_VMSLTU_VX = 0x68000057 | V_OPIVX,
+ OPC_VMSLT_VV = 0x6c000057 | V_OPIVV,
+ OPC_VMSLT_VX = 0x6c000057 | V_OPIVX,
+ OPC_VMSLEU_VV = 0x70000057 | V_OPIVV,
+ OPC_VMSLEU_VX = 0x70000057 | V_OPIVX,
+ OPC_VMSLE_VV = 0x74000057 | V_OPIVV,
+ OPC_VMSLE_VX = 0x74000057 | V_OPIVX,
+
+ OPC_VMSLEU_VI = 0x70000057 | V_OPIVI,
+ OPC_VMSLE_VI = 0x74000057 | V_OPIVI,
+ OPC_VMSGTU_VI = 0x78000057 | V_OPIVI,
+ OPC_VMSGTU_VX = 0x78000057 | V_OPIVX,
+ OPC_VMSGT_VI = 0x7c000057 | V_OPIVI,
+ OPC_VMSGT_VX = 0x7c000057 | V_OPIVX,
+
OPC_VMV_V_V = 0x5e000057 | V_OPIVV,
OPC_VMV_V_I = 0x5e000057 | V_OPIVI,
OPC_VMV_V_X = 0x5e000057 | V_OPIVX,
@@ -324,6 +310,97 @@ typedef enum {
OPC_VMVNR_V = 0x9e000057 | V_OPIVI,
} RISCVInsn;
+static const struct {
+ RISCVInsn op;
+ bool swap;
+} tcg_cmpcond_to_rvv_vv[] = {
+ [TCG_COND_EQ] = { OPC_VMSEQ_VV, false },
+ [TCG_COND_NE] = { OPC_VMSNE_VV, false },
+ [TCG_COND_LT] = { OPC_VMSLT_VV, false },
+ [TCG_COND_GE] = { OPC_VMSLE_VV, true },
+ [TCG_COND_GT] = { OPC_VMSLT_VV, true },
+ [TCG_COND_LE] = { OPC_VMSLE_VV, false },
+ [TCG_COND_LTU] = { OPC_VMSLTU_VV, false },
+ [TCG_COND_GEU] = { OPC_VMSLEU_VV, true },
+ [TCG_COND_GTU] = { OPC_VMSLTU_VV, true },
+ [TCG_COND_LEU] = { OPC_VMSLEU_VV, false }
+};
+
+static const struct {
+ RISCVInsn op;
+ int min;
+ int max;
+ bool adjust;
+} tcg_cmpcond_to_rvv_vi[] = {
+ [TCG_COND_EQ] = { OPC_VMSEQ_VI, -16, 15, false },
+ [TCG_COND_NE] = { OPC_VMSNE_VI, -16, 15, false },
+ [TCG_COND_GT] = { OPC_VMSGT_VI, -16, 15, false },
+ [TCG_COND_LE] = { OPC_VMSLE_VI, -16, 15, false },
+ [TCG_COND_LT] = { OPC_VMSLE_VI, -15, 16, true },
+ [TCG_COND_GE] = { OPC_VMSGT_VI, -15, 16, true },
+ [TCG_COND_LEU] = { OPC_VMSLEU_VI, 0, 15, false },
+ [TCG_COND_GTU] = { OPC_VMSGTU_VI, 0, 15, false },
+ [TCG_COND_LTU] = { OPC_VMSLEU_VI, 1, 16, true },
+ [TCG_COND_GEU] = { OPC_VMSGTU_VI, 1, 16, true },
+};
+
+/* test if a constant matches the constraint */
+static bool tcg_target_const_match(int64_t val, int ct,
+ TCGType type, TCGCond cond, int vece)
+{
+ if (ct & TCG_CT_CONST) {
+ return 1;
+ }
+ if ((ct & TCG_CT_CONST_ZERO) && val == 0) {
+ return 1;
+ }
+ /*
+ * Sign extended from 12 bits: [-0x800, 0x7ff].
+ * Used for most arithmetic, as this is the isa field.
+ */
+ if ((ct & TCG_CT_CONST_S12) && val >= -0x800 && val <= 0x7ff) {
+ return 1;
+ }
+ /*
+ * Sign extended from 12 bits, negated: [-0x7ff, 0x800].
+ * Used for subtraction, where a constant must be handled by ADDI.
+ */
+ if ((ct & TCG_CT_CONST_N12) && val >= -0x7ff && val <= 0x800) {
+ return 1;
+ }
+ /*
+ * Sign extended from 12 bits, +/- matching: [-0x7ff, 0x7ff].
+ * Used by addsub2 and movcond, which may need the negative value,
+ * and requires the modified constant to be representable.
+ */
+ if ((ct & TCG_CT_CONST_M12) && val >= -0x7ff && val <= 0x7ff) {
+ return 1;
+ }
+ /*
+ * Inverse of sign extended from 12 bits: ~[-0x800, 0x7ff].
+ * Used to map ANDN back to ANDI, etc.
+ */
+ if ((ct & TCG_CT_CONST_J12) && ~val >= -0x800 && ~val <= 0x7ff) {
+ return 1;
+ }
+ /*
+ * Sign extended from 5 bits: [-0x10, 0x0f].
+ * Used for vector-immediate.
+ */
+ if ((ct & TCG_CT_CONST_S5) && val >= -0x10 && val <= 0x0f) {
+ return 1;
+ }
+ /*
+ * Used for vector compare OPIVI instructions.
+ */
+ if ((ct & TCG_CT_CONST_CMP_VI) &&
+ val >= tcg_cmpcond_to_rvv_vi[cond].min &&
+ val <= tcg_cmpcond_to_rvv_vi[cond].max) {
+ return true;
+ }
+ return 0;
+}
+
/*
* RISC-V immediate and instruction encoders (excludes 16-bit RVC)
*/
@@ -592,6 +669,18 @@ static void tcg_out_opc_vi(TCGContext *s, RISCVInsn opc, TCGReg vd,
tcg_out32(s, encode_vi(opc, vd, imm, vs2, vm));
}
+static void tcg_out_opc_vim_mask(TCGContext *s, RISCVInsn opc, TCGReg vd,
+ TCGReg vs2, int32_t imm)
+{
+ tcg_out32(s, encode_vi(opc, vd, imm, vs2, false));
+}
+
+static void tcg_out_opc_vvm_mask(TCGContext *s, RISCVInsn opc, TCGReg vd,
+ TCGReg vs2, TCGReg vs1)
+{
+ tcg_out32(s, encode_v(opc, vd, vs1, vs2, false));
+}
+
/*
* Only unit-stride addressing implemented; may extend in future.
*/
@@ -2322,6 +2411,51 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
riscv_set_vec_config_vl(s, type);
tcg_out_opc_vi(s, OPC_VXOR_VI, a0, a1, -1, true);
break;
+ case INDEX_op_cmpsel_vec:
+ TCGArg a3, a4;
+ int c3, c4;
+ TCGCond cond;
+
+ a3 = args[3];
+ a4 = args[4];
+ c3 = const_args[3];
+ c4 = const_args[4];
+ cond = args[5];
+ riscv_set_vec_config_vl_vece(s, type, vece);
+
+ /* Use only vmerge_vim if possible, by inverting the test. */
+ if (c4 && !c3) {
+ cond = tcg_invert_cond(cond);
+ a3 = a4;
+ a4 = args[3];
+ c3 = true;
+ c4 = false;
+ }
+
+ /* Perform the comparison into V0 mask. */
+ if (const_args[2]) {
+ tcg_out_opc_vi(s, tcg_cmpcond_to_rvv_vi[cond].op,
+ TCG_REG_V0, a1,
+ a2 - tcg_cmpcond_to_rvv_vi[cond].adjust, true);
+ } else if (tcg_cmpcond_to_rvv_vv[cond].swap) {
+ tcg_out_opc_vv(s, tcg_cmpcond_to_rvv_vv[cond].op,
+ TCG_REG_V0, a2, a1, true);
+ } else {
+ tcg_out_opc_vv(s, tcg_cmpcond_to_rvv_vv[cond].op,
+ TCG_REG_V0, a1, a2, true);
+ }
+ if (c3) {
+ if (c4) {
+ tcg_out_opc_vi(s, OPC_VMV_V_I, a0, TCG_REG_V0, a4, true);
+ a4 = a0;
+ }
+ /* vd[i] == v0.mask[i] ? imm : vs2[i] */
+ tcg_out_opc_vim_mask(s, OPC_VMERGE_VIM, a0, a4, a3);
+ } else {
+ /* vd[i] == v0.mask[i] ? vs1[i] : vs2[i] */
+ tcg_out_opc_vvm_mask(s, OPC_VMERGE_VVM, a0, a4, a3);
+ }
+ break;
case INDEX_op_mov_vec: /* Always emitted via tcg_out_mov. */
case INDEX_op_dup_vec: /* Always emitted via tcg_out_dup_vec. */
default:
@@ -2332,10 +2466,27 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
void tcg_expand_vec_op(TCGOpcode opc, TCGType type, unsigned vece,
TCGArg a0, ...)
{
+ va_list va;
+ TCGv_vec v0, v1;
+ TCGArg a2, a3;
+
+ va_start(va, a0);
+ v0 = temp_tcgv_vec(arg_temp(a0));
+ v1 = temp_tcgv_vec(arg_temp(va_arg(va, TCGArg)));
+ a2 = va_arg(va, TCGArg);
+
switch (opc) {
+ case INDEX_op_cmp_vec:
+ a3 = va_arg(va, TCGArg);
+ vec_gen_6(INDEX_op_cmpsel_vec, type, vece, tcgv_vec_arg(v0),
+ tcgv_vec_arg(v1), a2,
+ tcgv_i64_arg(tcg_constant_i64(-1)),
+ tcgv_i64_arg(tcg_constant_i64(0)), a3);
+ break;
default:
g_assert_not_reached();
}
+ va_end(va);
}
int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)
@@ -2347,7 +2498,10 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)
case INDEX_op_or_vec:
case INDEX_op_xor_vec:
case INDEX_op_not_vec:
+ case INDEX_op_cmpsel_vec:
return 1;
+ case INDEX_op_cmp_vec:
+ return -1;
default:
return 0;
}
@@ -2506,6 +2660,10 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode op)
case INDEX_op_or_vec:
case INDEX_op_xor_vec:
return C_O1_I2(v, v, v);
+ case INDEX_op_cmp_vec:
+ return C_O1_I2(v, v, vL);
+ case INDEX_op_cmpsel_vec:
+ return C_O1_I4(v, v, vL, vK, vK);
default:
g_assert_not_reached();
}
diff --git a/tcg/riscv/tcg-target.h b/tcg/riscv/tcg-target.h
index acb8dfdf16..94034504b2 100644
--- a/tcg/riscv/tcg-target.h
+++ b/tcg/riscv/tcg-target.h
@@ -164,7 +164,7 @@ typedef enum {
#define TCG_TARGET_HAS_sat_vec 0
#define TCG_TARGET_HAS_minmax_vec 0
#define TCG_TARGET_HAS_bitsel_vec 0
-#define TCG_TARGET_HAS_cmpsel_vec 0
+#define TCG_TARGET_HAS_cmpsel_vec 1
#define TCG_TARGET_HAS_tst_vec 0
diff --git a/tcg/tcg-internal.h b/tcg/tcg-internal.h
index 9b0d982f65..512128e8a7 100644
--- a/tcg/tcg-internal.h
+++ b/tcg/tcg-internal.h
@@ -102,5 +102,7 @@ void tcg_gen_op6(TCGOpcode, TCGArg, TCGArg, TCGArg, TCGArg, TCGArg, TCGArg);
void vec_gen_2(TCGOpcode, TCGType, unsigned, TCGArg, TCGArg);
void vec_gen_3(TCGOpcode, TCGType, unsigned, TCGArg, TCGArg, TCGArg);
void vec_gen_4(TCGOpcode, TCGType, unsigned, TCGArg, TCGArg, TCGArg, TCGArg);
+void vec_gen_6(TCGOpcode opc, TCGType type, unsigned vece, TCGArg r,
+ TCGArg a, TCGArg b, TCGArg c, TCGArg d, TCGArg e);
#endif /* TCG_INTERNAL_H */
diff --git a/tcg/tcg-op-vec.c b/tcg/tcg-op-vec.c
index 84af210bc0..851322878c 100644
--- a/tcg/tcg-op-vec.c
+++ b/tcg/tcg-op-vec.c
@@ -172,7 +172,7 @@ void vec_gen_4(TCGOpcode opc, TCGType type, unsigned vece,
op->args[3] = c;
}
-static void vec_gen_6(TCGOpcode opc, TCGType type, unsigned vece, TCGArg r,
+void vec_gen_6(TCGOpcode opc, TCGType type, unsigned vece, TCGArg r,
TCGArg a, TCGArg b, TCGArg c, TCGArg d, TCGArg e)
{
TCGOp *op = tcg_emit_op(opc, 6);
--
2.43.0On 9/4/24 07:27, LIU Zhiwei wrote:
> @@ -2322,6 +2411,51 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
> riscv_set_vec_config_vl(s, type);
> tcg_out_opc_vi(s, OPC_VXOR_VI, a0, a1, -1, true);
> break;
> + case INDEX_op_cmpsel_vec:
> + TCGArg a3, a4;
> + int c3, c4;
> + TCGCond cond;
While I suppose this compiles, it's not great to have new variables added randomly within
a switch. At minimum, add { } around the block, but consider breaking out a separate
tcg_out_cmpsel function, akin to tcg_out_movcond et al.
> @@ -2332,10 +2466,27 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
> void tcg_expand_vec_op(TCGOpcode opc, TCGType type, unsigned vece,
> TCGArg a0, ...)
> {
> + va_list va;
> + TCGv_vec v0, v1;
> + TCGArg a2, a3;
> +
> + va_start(va, a0);
> + v0 = temp_tcgv_vec(arg_temp(a0));
> + v1 = temp_tcgv_vec(arg_temp(va_arg(va, TCGArg)));
> + a2 = va_arg(va, TCGArg);
> +
> switch (opc) {
> + case INDEX_op_cmp_vec:
> + a3 = va_arg(va, TCGArg);
> + vec_gen_6(INDEX_op_cmpsel_vec, type, vece, tcgv_vec_arg(v0),
> + tcgv_vec_arg(v1), a2,
> + tcgv_i64_arg(tcg_constant_i64(-1)),
> + tcgv_i64_arg(tcg_constant_i64(0)), a3);
> + break;
> default:
> g_assert_not_reached();
> }
> + va_end(va);
> }
Better to use "TCGArg a0, a1". Converting through arg_tmp + temp_tcgv_vec to v0/v1 and
then undoing that with tcgv_vec_arg is confusing.
r~
On 2024/9/5 15:12, Richard Henderson wrote:
> On 9/4/24 07:27, LIU Zhiwei wrote:
>> @@ -2322,6 +2411,51 @@ static void tcg_out_vec_op(TCGContext *s,
>> TCGOpcode opc,
>> riscv_set_vec_config_vl(s, type);
>> tcg_out_opc_vi(s, OPC_VXOR_VI, a0, a1, -1, true);
>> break;
>> + case INDEX_op_cmpsel_vec:
>> + TCGArg a3, a4;
>> + int c3, c4;
>> + TCGCond cond;
>
> While I suppose this compiles, it's not great to have new variables
> added randomly within a switch. At minimum, add { } around the block,
> but consider breaking out a separate tcg_out_cmpsel function, akin to
> tcg_out_movcond et al.
OK.
>
>> @@ -2332,10 +2466,27 @@ static void tcg_out_vec_op(TCGContext *s,
>> TCGOpcode opc,
>> void tcg_expand_vec_op(TCGOpcode opc, TCGType type, unsigned vece,
>> TCGArg a0, ...)
>> {
>> + va_list va;
>> + TCGv_vec v0, v1;
>> + TCGArg a2, a3;
>> +
>> + va_start(va, a0);
>> + v0 = temp_tcgv_vec(arg_temp(a0));
>> + v1 = temp_tcgv_vec(arg_temp(va_arg(va, TCGArg)));
>> + a2 = va_arg(va, TCGArg);
>> +
>> switch (opc) {
>> + case INDEX_op_cmp_vec:
>> + a3 = va_arg(va, TCGArg);
>> + vec_gen_6(INDEX_op_cmpsel_vec, type, vece, tcgv_vec_arg(v0),
>> + tcgv_vec_arg(v1), a2,
>> + tcgv_i64_arg(tcg_constant_i64(-1)),
>> + tcgv_i64_arg(tcg_constant_i64(0)), a3);
>> + break;
>> default:
>> g_assert_not_reached();
>> }
>> + va_end(va);
>> }
>
> Better to use "TCGArg a0, a1". Converting through arg_tmp +
> temp_tcgv_vec to v0/v1 and then undoing that with tcgv_vec_arg is
> confusing.
OK.
Thanks,
Zhiwei
>
>
> r~
© 2016 - 2024 Red Hat, Inc.