[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

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

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

[paperchalice]

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

266

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

public:
   /// generated by TableGen).
   ///
   /// \note The mapping RC -> RegBank could be built while adding the
   /// coverage for the register banks. However, we do not do it, because,
   /// at least for now, we only need this information for register classes
   /// 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");
   }
 
   /// Get the cost of a copy from \p B to \p A, or put differently,
   /// get the cost of A = COPY B. Since register banks may cover
   /// different size, \p Size specifies what will be the size in bits

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

if (&A == &AArch64::FPRRegBank && &B == &AArch64::GPRRegBank)
     // FMOVDXr or FMOVSWr.
     return 4;
 
   return RegisterBankInfo::copyCost(A, B, Size);
 }
 
 const RegisterBank &
 AArch64RegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                                LLT) const {
+                                                LLT Ty) const {
   switch (RC.getID()) {
-  case AArch64::FPR8RegClassID:
-  case AArch64::FPR16RegClassID:
-  case AArch64::FPR16_loRegClassID:
-  case AArch64::FPR32_with_hsub_in_FPR16_loRegClassID:
-  case AArch64::FPR32RegClassID:
-  case AArch64::FPR64RegClassID:
-  case AArch64::FPR64_loRegClassID:
-  case AArch64::FPR128RegClassID:
-  case AArch64::FPR128_loRegClassID:
-  case AArch64::DDRegClassID:
-  case AArch64::DDDRegClassID:
-  case AArch64::DDDDRegClassID:
-  case AArch64::QQRegClassID:
-  case AArch64::QQQRegClassID:
-  case AArch64::QQQQRegClassID:
-    return getRegBank(AArch64::FPRRegBankID);
-  case AArch64::GPR32commonRegClassID:
-  case AArch64::GPR32RegClassID:
-  case AArch64::GPR32spRegClassID:
-  case AArch64::GPR32sponlyRegClassID:
-  case AArch64::GPR32argRegClassID:
-  case AArch64::GPR32allRegClassID:
-  case AArch64::GPR64commonRegClassID:
-  case AArch64::GPR64RegClassID:
-  case AArch64::GPR64spRegClassID:
-  case AArch64::GPR64sponlyRegClassID:
-  case AArch64::GPR64argRegClassID:
-  case AArch64::GPR64allRegClassID:
-  case AArch64::GPR64noipRegClassID:
-  case AArch64::GPR64common_and_GPR64noipRegClassID:
-  case AArch64::GPR64noip_and_tcGPR64RegClassID:
-  case AArch64::tcGPR64RegClassID:
-  case AArch64::rtcGPR64RegClassID:
   case AArch64::WSeqPairsClassRegClassID:
   case AArch64::XSeqPairsClassRegClassID:
     return getRegBank(AArch64::GPRRegBankID);
-  case AArch64::CCRRegClassID:
-    return getRegBank(AArch64::CCRegBankID);
   default:
-    llvm_unreachable("Register class not supported");
+    return AArch64GenRegisterBankInfo::getRegBankFromRegClass(RC, Ty);
   }
 }
 
 RegisterBankInfo::InstructionMappings
 AArch64RegisterBankInfo::getInstrAlternativeMappings(
     const MachineInstr &MI) const {
   const MachineFunction &MF = *MI.getParent()->getParent();
   const TargetSubtargetInfo &STI = MF.getSubtarget();

llvm/lib/Target/ARM/ARMRegisterBankInfo.h

 #include "ARMGenRegisterBank.inc"
 };
 
 /// This class provides the information for the target register banks.
 class ARMRegisterBankInfo final : public ARMGenRegisterBankInfo {
 public:
   ARMRegisterBankInfo(const TargetRegisterInfo &TRI);
 
-  const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT) const override;
-
   const InstructionMapping &
   getInstrMapping(const MachineInstr &MI) const override;
 };
 } // End llvm namespace.
 #endif

llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp

#ifndef NDEBUG
     ARM::checkPartialMappings();
     ARM::checkValueMappings();
 #endif
   };
 
   llvm::call_once(InitializeRegisterBankFlag, InitializeRegisterBankOnce);
 }
 
