[GISel][TableGen] Generate getRegBankFromRegClass #99896

redstar · 2024-07-22T16:51:51Z

Generating the mapping from a register class to a register bank is complex:

there can be lots of register classes
the mapping may be ambiguos
- a register class can span several register banks (e.g. a register class containing all registers)
- the type information is not enough to decide which register bank to map to (e.g. a register class containing floating point and vector registers, and all register can represent a f64 value)

The approach taken here is to encode the register banks in an array indexed by the ID of the register class. To save space, the entries are packed into chunks of size 2^n.

llvmbot · 2024-07-22T16:52:21Z

@llvm/pr-subscribers-llvm-globalisel
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-backend-powerpc

Author: Kai Nacke (redstar)

Changes

Generating the mapping from a register class to a register bank is complex:

there can be lots of register classes
the mapping may be ambiguos
- a register class can span several register banks (e.g. a register class containing all registers)
- the type information is not enough to decide which register bank to map to (e.g. a register class containing floating point and vector registers, and all register can represent a f64 value)

The approach taken here is to encode the register banks in an array indexed by the ID of the register class. To save space, the entries are packed into chunks of size 2^n.

Patch is 21.98 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99896.diff

17 Files Affected:

(modified) llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp (+2-47)
(modified) llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h (+1-1)
(modified) llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp (-38)
(modified) llvm/lib/Target/ARM/ARMRegisterBankInfo.h (-3)
(modified) llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.cpp (-6)
(modified) llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.h (-3)
(modified) llvm/lib/Target/Mips/MipsRegisterBankInfo.cpp (-29)
(modified) llvm/lib/Target/Mips/MipsRegisterBankInfo.h (-3)
(modified) llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp (+1-19)
(modified) llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.h (+1)
(modified) llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp (-45)
(modified) llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.h (-3)
(modified) llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.cpp (-12)
(modified) llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.h (-2)
(modified) llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp (-27)
(modified) llvm/lib/Target/X86/GISel/X86RegisterBankInfo.h (-3)
(modified) llvm/utils/TableGen/RegisterBankEmitter.cpp (+86-2)

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 5616d063f70bc..d4445b0a89654 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -240,57 +240,12 @@ unsigned AArch64RegisterBankInfo::copyCost(const RegisterBank &A,
 
 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::FPR128RegClassID:
-  case AArch64::FPR64_loRegClassID:
-  case AArch64::FPR128_loRegClassID:
-  case AArch64::FPR128_0to7RegClassID:
-  case AArch64::DDRegClassID:
-  case AArch64::DDDRegClassID:
-  case AArch64::DDDDRegClassID:
-  case AArch64::QQRegClassID:
-  case AArch64::QQQRegClassID:
-  case AArch64::QQQQRegClassID:
-  case AArch64::ZPRRegClassID:
-  case AArch64::ZPR_3bRegClassID:
-    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::tcGPRx16x17RegClassID:
-  case AArch64::tcGPRx17RegClassID:
-  case AArch64::tcGPRnotx16RegClassID:
-  case AArch64::WSeqPairsClassRegClassID:
-  case AArch64::XSeqPairsClassRegClassID:
-  case AArch64::MatrixIndexGPR32_8_11RegClassID:
-  case AArch64::MatrixIndexGPR32_12_15RegClassID:
-  case AArch64::GPR64_with_sub_32_in_MatrixIndexGPR32_8_11RegClassID:
-  case AArch64::GPR64_with_sub_32_in_MatrixIndexGPR32_12_15RegClassID:
     return getRegBank(AArch64::GPRRegBankID);
-  case AArch64::CCRRegClassID:
-    return getRegBank(AArch64::CCRegBankID);
   default:
-    llvm_unreachable("Register class not supported");
+    return AArch64GenRegisterBankInfo::getRegBankFromRegClass(RC, Ty);
   }
 }
 
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h
index 0d89f540650a9..941499b08d05d 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h
@@ -150,7 +150,7 @@ class AArch64RegisterBankInfo final : public AArch64GenRegisterBankInfo {
                     TypeSize Size) const override;
 
   const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT) const override;
