Skip to content

[MC] Reorder TARGETInstrTable to shrink MCInstrDesc::ImplicitOffset #171199

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

Merged
jayfoad merged 1 commit into from
Dec 10, 2025
+38 −24
Merged

[MC] Reorder TARGETInstrTable to shrink MCInstrDesc::ImplicitOffset #171199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion llvm/include/llvm/MC/MCInstrDesc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion llvm/test/TableGen/RegClassByHwMode.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/TableGen/target-specialized-pseudos.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 },

Expand Down
46 changes: 30 additions & 16 deletions llvm/utils/TableGen/InstrInfoEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 % "
Copy link
Contributor

@perlfu perlfu Dec 9, 2025

Choose a reason for hiding this comment

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

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.)

Copy link
Contributor Author

@jayfoad jayfoad Dec 9, 2025

Choose a reason for hiding this comment

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

That's how C works - you only need parentheses for sizeof a type, not for sizeof a variable.

Copy link
Contributor

@perlfu perlfu Dec 9, 2025

Choose a reason for hiding this comment

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

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.)

Copy link
Contributor Author

@jayfoad jayfoad Dec 9, 2025

Choose a reason for hiding this comment

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

Thanks. My personal preference is not to add the parens where they're not required.

"sizeof(MCOperandInfo)];\n";
OS << " static_assert(alignof(MCInstrDesc) >= alignof(MCOperandInfo), "
Copy link
Contributor

@perlfu perlfu Dec 9, 2025

Choose a reason for hiding this comment

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

Is this assertion still correct/relevant?

Copy link
Contributor Author

@jayfoad jayfoad Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

"\"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 << "};";
}

Expand All @@ -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";
Expand All @@ -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) {
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
Loading