[llvm] c09f1fc - [GlobalISel][Tablegen] Fix SameOperandMatcher's isIdentical check
Konstantin Schwarz via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 28 04:37:46 PDT 2021
Author: Konstantin Schwarz
Date: 2021-10-28T13:37:12+02:00
New Revision: c09f1fc74c9b16ffbfa54827e4739c7576cc890b
URL: https://github.com/llvm/llvm-project/commit/c09f1fc74c9b16ffbfa54827e4739c7576cc890b
DIFF: https://github.com/llvm/llvm-project/commit/c09f1fc74c9b16ffbfa54827e4739c7576cc890b.diff
LOG: [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
Added:
llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand-invalid.td
llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td
Modified:
llvm/utils/TableGen/GlobalISelEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand-invalid.td b/llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand-invalid.td
new file mode 100644
index 0000000000000..44e1b08bdd829
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand-invalid.td
@@ -0,0 +1,102 @@
+// RUN: llvm-tblgen %s -gen-global-isel -optimize-match-table=true -I %p/../../include -I %p/Common -o - | FileCheck %s
+
+include "llvm/Target/Target.td"
+include "GlobalISelEmitterCommon.td"
+
+def InstTwoOperands : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
+def InstThreeOperands : I<(outs GPR32:$dst), (ins GPR32:$cond, GPR32:$src,GPR32:$src2), []>;
+
+// CHECK: GIM_Try, /*On fail goto*//*Label 0*/ 219,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_SELECT,
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 1*/ 189,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckIsSameOperand, /*MI*/0, /*OpIdx*/3, /*OtherMI*/2, /*OtherOpIdx*/2,
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 2*/ 108, // Rule ID 1 //
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_ICMP,
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/3, /*Type*/GILLT_s32,
+// CHECK-NEXT: // MIs[1] Operand 1
+// CHECK-NEXT: GIM_CheckCmpPredicate, /*MI*/1, /*Op*/1, /*Predicate*/CmpInst::ICMP_EQ,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckConstantInt, /*MI*/1, /*Op*/3, 0,
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/2, /*MI*/0, /*OpIdx*/2, // MIs[2]
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/2, TargetOpcode::G_SUB,
+// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/1, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/1, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
+// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/2,
+// 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)
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::InstThreeOperands,
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // cond
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/1, // src1
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/2, // src2
+// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
+// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
+// CHECK-NEXT: // GIR_Coverage, 1,
+// CHECK-NEXT: GIR_Done,
+// CHECK-NEXT: // Label 2: @108
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/ 188, // Rule ID 2 //
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, TargetOpcode::G_ICMP,
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckType, /*MI*/1, /*Op*/3, /*Type*/GILLT_s32,
+// CHECK-NEXT: // MIs[1] Operand 1
+// CHECK-NEXT: GIM_CheckCmpPredicate, /*MI*/1, /*Op*/1, /*Predicate*/CmpInst::ICMP_NE,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckConstantInt, /*MI*/1, /*Op*/3, 0,
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/2, /*MI*/0, /*OpIdx*/2, // MIs[2]
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/2, TargetOpcode::G_SUB,
+// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/1, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/1, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/2, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/1,
+// CHECK-NEXT: GIM_CheckIsSafeToFold, /*InsnID*/2,
+// 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)
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::InstThreeOperands,
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // cond
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/1, // src1
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/2, /*OpIdx*/2, // src2
+// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
+// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
+// CHECK-NEXT: // GIR_Coverage, 2,
+// CHECK-NEXT: GIR_Done,
+// CHECK-NEXT: // Label 3: @188
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: // Label 1: @189
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 4*/ 218, // Rule ID 0 //
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/3, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/1, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/2, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/3, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT: // (select:{ *:[i32] } GPR32:{ *:[i32] }:$cond, GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2) => (InstThreeOperands:{ *:[i32] } GPR32:{ *:[i32] }:$cond, GPR32:{ *:[i32] }:$src1, GPR32:{ *:[i32] }:$src2)
+// CHECK-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/MyTarget::InstThreeOperands,
+// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
+// CHECK-NEXT: // GIR_Coverage, 0,
+// CHECK-NEXT: GIR_Done,
+// CHECK-NEXT: // Label 4: @218
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: // Label 0: @219
+// CHECK-NEXT: GIM_Reject,
+// CHECK-NEXT: }
+def : Pat<(i32 (select GPR32:$cond, GPR32:$src1, GPR32:$src2)),
+ (InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;
+
+def : Pat<(i32 (select (i32 (setcc GPR32:$cond, (i32 0), (OtherVT SETEQ))),
+ (i32 (sub GPR32:$src1, GPR32:$src2)),
+ GPR32:$src2)),
+ (InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;
+
+def : Pat<(i32 (select (i32 (setcc GPR32:$cond, (i32 0), (OtherVT SETNE))),
+ (i32 (sub GPR32:$src1, GPR32:$src2)),
+ GPR32:$src2)),
+ (InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;
diff --git a/llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td b/llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td
new file mode 100644
index 0000000000000..167ab83b862b0
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td
@@ -0,0 +1,26 @@
+// RUN: llvm-tblgen %s -gen-global-isel -optimize-match-table=true -I %p/../../include -I %p/Common -o - | FileCheck %s
+
+include "llvm/Target/Target.td"
+include "GlobalISelEmitterCommon.td"
+
+def InstTwoOperands : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
+def InstThreeOperands : I<(outs GPR32:$dst), (ins GPR32:$cond, GPR32:$src,GPR32:$src2), []>;
+
+// Make sure the GIM_CheckIsSameOperand check is not hoisted into the common header group
+
+// CHECK: GIM_Try, /*On fail goto*//*Label 1*/
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NOT: GIM_CheckIsSameOperand
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 2*/
+// CHECK: GIM_CheckIsSameOperand, /*MI*/0, /*OpIdx*/3, /*OtherMI*/2, /*OtherOpIdx*/1,
+// CHECK: // Label 2
+// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/
+// CHECK: GIM_CheckIsSameOperand, /*MI*/0, /*OpIdx*/3, /*OtherMI*/2, /*OtherOpIdx*/2,
+// CHECK: // Label 1
+def : Pat<(i32 (select GPR32:$cond, GPR32:$src1, GPR32:$src2)),
+ (InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$src2)>;
+
+def : Pat<(i32 (select (i32 (setcc GPR32:$cond, (i32 0), (OtherVT SETEQ))),
+ (i32 (add GPR32:$src1, GPR32:$const)),
+ GPR32:$src1)),
+ (InstThreeOperands GPR32:$cond, GPR32:$src1, GPR32:$const)>;
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index 693073672fc11..d08186b7094b2 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -1212,11 +1212,13 @@ PredicateListMatcher<OperandPredicateMatcher>::getNoPredicateComment() const {
/// one as another.
class SameOperandMatcher : public OperandPredicateMatcher {
std::string MatchingName;
+ unsigned OrigOpIdx;
public:
- SameOperandMatcher(unsigned InsnVarID, unsigned OpIdx, StringRef MatchingName)
+ SameOperandMatcher(unsigned InsnVarID, unsigned OpIdx, StringRef MatchingName,
+ unsigned OrigOpIdx)
: OperandPredicateMatcher(OPM_SameOperand, InsnVarID, OpIdx),
- MatchingName(MatchingName) {}
+ MatchingName(MatchingName), OrigOpIdx(OrigOpIdx) {}
static bool classof(const PredicateMatcher *P) {
return P->getKind() == OPM_SameOperand;
@@ -1227,6 +1229,7 @@ class SameOperandMatcher : public OperandPredicateMatcher {
bool isIdentical(const PredicateMatcher &B) const override {
return OperandPredicateMatcher::isIdentical(B) &&
+ OrigOpIdx == cast<SameOperandMatcher>(&B)->OrigOpIdx &&
MatchingName == cast<SameOperandMatcher>(&B)->MatchingName;
}
};
@@ -3291,7 +3294,8 @@ void RuleMatcher::defineOperand(StringRef SymbolicName, OperandMatcher &OM) {
// If the operand is already defined, then we must ensure both references in
// the matcher have the exact same node.
- OM.addPredicate<SameOperandMatcher>(OM.getSymbolicName());
+ OM.addPredicate<SameOperandMatcher>(
+ OM.getSymbolicName(), getOperandMatcher(OM.getSymbolicName()).getOpIdx());
}
void RuleMatcher::definePhysRegOperand(Record *Reg, OperandMatcher &OM) {
More information about the llvm-commits
mailing list