+                                             LLT Ty) const override;
 
   InstructionMappings
   getInstrAlternativeMappings(const MachineInstr &MI) const override;
diff --git a/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp b/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp
index 9234881c9407e..447db18b8defa 100644
--- a/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp
@@ -170,44 +170,6 @@ ARMRegisterBankInfo::ARMRegisterBankInfo(const TargetRegisterInfo &TRI) {
   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 tcGPRRegClassID:
-  case tcGPRnotr12RegClassID:
-  case tGPRRegClassID:
-  case tGPREvenRegClassID:
-  case tGPROddRegClassID:
-  case tGPR_and_tGPREvenRegClassID:
-  case tGPR_and_tGPROddRegClassID:
-  case tGPREven_and_tcGPRRegClassID:
-  case tGPROdd_and_tcGPRRegClassID:
-  case tGPREven_and_tcGPRnotr12RegClassID:
-    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();
diff --git a/llvm/lib/Target/ARM/ARMRegisterBankInfo.h b/llvm/lib/Target/ARM/ARMRegisterBankInfo.h
index c56134aab38c6..2694174623c5c 100644
--- a/llvm/lib/Target/ARM/ARMRegisterBankInfo.h
+++ b/llvm/lib/Target/ARM/ARMRegisterBankInfo.h
@@ -32,9 +32,6 @@ 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;
 };
diff --git a/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.cpp b/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.cpp
index e7e5bb19c3a07..16d009ab81adc 100644
--- a/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.cpp
+++ b/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.cpp
@@ -58,12 +58,6 @@ const RegisterBankInfo::ValueMapping ValueMappings[] = {
 M68kRegisterBankInfo::M68kRegisterBankInfo(const TargetRegisterInfo &TRI)
     : M68kGenRegisterBankInfo() {}
 
-const RegisterBank &
-M68kRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT) const {
-  return getRegBank(M68k::GPRRegBankID);
-}
-
 const RegisterBankInfo::InstructionMapping &
 M68kRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
   auto Opc = MI.getOpcode();
diff --git a/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.h b/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.h
index 493c139f018cd..6122db8b48989 100644
--- a/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.h
+++ b/llvm/lib/Target/M68k/GISel/M68kRegisterBankInfo.h
@@ -35,9 +35,6 @@ class M68kRegisterBankInfo final : public M68kGenRegisterBankInfo {
 public:
   M68kRegisterBankInfo(const TargetRegisterInfo &TRI);
 
-  const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT) const override;
-
   const InstructionMapping &
   getInstrMapping(const MachineInstr &MI) const override;
 };
diff --git a/llvm/lib/Target/Mips/MipsRegisterBankInfo.cpp b/llvm/lib/Target/Mips/MipsRegisterBankInfo.cpp
index 62b58cba9f24a..4aecaf18db480 100644
--- a/llvm/lib/Target/Mips/MipsRegisterBankInfo.cpp
+++ b/llvm/lib/Target/Mips/MipsRegisterBankInfo.cpp
@@ -75,35 +75,6 @@ using namespace llvm;
 
 MipsRegisterBankInfo::MipsRegisterBankInfo(const TargetRegisterInfo &TRI) {}
 
