[TableGen][RegisterInfoEmitter] Make entries of base register class tables human-readable.

[kosarev]

Summary

Helps tracking changes in the tables on adding new register classes and
updating BaseClassOrder values.

Also eliminates tables translating base register class indexes into
TargetRegisterClass pointers.

[arsenm]

Does this improve compile time at all?

[kosarev]

Does this improve compile time at all?

Removing a level of indirection shouldn't it make slower and switching from uint8_t to uint16_t shouldn't cause any noticeable difference in compile time I think. Generating the tables should take same amount of time or less with the new code as it now stops traversing the base classes as soon as we found one that contains the register.

[critson]

The table indirection is about trying to keep memory usage/binary size down.
Since we have ~9000 registers, this increases the size of the tables ~18kb.

The code looks fine and I will accept it, but can you clarify the objective/context of the change (here or offline)?
e.g. are you trying to edit these tables by hand / have LIT tests that contains the table gen output?

[kosarev]

are you trying to edit these tables by hand / have LIT tests that contains the table gen output?

No, I needed to see how D156099 affects the choice of base classes, which is virtually impossible to do when they are represented as a long line of raw numbers.

The table indirection is about trying to keep memory usage/binary size down.
Since we have ~9000 registers, this increases the size of the tables ~18kb.

Yeah, I think considering the size of other tables it's not worth the extra complexity.

[critson]

LGTM

llvm/test/TableGen/RegisterInfoEmitter-BaseClassOrder.td

 
 // BaseC defined order overrides BaseA and BaseB
 def BaseC : RegisterClass<"MyTarget", [i32], 32, (sequence "R%u", 2, 3)> {
   let BaseClassOrder = 0;
 }
 
 def MyTarget : Target;
 
-// CHECK: static const TargetRegisterClass *BaseClasses[4] = {
-// CHECK-NEXT:   nullptr,
-// CHECK-NEXT:   &MyTarget::BaseCRegClass,
-// CHECK-NEXT:   &MyTarget::BaseARegClass,
-// CHECK-NEXT:   &MyTarget::BaseBRegClass,
-// CHECK-NEXT: }
-// CHECK-NEXT: static const uint8_t Mapping[8] = {
-// CHECK-NEXT:   0,2,2,1,1,3,3,0, };
+// CHECK:      static const uint16_t Mapping[8] = {
+// CHECK-NEXT:   InvalidRegClassID,  // NoRegister
+// CHECK-NEXT:   MyTarget::BaseARegClassID,  // R0
+// CHECK-NEXT:   MyTarget::BaseARegClassID,  // R1
+// CHECK-NEXT:   MyTarget::BaseCRegClassID,  // R2
+// CHECK-NEXT:   MyTarget::BaseCRegClassID,  // R3
+// CHECK-NEXT:   MyTarget::BaseBRegClassID,  // R4
+// CHECK-NEXT:   MyTarget::BaseBRegClassID,  // R5
+// CHECK-NEXT:   InvalidRegClassID,  // R6
+// CHECK-NEXT: };

llvm/utils/TableGen/RegisterInfoEmitter.cpp

RegisterInfoEmitter::runTargetDesc(raw_ostream &OS, CodeGenTarget &Target,
   if (!RegisterClasses.empty()) {
     // Collect base classes
     SmallVector<const CodeGenRegisterClass*> BaseClasses;
     for (const auto &RC : RegisterClasses) {
       if (RC.getBaseClassOrder())
         BaseClasses.push_back(&RC);
     }
     if (!BaseClasses.empty()) {
-      // Represent class indexes with uint8_t and allocate one index for nullptr
-      assert(BaseClasses.size() <= UINT8_MAX && "Too many base register classes");
+      assert(BaseClasses.size() < UINT16_MAX &&
+             "Too many base register classes");
 
       // Apply order
       struct BaseClassOrdering {
         bool operator()(const CodeGenRegisterClass *LHS, const CodeGenRegisterClass *RHS) const {
           return std::pair(*LHS->getBaseClassOrder(), LHS->EnumValue)
                < std::pair(*RHS->getBaseClassOrder(), RHS->EnumValue);
         }
       };
       llvm::stable_sort(BaseClasses, BaseClassOrdering());
 
-      // Build mapping for Regs (+1 for NoRegister)
-      std::vector<uint8_t> Mapping(Regs.size() + 1, 0);
-      for (int RCIdx = BaseClasses.size() - 1; RCIdx >= 0; --RCIdx) {
-        for (const auto Reg : BaseClasses[RCIdx]->getMembers())
-          Mapping[Reg->EnumValue] = RCIdx + 1;
-      }
-
       OS << "\n// Register to base register class mapping\n\n";
       OS << "\n";
       OS << "const TargetRegisterClass *" << ClassName
          << "::getPhysRegBaseClass(MCRegister Reg)"
          << " const {\n";
-      OS << "  static const TargetRegisterClass *BaseClasses[" << (BaseClasses.size() + 1) << "] = {\n";
-      OS << "    nullptr,\n";
-      for (const auto RC : BaseClasses)
-        OS << "    &" << RC->getQualifiedName() << "RegClass,\n";
-      OS << "  };\n";
-      OS << "  static const uint8_t Mapping[" << Mapping.size() << "] = {\n    ";
-      for (const uint8_t Value : Mapping)
-        OS << (unsigned)Value << ",";
-      OS << "  };\n\n";
-      OS << "  assert(Reg < sizeof(Mapping));\n";
-      OS << "  return BaseClasses[Mapping[Reg]];\n";
-      OS << "}\n";
+      OS << "  static const uint16_t InvalidRegClassID = UINT16_MAX;\n\n";
+      OS << "  static const uint16_t Mapping[" << Regs.size() + 1 << "] = {\n";
+      OS << "    InvalidRegClassID,  // NoRegister\n";
+      for (const CodeGenRegister &Reg : Regs) {
+        const CodeGenRegisterClass *BaseRC = nullptr;
+        for (const CodeGenRegisterClass *RC : BaseClasses) {
+          if (is_contained(RC->getMembers(), &Reg)) {
+            BaseRC = RC;
+            break;
+          }
+        }
+
+        OS << "    "
+           << (BaseRC ? BaseRC->getQualifiedName() + "RegClassID"
+                      : "InvalidRegClassID")
+           << ",  // " << Reg.getName() << "\n";
+      }
+      OS << "  };\n\n"
+            "  assert(Reg < ArrayRef(Mapping).size());\n"
+            "  unsigned RCID = Mapping[Reg];\n"
+            "  if (RCID == InvalidRegClassID)\n"
+            "    return nullptr;\n"
+            "  return RegisterClasses[RCID];\n"
+            "}\n";
     }
   }
 
   // Emit the constructor of the class...
   OS << "extern const MCRegisterDesc " << TargetName << "RegDesc[];\n";
   OS << "extern const int16_t " << TargetName << "RegDiffLists[];\n";
   OS << "extern const LaneBitmask " << TargetName << "LaneMaskLists[];\n";
   OS << "extern const char " << TargetName << "RegStrings[];\n";