[TableGen] Handle duplicate rules in combiners

llvm/test/TableGen/GlobalISelCombinerEmitter/misc/redundant-combine-in-list.td

@@ -0,0 +1,30 @@
+// RUN: llvm-tblgen -I %p/../../../../include -gen-global-isel-combiner \
+// RUN:     -combiners=Combiner %s 2>&1 | FileCheck %s
+
+include "llvm/Target/Target.td"
+include "llvm/Target/GlobalISel/Combine.td"
+
+// Check we don't crash if a combine is present twice in the list.
+
+def MyTargetISA : InstrInfo;
+def MyTarget : Target { let InstructionSet = MyTargetISA; }
+
+def dummy;
+
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: warning: skipping rule 'Foo' because it has already been processed
+def Foo : GICombineRule<
+  (defs root:$root),
+  (match (G_ZEXT $root, $x)),
+  (apply (G_TRUNC $root, $x))>;
+
+def Bar : GICombineRule<
+  (defs root:$root),
+  (match (G_TRUNC $root, $x)),
+  (apply (G_ZEXT $root, $x))>;
+
+def FooBar : GICombineGroup<[ Foo, Bar ]>;
+
+def Combiner: GICombiner<"GenMyCombiner", [
+  FooBar,
+  Foo
+]>;

llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp

@@ -3310,6 +3310,10 @@ class GICombinerEmitter final : public GlobalISelMatchTableExecutorEmitter {
   // combine rule used to disable/enable it.
   std::vector<std::pair<unsigned, std::string>> AllCombineRules;
 
+  // Keep track of all rules we've seen so far to ensure we don't process
+  // the same rule twice.
+  StringSet<> RulesSeen;
+
   MatchTable buildMatchTable(MutableArrayRef<RuleMatcher> Rules);
 
   void emitRuleConfigImpl(raw_ostream &OS);
@@ -3627,27 +3631,37 @@ void GICombinerEmitter::gatherRules(
     std::vector<RuleMatcher> &ActiveRules,
     const std::vector<Record *> &&RulesAndGroups) {
   for (Record *Rec : RulesAndGroups) {
-    if (Rec->isValueUnset("Rules")) {
-      AllCombineRules.emplace_back(NextRuleID, Rec->getName().str());
-      CombineRuleBuilder CRB(Target, SubtargetFeatures, *Rec, NextRuleID++,
-                             ActiveRules);
+    if (!Rec->isValueUnset("Rules")) {
+      gatherRules(ActiveRules, Rec->getValueAsListOfDefs("Rules"));
+      continue;
+    }
 
-      if (!CRB.parseAll()) {
-        assert(ErrorsPrinted && "Parsing failed without errors!");
-        continue;
-      }
+    StringRef RuleName = Rec->getName();
+    if (!RulesSeen.insert(RuleName).second) {
+      PrintWarning(Rec->getLoc(),
+                   "skipping rule '" + Rec->getName() +
+                       "' because it has already been processed");
+      continue;
+    }
 
-      if (StopAfterParse) {
-        CRB.print(outs());
-        continue;
-      }
+    AllCombineRules.emplace_back(NextRuleID, Rec->getName().str());
+    CombineRuleBuilder CRB(Target, SubtargetFeatures, *Rec, NextRuleID++,
+                           ActiveRules);
 
-      if (!CRB.emitRuleMatchers()) {
-        assert(ErrorsPrinted && "Emission failed without errors!");
-        continue;
-      }
-    } else
-      gatherRules(ActiveRules, Rec->getValueAsListOfDefs("Rules"));
+    if (!CRB.parseAll()) {
+      assert(ErrorsPrinted && "Parsing failed without errors!");
+      continue;
+    }
+
+    if (StopAfterParse) {
+      CRB.print(outs());
+      continue;
+    }
+
+    if (!CRB.emitRuleMatchers()) {
+      assert(ErrorsPrinted && "Emission failed without errors!");
+      continue;
+    }
   }
 }