Skip to content

[MC] Use StringTable for MCSchedClassDesc names #137012

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
Open

[MC] Use StringTable for MCSchedClassDesc names #137012

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
236ea03
[MC] Use StringTable for MCSchedClassDesc names
jayfoad Apr 23, 2025
0c4b9d2
Move name table into MCSubtargetInfo
jayfoad Apr 24, 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
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class TargetSubtargetInfo : public MCSubtargetInfo {
const MCWriteProcResEntry *WPR,
const MCWriteLatencyEntry *WL,
const MCReadAdvanceEntry *RA, const InstrStage *IS,
const unsigned *OC, const unsigned *FP);
const unsigned *OC, const unsigned *FP, StringTable NT);

public:
// AntiDepBreakMode - Type of anti-dependence breaking that should
Expand Down
2 changes: 1 addition & 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 @@ -123,7 +123,7 @@ struct MCSchedClassDesc {
static const unsigned short VariantNumMicroOps = InvalidNumMicroOps - 1;

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
const char* Name;
unsigned NameOffset;
#endif
uint16_t NumMicroOps : 13;
uint16_t BeginGroup : 1;
Expand Down
13 changes: 12 additions & 1 deletion llvm/include/llvm/MC/MCSubtargetInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringTable.h"
#include "llvm/MC/MCInstrItineraries.h"
#include "llvm/MC/MCSchedule.h"
#include "llvm/TargetParser/SubtargetFeature.h"
Expand Down Expand Up @@ -93,6 +94,10 @@ class MCSubtargetInfo {
FeatureBitset FeatureBits; // Feature bits for current CPU + FS
std::string FeatureString; // Feature string

// General purpose name table, currently only used to store MCSchedClassDesc
// names when assertions are enabled.
StringTable NameTable;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we guard this def as well with if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)? And in the constructor, assert that the NT passed is empty() in non-assert builds optionally?


public:
MCSubtargetInfo(const MCSubtargetInfo &) = default;
MCSubtargetInfo(const Triple &TT, StringRef CPU, StringRef TuneCPU,
Expand All @@ -101,7 +106,7 @@ class MCSubtargetInfo {
ArrayRef<SubtargetSubTypeKV> PD,
const MCWriteProcResEntry *WPR, const MCWriteLatencyEntry *WL,
const MCReadAdvanceEntry *RA, const InstrStage *IS,
const unsigned *OC, const unsigned *FP);
const unsigned *OC, const unsigned *FP, StringTable NT);
MCSubtargetInfo() = delete;
MCSubtargetInfo &operator=(const MCSubtargetInfo &) = delete;
MCSubtargetInfo &operator=(MCSubtargetInfo &&) = delete;
Expand Down Expand Up @@ -226,6 +231,12 @@ class MCSubtargetInfo {
return 0;
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
StringRef getSchedClassName(const MCSchedClassDesc *SCDesc) const {
return NameTable[SCDesc->NameOffset];
}
#endif

/// Check whether the CPU string is valid.
virtual bool isCPUStringValid(StringRef CPU) const {
auto Found = llvm::lower_bound(ProcDesc, CPU);
Expand Down
3 changes: 2 additions & 1 deletion llvm/include/llvm/TableGen/StringToOffsetTable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ class StringToOffsetTable {
// `static` and `constexpr`. Both `Name` and (`Name` + "Storage") must be
// valid identifiers to declare.
void EmitStringTableDef(raw_ostream &OS, const Twine &Name,
const Twine &Indent = "") const;
const Twine &Indent = "",
StringRef Linkage = "static") const;

// Emit the string as one single string.
void EmitString(raw_ostream &O) const;
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/CodeGen/TargetSubtargetInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ TargetSubtargetInfo::TargetSubtargetInfo(
ArrayRef<StringRef> PN, ArrayRef<SubtargetFeatureKV> PF,
ArrayRef<SubtargetSubTypeKV> PD, const MCWriteProcResEntry *WPR,
const MCWriteLatencyEntry *WL, const MCReadAdvanceEntry *RA,
const InstrStage *IS, const unsigned *OC, const unsigned *FP)
: MCSubtargetInfo(TT, CPU, TuneCPU, FS, PN, PF, PD, WPR, WL, RA, IS, OC,
FP) {}
const InstrStage *IS, const unsigned *OC, const unsigned *FP,
StringTable NT)
: MCSubtargetInfo(TT, CPU, TuneCPU, FS, PN, PF, PD, WPR, WL, RA, IS, OC, FP,
NT) {}

TargetSubtargetInfo::~TargetSubtargetInfo() = default;

Expand Down
17 changes: 10 additions & 7 deletions llvm/lib/MC/MCSubtargetInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,19 @@ void MCSubtargetInfo::setDefaultFeatures(StringRef CPU, StringRef TuneCPU,
FeatureString = std::string(FS);
}

MCSubtargetInfo::MCSubtargetInfo(
const Triple &TT, StringRef C, StringRef TC, StringRef FS,
ArrayRef<StringRef> PN, ArrayRef<SubtargetFeatureKV> PF,
ArrayRef<SubtargetSubTypeKV> PD, const MCWriteProcResEntry *WPR,
const MCWriteLatencyEntry *WL, const MCReadAdvanceEntry *RA,
const InstrStage *IS, const unsigned *OC, const unsigned *FP)
MCSubtargetInfo::MCSubtargetInfo(const Triple &TT, StringRef C, StringRef TC,
StringRef FS, ArrayRef<StringRef> PN,
ArrayRef<SubtargetFeatureKV> PF,
ArrayRef<SubtargetSubTypeKV> PD,
const MCWriteProcResEntry *WPR,
const MCWriteLatencyEntry *WL,
const MCReadAdvanceEntry *RA,
const InstrStage *IS, const unsigned *OC,
const unsigned *FP, StringTable NT)
: TargetTriple(TT), CPU(std::string(C)), TuneCPU(std::string(TC)),
ProcNames(PN), ProcFeatures(PF), ProcDesc(PD), WriteProcResTable(WPR),
WriteLatencyTable(WL), ReadAdvanceTable(RA), Stages(IS),
OperandCycles(OC), ForwardingPaths(FP) {
OperandCycles(OC), ForwardingPaths(FP), NameTable(NT) {
InitMCProcessorInfo(CPU, TuneCPU, FS);
}

Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/MCA/InstrBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ 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
<< " (write index #" << I << ")\n";
WithColor::note() << "found in scheduling class "
<< STI.getSchedClassName(&SCDesc) << " (write index #"
<< I << ")\n";
#endif
continue;
}
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/TableGen/StringToOffsetTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ unsigned StringToOffsetTable::GetOrAddStringOffset(StringRef Str,
}

void StringToOffsetTable::EmitStringTableDef(raw_ostream &OS, const Twine &Name,
const Twine &Indent) const {
const Twine &Indent,
StringRef Linkage) const {
OS << formatv(R"(
#ifdef __GNUC__
#pragma GCC diagnostic push
Expand Down Expand Up @@ -78,10 +79,10 @@ void StringToOffsetTable::EmitStringTableDef(raw_ostream &OS, const Twine &Name,
#pragma GCC diagnostic pop
#endif

{0}static constexpr llvm::StringTable {1} =
{0}{2} constexpr llvm::StringTable {1} =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we renumber 1 and 2 and swap the args to formatv()? Just that its easier to follow if they are in increasing order to the extent possible,

{0} {1}Storage;
)",
Indent, Name);
Indent, Name, Linkage);
}

void StringToOffsetTable::EmitString(raw_ostream &O) const {
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 /* InvalidSchedClass */) 8191, false, false, false, 0, 0, 0, 0, 0, 0},
// CHECK-NEXT: {DBGFIELD(19 /* Inst_A */) 1, false, false, false, 0, 0, 1, 1, 0, 0}, // #1
// CHECK-NEXT: {DBGFIELD(26 /* Inst_B */) 1, false, false, false, 0, 0, 2, 1, 0, 0}, // #2
// CHECK-NEXT: {DBGFIELD(33 /* Inst_C */) 1, false, false, false, 0, 0, 1, 1, 1, 1}, // #3
// CHECK-NEXT: }; // SchedModel_ASchedClasses

