Skip to content

[MC] Use StringTable to reduce dynamic relocations #144202

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+53 −25
Open

[MC] Use StringTable to reduce dynamic relocations #144202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
551c831
[MC] Use StringTable to reduce dynamic relocations
rnk Jun 14, 2025
d9e4f0f
fix tblgen tests
rnk Jun 14, 2025
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
14 changes: 13 additions & 1 deletion llvm/include/llvm/MC/MCSchedule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#define LLVM_MC_MCSCHEDULE_H

#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringTable.h"
#include "llvm/MC/MCInstrDesc.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/ErrorHandling.h"
Expand Down Expand Up @@ -124,7 +125,7 @@ struct MCSchedClassDesc {
static const unsigned short VariantNumMicroOps = InvalidNumMicroOps - 1;

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
const char* Name;
uint32_t NameOff;
#endif
uint16_t NumMicroOps : 13;
uint16_t BeginGroup : 1;
Expand Down Expand Up @@ -324,6 +325,9 @@ struct MCSchedModel {
const MCSchedClassDesc *SchedClassTable;
unsigned NumProcResourceKinds;
unsigned NumSchedClasses;
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
const StringTable *SchedClassNames;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the field is declared conditionally here, but then initialized unconditionally both via TableGen and in the test file?

Copy link
Member

@compnerd compnerd Jun 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This seems like a concern - hopefully a -Asserts build would just fail and catch this?

// Instruction itinerary tables used by InstrItineraryData.
friend class InstrItineraryData;
const InstrItinerary *InstrItineraries;
Expand Down Expand Up @@ -368,6 +372,14 @@ struct MCSchedModel {
return &SchedClassTable[SchedClassIdx];
}

StringRef getSchedClassName(unsigned SchedClassIdx) const {
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
return (*SchedClassNames)[SchedClassTable[SchedClassIdx].NameOff];
#else
return "<unknown>";
#endif
}

/// Returns the latency value for the scheduling class.
LLVM_ABI static int computeInstrLatency(const MCSubtargetInfo &STI,
const MCSchedClassDesc &SCDesc);
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/MC/MCSchedule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const MCSchedModel MCSchedModel::Default = {DefaultIssueWidth,
0,
0,
nullptr,
nullptr,
nullptr};

int MCSchedModel::computeInstrLatency(const MCSubtargetInfo &STI,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/MCA/InstrBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ static void initializeUsedResources(InstrDesc &ID,
WithColor::warning()
<< "Ignoring invalid write of zero cycles on processor resource "
<< PR.Name << "\n";
WithColor::note() << "found in scheduling class " << SCDesc.Name
WithColor::note() << "found in scheduling class "
<< SM.getSchedClassName(ID.SchedClassID)
<< " (write index #" << I << ")\n";
#endif
continue;
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/TableGen/CompressWriteLatencyEntry.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ def Read_D : SchedRead;
// CHECK-NEXT: }; // MyTargetReadAdvanceTable

// CHECK: static const llvm::MCSchedClassDesc SchedModel_ASchedClasses[] = {
// CHECK-NEXT: {DBGFIELD("InvalidSchedClass") 8191, false, false, false, 0, 0, 0, 0, 0, 0},
// CHECK-NEXT: {DBGFIELD("Inst_A") 1, false, false, false, 0, 0, 1, 1, 0, 0}, // #1
// CHECK-NEXT: {DBGFIELD("Inst_B") 1, false, false, false, 0, 0, 2, 1, 0, 0}, // #2
// CHECK-NEXT: {DBGFIELD("Inst_C") 1, false, false, false, 0, 0, 1, 1, 1, 1}, // #3
// CHECK-NEXT: {DBGFIELD(1) 8191, false, false, false, 0, 0, 0, 0, 0, 0},
// CHECK-NEXT: {DBGFIELD(/*Inst_A*/ {{[0-9]+}}) 1, false, false, false, 0, 0, 1, 1, 0, 0}, // #1
// CHECK-NEXT: {DBGFIELD(/*Inst_B*/ {{[0-9]+}}) 1, false, false, false, 0, 0, 2, 1, 0, 0}, // #2
// CHECK-NEXT: {DBGFIELD(/*Inst_C*/ {{[0-9]+}}) 1, false, false, false, 0, 0, 1, 1, 1, 1}, // #3
// CHECK-NEXT: }; // SchedModel_ASchedClasses

let SchedModel = SchedModel_A in {
Expand Down
22 changes: 11 additions & 11 deletions llvm/test/TableGen/InvalidMCSchedClassDesc.td
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// RUN: llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s
// Check if it is valid MCSchedClassDesc if didn't have the resources.
// Check if it is valid MCSchedClassDesc if didn't have the resources.

include "llvm/Target/Target.td"

def MyTarget : Target;

let OutOperandList = (outs), InOperandList = (ins) in {
def Inst_A : Instruction;
def Inst_B : Instruction;
def Inst_A : Instruction;
def Inst_B : Instruction;
}

let CompleteModel = 0 in {
Expand All @@ -18,27 +18,27 @@ let CompleteModel = 0 in {

// Inst_B didn't have the resoures, and it is invalid.
// CHECK: SchedModel_ASchedClasses[] = {
// CHECK: {DBGFIELD("Inst_A") 1
// CHECK-NEXT: {DBGFIELD("Inst_B") 8191
// CHECK: {DBGFIELD(/*Inst_A*/ 19) 1
// CHECK-NEXT: {DBGFIELD(/*Inst_B*/ 26) 8191
let SchedModel = SchedModel_A in {
def Write_A : SchedWriteRes<[]>;
def : InstRW<[Write_A], (instrs Inst_A)>;
}

// Inst_A didn't have the resoures, and it is invalid.
// CHECK: SchedModel_BSchedClasses[] = {
// CHECK: {DBGFIELD("Inst_A") 8191
// CHECK-NEXT: {DBGFIELD("Inst_B") 1
// CHECK: {DBGFIELD(/*Inst_A*/ 19) 8191
// CHECK-NEXT: {DBGFIELD(/*Inst_B*/ 26) 1
let SchedModel = SchedModel_B in {
def Write_B: SchedWriteRes<[]>;
def Write_B: SchedWriteRes<[]>;
def : InstRW<[Write_B], (instrs Inst_B)>;
}

// CHECK: SchedModel_CSchedClasses[] = {
// CHECK: {DBGFIELD("Inst_A") 1
// CHECK-NEXT: {DBGFIELD("Inst_B") 1
// CHECK: {DBGFIELD(/*Inst_A*/ 19) 1
// CHECK-NEXT: {DBGFIELD(/*Inst_B*/ 26) 1
let SchedModel = SchedModel_C in {
def Write_C: SchedWriteRes<[]>;
def Write_C: SchedWriteRes<[]>;
def : InstRW<[Write_C], (instrs Inst_A, Inst_B)>;
}

Expand Down
9 changes: 5 additions & 4 deletions llvm/tools/llvm-exegesis/lib/Analysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ void Analysis::printInstructionRowCsv(const size_t PointId,
std::tie(SchedClassId, std::ignore) = ResolvedSchedClass::resolveSchedClassId(
State_.getSubtargetInfo(), State_.getInstrInfo(), MCI);
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
const MCSchedClassDesc *const SCDesc =
State_.getSubtargetInfo().getSchedModel().getSchedClassDesc(SchedClassId);
writeEscaped<kEscapeCsv>(OS, SCDesc->Name);
StringRef SCDescName =
State_.getSubtargetInfo().getSchedModel().getSchedClassName(SchedClassId);
writeEscaped<kEscapeCsv>(OS, SCDescName);
#else
OS << SchedClassId;
#endif
Expand Down Expand Up @@ -563,7 +563,8 @@ Error Analysis::run<Analysis::PrintSchedClassInconsistencies>(
OS << "<div class=\"inconsistency\"><p>Sched Class <span "
"class=\"sched-class-name\">";
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
writeEscaped<kEscapeHtml>(OS, RSCAndPoints.RSC.SCDesc->Name);
writeEscaped<kEscapeHtml>(OS, SI.getSchedModel().getSchedClassName(
RSCAndPoints.RSC.SchedClassId));
#else
OS << RSCAndPoints.RSC.SchedClassId;
#endif
Expand Down
21 changes: 17 additions & 4 deletions llvm/utils/TableGen/SubtargetEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "llvm/Support/raw_ostream.h"
#include "llvm/TableGen/Error.h"
#include "llvm/TableGen/Record.h"
#include "llvm/TableGen/StringToOffsetTable.h"
#include "llvm/TableGen/TableGenBackend.h"
#include "llvm/TargetParser/SubtargetFeature.h"
#include <algorithm>
Expand Down Expand Up @@ -1456,6 +1457,10 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
}
OS << "}; // " << Target << "ReadAdvanceTable\n";

// Pool all SchedClass names in a string table.
StringToOffsetTable StrTab;
unsigned InvalidNameOff = StrTab.GetOrAddStringOffset("InvalidSchedClass");

// Emit a SchedClass table for each processor.
for (const auto &[Idx, Proc] : enumerate(SchedModels.procModels())) {
if (!Proc.hasInstrSchedModel())
Expand All @@ -1473,14 +1478,15 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
// name and position.
assert(SchedModels.getSchedClass(0).Name == "NoInstrModel" &&
"invalid class not first");
OS << " {DBGFIELD(\"InvalidSchedClass\") "
OS << " {DBGFIELD(" << InvalidNameOff << ") "
<< MCSchedClassDesc::InvalidNumMicroOps
<< ", false, false, false, 0, 0, 0, 0, 0, 0},\n";

for (unsigned SCIdx = 1, SCEnd = SCTab.size(); SCIdx != SCEnd; ++SCIdx) {
MCSchedClassDesc &MCDesc = SCTab[SCIdx];
const CodeGenSchedClass &SchedClass = SchedModels.getSchedClass(SCIdx);
OS << " {DBGFIELD(\"" << SchedClass.Name << "\") ";
unsigned NameOff = StrTab.GetOrAddStringOffset(SchedClass.Name);
OS << " {DBGFIELD(/*" << SchedClass.Name << "*/ " << NameOff << ") ";
if (SchedClass.Name.size() < 18)
OS.indent(18 - SchedClass.Name.size());
OS << MCDesc.NumMicroOps << ", " << (MCDesc.BeginGroup ? "true" : "false")
Expand All @@ -1495,6 +1501,8 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
}
OS << "}; // " << Proc.ModelName << "SchedClasses\n";
}

StrTab.EmitStringTableDef(OS, Target + "SchedClassNames");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move this behind the NDEBUG check as well, or do we just rely on it being DCEd? (Do we need to suppress a warning about an unused static in release builds possibly?)

Copy link
Member

Choose a reason for hiding this comment

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

I don’t think that DCE would occur in the dynamic build. This is an exposed interface, and unless it was marked as infused or static, the linker cannot know it it’s safe to remove.

}

void SubtargetEmitter::emitProcessorModels(raw_ostream &OS) {
Expand Down Expand Up @@ -1548,6 +1556,8 @@ void SubtargetEmitter::emitProcessorModels(raw_ostream &OS) {
else
OS << " nullptr, nullptr, 0, 0,"
<< " // No instruction-level machine model.\n";
OS << " DBGVAL_OR_NULLPTR(&" << Target
<< "SchedClassNames), // SchedClassNames\n";
if (PM.hasItineraries())
OS << " " << PM.ItinsDef->getName() << ",\n";
else
Expand All @@ -1569,8 +1579,10 @@ void SubtargetEmitter::emitSchedModel(raw_ostream &OS) {
<< "#endif\n"
<< "#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)\n"
<< "#define DBGFIELD(x) x,\n"
<< "#define DBGVAL_OR_NULLPTR(x) x\n"
<< "#else\n"
<< "#define DBGFIELD(x)\n"
<< "#define DBGVAL_OR_NULLPTR(x) nullptr\n"
<< "#endif\n";

if (SchedModels.hasItineraries()) {
Expand All @@ -1588,10 +1600,11 @@ void SubtargetEmitter::emitSchedModel(raw_ostream &OS) {
}
emitSchedClassTables(SchedTables, OS);

OS << "\n#undef DBGFIELD\n";

// Emit the processor machine model
emitProcessorModels(OS);

OS << "\n#undef DBGFIELD\n";
OS << "\n#undef DBGVAL_OR_NULLPTR\n";
}

static void emitPredicateProlog(const RecordKeeper &Records, raw_ostream &OS) {
Expand Down
Loading