-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 use operands are floating point registers.
 // Def operands are general purpose.
 static bool isFloatingPointOpcodeUse(unsigned Opc) {
diff --git a/llvm/lib/Target/Mips/MipsRegisterBankInfo.h b/llvm/lib/Target/Mips/MipsRegisterBankInfo.h
index bc424b93f6056..1e07962fd02b3 100644
--- a/llvm/lib/Target/Mips/MipsRegisterBankInfo.h
+++ b/llvm/lib/Target/Mips/MipsRegisterBankInfo.h
@@ -32,9 +32,6 @@ 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;
 
diff --git a/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp b/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp
index 125a49de7b27d..4a004f8960562 100644
--- a/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp
+++ b/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp
@@ -34,13 +34,6 @@ const RegisterBank &
 PPCRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
                                             LLT Ty) const {
   switch (RC.getID()) {
-  case PPC::G8RCRegClassID:
-  case PPC::G8RC_NOX0RegClassID:
-  case PPC::G8RC_and_G8RC_NOX0RegClassID:
-  case PPC::GPRCRegClassID:
-  case PPC::GPRC_NOR0RegClassID:
-  case PPC::GPRC_and_GPRC_NOR0RegClassID:
-    return getRegBank(PPC::GPRRegBankID);
   case PPC::VSFRCRegClassID:
   case PPC::SPILLTOVSRRC_and_VSFRCRegClassID:
   case PPC::SPILLTOVSRRC_and_VFRCRegClassID:
@@ -50,19 +43,8 @@ PPCRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
   case PPC::VSSRCRegClassID:
   case PPC::F4RCRegClassID:
     return getRegBank(PPC::FPRRegBankID);
-  case PPC::VSRCRegClassID:
-  case PPC::VRRCRegClassID:
-  case PPC::VRRC_with_sub_64_in_SPILLTOVSRRCRegClassID:
-  case PPC::VSRC_with_sub_64_in_SPILLTOVSRRCRegClassID:
-  case PPC::SPILLTOVSRRCRegClassID:
-  case PPC::VSLRCRegClassID:
-  case PPC::VSLRC_with_sub_64_in_SPILLTOVSRRCRegClassID:
-    return getRegBank(PPC::VECRegBankID);
-  case PPC::CRRCRegClassID:
-  case PPC::CRBITRCRegClassID:
-    return getRegBank(PPC::CRRegBankID);
   default:
-    llvm_unreachable("Unexpected register class");
+    return PPCGenRegisterBankInfo::getRegBankFromRegClass(RC, Ty);
   }
 }
 
diff --git a/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.h b/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.h
index 1477fdca917d7..332a34fe022dd 100644
--- a/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.h
+++ b/llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.h
@@ -67,6 +67,7 @@ class PPCRegisterBankInfo final : public PPCGenRegisterBankInfo {
 
   const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
                                              LLT Ty) const override;
+
   const InstructionMapping &
   getInstrMapping(const MachineInstr &MI) const override;
 
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
index d25e96525399e..43bbc8589e7e2 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
@@ -112,51 +112,6 @@ using namespace llvm;
 RISCVRegisterBankInfo::RISCVRegisterBankInfo(unsigned HwMode)
     : RISCVGenRegisterBankInfo(HwMode) {}
 
