[globalisel] Prevent infinite recursion in register bank emitter.

[rovka]

Ugh, hasSubClass looks a lot less friendly now, but it's probably too late to change terminology. I think it would be wise to add a comment to hasSubClass's documentation saying that it's not necessarily anti-symmetric, to avoid this kind of issue in the future.
/end rant

LGTM.

test/TableGen/RegisterBankEmitter.td

11

Nitpick: were you going to use Size instead of 32 here? :)

[dsanders]

I haven't posted this to the list yet since I created it to illustrate an alternative solution to D29049 and I've taken bits of Tom's patch without asking. If it solves his problem then he can either update his patch or he can give me permission to use the bit I took from his patch in which case I'll re-post to the list.

test/TableGen/RegisterBankEmitter.td

11

It's setting RegisterClass.Size on both ClassA and ClassB which isn't available from the parameter list (the third argument is the alignment).

[rovka]

Ah, sorry, didn't see that one. I'll let you 2 sort it out then.

[dsanders]

No worries. Thanks for the review

test/TableGen/RegisterBankEmitter.td

+// RUN: llvm-tblgen -gen-register-bank -I %p/../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def MyTarget : Target;
+def R0 : Register<"r0">;
+// CHECK: GPRRegBankCoverageData
+// CHECK: 1u << (MyTarget::ClassARegClassID
+// CHECK: 1u << (MyTarget::ClassBRegClassID
+let Size = 32 in {
+  def ClassA : RegisterClass<"MyTarget", [i32], 32, (add R0)>;
+  def ClassB : RegisterClass<"MyTarget", [i1], 32, (add ClassA)>;
+}
+// CHECK: MyTarget::GPRRegBank
+def GPRRegBank : RegisterBank<"GPR", [ClassA]>;

utils/TableGen/RegisterBankEmitter.cpp

 ///
 /// \param RC The register class to search.
 /// \param Kind A debug string containing the path the visitor took to reach RC.
 /// \param VisitFn The action to take for each class visited. It may be called
 ///                multiple times for a given class if there are multiple paths
 ///                to the class.
 static void visitRegisterBankClasses(
     CodeGenRegBank &RegisterClassHierarchy, const CodeGenRegisterClass *RC,
-    const Twine Kind,
+    const Twine Kind, BitVector &Visited,
     std::function<void(const CodeGenRegisterClass *, StringRef)> VisitFn) {
+  if (Visited[RC->EnumValue]) {
+    DEBUG(dbgs() << "Not re-visiting " << RC->getName() << "\n");
+    return;
+  }
+  Visited[RC->EnumValue] = true;
+
   // Visit each explicitly named class.
   VisitFn(RC, Kind.str());
 
   for (const auto &PossibleSubclass : RegisterClassHierarchy.getRegClasses()) {
     std::string TmpKind =
         (Twine(Kind) + " (" + PossibleSubclass.getName() + ")").str();
 
     // Visit each subclass of an explicitly named class.
     if (RC != &PossibleSubclass && RC->hasSubClass(&PossibleSubclass))
       visitRegisterBankClasses(RegisterClassHierarchy, &PossibleSubclass,
                                TmpKind + " " + RC->getName() + " subclass",
-                               VisitFn);
+                               Visited, VisitFn);
 
     // Visit each class that contains only subregisters of RC with a common
     // subregister-index.
     //
     // More precisely, PossibleSubclass is a subreg-class iff Reg:SubIdx is in
     // PossibleSubclass for all registers Reg from RC using any
     // subregister-index SubReg
     for (const auto &SubIdx : RegisterClassHierarchy.getSubRegIndices()) {
void RegisterBankEmitter::run(raw_ostream &OS) {
   StringRef TargetName = Targets[0]->getName();
 
   std::vector<RegisterBank> Banks;
   for (const auto &V : Records.getAllDerivedDefinitions("RegisterBank")) {
     RegisterBank Bank(*V);
 
     for (const CodeGenRegisterClass *RC :
          Bank.getExplictlySpecifiedRegisterClasses(RegisterClassHierarchy)) {
+      BitVector Visited(RegisterClassHierarchy.getRegClasses().size());
       visitRegisterBankClasses(
-          RegisterClassHierarchy, RC, "explicit",
+          RegisterClassHierarchy, RC, "explicit", Visited,
           [&Bank](const CodeGenRegisterClass *RC, StringRef Kind) {
             DEBUG(dbgs() << "Added " << RC->getName() << "(" << Kind << ")\n");
             Bank.addRegisterClass(RC);
           });
     }
 
     Banks.push_back(Bank);
   }