[MC] Reorder TARGETInstrTable to shrink MCInstrDesc::ImplicitOffset #171199
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
Put ImplicitOps[] before OperandInfo[] in the generated TARGETInstrTable. This means that offsets to entries into the (small) ImplicitOps table do not need to skip over the (large) OperandInfo table. This allows shrinking ImplicitOffset from 32 bits to 16 bits (effectively reverting llvm#138127) which will allow expanding Opcode instead without increasing the size of MCInstrDesc.
Member
|
@llvm/pr-subscribers-llvm-mc @llvm/pr-subscribers-tablegen Author: Jay Foad (jayfoad) ChangesPut ImplicitOps[] before OperandInfo[] in the generated This allows shrinking ImplicitOffset from 32 bits to 16 bits Full diff: https://github.com/llvm/llvm-project/pull/171199.diff 4 Files Affected:
diff --git a/llvm/include/llvm/MC/MCInstrDesc.h b/llvm/include/llvm/MC/MCInstrDesc.h
index 5722213347d51..69dd3e2848832 100644
--- a/llvm/include/llvm/MC/MCInstrDesc.h
+++ b/llvm/include/llvm/MC/MCInstrDesc.h
@@ -211,7 +211,7 @@ class MCInstrDesc {
unsigned char NumImplicitUses; // Num of regs implicitly used
unsigned char NumImplicitDefs; // Num of regs implicitly defined
unsigned short OpInfoOffset; // Offset to info about operands
- unsigned int ImplicitOffset; // Offset to start of implicit op list
+ unsigned short ImplicitOffset; // Offset to start of implicit op list
uint64_t Flags; // Flags identifying machine instr class
uint64_t TSFlags; // Target Specific Flag values
diff --git a/llvm/test/TableGen/RegClassByHwMode.td b/llvm/test/TableGen/RegClassByHwMode.td
index ec723f8b70478..3282c6191216b 100644
--- a/llvm/test/TableGen/RegClassByHwMode.td
+++ b/llvm/test/TableGen/RegClassByHwMode.td
@@ -24,7 +24,7 @@ include "llvm/Target/Target.td"
// INSTRINFO-NEXT: } // namespace llvm::MyTarget
-// INSTRINFO: { [[LOAD_STACK_GUARD_OPCODE]], 1, 1, 0, 0, 0, 0, [[LOAD_STACK_GUARD_OP_INDEX:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::Rematerializable)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
+// INSTRINFO: { [[LOAD_STACK_GUARD_OPCODE]], 1, 1, 0, 0, 0, 0, MyTargetOpInfoBase + [[LOAD_STACK_GUARD_OP_INDEX:[0-9]+]], 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::Rematerializable)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
// INSTRINFO: /* [[LOAD_STACK_GUARD_OP_INDEX]] */ { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 },
// INSTRINFO: { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 }, { -1, 0, MCOI::OPERAND_IMMEDIATE, 0 }, { -1, 0, MCOI::OPERAND_IMMEDIATE, 0 },
diff --git a/llvm/test/TableGen/target-specialized-pseudos.td b/llvm/test/TableGen/target-specialized-pseudos.td
index 3953a36101fe0..bf55926f21983 100644
--- a/llvm/test/TableGen/target-specialized-pseudos.td
+++ b/llvm/test/TableGen/target-specialized-pseudos.td
@@ -22,13 +22,13 @@
// CHECK: extern const MyTargetInstrTable MyTargetDescs = {
// CHECK-NEXT: {
-// CHECK-NEXT: { [[MY_MOV_OPCODE]], 2, 1, 2, 0, 0, 0, {{[0-9]+}}, MyTargetImpOpBase + 0, 0|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // MY_MOV
-// CHECK-NEXT: { [[G_UBFX_OPCODE]], 4, 1, 0, 0, 0, 0, {{[0-9]+}}, MyTargetImpOpBase + 0, 0|(1ULL<<MCID::PreISelOpcode)|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // G_UBFX
+// CHECK-NEXT: { [[MY_MOV_OPCODE]], 2, 1, 2, 0, 0, 0, MyTargetOpInfoBase + {{[0-9]+}}, 0, 0|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // MY_MOV
+// CHECK-NEXT: { [[G_UBFX_OPCODE]], 4, 1, 0, 0, 0, 0, MyTargetOpInfoBase + {{[0-9]+}}, 0, 0|(1ULL<<MCID::PreISelOpcode)|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // G_UBFX
-// ALLCASES: { [[PATCHABLE_TYPED_EVENT_CALL_OPCODE]], 3, 0, 0, 0, 0, 0, [[PATCHABLE_TYPED_EVENT_CALL_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::Call)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::MayStore)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
-// ALLCASES: { [[PATCHABLE_EVENT_CALL_OPCODE]], 2, 0, 0, 0, 0, 0, [[PATCHABLE_EVENT_CALL_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::Call)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::MayStore)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
-// ALLCASES: { [[PREALLOCATED_ARG_OPCODE]], 3, 1, 0, 0, 0, 0, [[PREALLOCATED_ARG_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
-// ALLCASES: { [[LOAD_STACK_GUARD_OPCODE]], 1, 1, 0, 0, 0, 0, [[LOAD_STACK_GUARD_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::Rematerializable)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
+// ALLCASES: { [[PATCHABLE_TYPED_EVENT_CALL_OPCODE]], 3, 0, 0, 0, 0, 0, MyTargetOpInfoBase + [[PATCHABLE_TYPED_EVENT_CALL_OP_ENTRY:[0-9]+]], 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::Call)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::MayStore)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
+// ALLCASES: { [[PATCHABLE_EVENT_CALL_OPCODE]], 2, 0, 0, 0, 0, 0, MyTargetOpInfoBase + [[PATCHABLE_EVENT_CALL_OP_ENTRY:[0-9]+]], 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::Call)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::MayStore)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
+// ALLCASES: { [[PREALLOCATED_ARG_OPCODE]], 3, 1, 0, 0, 0, 0, MyTargetOpInfoBase + [[PREALLOCATED_ARG_OP_ENTRY:[0-9]+]], 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
+// ALLCASES: { [[LOAD_STACK_GUARD_OPCODE]], 1, 1, 0, 0, 0, 0, MyTargetOpInfoBase + [[LOAD_STACK_GUARD_OP_ENTRY:[0-9]+]], 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::Rematerializable)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
// CHECK: /* 0 */ { -1, 0, MCOI::OPERAND_UNKNOWN, 0 },
diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index 9cd6ad28b1be4..0faef33a386e7 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -959,13 +959,19 @@ void InstrInfoEmitter::run(raw_ostream &OS) {
OS << "struct " << TargetName << "InstrTable {\n";
OS << " MCInstrDesc Insts[" << NumberedInstructions.size() << "];\n";
+ OS << " static_assert(alignof(MCInstrDesc) >= alignof(MCPhysReg), "
+ "\"Unwanted padding between Insts and ImplicitOps\");\n";
+ OS << " MCPhysReg ImplicitOps[" << std::max(ImplicitListSize, 1U)
+ << "];\n";
+ // Emit enough padding to make ImplicitOps plus Padding add up to the size
+ // of a whole number of MCOperandInfo structs. This allows us to index into
+ // the OperandInfo array starting from the end of the Insts array, by
+ // biasing the indices by the OpInfoBase value calculated below.
+ OS << " char Padding[sizeof(MCOperandInfo) - sizeof ImplicitOps % "
+ "sizeof(MCOperandInfo)];\n";
OS << " static_assert(alignof(MCInstrDesc) >= alignof(MCOperandInfo), "
"\"Unwanted padding between Insts and OperandInfo\");\n";
OS << " MCOperandInfo OperandInfo[" << OperandInfoSize << "];\n";
- OS << " static_assert(alignof(MCOperandInfo) >= alignof(MCPhysReg), "
- "\"Unwanted padding between OperandInfo and ImplicitOps\");\n";
- OS << " MCPhysReg ImplicitOps[" << std::max(ImplicitListSize, 1U)
- << "];\n";
OS << "};";
}
@@ -991,9 +997,12 @@ void InstrInfoEmitter::run(raw_ostream &OS) {
// Emit all of the MCInstrDesc records in reverse ENUM ordering.
Timer.startTimer("Emit InstrDesc records");
- OS << "static_assert(sizeof(MCOperandInfo) % sizeof(MCPhysReg) == 0);\n";
- OS << "static constexpr unsigned " << TargetName << "ImpOpBase = sizeof "
- << TargetName << "InstrTable::OperandInfo / (sizeof(MCPhysReg));\n\n";
+ OS << "static_assert((sizeof " << TargetName
+ << "InstrTable::ImplicitOps + sizeof " << TargetName
+ << "InstrTable::Padding) % sizeof(MCOperandInfo) == 0);\n";
+ OS << "static constexpr unsigned " << TargetName << "OpInfoBase = (sizeof "
+ << TargetName << "InstrTable::ImplicitOps + sizeof " << TargetName
+ << "InstrTable::Padding) / sizeof(MCOperandInfo);\n\n";
OS << "extern const " << TargetName << "InstrTable " << TargetName
<< "Descs = {\n {\n";
@@ -1013,12 +1022,6 @@ void InstrInfoEmitter::run(raw_ostream &OS) {
OS << " }, {\n";
- // Emit all of the operand info records.
- Timer.startTimer("Emit operand info");
- EmitOperandInfo(OS, OperandInfoList);
-
- OS << " }, {\n";
-
// Emit all of the instruction's implicit uses and defs.
Timer.startTimer("Emit uses/defs");
for (auto &List : ImplicitLists) {
@@ -1028,6 +1031,17 @@ void InstrInfoEmitter::run(raw_ostream &OS) {
OS << '\n';
}
+ OS << " }, {\n";
+
+ // Emit the padding.
+ OS << " 0\n";
+
+ OS << " }, {\n";
+
+ // Emit all of the operand info records.
+ Timer.startTimer("Emit operand info");
+ EmitOperandInfo(OS, OperandInfoList);
+
OS << " }\n};\n\n";
// Emit the array of instruction names.
@@ -1291,11 +1305,11 @@ void InstrInfoEmitter::emitRecord(
// Emit the operand info offset.
OperandInfoTy OperandInfo = GetOperandInfo(Inst);
- OS << OperandInfoMap.find(OperandInfo)->second << ",\t";
+ OS << Target.getName() << "OpInfoBase + "
+ << OperandInfoMap.find(OperandInfo)->second << ",\t";
// Emit implicit operand base.
- OS << Target.getName() << "ImpOpBase + " << EmittedLists[ImplicitOps]
- << ",\t0";
+ OS << EmittedLists[ImplicitOps] << ",\t0";
// Emit all of the target independent flags...
if (Inst.isPreISelOpcode)
|
perlfu
reviewed
Dec 9, 2025
| // of a whole number of MCOperandInfo structs. This allows us to index into | ||
| // the OperandInfo array starting from the end of the Insts array, by | ||
| // biasing the indices by the OpInfoBase value calculated below. | ||
| OS << " char Padding[sizeof(MCOperandInfo) - sizeof ImplicitOps % " |
Contributor
There was a problem hiding this comment.
I know this is mostly inherited from the existing code, but there seems to be a mix of sizeof without brackets and with brackets. Perhaps we can tidy them all up?
(This also applies to many of the lines below.)
Contributor
Author
There was a problem hiding this comment.
That's how C works - you only need parentheses for sizeof a type, not for sizeof a variable.
Contributor
There was a problem hiding this comment.
Sure, I know syntactically it's OK -- but would it be more consistent to use brackets for them all? (Feel free to ignore me, I know this is just a nit.)
Contributor
Author
There was a problem hiding this comment.
Thanks. My personal preference is not to add the parens where they're not required.
| // biasing the indices by the OpInfoBase value calculated below. | ||
| OS << " char Padding[sizeof(MCOperandInfo) - sizeof ImplicitOps % " | ||
| "sizeof(MCOperandInfo)];\n"; | ||
| OS << " static_assert(alignof(MCInstrDesc) >= alignof(MCOperandInfo), " |
Contributor
There was a problem hiding this comment.
Is this assertion still correct/relevant?
Contributor
Author
There was a problem hiding this comment.
Yes. It's not completely obvious, but I think this is still required, otherwise there's no way we could index into the OperandInfo array starting from the end of the Insts array.
perlfu
approved these changes
Dec 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Put ImplicitOps[] before OperandInfo[] in the generated
TARGETInstrTable. This means that offsets to entries into the (small)
ImplicitOps table do not need to skip over the (large) OperandInfo
table.
This allows shrinking ImplicitOffset from 32 bits to 16 bits
(effectively reverting #138127) which will allow expanding Opcode
instead without increasing the size of MCInstrDesc.