Skip to content

Commit c09f1fc

Browse files
konstantinschwarzkonstantinschwarz
committed
[GlobalISel][Tablegen] Fix SameOperandMatcher's isIdentical check
During rule optimization, identical SameOperandMatchers are hoisted into a common group, however previously only one operand index was considered. Commutable patterns can introduce SameOperandMatcher checks where the second index is commuted, resulting in a different check that cannot be hoisted. Reviewed By: qcolombet Differential Revision: https://reviews.llvm.org/D111506
1 parent 4d50803 commit c09f1fc

File tree

3 files changed

+135
-3
lines changed

3 files changed

+135
-3
lines changed

‎llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand-invalid.td

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// RUN: llvm-tblgen %s -gen-global-isel -optimize-match-table=true -I %p/../../include -I %p/Common -o - | FileCheck %s
2+
3+
include "llvm/Target/Target.td"
4+
include "GlobalISelEmitterCommon.td"
5+
6+
def InstTwoOperands : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
7+
def InstThreeOperands : I<(outs GPR32:$dst), (ins GPR32:$cond, GPR32:$src,GPR32:$src2), []>;
8+
9+
// CHECK: GIM_Try, /*On fail goto*//*Label 0*/ 219,
10+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SELECT,
11+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
12+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
13+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
14+
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 1*/ 189,
15+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
16+
// CHECK-NEXT: GIM_CheckIsSameOperand, /*MI*/0, /*OpIdx*/3, /*OtherMI*/2, /*OtherOpIdx*/2,
17+
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 2*/ 108, // Rule ID 1 //
18+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
19+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_ICMP,
20+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
21+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/3, /*Type*/GILLT_s32,
22+
// CHECK-NEXT: // MIs[1] Operand 1
23+
// CHECK-NEXT: GIM_CheckCmpPredicate, /*MI*/1, /*Op*/1, /*Predicate*/CmpInst::ICMP_EQ,
24+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
25+
// CHECK-NEXT: GIM_CheckConstantInt, /*MI*/1, /*Op*/3, 0,
26+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/2, /*MI*/0, /*OpIdx*/2, // MIs[2]
27+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/2, TargetOpcode::G_SUB,
28+
// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/1, /*Type*/GILLT_s32,
29+
// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/2, /*Type*/GILLT_s32,
30+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/1, /*RC*/MyTarget::GPR32RegClassID,
31+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
32+
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
33+
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/2,
34+
// CHECK-NEXT: // (select:{ *:[i32] } (setcc:{ *:[i32] } GPR32:{ *:[i32] }:$cond, 0:{ *:[i32] }, SETEQ:{ *:[Other] }), (sub:{ *:[i32] } GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2), GPR32:{ *:[i32] }:$src2) => (InstThreeOperands:{ *:[i32] } GPR32:{ *:[i32] }:$cond, GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2)
35+
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::InstThreeOperands,
36+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
37+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // cond
38+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/1, // src1
39+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/2, // src2
40+
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
41+
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
42+
// CHECK-NEXT: // GIR_Coverage, 1,
43+
// CHECK-NEXT: GIR_Done,
44+
// CHECK-NEXT: // Label 2: @108
45+
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/ 188, // Rule ID 2 //
46+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
47+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_ICMP,
48+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
49+
// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/3, /*Type*/GILLT_s32,
50+
// CHECK-NEXT: // MIs[1] Operand 1
51+
// CHECK-NEXT: GIM_CheckCmpPredicate, /*MI*/1, /*Op*/1, /*Predicate*/CmpInst::ICMP_NE,
52+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
53+
// CHECK-NEXT: GIM_CheckConstantInt, /*MI*/1, /*Op*/3, 0,
54+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/2, /*MI*/0, /*OpIdx*/2, // MIs[2]
55+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/2, TargetOpcode::G_SUB,
56+
// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/1, /*Type*/GILLT_s32,
57+
// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/2, /*Type*/GILLT_s32,
58+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/1, /*RC*/MyTarget::GPR32RegClassID,
59+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
60+
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
61+
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/2,
62+
// CHECK-NEXT: // (select:{ *:[i32] } (setcc:{ *:[i32] } GPR32:{ *:[i32] }:$cond, 0:{ *:[i32] }, SETNE:{ *:[Other] }), (sub:{ *:[i32] } GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2), GPR32:{ *:[i32] }:$src2) => (InstThreeOperands:{ *:[i32] } GPR32:{ *:[i32] }:$cond, GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2)
63+
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::InstThreeOperands,
64+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
65+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // cond
66+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/1, // src1
67+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/2, // src2
68+
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
69+
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
70+
// CHECK-NEXT: // GIR_Coverage, 2,
71+
// CHECK-NEXT: GIR_Done,
72+
// CHECK-NEXT: // Label 3: @188
73+
// CHECK-NEXT: GIM_Reject,
74+
// CHECK-NEXT: // Label 1: @189
75+
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 4*/ 218, // Rule ID 0 //
76+
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/3, /*Type*/GILLT_s32,
77+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
78+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/1, /*RC*/MyTarget::GPR32RegClassID,
79+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
80+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/3, /*RC*/MyTarget::GPR32RegClassID,
81+
// CHECK-NEXT: // (select:{ *:[i32] } GPR32:{ *:[i32] }:$cond, GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2) => (InstThreeOperands:{ *:[i32] } GPR32:{ *:[i32] }:$cond, GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2)
82+
// CHECK-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/MyTarget::InstThreeOperands,
83+
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
84+
// CHECK-NEXT: // GIR_Coverage, 0,
85+
// CHECK-NEXT: GIR_Done,
86+
// CHECK-NEXT: // Label 4: @218
87+
// CHECK-NEXT: GIM_Reject,
88+
// CHECK-NEXT: // Label 0: @219
89+
// CHECK-NEXT: GIM_Reject,
90+
// CHECK-NEXT: }
91+
def : Pat<(i32 (select GPR32:$cond, GPR32:$src1, GPR32:$src2)),
92+
(InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;
93+
94+
def : Pat<(i32 (select (i32 (setcc GPR32:$cond, (i32 0), (OtherVT SETEQ))),
95+
(i32 (sub GPR32:$src1, GPR32:$src2)),
96+
GPR32:$src2)),
97+
(InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;
98+
99+
def : Pat<(i32 (select (i32 (setcc GPR32:$cond, (i32 0), (OtherVT SETNE))),
100+
(i32 (sub GPR32:$src1, GPR32:$src2)),
101+
GPR32:$src2)),
102+
(InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;

‎llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: llvm-tblgen %s -gen-global-isel -optimize-match-table=true -I %p/../../include -I %p/Common -o - | FileCheck %s
2+
3+
include "llvm/Target/Target.td"
4+
include "GlobalISelEmitterCommon.td"
5+
6+
def InstTwoOperands : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
7+
def InstThreeOperands : I<(outs GPR32:$dst), (ins GPR32:$cond, GPR32:$src,GPR32:$src2), []>;
8+
9+
// Make sure the GIM_CheckIsSameOperand check is not hoisted into the common header group
10+
11+
// CHECK: GIM_Try, /*On fail goto*//*Label 1*/
12+
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
13+
// CHECK-NOT: GIM_CheckIsSameOperand
14+
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 2*/
15+
// CHECK: GIM_CheckIsSameOperand, /*MI*/0, /*OpIdx*/3, /*OtherMI*/2, /*OtherOpIdx*/1,
16+
// CHECK: // Label 2
17+
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/
18+
// CHECK: GIM_CheckIsSameOperand, /*MI*/0, /*OpIdx*/3, /*OtherMI*/2, /*OtherOpIdx*/2,
19+
// CHECK: // Label 1
20+
def : Pat<(i32 (select GPR32:$cond, GPR32:$src1, GPR32:$src2)),
21+
(InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;
22+
23+
def : Pat<(i32 (select (i32 (setcc GPR32:$cond, (i32 0), (OtherVT SETEQ))),
24+
(i32 (add GPR32:$src1, GPR32:$const)),
25+
GPR32:$src1)),
26+
(InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$const)>;

‎llvm/utils/TableGen/GlobalISelEmitter.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,11 +1212,13 @@ PredicateListMatcher<OperandPredicateMatcher>::getNoPredicateComment() const {
12121212
/// one as another.
12131213
class SameOperandMatcher : public OperandPredicateMatcher {
12141214
std::string MatchingName;
1215+
unsigned OrigOpIdx;
12151216

12161217
public:
1217-
SameOperandMatcher(unsigned InsnVarID, unsigned OpIdx, StringRef MatchingName)
1218+
SameOperandMatcher(unsigned InsnVarID, unsigned OpIdx, StringRef MatchingName,
1219+
unsigned OrigOpIdx)
12181220
: OperandPredicateMatcher(OPM_SameOperand, InsnVarID, OpIdx),
1219-
MatchingName(MatchingName) {}
1221+
MatchingName(MatchingName), OrigOpIdx(OrigOpIdx) {}
12201222

12211223
static bool classof(const PredicateMatcher *P) {
12221224
return P->getKind() == OPM_SameOperand;
@@ -1227,6 +1229,7 @@ class SameOperandMatcher : public OperandPredicateMatcher {
12271229

12281230
bool isIdentical(const PredicateMatcher &B) const override {
12291231
return OperandPredicateMatcher::isIdentical(B) &&
1232+
OrigOpIdx == cast<SameOperandMatcher>(&B)->OrigOpIdx &&
12301233
MatchingName == cast<SameOperandMatcher>(&B)->MatchingName;
12311234
}
12321235
};
@@ -3291,7 +3294,8 @@ void RuleMatcher::defineOperand(StringRef SymbolicName, OperandMatcher &OM) {
32913294

32923295
// If the operand is already defined, then we must ensure both references in
32933296
// the matcher have the exact same node.
3294-
OM.addPredicate<SameOperandMatcher>(OM.getSymbolicName());
3297+
OM.addPredicate<SameOperandMatcher>(
3298+
OM.getSymbolicName(), getOperandMatcher(OM.getSymbolicName()).getOpIdx());
32953299
}
32963300

32973301
void RuleMatcher::definePhysRegOperand(Record *Reg, OperandMatcher &OM) {

0 commit comments

Comments
 (0)