-const RegisterBank &
-ARMRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                            LLT) const {
-  using namespace ARM;
-
-  switch (RC.getID()) {
-  case GPRRegClassID:
-  case GPRwithAPSRRegClassID:
-  case GPRnoipRegClassID:
-  case GPRnopcRegClassID:
-  case GPRnoip_and_GPRnopcRegClassID:
-  case rGPRRegClassID:
-  case GPRspRegClassID:
-  case GPRnoip_and_tcGPRRegClassID:
-  case tcGPRRegClassID:
-  case tGPRRegClassID:
-  case tGPREvenRegClassID:
-  case tGPROddRegClassID:
-  case tGPR_and_tGPREvenRegClassID:
-  case tGPR_and_tGPROddRegClassID:
-  case tGPREven_and_tcGPRRegClassID:
-  case tGPREven_and_GPRnoip_and_tcGPRRegClassID:
-  case tGPROdd_and_tcGPRRegClassID:
-    return getRegBank(ARM::GPRRegBankID);
-  case HPRRegClassID:
-  case SPR_8RegClassID:
-  case SPRRegClassID:
-  case DPR_8RegClassID:
-  case DPRRegClassID:
-  case QPRRegClassID:
-    return getRegBank(ARM::FPRRegBankID);
-  default:
-    llvm_unreachable("Unsupported register kind");
-  }
-
-  llvm_unreachable("Switch should handle all register classes");
-}
-
 const RegisterBankInfo::InstructionMapping &
 ARMRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
   auto Opc = MI.getOpcode();
 
   // Try the default logic for non-generic instructions that are either copies
   // or already have some operands assigned to banks.
   if (!isPreISelGenericOpcode(Opc) || Opc == TargetOpcode::G_PHI) {
     const InstructionMapping &Mapping = getInstrMappingImpl(MI);

llvm/lib/Target/Mips/MipsRegisterBankInfo.h

 #include "MipsGenRegisterBank.inc"
 };
 
 /// This class provides the information for the target register banks.
 class MipsRegisterBankInfo final : public MipsGenRegisterBankInfo {
 public:
   MipsRegisterBankInfo(const TargetRegisterInfo &TRI);
 
-  const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT) const override;
-
   const InstructionMapping &
   getInstrMapping(const MachineInstr &MI) const override;
 
   /// Here we have to narrowScalar s64 operands to s32, combine away G_MERGE or
   /// G_UNMERGE and erase instructions that became dead in the process. We
   /// manually assign bank to def operand of all new instructions that were
   /// created in the process since they will not end up in RegBankSelect loop.
   void applyMappingImpl(const OperandsMapper &OpdMapper) const override;

llvm/lib/Target/Mips/MipsRegisterBankInfo.cpp

 } // end namespace Mips
 } // end namespace llvm
 
 using namespace llvm;
 
 MipsRegisterBankInfo::MipsRegisterBankInfo(const TargetRegisterInfo &TRI)
     : MipsGenRegisterBankInfo() {}
 
-const RegisterBank &
-MipsRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT) const {
-  using namespace Mips;
-
-  switch (RC.getID()) {
-  case Mips::GPR32RegClassID:
-  case Mips::CPU16Regs_and_GPRMM16ZeroRegClassID:
-  case Mips::GPRMM16MovePPairFirstRegClassID:
-  case Mips::CPU16Regs_and_GPRMM16MovePPairSecondRegClassID:
-  case Mips::GPRMM16MoveP_and_CPU16Regs_and_GPRMM16ZeroRegClassID:
-  case Mips::GPRMM16MovePPairFirst_and_GPRMM16MovePPairSecondRegClassID:
-  case Mips::SP32RegClassID:
-  case Mips::GP32RegClassID:
-    return getRegBank(Mips::GPRBRegBankID);
-  case Mips::FGRCCRegClassID:
-  case Mips::FGR32RegClassID:
-  case Mips::FGR64RegClassID:
-  case Mips::AFGR64RegClassID:
-  case Mips::MSA128BRegClassID:
-  case Mips::MSA128HRegClassID:
-  case Mips::MSA128WRegClassID:
-  case Mips::MSA128DRegClassID:
-    return getRegBank(Mips::FPRBRegBankID);
-  default:
-    llvm_unreachable("Register class not supported");
-  }
-}
-
 // Instructions where all register operands are floating point.
 static bool isFloatingPointOpcode(unsigned Opc) {
   switch (Opc) {
   case TargetOpcode::G_FCONSTANT:
   case TargetOpcode::G_FADD:
   case TargetOpcode::G_FSUB:
   case TargetOpcode::G_FMUL:
   case TargetOpcode::G_FDIV:

llvm/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">;
+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 F1, ClassA)>;
 }
 
 // CHECK: GPRRegBankCoverageData
 // CHECK: MyTarget::ClassARegClassID
 // CHECK: MyTarget::ClassBRegClassID
