Skip to content

Commit d36d8d6

Browse files
mshockwavemshockwave
authored
[TableGen][GISel] Fix incorrect binding of predicate operands upon PredicateUsesOperands = 1 (#68125)
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.
1 parent 4e9054d commit d36d8d6

File tree

2 files changed

+96
-15
lines changed

2 files changed

+96
-15
lines changed

‎llvm/test/TableGen/GlobalISelEmitterCustomPredicate.td

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// CHECK: // PatFrag predicates.
66
// CHECK-NEXT: enum {
77
// CHECK-NEXT: GICXXPred_MI_Predicate_and_or_pat = GICXXPred_Invalid + 1,
8+
// CHECK-NEXT: GICXXPred_MI_Predicate_mul_pat,
89
// CHECK-NEXT: GICXXPred_MI_Predicate_or_oneuse,
910
// CHECK-NEXT: GICXXPred_MI_Predicate_patfrags_test_pat,
1011
// CHECK-NEXT: GICXXPred_MI_Predicate_sub3_pat,
@@ -15,6 +16,8 @@
1516
// CHECK: bool MyTargetInstructionSelector::testMIPredicate_MI(
1617
// CHECK: case GICXXPred_MI_Predicate_and_or_pat: {
1718
// CHECK: return doesComplexCheck(MI);
19+
// CHECK: case GICXXPred_MI_Predicate_mul_pat: {
20+
// CHECK: return doesComplexCheck(MI);
1821
// CHECK: case GICXXPred_MI_Predicate_or_oneuse: {
1922
// CHECK: return MRI.hasOneNonDBGUse(MI.getOperand(0).getReg());
2023
// CHECK: case GICXXPred_MI_Predicate_patfrags_test_pat: {
@@ -49,6 +52,7 @@ def D0 : MyReg<"d0", [S0, S1]>;
4952
def DRegs : MyClass<32, [i32], (sequence "D%u", 0, 0)>;
5053
def DOP : RegisterOperand<DRegs>;
5154
def AND_OR : I<(outs DRegs:$dst), (ins DOP:$src0, DOP:$src1, DOP:$src2), []>;
55+
def MUL_OR : I<(outs DRegs:$dst), (ins DOP:$src0, DOP:$src1, DOP:$src2), []>;
5256

5357
def or_oneuse : PatFrag<
5458
(ops node:$x, node:$y),
@@ -69,7 +73,7 @@ def and_or_pat : PatFrag<
6973
let PredicateCodeUsesOperands = 1;
7074
}
7175

72-
// CHECK: GIM_Try, /*On fail goto*//*Label 0*/ 99, // Rule ID 6 //
76+
// CHECK: GIM_Try, /*On fail goto*//*Label 0*/ 99, // Rule ID 7 //
7377
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
7478
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_AND,
7579
// CHECK-NEXT: // MIs[0] dst
@@ -135,6 +139,79 @@ def : Pat<
135139
(AND_OR DOP:$src0, DOP:$src1, DOP:$src2)
136140
>;
137141

142+
def mul_pat : PatFrag<
143+
(ops node:$x, node:$y),
144+
(mul node:$x, node:$y), [{ return foo(); }]> {
145+
let GISelPredicateCode = [{
146+
return doesComplexCheck(MI);
147+
}];
148+
let PredicateCodeUsesOperands = 1;
149+
}
150+
151+
// CHECK: GIM_Try, /*On fail goto*//*Label 2*/ 293, // Rule ID 4 //
152+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
153+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_MUL,
154+
// CHECK-NEXT: // MIs[0] dst
155+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
156+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/Test::DRegsRegClassID,
157+
// CHECK-NEXT: // MIs[0] Operand 1
158+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
159+
// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/1, /*StoreIdx*/0, // Name : pred:4:x
160+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
161+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/1, /*Expected*/3,
162+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_OR,
163+
// CHECK-NEXT: // MIs[1] Operand 0
164+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/0, /*Type*/GILLT_s32,
165+
// CHECK-NEXT: // MIs[1] src0
166+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/1, /*Type*/GILLT_s32,
167+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/1, /*RC*/Test::DRegsRegClassID,
168+
// CHECK-NEXT: // MIs[1] src1
169+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
170+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/Test::DRegsRegClassID,
171+
// CHECK-NEXT: // MIs[0] src2
172+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
173+
// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/2, /*StoreIdx*/1, // Name : pred:4:y
174+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/2, /*RC*/Test::DRegsRegClassID,
175+
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GICXXPred_MI_Predicate_mul_pat,
176+
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
177+
// 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)
178+
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::MUL_OR,
179+
180+
// CHECK: GIM_Try, /*On fail goto*//*Label 3*/ 388, // Rule ID 8 //
181+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
182+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_MUL,
183+
// CHECK-NEXT: // MIs[0] dst
184+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
185+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/Test::DRegsRegClassID,
186+
// CHECK-NEXT: // MIs[0] src2
187+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
188+
// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/1, /*StoreIdx*/1, // Name : pred:4:y
189+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/1, /*RC*/Test::DRegsRegClassID,
190+
// CHECK-NEXT: // MIs[0] Operand 2
191+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
192+
// CHECK-NEXT: GIM_RecordNamedOperand, /*MI*/0, /*Op*/2, /*StoreIdx*/0, // Name : pred:4:x
193+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/2, // MIs[1]
194+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/1, /*Expected*/3,
195+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_OR,
196+
// CHECK-NEXT: // MIs[1] Operand 0
197+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/0, /*Type*/GILLT_s32,
198+
// CHECK-NEXT: // MIs[1] src0
199+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/1, /*Type*/GILLT_s32,
200+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/1, /*RC*/Test::DRegsRegClassID,
201+
// CHECK-NEXT: // MIs[1] src1
202+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
203+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/Test::DRegsRegClassID,
204+
// CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GICXXPred_MI_Predicate_mul_pat,
205+
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
206+
// 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)
207+
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::MUL_OR,
208+
209+
// Test commutative patterns where named operands in the source pattern are not
210+
// directly bound to PatFrag's operands.
211+
def : Pat<
212+
(i32 (mul_pat (or DOP:$src0, DOP:$src1), DOP:$src2)),
213+
(MUL_OR DOP:$src0, DOP:$src1, DOP:$src2)
214+
>;
138215

