[TableGen] Implement getNamedOperandIdx with another table lookup. NFC. #151116
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use direct table lookup instead of a switch over all opcodes. In my Release+Asserts build this reduced the code size for AMDGPU::getNamedOperandIdx from 11422 to 80 bytes, and the total size of all its tables (including the jump table for the switch) from 243564 to 155020 bytes.
Member
|
@llvm/pr-subscribers-tablegen Author: Jay Foad (jayfoad) ChangesUse direct table lookup instead of a switch over all opcodes. In my Release+Asserts build this reduced the code size for Full diff: https://github.com/llvm/llvm-project/pull/151116.diff 2 Files Affected:
diff --git a/llvm/test/TableGen/get-named-operand-idx.td b/llvm/test/TableGen/get-named-operand-idx.td
index f5c5d93f9e522..ab23edd54c723 100644
--- a/llvm/test/TableGen/get-named-operand-idx.td
+++ b/llvm/test/TableGen/get-named-operand-idx.td
@@ -72,14 +72,10 @@ def InstD : InstBase {
// CHECK: {0, 1, 2, -1, -1, },
// CHECK: {-1, -1, -1, 0, 1, },
// CHECK: };
-// CHECK: switch(Opcode) {
-// CHECK: case MyNamespace::InstA:
-// CHECK: return OperandMap[0][static_cast<unsigned>(Name)];
-// CHECK: case MyNamespace::InstB:
-// CHECK: case MyNamespace::InstC:
-// CHECK: return OperandMap[1][static_cast<unsigned>(Name)];
-// CHECK: default: return -1;
-// CHECK: }
+// CHECK: static constexpr uint8_t InstructionIndex[] = {
+// CHECK: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+// CHECK: };
+// CHECK: return OperandMap[InstructionIndex[Opcode]][(unsigned)Name];
// CHECK: }
// CHECK: } // end namespace llvm::MyNamespace
// CHECK: #endif //GET_INSTRINFO_NAMED_OPS
diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index fa38d01dd9518..6f72b51963f87 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -250,29 +250,38 @@ void InstrInfoEmitter::emitOperandNameMappings(
// Map of operand names to their ID.
MapVector<StringRef, unsigned> OperandNameToID;
- /// The keys of this map is a map which have OpName ID values as their keys
- /// and instruction operand indices as their values. The values of this map
- /// are lists of instruction names. This map helps to unique entries among
+ /// A key in this map is a vector mapping OpName ID values to instruction
+ /// operand indices or -1 (but without any trailing -1 values which will be
+ /// added later). The corresponding value in this map is the index of that row
+ /// in the emitted OperandMap table. This map helps to unique entries among
/// instructions that have identical OpName -> Operand index mapping.
- std::map<std::map<unsigned, unsigned>, std::vector<StringRef>> OperandMap;
+ MapVector<SmallVector<int>, unsigned> OperandMap;
// Max operand index seen.
unsigned MaxOperandNo = 0;
// Fixed/Predefined instructions do not have UseNamedOperandTable enabled, so
- // we can just skip them.
+ // add a dummy map entry for them.
+ OperandMap.try_emplace({}, 0);
+ unsigned FirstTargetVal = TargetInstructions.front()->EnumVal;
+ SmallVector<unsigned> InstructionIndex(FirstTargetVal, 0);
for (const CodeGenInstruction *Inst : TargetInstructions) {
- if (!Inst->TheDef->getValueAsBit("UseNamedOperandTable"))
+ if (!Inst->TheDef->getValueAsBit("UseNamedOperandTable")) {
+ InstructionIndex.push_back(0);
continue;
- std::map<unsigned, unsigned> OpList;
+ }
+ SmallVector<int> OpList;
for (const auto &Info : Inst->Operands) {
unsigned ID =
OperandNameToID.try_emplace(Info.Name, OperandNameToID.size())
.first->second;
+ OpList.resize(std::max((unsigned)OpList.size(), ID + 1), -1);
OpList[ID] = Info.MIOperandNo;
MaxOperandNo = std::max(MaxOperandNo, Info.MIOperandNo);
}
- OperandMap[OpList].push_back(Inst->TheDef->getName());
+ auto [It, Inserted] =
+ OperandMap.try_emplace(std::move(OpList), OperandMap.size());
+ InstructionIndex.push_back(It->second);
}
const size_t NumOperandNames = OperandNameToID.size();
@@ -302,28 +311,22 @@ void InstrInfoEmitter::emitOperandNameMappings(
StringRef Type = MaxOperandNo <= INT8_MAX ? "int8_t" : "int16_t";
OS << " static constexpr " << Type << " OperandMap[][" << NumOperandNames
<< "] = {\n";
- for (const auto &Entry : OperandMap) {
- const std::map<unsigned, unsigned> &OpList = Entry.first;
-
+ for (const auto &[OpList, _] : OperandMap) {
// Emit a row of the OperandMap table.
OS << " {";
- for (unsigned ID = 0; ID < NumOperandNames; ++ID) {
- auto Iter = OpList.find(ID);
- OS << (Iter != OpList.end() ? (int)Iter->second : -1) << ", ";
- }
+ for (unsigned ID = 0; ID < NumOperandNames; ++ID)
+ OS << (ID < OpList.size() ? OpList[ID] : -1) << ", ";
OS << "},\n";
}
OS << " };\n";
- OS << " switch(Opcode) {\n";
- for (const auto &[TableIndex, Entry] : enumerate(OperandMap)) {
- for (StringRef Name : Entry.second)
- OS << " case " << Namespace << "::" << Name << ":\n";
- OS << " return OperandMap[" << TableIndex
- << "][static_cast<unsigned>(Name)];\n";
- }
- OS << " default: return -1;\n";
- OS << " }\n";
+ Type = OperandMap.size() <= UINT8_MAX + 1 ? "uint8_t" : "uint16_t";
+ OS << " static constexpr " << Type << " InstructionIndex[] = {";
+ for (auto [TableIndex, Entry] : enumerate(InstructionIndex))
+ OS << (TableIndex % 16 == 0 ? "\n " : " ") << Entry << ',';
+ OS << "\n };\n";
+
+ OS << " return OperandMap[InstructionIndex[Opcode]][(unsigned)Name];\n";
} else {
// There are no operands, so no need to emit anything
OS << " return -1;\n";
|
arsenm
approved these changes
Jul 29, 2025
jurahul
reviewed
Jul 29, 2025
| // add a dummy map entry for them. | ||
| OperandMap.try_emplace({}, 0); | ||
| unsigned FirstTargetVal = TargetInstructions.front()->EnumVal; | ||
| SmallVector<unsigned> InstructionIndex(FirstTargetVal, 0); |
Contributor
There was a problem hiding this comment.
Looks like there are ~316 generic opcodes, so we could squeeze the tables by another 316/632 bytes if desired by not encoding their index in the InstructionIndex table and doing:
if (Opcode < FirstTargetVal)
return -1;
return OperandMap[InstructionIndex[Opcode - FirstTargetVal]][(unsigned)Name];
WDYT? Is it wodth the addition 1/2 a KB?
Contributor
There was a problem hiding this comment.
In addition, there could be some additional coincidental compression. For AMDGPU, AV_MOV_B32_IMM_PSEUDO, the first opcode after the generic ones, does not have any named operands, so its index need not be encoded in the table as well..
Contributor
Author
I did consider this but my preference was to keep the code as simple as possible and non-branchy, even at the expense of slightly larger data. If you really want to save space in this table, maybe we should represent it with an extra field in MCInstrDesc instead of a separate table? It looks like MCInstrDesc currently has a two byte hole that we could use. |
jurahul
approved these changes
Jul 30, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/24357 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/21824 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use direct table lookup instead of a switch over all opcodes.
In my Release+Asserts build this reduced the code size for
AMDGPU::getNamedOperandIdx from 11422 to 80 bytes, and the total size of
all its tables (including the jump table for the switch) from 243564 to
155020 bytes.