Skip to content

[AsmPrinter][ELF] Support profile-guided section prefix for jump tables' (read-only) data sections #122215

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.

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
merged 23 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
5d207e9
[SDP]Introduce StaticDataSplitter pass and implemenet jump table spli…
mingmingl-llvm Jan 8, 2025
34b6b9b
Flag-gate the new pass and resolve review feedback
mingmingl-llvm Jan 9, 2025
dd74827
rely to upstream
mingmingl-llvm Jan 10, 2025
8d3a985
Emit jump table with section suffix
mingmingl-llvm Jan 11, 2025
1bacc51
Apply suggestions from code review
mingmingl-llvm Jan 11, 2025
8a85d1a
resolve review feedback
mingmingl-llvm Jan 11, 2025
e54dacb
Discover jump table by calling MachineOperand::isJTI()
mingmingl-llvm Jan 13, 2025
27ef86d
Introduce a TargetMachine option , and the command line flag option t…
mingmingl-llvm Jan 14, 2025
e816def
Emit jump tables into .hot and .unlikely sections respectively
mingmingl-llvm Jan 15, 2025
28defd8
merge base patch
mingmingl-llvm Jan 15, 2025
7f3e473
Add -mtriple=x86_64-unknown-linux-gnu for test
mingmingl-llvm Jan 16, 2025
1bef9b1
run clang format
mingmingl-llvm Jan 16, 2025
59a958a
Apply suggestions from code review
mingmingl-llvm Jan 17, 2025
2d06092
fix windows build bot failure
mingmingl-llvm Jan 17, 2025
9134ffa
resolve comments
mingmingl-llvm Jan 17, 2025
ea2d838
merge with main branch
mingmingl-llvm Jan 24, 2025
027ae56
resolve review feedback
mingmingl-llvm Jan 27, 2025
fd0034a
[NFCI]Refactor AsmPrinter around jump table emission
mingmingl-llvm Jan 27, 2025
a79a789
Merge branch 'main' into users/mingmingl-llvm/spr/nfcprecommit
mingmingl-llvm Jan 27, 2025
d569eff
display diff after refactor
mingmingl-llvm Jan 27, 2025
be96277
git merge main
mingmingl-llvm Jan 27, 2025
b919de9
resolve comments
mingmingl-llvm Jan 28, 2025
6bb4b32
Resolve comments. Also append a dot when -fno-unique-section-names is…
mingmingl-llvm Jan 28, 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: 2 additions & 0 deletions llvm/include/llvm/CodeGen/CommandFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ bool getEmitCallSiteInfo();

bool getEnableMachineFunctionSplitter();

bool getEnableStaticDataPartitioning();

bool getEnableDebugEntryValues();