139216
def sub3_pat : PatFrag<
140217
(ops node:$x, node:$y, node:$z),
@@ -146,7 +223,7 @@ def sub3_pat : PatFrag<
146223
let PredicateCodeUsesOperands = 1;
147224
}
148225

149-
// CHECK: GIM_Try, /*On fail goto*//*Label 2*/ 285, // Rule ID 0 //
226+
// CHECK: GIM_Try, /*On fail goto*//*Label 4*/ 475, // Rule ID 0 //
150227
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
151228
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SUB,
152229
// CHECK-NEXT: // MIs[0] dst
@@ -192,16 +269,16 @@ def patfrags_test_pat : PatFrags<
192269
let PredicateCodeUsesOperands = 1;
193270
}
194271

195-
// CHECK: GIM_Try, /*On fail goto*//*Label 3*/ 372, // Rule ID 1 //
272+
// CHECK: GIM_Try, /*On fail goto*//*Label 5*/ 562, // Rule ID 1 //
196273
// 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)
197274

198-
// CHECK: GIM_Try, /*On fail goto*//*Label 4*/ 459, // Rule ID 2 //
275+
// CHECK: GIM_Try, /*On fail goto*//*Label 6*/ 649, // Rule ID 2 //
199276
// 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)
200277

201-
// CHECK: GIM_Try, /*On fail goto*//*Label 5*/ 546, // Rule ID 4 //
278+
// CHECK: GIM_Try, /*On fail goto*//*Label 7*/ 736, // Rule ID 5 //
202279
// 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)
203280

204-
// CHECK: GIM_Try, /*On fail goto*//*Label 6*/ 633, // Rule ID 5 //
281+
// CHECK: GIM_Try, /*On fail goto*//*Label 8*/ 823, // Rule ID 6 //
205282
// 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)
206283

207284

‎llvm/utils/TableGen/GlobalISelEmitter.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
357357
/// to the number of named operands that predicate expects. Store locations in
358358
/// StoreIdxForName correspond to the order in which operand names appear in
359359
/// predicate's argument list.
360-
/// When we visit named leaf operand and WaitingForNamedOperands is not zero,
361-
/// add matcher that will record operand and decrease counter.
360+
/// When we visit named operand and WaitingForNamedOperands is not zero, add
361+
/// matcher that will record operand and decrease counter.
362362
unsigned WaitingForNamedOperands = 0;
363363
StringMap<unsigned> StoreIdxForName;
364364

@@ -997,6 +997,17 @@ Error GlobalISelEmitter::importChildMatcher(
997997
to_string(*SrcChild) + ")");
998998
}
999999

1000+
// Try look up SrcChild for a (named) predicate operand if there is any.
1001+
if (WaitingForNamedOperands) {
1002+
auto &ScopedNames = SrcChild->getNamesAsPredicateArg();
1003+
if (!ScopedNames.empty()) {
1004+
auto PA = ScopedNames.begin();
1005+
std::string Name = getScopedName(PA->getScope(), PA->getIdentifier());
1006+
OM.addPredicate<RecordNamedOperandMatcher>(StoreIdxForName[Name], Name);
1007+
--WaitingForNamedOperands;
1008+
}
1009+
}
1010+
10001011
// Check for nested instructions.
10011012
if (!SrcChild->isLeaf()) {
10021013
if (SrcChild->getOperator()->isSubClassOf("ComplexPattern")) {
@@ -1061,13 +1072,6 @@ Error GlobalISelEmitter::importChildMatcher(
10611072
if (auto *ChildDefInit = dyn_cast<DefInit>(SrcChild->getLeafValue())) {
10621073
auto *ChildRec = ChildDefInit->getDef();
10631074

1064-
if (WaitingForNamedOperands) {
1065-
auto PA = SrcChild->getNamesAsPredicateArg().begin();
1066-
std::string Name = getScopedName(PA->getScope(), PA->getIdentifier());
1067-
OM.addPredicate<RecordNamedOperandMatcher>(StoreIdxForName[Name], Name);
1068-
--WaitingForNamedOperands;
1069-
}
1070-
10711075
// Check for register classes.
10721076
if (ChildRec->isSubClassOf("RegisterClass") ||
10731077
ChildRec->isSubClassOf("RegisterOperand")) {

0 commit comments

Comments
 (0)