[PATCH] D111506: [GlobalISel][Tablegen] Fix SameOperandMatcher's isIdentical check

[Konstantin Schwarz via Phabricator via llvm-commits]

kschwarz created this revision.
kschwarz added reviewers: rtereshin, dsanders, qcolombet, arsenm.
Herald added a subscriber: rovka.
kschwarz requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111506

Files:
  llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td
  llvm/utils/TableGen/GlobalISelEmitter.cpp


Index: llvm/utils/TableGen/GlobalISelEmitter.cpp
===================================================================
--- llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -1212,11 +1212,13 @@
 /// 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 @@
 
   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 @@
 
   // 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) {
Index: llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td
===================================================================
--- /dev/null
+++ 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)>;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111506.378494.patch
Type: text/x-patch
Size: 3198 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211010/01f93d41/attachment.bin>