-const RegisterBank &
-RISCVRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                              LLT Ty) const {
-  switch (RC.getID()) {
-  default:
-    llvm_unreachable("Register class not supported");
-  case RISCV::GPRRegClassID:
-  case RISCV::GPRF16RegClassID:
-  case RISCV::GPRF32RegClassID:
-  case RISCV::GPRNoX0RegClassID:
-  case RISCV::GPRNoX0X2RegClassID:
-  case RISCV::GPRJALRRegClassID:
-  case RISCV::GPRJALRNonX7RegClassID:
-  case RISCV::GPRTCRegClassID:
-  case RISCV::GPRTCNonX7RegClassID:
-  case RISCV::GPRC_and_GPRTCRegClassID:
-  case RISCV::GPRCRegClassID:
-  case RISCV::GPRC_and_SR07RegClassID:
-  case RISCV::SR07RegClassID:
-  case RISCV::SPRegClassID:
-  case RISCV::GPRX0RegClassID:
-    return getRegBank(RISCV::GPRBRegBankID);
-  case RISCV::FPR64RegClassID:
-  case RISCV::FPR16RegClassID:
-  case RISCV::FPR32RegClassID:
-  case RISCV::FPR64CRegClassID:
-  case RISCV::FPR32CRegClassID:
-    return getRegBank(RISCV::FPRBRegBankID);
-  case RISCV::VMRegClassID:
-  case RISCV::VRRegClassID:
-  case RISCV::VRNoV0RegClassID:
-  case RISCV::VRM2RegClassID:
-  case RISCV::VRM2NoV0RegClassID:
-  case RISCV::VRM4RegClassID:
-  case RISCV::VRM4NoV0RegClassID:
-  case RISCV::VMV0RegClassID:
-  case RISCV::VRM2_with_sub_vrm1_0_in_VMV0RegClassID:
-  case RISCV::VRM4_with_sub_vrm1_0_in_VMV0RegClassID:
-  case RISCV::VRM8RegClassID:
-  case RISCV::VRM8NoV0RegClassID:
-  case RISCV::VRM8_with_sub_vrm1_0_in_VMV0RegClassID:
-    return getRegBank(RISCV::VRBRegBankID);
-  }
-}
-
 static const RegisterBankInfo::ValueMapping *getFPValueMapping(unsigned Size) {
   unsigned Idx;
   switch (Size) {
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.h b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.h
index abd0837395f66..79dddb73a2373 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.h
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.h
@@ -33,9 +33,6 @@ class RISCVRegisterBankInfo final : public RISCVGenRegisterBankInfo {
 public:
   RISCVRegisterBankInfo(unsigned HwMode);
 
-  const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT Ty) const override;
-
   const InstructionMapping &
   getInstrMapping(const MachineInstr &MI) const override;
 
diff --git a/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.cpp b/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.cpp
index ecd99f1840d7e..cc06aded901dc 100644
--- a/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.cpp
@@ -19,15 +19,3 @@
 
 #define GET_TARGET_REGBANK_IMPL
 #include "SPIRVGenRegisterBank.inc"
-
-using namespace llvm;
-
-// This required for .td selection patterns to work or we'd end up with RegClass
-// checks being redundant as all the classes would be mapped to the same bank.
-const RegisterBank &
-SPIRVRegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                              LLT Ty) const {
-  if (RC.getID() == SPIRV::TYPERegClassID)
-    return SPIRV::TYPERegBank;
-  return SPIRV::IDRegBank;
-}
diff --git a/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.h b/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.h
index 67ddcdefb7ddc..e5c1eb2b1e121 100644
--- a/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.h
+++ b/llvm/lib/Target/SPIRV/SPIRVRegisterBankInfo.h
@@ -31,8 +31,6 @@ class SPIRVGenRegisterBankInfo : public RegisterBankInfo {
 // This class provides the information for the target register banks.
 class SPIRVRegisterBankInfo final : public SPIRVGenRegisterBankInfo {
 public:
-  const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT Ty) const override;
 };
 } // namespace llvm
 #endif // LLVM_LIB_TARGET_SPIRV_SPIRVREGISTERBANKINFO_H
diff --git a/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp b/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
index 9e85424e76e62..61633a09d93cf 100644
--- a/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
+++ b/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
@@ -44,33 +44,6 @@ X86RegisterBankInfo::X86RegisterBankInfo(const TargetRegisterInfo &TRI) {
          "GPRs should hold up to 64-bit");
 }
 
