[TableGen][GISel] Fix incorrect binding of predicate operands upon PredicateUsesOperands = 1
#68125
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…redicateUsesOperands = 1` 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.
mshockwave
requested review from
dsandersllvm,
topperc,
Pierre-vh and
nitinjohnraj
|
@llvm/pr-subscribers-llvm-globalisel ChangesWhen Full diff: https://github.com/llvm/llvm-project/pull/68125.diff 2 Files Affected:
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")) {
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When
PredicateUsesOperandsis 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.