bool getValueTrackingVariableLocations();
Expand Down
3 changes: 3 additions & 0 deletions llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ class TargetLoweringObjectFileELF : public TargetLoweringObjectFile {

MCSection *getSectionForJumpTable(const Function &F,
const TargetMachine &TM) const override;
MCSection *
getSectionForJumpTable(const Function &F, const TargetMachine &TM,
const MachineJumpTableEntry *JTE) const override;
MCSection *getSectionForLSDA(const Function &F, const MCSymbol &FnSym,
const TargetMachine &TM) const override;

Expand Down
5 changes: 5 additions & 0 deletions llvm/include/llvm/Target/TargetLoweringObjectFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
namespace llvm {

struct Align;
struct MachineJumpTableEntry;
class Constant;
class DataLayout;
class Function;
Expand Down Expand Up @@ -135,6 +136,10 @@ class TargetLoweringObjectFile : public MCObjectFileInfo {

virtual MCSection *getSectionForJumpTable(const Function &F,
const TargetMachine &TM) const;
virtual MCSection *
getSectionForJumpTable(const Function &F, const TargetMachine &TM,
const MachineJumpTableEntry *JTE) const;

virtual MCSection *getSectionForLSDA(const Function &, const MCSymbol &,
const TargetMachine &) const {
return LSDASection;
Expand Down
4 changes: 4 additions & 0 deletions llvm/include/llvm/Target/TargetMachine.h
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,10 @@ class TargetMachine {
return Options.FunctionSections;
}

bool getEnableStaticDataPartitioning() const {
return Options.EnableStaticDataPartitioning;
}

/// Return true if visibility attribute should not be emitted in XCOFF,
/// corresponding to -mignore-xcoff-visibility.
bool getIgnoreXCOFFVisibility() const {
Expand Down
6 changes: 5 additions & 1 deletion llvm/include/llvm/Target/TargetOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ namespace llvm {
TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
EmulatedTLS(false), EnableTLSDESC(false), EnableIPRA(false),
EmitStackSizeSection(false), EnableMachineOutliner(false),
EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
EnableMachineFunctionSplitter(false),
EnableStaticDataPartitioning(false), SupportsDefaultOutlining(false),
EmitAddrsig(false), BBAddrMap(false), EmitCallSiteInfo(false),
SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
Expand Down Expand Up @@ -312,6 +313,9 @@ namespace llvm {
/// Enables the MachineFunctionSplitter pass.
unsigned EnableMachineFunctionSplitter : 1;

/// Enables the StaticDataSplitter pass.
unsigned EnableStaticDataPartitioning : 1;

/// Set if the target supports default outlining behaviour.
unsigned SupportsDefaultOutlining : 1;

Expand Down
32 changes: 27 additions & 5 deletions llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2853,7 +2853,6 @@ void AsmPrinter::emitConstantPool() {
// Print assembly representations of the jump tables used by the current
// function.
void AsmPrinter::emitJumpTableInfo() {
const DataLayout &DL = MF->getDataLayout();
const MachineJumpTableInfo *MJTI = MF->getJumpTableInfo();
if (!MJTI) return;
if (MJTI->getEntryKind() == MachineJumpTableInfo::EK_Inline) return;
Expand All @@ -2869,11 +2868,28 @@ void AsmPrinter::emitJumpTableInfo() {
MJTI->getEntryKind() == MachineJumpTableInfo::EK_LabelDifference64,
F);

SmallVector<unsigned> JumpTableIndices;
if (!TM.Options.EnableStaticDataPartitioning) {
SmallVector<unsigned> JumpTableIndices;
for (unsigned JTI = 0, JTSize = JT.size(); JTI < JTSize; ++JTI)
JumpTableIndices.push_back(JTI);
emitJumpTableImpl(*MJTI, JumpTableIndices, JTInDiffSection);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be written as emitJumpTableImpl(*MJTI, llvm::seq(0, JT.size()), JTInDiffSection);?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! TIL about llvm::seq. llvm::to_vector(llvm::seq(<range>)) works.

return;
}

SmallVector<unsigned> HotJumpTableIndices, ColdJumpTableIndices;
// When static data partitioning is enabled, collect jump table entries that
// go into the same section together to reduce the amount of section switch
// statements.
for (unsigned JTI = 0, JTSize = JT.size(); JTI < JTSize; ++JTI) {
JumpTableIndices.push_back(JTI);
if (JT[JTI].Hotness == MachineFunctionDataHotness::Cold) {
ColdJumpTableIndices.push_back(JTI);
} else {
HotJumpTableIndices.push_back(JTI);
}
}
emitJumpTableImpl(*MJTI, JumpTableIndices, JTInDiffSection);

emitJumpTableImpl(*MJTI, HotJumpTableIndices, JTInDiffSection);
emitJumpTableImpl(*MJTI, ColdJumpTableIndices, JTInDiffSection);
}

void AsmPrinter::emitJumpTableImpl(const MachineJumpTableInfo &MJTI,
Expand All @@ -2885,7 +2901,13 @@ void AsmPrinter::emitJumpTableImpl(const MachineJumpTableInfo &MJTI,
const TargetLoweringObjectFile &TLOF = getObjFileLowering();
const Function &F = MF->getFunction();
const std::vector<MachineJumpTableEntry> &JT = MJTI.getJumpTables();
MCSection *JumpTableSection = TLOF.getSectionForJumpTable(F, TM);
MCSection *JumpTableSection = nullptr;
if (TM.Options.EnableStaticDataPartitioning) {
JumpTableSection =
TLOF.getSectionForJumpTable(F, TM, &JT[*JumpTableIndices.begin()]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe s/*JumpTableIndices.begin()/JumpTableIndices.front()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

} else {
JumpTableSection = TLOF.getSectionForJumpTable(F, TM);
}

const DataLayout &DL = MF->getDataLayout();
if (JTInDiffSection) {
Expand Down
8 changes: 8 additions & 0 deletions llvm/lib/CodeGen/CommandFlags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ CGOPT(bool, EnableStackSizeSection)
CGOPT(bool, EnableAddrsig)
CGOPT(bool, EmitCallSiteInfo)
CGOPT(bool, EnableMachineFunctionSplitter)
CGOPT(bool, EnableStaticDataPartitioning)
CGOPT(bool, EnableDebugEntryValues)
CGOPT(bool, ForceDwarfFrameSection)
CGOPT(bool, XRayFunctionIndex)
Expand Down Expand Up @@ -480,6 +481,12 @@ codegen::RegisterCodeGenFlags::RegisterCodeGenFlags() {
cl::init(false));
CGBINDOPT(EnableMachineFunctionSplitter);

static cl::opt<bool> EnableStaticDataPartitioning(
"partition-static-data-sections",
cl::desc("Partition data sections using profile information."),
cl::init(false));
CGBINDOPT(EnableStaticDataPartitioning);

static cl::opt<bool> ForceDwarfFrameSection(
"force-dwarf-frame-section",
cl::desc("Always emit a debug frame section."), cl::init(false));
Expand Down Expand Up @@ -586,6 +593,7 @@ codegen::InitTargetOptionsFromCodeGenFlags(const Triple &TheTriple) {
Options.ExceptionModel = getExceptionModel();
Options.EmitStackSizeSection = getEnableStackSizeSection();
Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
Options.EnableStaticDataPartitioning = getEnableStaticDataPartitioning();
Options.EmitAddrsig = getEnableAddrsig();
Options.EmitCallSiteInfo = getEmitCallSiteInfo();
Options.EnableDebugEntryValues = getEnableDebugEntryValues();
Expand Down
23 changes: 4 additions & 19 deletions llvm/lib/CodeGen/StaticDataSplitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,11 @@ using namespace llvm;

#define DEBUG_TYPE "static-data-splitter"

STATISTIC(NumHotJumpTables, "Number of hot jump tables seen");
STATISTIC(NumColdJumpTables, "Number of cold jump tables seen");
STATISTIC(NumHotJumpTables, "Number of hot jump tables seen.");
STATISTIC(NumColdJumpTables, "Number of cold jump tables seen.");
STATISTIC(NumUnknownJumpTables,
"Number of jump tables with unknown hotness. Option "
"-static-data-default-hotness specifies the hotness.");

static cl::opt<MachineFunctionDataHotness> StaticDataDefaultHotness(
"static-data-default-hotness", cl::Hidden,
cl::desc("This option specifies the hotness of static data when profile "
"information is unavailable"),
cl::init(MachineFunctionDataHotness::Hot),
cl::values(clEnumValN(MachineFunctionDataHotness::Hot, "hot", "Hot"),
clEnumValN(MachineFunctionDataHotness::Cold, "cold", "Cold")));
"Number of jump tables with unknown hotness. They are from functions "
"without profile information.");

class StaticDataSplitter : public MachineFunctionPass {
const MachineBranchProbabilityInfo *MBPI = nullptr;
Expand Down Expand Up @@ -156,13 +148,6 @@ bool StaticDataSplitter::splitJumpTables(MachineFunction &MF) {
if (ProfileAvailable)
return splitJumpTablesWithProfiles(MF, *MJTI);

// If function profile is unavailable (e.g., module not instrumented, or new
// code paths lacking samples), -static-data-default-hotness specifies the
// hotness.
for (size_t JTI = 0; JTI < MJTI->getJumpTables().size(); JTI++)
MF.getJumpTableInfo()->updateJumpTableEntryHotness(
JTI, StaticDataDefaultHotness);

return true;
}

Expand Down
33 changes: 25 additions & 8 deletions llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "llvm/CodeGen/BasicBlockSectionUtils.h"
#include "llvm/CodeGen/MachineBasicBlock.h"
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineJumpTableInfo.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/MachineModuleInfoImpls.h"
#include "llvm/IR/Comdat.h"
Expand Down Expand Up @@ -648,7 +649,8 @@ static StringRef getSectionPrefixForGlobal(SectionKind Kind, bool IsLarge) {
static SmallString<128>
getELFSectionNameForGlobal(const GlobalObject *GO, SectionKind Kind,
Mangler &Mang, const TargetMachine &TM,
unsigned EntrySize, bool UniqueSectionName) {
unsigned EntrySize, bool UniqueSectionName,
const MachineJumpTableEntry *JTE) {
SmallString<128> Name =
getSectionPrefixForGlobal(Kind, TM.isLargeGlobalValue(GO));
if (Kind.isMergeableCString()) {
Expand All @@ -669,7 +671,15 @@ getELFSectionNameForGlobal(const GlobalObject *GO, SectionKind Kind,

bool HasPrefix = false;
if (const auto *F = dyn_cast<Function>(GO)) {
if (std::optional<StringRef> Prefix = F->getSectionPrefix()) {
// Jump table hotness takes precedence over its enclosing function's hotness
// if both are available.
if (JTE) {
if (JTE->Hotness == MachineFunctionDataHotness::Hot) {
raw_svector_ostream(Name) << ".hot";
} else if (JTE->Hotness == MachineFunctionDataHotness::Cold) {
raw_svector_ostream(Name) << ".unlikely";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen to the case where JTE->Hotness == MachineFunctionDataHotness::Unknown? I think the behaviour we want is to fall back to the section prefix. Can you add a test if you agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes sense. Done.

The updated PR has @bar for test coverage.

} else if (std::optional<StringRef> Prefix = F->getSectionPrefix()) {
raw_svector_ostream(Name) << '.' << *Prefix;
HasPrefix = true;
}
Expand Down Expand Up @@ -767,8 +777,8 @@ calcUniqueIDUpdateFlagsAndSize(const GlobalObject *GO, StringRef SectionName,
// implicitly for this symbol e.g. .rodata.str1.1, then we don't need
// to unique the section as the entry size for this symbol will be
// compatible with implicitly created sections.
SmallString<128> ImplicitSectionNameStem =
getELFSectionNameForGlobal(GO, Kind, Mang, TM, EntrySize, false);
SmallString<128> ImplicitSectionNameStem = getELFSectionNameForGlobal(
GO, Kind, Mang, TM, EntrySize, false, /*MJTE=*/nullptr);
if (SymbolMergeable &&
Ctx.isELFImplicitMergeableSectionNamePrefix(SectionName) &&
SectionName.starts_with(ImplicitSectionNameStem))
Expand Down Expand Up @@ -874,7 +884,8 @@ MCSection *TargetLoweringObjectFileELF::getExplicitSectionGlobal(
static MCSectionELF *selectELFSectionForGlobal(
MCContext &Ctx, const GlobalObject *GO, SectionKind Kind, Mangler &Mang,
const TargetMachine &TM, bool EmitUniqueSection, unsigned Flags,
unsigned *NextUniqueID, const MCSymbolELF *AssociatedSymbol) {
unsigned *NextUniqueID, const MCSymbolELF *AssociatedSymbol,
const MachineJumpTableEntry *MJTE = nullptr) {

auto [Group, IsComdat, ExtraFlags] = getGlobalObjectInfo(GO, TM);
Flags |= ExtraFlags;
Expand All @@ -893,7 +904,7 @@ static MCSectionELF *selectELFSectionForGlobal(
}
}
SmallString<128> Name = getELFSectionNameForGlobal(
GO, Kind, Mang, TM, EntrySize, UniqueSectionName);
GO, Kind, Mang, TM, EntrySize, UniqueSectionName, MJTE);

// Use 0 as the unique ID for execute-only text.
if (Kind.isExecuteOnly())
Expand Down Expand Up @@ -967,17 +978,23 @@ MCSection *TargetLoweringObjectFileELF::getUniqueSectionForFunction(

MCSection *TargetLoweringObjectFileELF::getSectionForJumpTable(
const Function &F, const TargetMachine &TM) const {
return getSectionForJumpTable(F, TM, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: document the param name for nullptr like you have elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, and did the same for nullptr in TargetLoweringObjectFile.cpp at line 351.

}

MCSection *TargetLoweringObjectFileELF::getSectionForJumpTable(
const Function &F, const TargetMachine &TM,
const MachineJumpTableEntry *JTE) const {
// If the function can be removed, produce a unique section so that
// the table doesn't prevent the removal.
const Comdat *C = F.getComdat();
bool EmitUniqueSection = TM.getFunctionSections() || C;
if (!EmitUniqueSection)
if (!EmitUniqueSection && !TM.getEnableStaticDataPartitioning())
return ReadOnlySection;

return selectELFSectionForGlobal(getContext(), &F, SectionKind::getReadOnly(),
getMangler(), TM, EmitUniqueSection,
ELF::SHF_ALLOC, &NextUniqueID,
/* AssociatedSymbol */ nullptr);
/* AssociatedSymbol */ nullptr, JTE);
}

MCSection *TargetLoweringObjectFileELF::getSectionForLSDA(
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/TargetPassConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ void TargetPassConfig::addMachinePasses() {
}
}
addPass(createMachineFunctionSplitterPass());
if (SplitStaticData)
if (SplitStaticData || TM->Options.EnableStaticDataPartitioning)
addPass(createStaticDataSplitterPass());
}
// We run the BasicBlockSections pass if either we need BB sections or BB
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/TargetLoweringObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,12 @@ TargetLoweringObjectFile::SectionForGlobal(const GlobalObject *GO,

MCSection *TargetLoweringObjectFile::getSectionForJumpTable(
const Function &F, const TargetMachine &TM) const {
return getSectionForJumpTable(F, TM, nullptr);
}

MCSection *TargetLoweringObjectFile::getSectionForJumpTable(
const Function &F, const TargetMachine &TM,
const MachineJumpTableEntry *JTE) const {
Align Alignment(1);
return getSectionForConstant(F.getDataLayout(),
SectionKind::getReadOnly(), /*C=*/nullptr,
Expand Down
Loading
Loading