-const RegisterBank &
-X86RegisterBankInfo::getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                            LLT) const {
-
-  if (X86::GR8RegClass.hasSubClassEq(&RC) ||
-      X86::GR16RegClass.hasSubClassEq(&RC) ||
-      X86::GR32RegClass.hasSubClassEq(&RC) ||
-      X86::GR64RegClass.hasSubClassEq(&RC) ||
-      X86::LOW32_ADDR_ACCESSRegClass.hasSubClassEq(&RC) ||
-      X86::LOW32_ADDR_ACCESS_RBPRegClass.hasSubClassEq(&RC))
-    return getRegBank(X86::GPRRegBankID);
-
-  if (X86::FR32XRegClass.hasSubClassEq(&RC) ||
-      X86::FR64XRegClass.hasSubClassEq(&RC) ||
-      X86::VR128XRegClass.hasSubClassEq(&RC) ||
-      X86::VR256XRegClass.hasSubClassEq(&RC) ||
-      X86::VR512RegClass.hasSubClassEq(&RC))
-    return getRegBank(X86::VECRRegBankID);
-
-  if (X86::RFP80RegClass.hasSubClassEq(&RC) ||
-      X86::RFP32RegClass.hasSubClassEq(&RC) ||
-      X86::RFP64RegClass.hasSubClassEq(&RC))
-    return getRegBank(X86::PSRRegBankID);
-
-  llvm_unreachable("Unsupported register kind yet.");
-}
-
 // \returns true if a given intrinsic only uses and defines FPRs.
 static bool isFPIntrinsic(const MachineRegisterInfo &MRI,
                           const MachineInstr &MI) {
diff --git a/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.h b/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.h
index 8f38e717e36b0..f30e9fea1a597 100644
--- a/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.h
+++ b/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.h
@@ -81,9 +81,6 @@ class X86RegisterBankInfo final : public X86GenRegisterBankInfo {
 public:
   X86RegisterBankInfo(const TargetRegisterInfo &TRI);
 
-  const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
-                                             LLT) const override;
-
   InstructionMappings
   getInstrAlternativeMappings(const MachineInstr &MI) const override;
 
diff --git a/llvm/utils/TableGen/RegisterBankEmitter.cpp b/llvm/utils/TableGen/RegisterBankEmitter.cpp
index 5546e727af384..eedc84f27419b 100644
--- a/llvm/utils/TableGen/RegisterBankEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterBankEmitter.cpp
@@ -16,6 +16,7 @@
 #include "Common/InfoByHwMode.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
@@ -151,6 +152,9 @@ void RegisterBankEmitter::emitBaseClassDefinition(
   OS << "private:\n"
      << "  static const RegisterBank *RegBanks[];\n"
      << "  static const unsigned Sizes[];\n\n"
+     << "public:\n"
+     << "  const RegisterBank &getRegBankFromRegClass(const "
+        "TargetRegisterClass &RC, LLT Ty) const override;\n"
      << "protected:\n"
      << "  " << TargetName << "GenRegisterBankInfo(unsigned HwMode = 0);\n"
      << "\n";
@@ -287,8 +291,88 @@ void RegisterBankEmitter::emitBaseClassImplementation(
      << "  for (auto RB : enumerate(RegBanks))\n"
      << "    assert(RB.index() == RB.value()->getID() && \"Index != ID\");\n"
      << "#endif // NDEBUG\n"
-     << "}\n"
-     << "} // end namespace llvm\n";
+     << "}\n";
+
+  uint32_t NoRegBanks = Banks.size();
+  uint32_t BitSize = NextPowerOf2(Log2_32(NoRegBanks));
+  uint32_t ElemsPerWord = 32 / BitSize;
+  uint32_t BitMask = (1 << BitSize) - 1;
+  bool HasAmbigousOrMissingEntry = false;
+  struct Entry {
+    std::string RCIdName;
+    std::string RBIdName;
+  };
+  std::vector<Entry> Entries;
+  for (const auto &Bank : Banks)
+    for (const auto *RC : Bank.register_classes()) {
+      if (RC->EnumValue >= Entries.size())
+        Entries.resize(RC->EnumValue+1);
+      Entry &E = Entries[RC->EnumValue];
+      E.RCIdName = RC->getIdName();
+      if (!E.RBIdName.empty()) {
+        HasAmbigousOrMissingEntry = true;
+        E.RBIdName = "InvalidRegBankID";
+      } else {
+        E.RBIdName = (TargetName + "::" + Bank.getEnumeratorName()).str();
+      }
+    }
+  for (auto &E : Entries) {
+    if (E.RBIdName.empty()) {
+      HasAmbigousOrMissingEntry = true;
+      E.RBIdName = "InvalidRegBankID";
+    }
+  }
+  OS << "co...
[truncated]

redstar · 2024-07-22T16:54:38Z

I know that test cases are missing. I'll add them when it becomes clear that this is the way to go.

On Power, building targets all + m68k, this change adds 1472 bytes to llc.

Possible alternative approaches which I have not examined:

compact the table further (increases access time)
generate a large switch
generate a perfect hash function (may not save much space)

github-actions · 2024-07-22T16:55:43Z

✅ With the latest revision this PR passed the C/C++ code formatter.

arsenm

Does this improve compile time at all? I assume this can't deal with the AMDGPU ambiguous i1 case?

arsenm · 2024-07-22T16:54:09Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+        OS << "," << TrailingComment << "\n";
+    } else {
+      OS << " |" << TrailingComment << "\n";


Suggested change

OS << "," << TrailingComment << "\n";

} else {

OS << " |" << TrailingComment << "\n";

OS << "," << TrailingComment << '\n';

} else {

OS << " |" << TrailingComment << '\n';

llvm/utils/TableGen/RegisterBankEmitter.cpp

arsenm · 2024-07-22T16:55:25Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+  OS << "  static const uint32_t RegClass2RegBank["
+     << TableSize << "] = {\n";
+  uint32_t Shift = 32 - BitSize;
+  bool First = true;


Use ListSeparator?

Doesn't work because the separator ',' + TrailingComment + '\n' is not fixed.

tschuett · 2024-07-22T17:03:30Z

Could you put the output for e.g. PowerPC here:
https://github.com/llvm/llvm-project/tree/main/llvm/test/TableGen
Just for our eyes.

redstar · 2024-07-22T17:56:43Z

Does this improve compile time at all? I assume this can't deal with the AMDGPU ambiguous i1 case?

I have not yet check the compile time. My main interest is to generate more code from the TableGen defintion. Well, I now have a Mac since a week, so I should now be able to measure the compile time impact....

The AMDGPU i1 case is indeed not handled. I found no reliable way to handle this exception. AArch64 and PowerPC also need some special handling because of register classes which could be mapped to several register banks. One way to solve the latter problem could be to add more information to TableGen, but I made no effort in this direction so far.

redstar · 2024-07-22T18:00:37Z

Could you put the output for e.g. PowerPC here: https://github.com/llvm/llvm-project/tree/main/llvm/test/TableGen Just for our eyes.

Done, see the last function in file PPCGenRegisterBank.inc.

arsenm · 2024-07-22T18:09:44Z

llvm/test/TableGen/PPCGenRegisterBank.inc

@@ -0,0 +1,176 @@
+/*===- TableGen'erated file -------------------------------------*- C++ -*-===*\


This is just a generated output, it's not a test. The test needs to be the tablegen input checking the generated output

For sure, that was just to show the generated code as requested above.

Generating the mapping from a register class to a register bank is complex: - there can be lots of register classes - the mapping may be ambiguos - a register class can span several register banks (e.g. a register class containing all registers) - the type information is not enough to decide which register bank to map to (e.g. a register class containing floating point and vector registers, and all register can represent a f64 value) The approach taken here is to encode the register banks in an array indexed by the ID of the register class. To save space, the entries are packed into chunks of size 2^n. See llvm/llvm-project#99896

Pierre-vh

Just some style nits. I'm not familiar enough with this part of GISel to comment on the implementation

Pierre-vh · 2024-07-23T06:04:37Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

-     << "} // end namespace llvm\n";
+     << "}\n";
+
+  uint32_t NoRegBanks = Banks.size();


very very tiny nit, but at first I thought this meant "no reg bank" as if it's a negative.
Maybe use N or Num?

Pierre-vh · 2024-07-23T06:05:09Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+    std::string RCIdName;
+    std::string RBIdName;
+  };
+  std::vector<Entry> Entries;


SmallVector is generally preferred

Pierre-vh · 2024-07-23T06:05:33Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+    std::string RBIdName;
+  };
+  std::vector<Entry> Entries;
+  for (const auto &Bank : Banks)


missing {} for this for loop

Pierre-vh · 2024-07-23T06:06:44Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+  OS << "const RegisterBank &\n"
+     << TargetName << "GenRegisterBankInfo::getRegBankFromRegClass"
+     << "(const TargetRegisterClass &RC, LLT) const {\n";
+  if (HasAmbigousOrMissingEntry)


Pierre-vh · 2024-07-23T06:07:11Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+  OS << "    unsigned RegBankID = (RegClass2RegBank[RegClassID / "
+     << ElemsPerWord << "] >> ((RegClassID % " << ElemsPerWord << ") * "
+     << BitSize << ")) & " << BitMask << ";\n";
+  if (HasAmbigousOrMissingEntry)


arsenm

lgtm with nits addressed

arsenm · 2024-07-24T16:28:20Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+  }
+  OS << "const RegisterBank &\n"
+     << TargetName << "GenRegisterBankInfo::getRegBankFromRegClass"
+     << "(const TargetRegisterClass &RC, LLT) const {\n";


Don't need the <<, can just rely on the string concat

Generating the mapping from a register class to a register bank is complex: - there can be lots of register classes - the mapping may be ambiguos - a register class can span several register banks (e.g. a register class containing all registers) - the type information is not enough to decide which register bank to map to (e.g. a register class containing floating point and vector registers, and all register can represent a f64 value) The approach taken here is to encode the register banks in an array indexed by the ID of the register class. To save space, the entries are packed into chunks of size 2^n.

topperc · 2024-07-25T15:09:01Z

llvm/utils/TableGen/RegisterBankEmitter.cpp

+       << TargetName + "::InvalidRegBankID) & " << BitMask << ";\n";
+  }
+  unsigned TableSize =
+      Entries.size() / ElemsPerWord + ((Entries.size() % ElemsPerWord) > 0);


use divideCeil?

Good point, I'll create a follow-up PR.

Summary: Generating the mapping from a register class to a register bank is complex: - there can be lots of register classes - the mapping may be ambiguos - a register class can span several register banks (e.g. a register class containing all registers) - the type information is not enough to decide which register bank to map to (e.g. a register class containing floating point and vector registers, and all register can represent a f64 value) The approach taken here is to encode the register banks in an array indexed by the ID of the register class. To save space, the entries are packed into chunks of size 2^n. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250585

Generating the mapping from a register class to a register bank is complex: - there can be lots of register classes - the mapping may be ambiguos - a register class can span several register banks (e.g. a register class containing all registers) - the type information is not enough to decide which register bank to map to (e.g. a register class containing floating point and vector registers, and all register can represent a f64 value) The approach taken here is to encode the register banks in an array indexed by the ID of the register class. To save space, the entries are packed into chunks of size 2^n. See llvm/llvm-project#99896

redstar requested review from aemerson, arsenm and tschuett July 22, 2024 16:51

redstar self-assigned this Jul 22, 2024

llvmbot added backend:ARM backend:AArch64 backend:RISC-V backend:PowerPC backend:X86 backend:m68k backend:SPIR-V labels Jul 22, 2024

arsenm requested a review from Pierre-vh July 22, 2024 16:57

arsenm reviewed Jul 22, 2024

View reviewed changes

tschuett added the llvm:globalisel label Jul 22, 2024

arsenm reviewed Jul 22, 2024

View reviewed changes

Pierre-vh reviewed Jul 23, 2024

View reviewed changes

arsenm reviewed Jul 23, 2024

View reviewed changes

arsenm reviewed Jul 24, 2024

View reviewed changes

arsenm approved these changes Jul 24, 2024

View reviewed changes

redstar force-pushed the knacke/generateregbank branch from 19e54b4 to a951a19 Compare July 25, 2024 13:37

redstar merged commit a79db96 into llvm:main Jul 25, 2024
5 of 7 checks passed

topperc reviewed Jul 25, 2024

View reviewed changes

		@@ -0,0 +1,176 @@
		/===- TableGen'erated file -------------------------------------- C++ --===\

[GISel][TableGen] Generate getRegBankFromRegClass #99896

[GISel][TableGen] Generate getRegBankFromRegClass #99896

Uh oh!

Conversation

redstar commented Jul 22, 2024

Uh oh!

llvmbot commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

redstar commented Jul 22, 2024

Uh oh!

github-actions bot commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tschuett commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

redstar commented Jul 22, 2024

Uh oh!

redstar commented Jul 22, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pierre-vh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Jul 22, 2024 •

edited

Loading

github-actions bot commented Jul 22, 2024 •

edited

Loading

tschuett commented Jul 22, 2024 •

edited

Loading