[TableGen] generate `getRegBankFromRegClass`

[paperchalice]

Summary

Generate getRegBankFromRegClass, referenced from ARM implementation. Currently LLT Ty is unused.

[gargaroff]

Do you plan to revive this at some point? It would be great to move more of the regbank boilerplate from C++ to TableGen

[paperchalice]

Raise warning when two register banks are overlapped.

[paperchalice]

Try remove some hand-write implementations.

[paperchalice]

Failed Tests (4):

cfi-devirt-lld-thinlto-x86_64 :: icall/bad-signature.c
cfi-devirt-lld-x86_64 :: icall/bad-signature.c
cfi-standalone-lld-thinlto-x86_64 :: icall/bad-signature.c
cfi-standalone-lld-x86_64 :: icall/bad-signature.c

[paperchalice]

Keep X86 implementation. Because 4 lld tests failed.

[craig.topper]

Please upload the patch with full context using -U99999 as documented here https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

[paperchalice]

Update patch with full context.

[paquette]

When I build llc with this patch, with AMDGPU as a target, I get a lot of warnings.

E.g.

Included from /Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPU.td:1439:
/Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPURegisterBanks.td:13:1: warning: Register class 'AV_128' is already mapped to register bank 'AGPR'.
def VGPRRegBank : RegisterBank<"VGPR",
^
Included from /Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPU.td:1439:
/Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPURegisterBanks.td:13:1: warning: Register class 'AV_160' is already mapped to register bank 'AGPR'.
def VGPRRegBank : RegisterBank<"VGPR",
^

Is that expected?

[arsenm]

In D99833#2686416, @paquette wrote:

When I build llc with this patch, with AMDGPU as a target, I get a lot of warnings.

E.g.

Included from /Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPU.td:1439:
/Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPURegisterBanks.td:13:1: warning: Register class 'AV_128' is already mapped to register bank 'AGPR'.
def VGPRRegBank : RegisterBank<"VGPR",
^
Included from /Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPU.td:1439:
/Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPURegisterBanks.td:13:1: warning: Register class 'AV_160' is already mapped to register bank 'AGPR'.
def VGPRRegBank : RegisterBank<"VGPR",
^

Is that expected?

AV_* classes are for operands that can be one of two different regbanks. You can't unambiguously go back from the class to the register bank. At the moment these are unallocatable classes, although this will probably be changing soon. I'm not really sure what this code should do in this case.

[paquette]

llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp

266 ↗

