[llvm] [TableGen][GISel] Fix incorrect binding of predicate operands upon `PredicateUsesOperands = 1` (PR #68125)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 3 09:22:15 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-globalisel
<details>
<summary>Changes</summary>
When `PredicateUsesOperands` is set to true, GlobalISelEmitter preserves the original index of predicate operands and uses that information on each predicate usage. However, previously it only looked up the original index for "actual" operands (i.e. operands of a predicate usage) that are leaf nodes, which is an incorrect assumption.
This patch fix it by generalizing the acceptable kinds of actual operands for predicate as well as checking the existance of bound predicate operands.
---
Full diff: https://github.com/llvm/llvm-project/pull/68125.diff
2 Files Affected:
- (modified) llvm/test/TableGen/GlobalISelEmitterCustomPredicate.td (+83-6)
- (modified) llvm/utils/TableGen/GlobalISelEmitter.cpp (+13-9)
``````````diff
diff --git a/llvm/test/TableGen/GlobalISelEmitterCustomPredicate.td b/llvm/test/TableGen/GlobalISelEmitterCustomPredicate.td
index 2b6b3ed977ebf7b..d07ef4e300ee5fc 100644
--- a/llvm/test/TableGen/GlobalISelEmitterCustomPredicate.td
+++ b/llvm/test/TableGen/GlobalISelEmitterCustomPredicate.td
@@ -5,6 +5,7 @@
// CHECK: // PatFrag predicates.
// CHECK-NEXT: enum {
// CHECK-NEXT: GICXXPred_MI_Predicate_and_or_pat = GICXXPred_Invalid + 1,
+// CHECK-NEXT: GICXXPred_MI_Predicate_mul_pat,
// CHECK-NEXT: GICXXPred_MI_Predicate_or_oneuse,
// CHECK-NEXT: GICXXPred_MI_Predicate_patfrags_test_pat,
// CHECK-NEXT: GICXXPred_MI_Predicate_sub3_pat,
@@ -15,6 +16,8 @@
// CHECK: bool MyTargetInstructionSelector::testMIPredicate_MI(
// CHECK: case GICXXPred_MI_Predicate_and_or_pat: {
// CHECK: return doesComplexCheck(MI);
+// CHECK: case GICXXPred_MI_Predicate_mul_pat: {
+// CHECK: return doesComplexCheck(MI);
// CHECK: case GICXXPred_MI_Predicate_or_oneuse: {
// CHECK: return MRI.hasOneNonDBGUse(MI.getOperand(0).getReg());
// CHECK: case GICXXPred_MI_Predicate_patfrags_test_pat: {
@@ -49,6 +52,7 @@ def D0 : MyReg<"d0", [S0, S1]>;
def DRegs : MyClass<32, [i32], (sequence "D%u", 0, 0)>;
def DOP : RegisterOperand<DRegs>;
def AND_OR : I<(outs DRegs:$dst), (ins DOP:$src0, DOP:$src1, DOP:$src2), []>;
+def MUL_OR : I<(outs DRegs:$dst), (ins DOP:$src0, DOP:$src1, DOP:$src2), []>;
def or_oneuse : PatFrag<
(ops node:$x, node:$y),
@@ -69,7 +73,7 @@ def and_or_pat : PatFrag<
let PredicateCodeUsesOperands = 1;
}
-// CHECK: GIM_Try, /*On fail goto*//*Label 0*/ 99, // Rule ID 6 //
+// CHECK: GIM_Try, /*On fail goto*//*Label 0*/ 99, // Rule ID 7 //
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_AND,
// CHECK-NEXT: // MIs[0] dst
@@ -135,6 +139,79 @@ def : Pat<
(AND_OR DOP:$src0, DOP:$src1, DOP:$src2)
>;
+def mul_pat : PatFrag<
+ (ops node:$x, node:$y),
+ (mul node:$x, node:$y), [{ return foo(); }]> {
+ let GISelPredicateCode = [{
+ return doesComplexCheck(MI);
+ }];
+ let PredicateCodeUsesOperands = 1;
+}
+
+// CHECK: GIM_Try, /*On fail goto*//*Label 2*/ 293, // Rule ID 4 //
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_MUL,
+// CHECK-NEXT: // MIs[0] dst
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: // MIs[0] Operand 1
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/1, /*StoreIdx*/0, // Name : pred:4:x
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/1, /*Expected*/3,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_OR,
+// CHECK-NEXT: // MIs[1] Operand 0
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/0, /*Type*/GILLT_s32,
+// CHECK-NEXT: // MIs[1] src0
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/1, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/1, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: // MIs[1] src1
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: // MIs[0] src2
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/2, /*StoreIdx*/1, // Name : pred:4:y
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/2, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GICXXPred_MI_Predicate_mul_pat,
+// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
+// CHECK-NEXT: // (mul:{ *:[i32] } (or:{ *:[i32] } DOP:{ *:[i32] }:$src0, DOP:{ *:[i32] }:$src1):$pred:4:x, DOP:{ *:[i32] }:$src2:$pred:4:y)<<P:4:Predicate_mul_pat>> => (MUL_OR:{ *:[i32] } DOP:{ *:[i32] }:$src0, DOP:{ *:[i32] }:$src1, DOP:{ *:[i32] }:$src2)
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::MUL_OR,
+
+// CHECK: GIM_Try, /*On fail goto*//*Label 3*/ 388, // Rule ID 8 //
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_MUL,
+// CHECK-NEXT: // MIs[0] dst
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: // MIs[0] src2
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/1, /*StoreIdx*/1, // Name : pred:4:y
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/1, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: // MIs[0] Operand 2
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/2, /*StoreIdx*/0, // Name : pred:4:x
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/2, // MIs[1]
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/1, /*Expected*/3,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_OR,
+// CHECK-NEXT: // MIs[1] Operand 0
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/0, /*Type*/GILLT_s32,
+// CHECK-NEXT: // MIs[1] src0
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/1, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/1, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: // MIs[1] src1
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/Test::DRegsRegClassID,
+// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GICXXPred_MI_Predicate_mul_pat,
+// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
+// CHECK-NEXT: // (mul:{ *:[i32] } DOP:{ *:[i32] }:$src2:$pred:4:y, (or:{ *:[i32] } DOP:{ *:[i32] }:$src0, DOP:{ *:[i32] }:$src1):$pred:4:x)<<P:4:Predicate_mul_pat>> => (MUL_OR:{ *:[i32] } DOP:{ *:[i32] }:$src0, DOP:{ *:[i32] }:$src1, DOP:{ *:[i32] }:$src2)
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::MUL_OR,
+
+// Test commutative patterns where named operands in the source pattern are not
+// directly bound to PatFrag's operands.
+def : Pat<
+ (i32 (mul_pat (or DOP:$src0, DOP:$src1), DOP:$src2)),
+ (MUL_OR DOP:$src0, DOP:$src1, DOP:$src2)
+>;
def sub3_pat : PatFrag<
(ops node:$x, node:$y, node:$z),
@@ -146,7 +223,7 @@ def sub3_pat : PatFrag<
let PredicateCodeUsesOperands = 1;
}
-// CHECK: GIM_Try, /*On fail goto*//*Label 2*/ 285, // Rule ID 0 //
+// CHECK: GIM_Try, /*On fail goto*//*Label 4*/ 475, // Rule ID 0 //
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SUB,
// CHECK-NEXT: // MIs[0] dst
@@ -192,16 +269,16 @@ def patfrags_test_pat : PatFrags<
let PredicateCodeUsesOperands = 1;
}
-// CHECK: GIM_Try, /*On fail goto*//*Label 3*/ 372, // Rule ID 1 //
+// CHECK: GIM_Try, /*On fail goto*//*Label 5*/ 562, // Rule ID 1 //
// CHECK: // (xor:{ *:[i32] } (add:{ *:[i32] } i32:{ *:[i32] }:$src0:$pred:2:x, i32:{ *:[i32] }:$src1:$pred:2:y), i32:{ *:[i32] }:$src2:$pred:2:z)<<P:2:Predicate_patfrags_test_pat>> => (PATFRAGS:{ *:[i32] } i32:{ *:[i32] }:$src0, i32:{ *:[i32] }:$src1, i32:{ *:[i32] }:$src2)
-// CHECK: GIM_Try, /*On fail goto*//*Label 4*/ 459, // Rule ID 2 //
+// CHECK: GIM_Try, /*On fail goto*//*Label 6*/ 649, // Rule ID 2 //
// CHECK: // (xor:{ *:[i32] } (sub:{ *:[i32] } i32:{ *:[i32] }:$src0:$pred:2:x, i32:{ *:[i32] }:$src1:$pred:2:y), i32:{ *:[i32] }:$src2:$pred:2:z)<<P:2:Predicate_patfrags_test_pat>> => (PATFRAGS:{ *:[i32] } i32:{ *:[i32] }:$src0, i32:{ *:[i32] }:$src1, i32:{ *:[i32] }:$src2)
-// CHECK: GIM_Try, /*On fail goto*//*Label 5*/ 546, // Rule ID 4 //
+// CHECK: GIM_Try, /*On fail goto*//*Label 7*/ 736, // Rule ID 5 //
// CHECK: // (xor:{ *:[i32] } i32:{ *:[i32] }:$src2:$pred:2:z, (add:{ *:[i32] } i32:{ *:[i32] }:$src0:$pred:2:x, i32:{ *:[i32] }:$src1:$pred:2:y))<<P:2:Predicate_patfrags_test_pat>> => (PATFRAGS:{ *:[i32] } i32:{ *:[i32] }:$src0, i32:{ *:[i32] }:$src1, i32:{ *:[i32] }:$src2)
-// CHECK: GIM_Try, /*On fail goto*//*Label 6*/ 633, // Rule ID 5 //
+// CHECK: GIM_Try, /*On fail goto*//*Label 8*/ 823, // Rule ID 6 //
// CHECK: // (xor:{ *:[i32] } i32:{ *:[i32] }:$src2:$pred:2:z, (sub:{ *:[i32] } i32:{ *:[i32] }:$src0:$pred:2:x, i32:{ *:[i32] }:$src1:$pred:2:y))<<P:2:Predicate_patfrags_test_pat>> => (PATFRAGS:{ *:[i32] } i32:{ *:[i32] }:$src0, i32:{ *:[i32] }:$src1, i32:{ *:[i32] }:$src2)
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index 1dbd821f20fdc55..2ea48904466af45 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -357,8 +357,8 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
/// to the number of named operands that predicate expects. Store locations in
/// StoreIdxForName correspond to the order in which operand names appear in
/// predicate's argument list.
- /// When we visit named leaf operand and WaitingForNamedOperands is not zero,
- /// add matcher that will record operand and decrease counter.
+ /// When we visit named operand and WaitingForNamedOperands is not zero, add
+ /// matcher that will record operand and decrease counter.
unsigned WaitingForNamedOperands = 0;
StringMap<unsigned> StoreIdxForName;
@@ -997,6 +997,17 @@ Error GlobalISelEmitter::importChildMatcher(
to_string(*SrcChild) + ")");
}
+ // Try look up SrcChild for a (named) predicate operand if there is any.
+ if (WaitingForNamedOperands) {
+ auto &ScopedNames = SrcChild->getNamesAsPredicateArg();
+ if (!ScopedNames.empty()) {
+ auto PA = ScopedNames.begin();
+ std::string Name = getScopedName(PA->getScope(), PA->getIdentifier());
+ OM.addPredicate<RecordNamedOperandMatcher>(StoreIdxForName[Name], Name);
+ --WaitingForNamedOperands;
+ }
+ }
+
// Check for nested instructions.
if (!SrcChild->isLeaf()) {
if (SrcChild->getOperator()->isSubClassOf("ComplexPattern")) {
@@ -1061,13 +1072,6 @@ Error GlobalISelEmitter::importChildMatcher(
if (auto *ChildDefInit = dyn_cast<DefInit>(SrcChild->getLeafValue())) {
auto *ChildRec = ChildDefInit->getDef();
- if (WaitingForNamedOperands) {
- auto PA = SrcChild->getNamesAsPredicateArg().begin();
- std::string Name = getScopedName(PA->getScope(), PA->getIdentifier());
- OM.addPredicate<RecordNamedOperandMatcher>(StoreIdxForName[Name], Name);
- --WaitingForNamedOperands;
- }
-
// Check for register classes.
if (ChildRec->isSubClassOf("RegisterClass") ||
ChildRec->isSubClassOf("RegisterOperand")) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/68125
More information about the llvm-commits
mailing list