Skip to content

TableGen support for RegisterBankInfo #71357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from
Closed

TableGen support for RegisterBankInfo #71357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d6f25b0
renamed emitRBIIMPL and other small changes
Nov 6, 2023
7bee10e
added const keyword to emitted arrays
Nov 6, 2023
e74d925
clang-format formatting
Nov 6, 2023
241919d
fixing clang-format complaint
Nov 6, 2023
7ed514c
Updating RegisterBankEmitter.td to include CHECKs
Nov 6, 2023
1f52ef1
added comment pointing at discourse RFC
Nov 6, 2023
ffe12e4
Fixed clang-format complaint
Nov 6, 2023
1280d9c
added more tests to RegisterBankEmitter.td
Nov 6, 2023
b5dc5f3
one more RegisterBankEmitter.td test
Nov 6, 2023
f701940
the emitter now writes #error lines if it detects either a non-Simple
Nov 7, 2023
6141ee2
resolving PR comments
Nov 8, 2023
775e71e
Added support for ValueMappings
Nov 8, 2023
18ff040
fixed clang format complaint
Nov 9, 2023
4250c64
added RegisterBankEmitter test
Nov 9, 2023
baa94ec
Improved comment.
Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions llvm/test/TableGen/RegisterBankEmitter.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,19 @@ let Size = 32 in {
// CHECK: MyTarget::ClassARegClassID
// CHECK: MyTarget::ClassBRegClassID
def GPRRegBank : RegisterBank<"GPR", [ClassA]>;

// CHECK: enum PartialMappingIdx
// CHECK: PMI_ClassA = 0,
// CHECK: const RegisterBankInfo::PartialMapping PartialMappings
// CHECK: { 0, 32, GPRRegBank },
// CHECK: const PartialMappingIdx BankIDToFirstRegisterClassIdx
// CHECK: PMI_ClassA,
// CHECK: const int BankIDToRegisterClassCount
// CHECK: 1,
// CHECK: const RegisterBankInfo::ValueMapping ValueMappings
// CHECK: PMI_ClassA
// CHECK: PMI_ClassA
// CHECK: PMI_ClassA
// CHECK: enum ValueMappingIdx
// CHECK: VMI_ClassA

190 changes: 173 additions & 17 deletions llvm/utils/TableGen/RegisterBankEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//===----------------------------------------------------------------------===//
//
// This tablegen backend is responsible for emitting a description of a target
// register bank for a code generator.
// register bank and register bank info for a code generator.
//
//===----------------------------------------------------------------------===//

Expand Down Expand Up @@ -46,7 +46,9 @@ class RegisterBank {
/// Get the human-readable name for the bank.
StringRef getName() const { return TheDef.getValueAsString("Name"); }
/// Get the name of the enumerator in the ID enumeration.
std::string getEnumeratorName() const { return (TheDef.getName() + "ID").str(); }
std::string getEnumeratorName() const {
return (TheDef.getName() + "ID").str();
}

/// Get the name of the array holding the register class coverage data;
std::string getCoverageArrayName() const {
Expand Down Expand Up @@ -107,12 +109,18 @@ class RegisterBankEmitter {
CodeGenTarget Target;
RecordKeeper &Records;

void emitHeader(raw_ostream &OS, const StringRef TargetName,
void emitHeader(raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks);
void emitBaseClassDefinition(raw_ostream &OS, const StringRef TargetName,
void emitBaseClassDefinition(raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks);
void emitBaseClassImplementation(raw_ostream &OS, const StringRef TargetName,
std::vector<RegisterBank> &Banks);
void emitBaseClassImplementation(raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks);
void emitRBIHeader(raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks);
void emitRBIPartialMappings(raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks);
void emitRBIValueMappings(raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks);

public:
RegisterBankEmitter(RecordKeeper &R) : Target(R), Records(R) {}
Expand All @@ -124,8 +132,7 @@ class RegisterBankEmitter {

/// Emit code to declare the ID enumeration and external global instance
/// variables.
void RegisterBankEmitter::emitHeader(raw_ostream &OS,
const StringRef TargetName,
void RegisterBankEmitter::emitHeader(raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks) {
// <Target>RegisterBankInfo.h
OS << "namespace llvm {\n"
Expand All @@ -144,7 +151,7 @@ void RegisterBankEmitter::emitHeader(raw_ostream &OS,

/// Emit declarations of the <Target>GenRegisterBankInfo class.
void RegisterBankEmitter::emitBaseClassDefinition(
raw_ostream &OS, const StringRef TargetName,
raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks) {
OS << "private:\n"
<< " static const RegisterBank *RegBanks[];\n"
Expand Down Expand Up @@ -213,7 +220,7 @@ static void visitRegisterBankClasses(

void RegisterBankEmitter::emitBaseClassImplementation(
raw_ostream &OS, StringRef TargetName,
std::vector<RegisterBank> &Banks) {
const std::vector<RegisterBank> &Banks) {
const CodeGenRegBank &RegisterClassHierarchy = Target.getRegBank();
const CodeGenHwModes &CGH = Target.getHwModes();

Expand All @@ -229,10 +236,13 @@ void RegisterBankEmitter::emitBaseClassImplementation(
OS << "const uint32_t " << Bank.getCoverageArrayName() << "[] = {\n";
unsigned LowestIdxInWord = 0;
for (const auto &RCs : RCsGroupedByWord) {
OS << " // " << LowestIdxInWord << "-" << (LowestIdxInWord + 31) << "\n";
OS << " // " << LowestIdxInWord << "-" << (LowestIdxInWord + 31)
<< "\n";
for (const auto &RC : RCs) {
OS << " (1u << (" << RC->getQualifiedIdName() << " - "
<< LowestIdxInWord << ")) |\n";
std::string QualifiedRegClassID =
(Twine(RC->Namespace) + "::" + RC->getName() + "RegClassID").str();
OS << " (1u << (" << QualifiedRegClassID << " - " << LowestIdxInWord
<< ")) |\n";
}
OS << " 0,\n";
LowestIdxInWord += 32;
Expand All @@ -244,7 +254,7 @@ void RegisterBankEmitter::emitBaseClassImplementation(
for (const auto &Bank : Banks) {
std::string QualifiedBankID =
(TargetName + "::" + Bank.getEnumeratorName()).str();
OS << "constexpr RegisterBank " << Bank.getInstanceVarName() << "(/* ID */ "
OS << "const RegisterBank " << Bank.getInstanceVarName() << "(/* ID */ "
<< QualifiedBankID << ", /* Name */ \"" << Bank.getName() << "\", "
<< "/* CoveredRegClasses */ " << Bank.getCoverageArrayName()
<< ", /* NumRegClasses */ "
Expand Down Expand Up @@ -289,6 +299,138 @@ void RegisterBankEmitter::emitBaseClassImplementation(
<< "} // end namespace llvm\n";
}

// This emitter generates PartialMappings, PartialMappingIdx,
// BankIDToCopyMapIdx and BankIDToRegisterClassCount from the .td files.
// However it requires that the .td files fully describe their RegisterBanks
// and otherwise emits #error lines for the offending Registers.
//
// These tables and enums are enabled by GET_REGBANKINFO_DECLARATIONS,
// GET_REGBANKINFO_PARTIALMAPPINGS and GET_REGBANKINFO_VALUEMAPPINGS
// So a backend which doesn't fully describe its RegisterBanks
// will not break if it doesn't define these macros.
//
// This was discussed in https://discourse.llvm.org/t/74459
void RegisterBankEmitter::emitRBIHeader(
raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks) {
const CodeGenRegBank &RegisterClassHierarchy = Target.getRegBank();

OS << "namespace llvm {\n"
<< "namespace " << TargetName << " {\n"
<< "enum PartialMappingIdx {\n"
<< " PMI_None = -1,\n";

// Banks and Register Classes are *not* emitted in their original text order
int ID = 0;
for (const auto &Bank : Banks) {
for (const CodeGenRegisterClass *RC :
Bank.getExplicitlySpecifiedRegisterClasses(RegisterClassHierarchy)) {
OS << " PMI_" << RC->getName() << " = " << ID++ << ",\n";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that a PartialMappingIdx is supposed to be a register bank and size not a register class. ARM and Mips name all of theirs using register class names. PowerPC uses more generic names like FPR32, FPR64, and VEC128.

The possible sizes for a register bank in the partial mapping index should be determined by the sizes of any register classes in the register bank. This should include the inferred register classes.

Because the sizes of the register classes determine the size for the partial mapping index, there is a correspondence.

@qcolombet @aemerson @dsandersllvm can you confirm my understanding?

Copy link
Contributor Author

@CBSears CBSears Nov 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PowerPC’s PartialMappingIdx in PPCRegBankInfo.h doesn’t match the Register Classes in PPCRegisterBanks.td

One of the things about having a TableGen emitter is that there will be some enforced standardization of naming. That standard could go one way or it could go another. But in the end, hopefully all of the backends follow the resulting convention which will make code sharing easier.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PowerPC’s PartialMappingIdx in PPCRegBankInfo.h doesn’t match the Register Classes in PPCRegisterBanks.td

That's because a partial mapping index is not a register class. It's a register bank and size.

It's supposed to describe the contents of the corresponding row in partial mapping table. Each row in the table is a register bank, a length, and a start index. The start index is always 0. The partial mapping index names should be the register bank and the length concatenated together. So they describe the row contents.

The RISC-V partial indices should be named GPRB32, GPRB64, FPRB32, and FPRB64

Copy link
Contributor Author

@CBSears CBSears Nov 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Mips:

enum PartialMappingIdx {
  PMI_GPR,
  PMI_SPR,
  PMI_DPR,
  PMI_MSA,
  PMI_Min = PMI_GPR,
};

I don't think it really matters which way as long as there's one consistent way. Right now, without an emitter laying down the law, the backends have wandered off in different directions. They compile and they are internally consistent but it makes sharing code harder.

I also don't think that when we pick one standard, that the resulting edits are going to be anything but an NFC.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm well aware that everything is inconsistent. I'm trying to explain what I believe AArch64 intended when they started this project.

I believe the intent of the register class lists in *RegisterBanks.td was to allow targets to list the minimum number of register classes to allow tablegen to find the rest of the related to registers. This is why RISC-V uses FPR64 only and AArch64 uses QQQQ. Those are the largest super classes that the FPR registers on those targets belong to. Tablegen can walk down the subregister information from there and find all of the register classes that need to belong to the bank. This is expressed in the CoverageData table. The type lists on the register classes should have nothing to do with this.

The union of the sizes from all of the covered register classes should be used to fill in the partial mapping table. Tblgen infers that the FPR register bank for RISC-V includes the following register classes

const uint32_t FPRRegBankCoverageData[] = {                                      
    // 0-31                                                                      
    (1u << (RISCV::FPR64RegClassID - 0)) |                                       
    (1u << (RISCV::FPR16RegClassID - 0)) |                                       
    (1u << (RISCV::FPR32RegClassID - 0)) |                                       
    (1u << (RISCV::FPR64CRegClassID - 0)) |                                      
    (1u << (RISCV::FPR32CRegClassID - 0)) |                                      
    0,                                                                           
    // 32-63                                                                     
    0,                                                                           
    // 64-95                                                                     
    0,                                                                           
};

Examining the sizes of those 6 register classes will reveal 3 unique sizes, 16, 32, and 64. So RISC-V should have PMI_FPR16, PMI_FPR32, and PMI_FPR64 as partial mapping indices for the FPR bank. I'm aware that PMI_FPR16 is missing today. I will add it soon.

For GPR on RISC-V we there are many register classes but they all have the same sizes which comes from HwMode. I think we should walk through all of the register classes in GPRRegBankCoverageData and for each class look at the possible sizes HwMode gives for them. They should all give the same two values, 32 and 64. So we should have PMI_GPR32 and PMI_GPR64.

On AArch64, FPRRegBankCoverageData contains a bunch of register classes inferred by QQQQ. I bet if we check the sizes of the all of those classes we will find sizes 8, 16, 32, 64, 128, 256, and 512. All of the sizes for FPR in AArch64's partial mapping index for FPR. Except 8 which I think is just missing from GlobalISel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The partial mapping should just be a size. However, I am not sure what the ultimate value of this level of detail provides. As it stands we have a ton of infrastructure to produce a bunch of register sizes, but then they're not actually useful to code that needs to use them.

RegBankSelect tries to pre-create registers for you with appropriate sizes, but that doesn't preserve the type if you're doing a vector breakdown for example. You also aren't free to just directly use the registers that were created for you, and it doesn't compose with constructing new instructions. For example if you try to split a 64-bit operation into operations on 2 32-bit pieces, you may end up with 2 32-bit registers but you ultimately need to figure out how to undo the split to produce a valid merge instruction

}
}
OS << "};\n";
OS << "} // end namespace " << TargetName << "\n"
<< "} // end namespace llvm\n";
}

void RegisterBankEmitter::emitRBIPartialMappings(
raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks) {
const CodeGenRegBank &RegisterClassHierarchy = Target.getRegBank();

OS << "namespace llvm {\n"
<< "namespace " << TargetName << " {\n"
<< "const RegisterBankInfo::PartialMapping PartialMappings[] = {\n";
for (const auto &Bank : Banks) {
for (const CodeGenRegisterClass *RC :
Bank.getExplicitlySpecifiedRegisterClasses(RegisterClassHierarchy)) {
if (!RC->RSI.isSimple()) {
OS << " #error non-Simple() RegisterClass " << RC->getName() << "\n";
} else if (RC->getValueTypes()[0].getSimple() == MVT::Untyped) {
OS << " #error Untyped RegisterClass " << RC->getName() << "\n";
} else {
// StartIdx is currently 0 in all of the in-tree backends
OS << " { 0, " << RC->RSI.getSimple().RegSize << ", "
<< Bank.getInstanceVarName() << " },\n";
}
}
}
OS << "};\n\n";

// emit PartialMappingIdx of the first Register Class of each Register Bank
OS << "const PartialMappingIdx BankIDToFirstRegisterClassIdx[] = {\n";
for (const auto &Bank : Banks) {
OS << " PMI_"
<< Bank.getExplicitlySpecifiedRegisterClasses(RegisterClassHierarchy)[0]
->getName()
<< ",\n";
}
OS << "};\n\n";

// emit count of Register Classes of each Register Bank
OS << "const int BankIDToRegisterClassCount[] = {\n";
for (const auto &Bank : Banks) {
OS << " "
<< Bank.getExplicitlySpecifiedRegisterClasses(RegisterClassHierarchy)
.size()
<< ",\n";
}
OS << "};\n\n";

OS << "} // end namespace " << TargetName << "\n"
<< "} // end namespace llvm\n";
}

// This supports ValueMappings for the simple cases.
// For the complex cases, GET_REGBANKINFO_VALUEMAPPINGS should be left
// undefined and the ValueMapping tables and enums must be hand crafted.
void RegisterBankEmitter::emitRBIValueMappings(
raw_ostream &OS, StringRef TargetName,
const std::vector<RegisterBank> &Banks) {
const CodeGenRegBank &RegisterClassHierarchy = Target.getRegBank();

OS << "namespace llvm {\n"
<< "namespace " << TargetName << " {\n"
<< "const RegisterBankInfo::ValueMapping ValueMappings[] = {\n"
<< " { nullptr, 0 },\n";
for (const auto &Bank : Banks) {
for (const CodeGenRegisterClass *RC :
Bank.getExplicitlySpecifiedRegisterClasses(RegisterClassHierarchy)) {
if (!RC->RSI.isSimple()) {
OS << " #error non-Simple() RegisterClass " << RC->getName() << "\n";
} else if (RC->getValueTypes()[0].getSimple() == MVT::Untyped) {
OS << " #error Untyped RegisterClass " << RC->getName() << "\n";
} else {
OS << " { &PartialMappings[PMI_" << RC->getName() << "], 1},\n"
<< " { &PartialMappings[PMI_" << RC->getName() << "], 1},\n"
<< " { &PartialMappings[PMI_" << RC->getName() << "], 1},\n";
}
}
}
OS << "};\n\n";

OS << "enum ValueMappingIdx = {\n"
<< " VMI_Invalid = 0,\n";
int Offset = 1;
for (const auto &Bank : Banks) {
for (const CodeGenRegisterClass *RC :
Bank.getExplicitlySpecifiedRegisterClasses(RegisterClassHierarchy)) {
if (!RC->RSI.isSimple()) {
OS << " #error non-Simple() RegisterClass " << RC->getName() << "\n";
} else if (RC->getValueTypes()[0].getSimple() == MVT::Untyped) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure you can assume a register class has any set types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TableGen RegisterClass constructor has

  if (TypeList.empty())
    PrintFatalError(R->getLoc(), "RegTypes list must not be empty!");

BTW, this logic is to detect the AArch64 case and AArch64 XSeqPairsClass indeed has

def XSeqPairsClass : RegisterClass<"AArch64", [untyped], 64,

OS << " #error Untyped RegisterClass " << RC->getName() << "\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single quotes for single characters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But "\n" is used elsewhere in this file and '\n' isn't. So I'm inclined to just follow their example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'\n' calls a different operator<< which is usually inlined vs. one that usually isn't. "\n" is more expensive

} else {
OS << " VMI_" << RC->getName() << " = " << Offset << ",\n";
Offset += 3;
}
}
}
OS << "};\n\n";

OS << "} // end namespace " << TargetName << "\n"
<< "} // end namespace llvm\n";
}

void RegisterBankEmitter::run(raw_ostream &OS) {
StringRef TargetName = Target.getName();
const CodeGenRegBank &RegisterClassHierarchy = Target.getRegBank();
Expand Down Expand Up @@ -330,7 +472,8 @@ void RegisterBankEmitter::run(raw_ostream &OS) {
}

Records.startTimer("Emit output");
emitSourceFileHeader("Register Bank Source Fragments", OS);
emitSourceFileHeader("Register Bank And Register Bank Info Source Fragments",
OS);
OS << "#ifdef GET_REGBANK_DECLARATIONS\n"
<< "#undef GET_REGBANK_DECLARATIONS\n";
emitHeader(OS, TargetName, Banks);
Expand All @@ -342,8 +485,21 @@ void RegisterBankEmitter::run(raw_ostream &OS) {
<< "#ifdef GET_TARGET_REGBANK_IMPL\n"
<< "#undef GET_TARGET_REGBANK_IMPL\n";
emitBaseClassImplementation(OS, TargetName, Banks);
OS << "#endif // GET_TARGET_REGBANK_IMPL\n";
OS << "#endif // GET_TARGET_REGBANK_IMPL\n\n"
<< "#ifdef GET_REGBANKINFO_DECLARATIONS\n"
<< "#undef GET_REGBANKINFO_DECLARATIONS\n";
emitRBIHeader(OS, TargetName, Banks);
OS << "#endif // GET_REGBANKINFO_DECLARATIONS\n\n"
<< "#ifdef GET_REGBANKINFO_PARTIALMAPPINGS\n"
<< "#undef GET_REGBANKINFO_PARTIALMAPPINGS\n";
emitRBIPartialMappings(OS, TargetName, Banks);
OS << "#endif // GET_REGBANKINFO_PARTIALMAPPINGS\n"
<< "#ifdef GET_REGBANKINFO_VALUEMAPPINGS\n"
<< "#undef GET_REGBANKINFO_VALUEMAPPINGS\n";
emitRBIValueMappings(OS, TargetName, Banks);
OS << "#endif // GET_REGBANKINFO_VALUEMAPPINGS\n";
}

static TableGen::Emitter::OptClass<RegisterBankEmitter>
X("gen-register-bank", "Generate registers bank descriptions");
X("gen-register-bank",
"Generate register bank and register bank info descriptions");