(On Diff #336713)

This register class is missing from the table-generated version of this function in AArch64.

[arsenm]

In D99833#2686464, @arsenm wrote:

In D99833#2686416, @paquette wrote:

When I build llc with this patch, with AMDGPU as a target, I get a lot of warnings.

E.g.

Included from /Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPU.td:1439:
/Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPURegisterBanks.td:13:1: warning: Register class 'AV_128' is already mapped to register bank 'AGPR'.
def VGPRRegBank : RegisterBank<"VGPR",
^
Included from /Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPU.td:1439:
/Users/gruyere/llvm-project/llvm/lib/Target/AMDGPU/AMDGPURegisterBanks.td:13:1: warning: Register class 'AV_160' is already mapped to register bank 'AGPR'.
def VGPRRegBank : RegisterBank<"VGPR",
^

Is that expected?

AV_* classes are for operands that can be one of two different regbanks. You can't unambiguously go back from the class to the register bank. At the moment these are unallocatable classes, although this will probably be changing soon. I'm not really sure what this code should do in this case.

Saying these are AGPR bank is definitely the wrong answer though. If it's going to be wrong, just saying VGPR would be a better choice

[paperchalice]

add missing WSeqPairsClass, XSeqPairsClass for GPRRegBank in AArch64

[RKSimon]

llvm/utils/TableGen/RegisterBankEmitter.cpp

305

QualifiedBankID is only used here - why create a std::string when you could just pass each of the the components to OS?

[RKSimon]

llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp

266 ↗

(On Diff #336713)

@paperchalice Have you been able to move these remaining classes to tablegen yet?

[paperchalice]

llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp

266 ↗

(On Diff #336713)

Some tests failed if changing def GPRRegBank : RegisterBank<"GPR", [GPR64all]>; to def GPRRegBank : RegisterBank<"GPR", [GPR64all, WSeqPairsClass, XSeqPairsClass]>;， because the code assume GPR is 64-bit, but the size of XSeqPairsClass is 128.

llvm/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h

   /// that are used in the description of instruction. In other words,
   /// there are just a handful of them and we do not want to waste space.
   ///
-  /// \todo This should be TableGen'ed.
   virtual const RegisterBank &
   getRegBankFromRegClass(const TargetRegisterClass &RC, LLT Ty) const {
     llvm_unreachable("The target must override this method");

llvm/test/TableGen/RegisterBankEmitter.td

 
 def MyTarget : Target;
 def R0 : Register<"r0">;
+def F1 : Register<"f1">;
+
 let Size = 32 in {
   def ClassA : RegisterClass<"MyTarget", [i32], 32, (add R0)>;
   def ClassB : RegisterClass<"MyTarget", [i1], 32, (add ClassA)>;
+  def ClassC : RegisterClass<"MyTarget", [i32], 32, (add R0)>;
+
+  def ClassD : RegisterClass<"MyTarget", [i32], 32, (add F1)>;
 }
 
 // CHECK: GPRRegBankCoverageData
 // CHECK: MyTarget::ClassARegClassID
 // CHECK: MyTarget::ClassBRegClassID
+
+// CHECK: MyTargetGenRegisterBankInfo::getRegBankFromRegClass
+// CHECK: case MyTarget::ClassDRegClassID:
+// CHECK-NEXT: return getRegBank(MyTarget::FPRRegBankID);
+
+// CHECK: case MyTarget::ClassARegClassID:
+// CHECK-NEXT: case MyTarget::ClassBRegClassID:
+// CHECK-NEXT: case MyTarget::ClassCRegClassID:
+// CHECK-NEXT: return getRegBank(MyTarget::GPRRegBankID);
 def GPRRegBank : RegisterBank<"GPR", [ClassA]>;
+def FPRRegBank : RegisterBank<"FPR", [ClassA, ClassD]>;

llvm/utils/TableGen/RegisterBankEmitter.cpp

      << "protected:\n"
      << "  " << TargetName << "GenRegisterBankInfo();\n"
      << "\n";
+  
+  OS << "  virtual const RegisterBank &\n"
+     << "  getRegBankFromRegClass(const TargetRegisterClass &RC, LLT Ty) const override;\n"
+     << "\n";
 }
 
 /// Visit each register class belonging to the given register bank.

      << "  for (const auto &RB : RegBanks)\n"
      << "    assert(Index++ == RB->getID() && \"Index != ID\");\n"
      << "#endif // NDEBUG\n"
-     << "}\n"
-     << "} // end namespace llvm\n";
+     << "}\n\n";
+
+  // generate getRegBankFromRegClass
+  {
+    OS << "const RegisterBank &\n"
+       << TargetName << "GenRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC, LLT Ty) const {\n"
+       << "  switch (RC.getID()) {\n";
+
+      for (const auto &Bank : Banks) {
+        std::string QualifiedBankID =
+          (TargetName + "::" + Bank.getEnumeratorName()).str();
+        for (const auto &RC : Bank.register_classes()) {
+          std::string QualifiedRegClassID =
+            (Twine(RC->Namespace) + "::" + RC->getName() + "RegClassID").str();
+          OS << "  case " << QualifiedRegClassID << ":\n";
+        }
+        OS << "    return getRegBank("<< QualifiedBankID << ");\n";
+      }
+
+    OS << ""
+       << "  default:\n"
+       << "    llvm_unreachable(\"Register class not supported\");\n"
+       << "  }\n"
+       << "}\n";
+  }
+  
+
+  OS << "} // end namespace llvm\n";
 }
 
 void RegisterBankEmitter::run(raw_ostream &OS) {