+
+// CHECK: MyTargetGenRegisterBankInfo::getRegBankFromRegClass
+// CHECK: case MyTarget::ClassARegClassID:
+// CHECK: return getRegBank(MyTarget::FPRRegBankID);
+
 def GPRRegBank : RegisterBank<"GPR", [ClassA]>;
+def FPRRegBank : RegisterBank<"FPR", [ClassC]>;

llvm/utils/TableGen/RegisterBankEmitter.cpp

 //===- RegisterBankEmitter.cpp - Generate a Register Bank Desc. -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This tablegen backend is responsible for emitting a description of a target
 // register bank for a code generator.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/BitVector.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 
 #include "CodeGenHwModes.h"
 #include "CodeGenRegisters.h"
 #include "CodeGenTarget.h"
 

 void RegisterBankEmitter::emitBaseClassDefinition(
     raw_ostream &OS, const StringRef TargetName,
     const std::vector<RegisterBank> &Banks) {
   OS << "private:\n"
      << "  static RegisterBank *RegBanks[];\n\n"
      << "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.
 ///
 /// A class belongs to the bank iff any of these apply:
 /// * It is explicitly specified
 /// * It is a subclass of a class that is a member.
 /// * It is a class containing subregisters of the registers of a class that
OS << TargetName << "GenRegisterBankInfo::" << TargetName
      << "    : RegisterBankInfo(RegBanks, " << TargetName
      << "::NumRegisterBanks) {\n"
      << "  // Assert that RegBank indices match their ID's\n"
      << "#ifndef NDEBUG\n"
      << "  unsigned Index = 0;\n"
      << "  for (const auto &RB : RegBanks)\n"
      << "    assert(Index++ == RB->getID() && \"Index != ID\");\n"
      << "#endif // NDEBUG\n"
-     << "}\n"
-     << "} // end namespace llvm\n";
+     << "}\n\n";
+
+  OS << "const RegisterBank &\n"
+     << TargetName
+     << "GenRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass "
+        "&RC, LLT Ty) const {\n"
+     << "  switch (RC.getID()) {\n";
+
+  std::map<std::string, StringRef> RCMap;
+  for (const auto &Bank : Banks) {
+    for (const auto &RC : Bank.register_classes()) {
+      std::string QualifiedRegClassID =
+          (Twine(RC->Namespace) + "::" + RC->getName() + "RegClassID").str();
+      if (RCMap.find(QualifiedRegClassID) == RCMap.end()) {
+        OS << "  case " << QualifiedRegClassID << ":\n";
+        RCMap.insert({QualifiedRegClassID, Bank.getName()});
+      } else {
+        std::string WarnStr = formatv(
+            "Register class '{0}' is already mapped to register bank '{1}'.",
+            RC->getName(), RCMap[QualifiedRegClassID]);
+        PrintWarning(Bank.getDef().getLoc(), WarnStr);
+      }
+    }
+    OS << "    return getRegBank(" << TargetName
+       << "::" << Bank.getEnumeratorName() << ");\n";
+  }
+
+  OS << ""
+     << "  default:\n"
+     << "    llvm_unreachable(\"Register class not supported\");\n"
+     << "  }\n"
+     << "}\n";
+
+  OS << "} // end namespace llvm\n";
 }
 
 void RegisterBankEmitter::run(raw_ostream &OS) {
   StringRef TargetName = Target.getName();
   const CodeGenRegBank &RegisterClassHierarchy = Target.getRegBank();
 
   Records.startTimer("Analyze records");
   std::vector<RegisterBank> Banks;