let SchedModel = SchedModel_A in {
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/TableGen/InvalidMCSchedClassDesc.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,25 @@ 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(19 /* Inst_A */) 1
// CHECK-NEXT: {DBGFIELD(26 /* Inst_B */) 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(19 /* Inst_A */) 8191
// CHECK-NEXT: {DBGFIELD(26 /* Inst_B */) 1
let SchedModel = SchedModel_B in {
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(19 /* Inst_A */) 1
// CHECK-NEXT: {DBGFIELD(26 /* Inst_B */) 1
let SchedModel = SchedModel_C in {
def Write_C: SchedWriteRes<[]>;
def : InstRW<[Write_C], (instrs Inst_A, Inst_B)>;
Expand Down
8 changes: 5 additions & 3 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,10 @@ 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 auto &SI = State_.getSubtargetInfo();
const MCSchedClassDesc *const SCDesc =
State_.getSubtargetInfo().getSchedModel().getSchedClassDesc(SchedClassId);
writeEscaped<kEscapeCsv>(OS, SCDesc->Name);
SI.getSchedModel().getSchedClassDesc(SchedClassId);
writeEscaped<kEscapeCsv>(OS, SI.getSchedClassName(SCDesc));
#else
OS << SchedClassId;
#endif
Expand Down Expand Up @@ -563,7 +564,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.getSchedClassName(RSCAndPoints.RSC.SCDesc));
#else
OS << RSCAndPoints.RSC.SchedClassId;
#endif
Expand Down
2 changes: 1 addition & 1 deletion llvm/unittests/CodeGen/MFCommon.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class BogusSubtarget : public TargetSubtargetInfo {
public:
BogusSubtarget(TargetMachine &TM)
: TargetSubtargetInfo(Triple(""), "", "", "", {}, {}, {}, nullptr,
nullptr, nullptr, nullptr, nullptr, nullptr),
nullptr, nullptr, nullptr, nullptr, nullptr, ""),
FL(), TL(TM) {}
~BogusSubtarget() override {}

Expand Down
2 changes: 1 addition & 1 deletion llvm/unittests/Target/AArch64/AArch64InstPrinterTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ static std::string AArch64InstPrinterTestPrintAlignedLabel(uint64_t value) {
MCSubtargetInfo STI(Triple(""), "", "", "", {},
ArrayRef((SubtargetFeatureKV *)NULL, (size_t)0),
ArrayRef((SubtargetSubTypeKV *)NULL, (size_t)0), NULL,
NULL, NULL, NULL, NULL, NULL);
NULL, NULL, NULL, NULL, NULL, "");
MCContext Ctx(Triple(""), &MAI, &MRI, &STI);
MCInst MI;

Expand Down
27 changes: 21 additions & 6 deletions llvm/utils/TableGen/SubtargetEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,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 @@ -1430,6 +1431,7 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
OS << "}; // " << Target << "ReadAdvanceTable\n";

// Emit a SchedClass table for each processor.
StringToOffsetTable NameTable;
for (const auto &[Idx, Proc] : enumerate(SchedModels.procModels())) {
if (!Proc.hasInstrSchedModel())
continue;
Expand All @@ -1446,14 +1448,17 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
// name and position.
assert(SchedModels.getSchedClass(0).Name == "NoInstrModel" &&
"invalid class not first");
OS << " {DBGFIELD(\"InvalidSchedClass\") "
unsigned NameOffset = NameTable.GetOrAddStringOffset("InvalidSchedClass");
OS << " {DBGFIELD(" << NameOffset << " /* InvalidSchedClass */) "
<< 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 << "\") ";
NameOffset = NameTable.GetOrAddStringOffset(SchedClass.Name);
OS << " {DBGFIELD(" << NameOffset << " /* " << SchedClass.Name
<< " */) ";
if (SchedClass.Name.size() < 18)
OS.indent(18 - SchedClass.Name.size());
OS << MCDesc.NumMicroOps << ", " << (MCDesc.BeginGroup ? "true" : "false")
Expand All @@ -1468,6 +1473,14 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
}
OS << "}; // " << Proc.ModelName << "SchedClasses\n";
}

OS << "\n#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)\n";
NameTable.EmitStringTableDef(OS, Target + "NameTable", /*Indent=*/"",
"extern");
OS << "\n#else\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

C++ raw string might make this easier to read

OS << "\nextern constexpr llvm::StringTable " << Target
Copy link
Contributor

Choose a reason for hiding this comment

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

extern constexpr? I'm surprised this even compiles.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that could also be the cause of the link failure seen above

<< "NameTable = \"\";\n";
OS << "\n#endif\n";
}

