-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[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
[AsmPrinter][ELF] Support profile-guided section prefix for jump tables' (read-only) data sections #122215
Changes from 21 commits
5d207e9
34b6b9b
dd74827
8d3a985
1bacc51
8a85d1a
e54dacb
27ef86d
e816def
28defd8
7f3e473
1bef9b1
59a958a
2d06092
9134ffa
ea2d838
027ae56
fd0034a
a79a789
d569eff
be96277
b919de9
6bb4b32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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); | ||
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, | ||
|
@@ -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()]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -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()) { | ||
|
@@ -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"; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What will happen to the case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes sense. Done. The updated PR has |
||
} else if (std::optional<StringRef> Prefix = F->getSectionPrefix()) { | ||
raw_svector_ostream(Name) << '.' << *Prefix; | ||
HasPrefix = true; | ||
} | ||
|
@@ -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)) | ||
|
@@ -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; | ||
|
@@ -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()) | ||
|
@@ -967,17 +978,23 @@ MCSection *TargetLoweringObjectFileELF::getUniqueSectionForFunction( | |
|
||
MCSection *TargetLoweringObjectFileELF::getSectionForJumpTable( | ||
const Function &F, const TargetMachine &TM) const { | ||
return getSectionForJumpTable(F, TM, nullptr); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: document the param name for nullptr like you have elsewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done, and did the same for |
||
} | ||
|
||
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( | ||
|
There was a problem hiding this comment.
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);
?There was a problem hiding this comment.
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.