void SubtargetEmitter::emitProcessorModels(raw_ostream &OS) {
Expand Down Expand Up @@ -1952,9 +1965,9 @@ void SubtargetEmitter::emitGenMCSubtargetInfo(raw_ostream &OS) {
<< " const MCWriteProcResEntry *WPR,\n"
<< " const MCWriteLatencyEntry *WL,\n"
<< " const MCReadAdvanceEntry *RA, const InstrStage *IS,\n"
<< " const unsigned *OC, const unsigned *FP) :\n"
<< " const unsigned *OC, const unsigned *FP, StringTable NT) :\n"
<< " MCSubtargetInfo(TT, CPU, TuneCPU, FS, PN, PF, PD,\n"
<< " WPR, WL, RA, IS, OC, FP) { }\n\n"
<< " WPR, WL, RA, IS, OC, FP, NT) { }\n\n"
<< " unsigned resolveVariantSchedClass(unsigned SchedClass,\n"
<< " const MCInst *MI, const MCInstrInfo *MCII,\n"
<< " unsigned CPUID) const override {\n"
Expand Down Expand Up @@ -2059,7 +2072,7 @@ void SubtargetEmitter::run(raw_ostream &OS) {
<< "ForwardingPaths";
} else
OS << "nullptr, nullptr, nullptr";
OS << ");\n}\n\n";
OS << ", " << Target << "NameTable);\n}\n\n";

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

Expand Down Expand Up @@ -2159,6 +2172,8 @@ void SubtargetEmitter::run(raw_ostream &OS) {
OS << "extern const unsigned " << Target << "ForwardingPaths[];\n";
}

OS << "extern const llvm::StringTable " << Target << "NameTable;\n";

OS << ClassName << "::" << ClassName << "(const Triple &TT, StringRef CPU, "
<< "StringRef TuneCPU, StringRef FS)\n";

Expand Down Expand Up @@ -2190,7 +2205,7 @@ void SubtargetEmitter::run(raw_ostream &OS) {
<< "ForwardingPaths";
} else
OS << "nullptr, nullptr, nullptr";
OS << ") {}\n\n";
OS << ", " << Target << "NameTable) {}\n\n";

emitSchedModelHelpers(ClassName, OS);
emitHwModeCheck(ClassName, OS);
Expand Down
Loading