Skip to content

Commit 0f74d19

Browse files
author
Fabian Parzefall
committed
[BOLT] Generate sections for multiple fragments
This patch adds support to generate any number of sections that are assigned to fragments of functions that are split more than two-way. With this, a function's *nth* split fragment goes into section `.text.cold.n`. This also changes `FunctionLayout::erase` to make sure, that there are no empty fragments at the end of the function. This sometimes happens when blocks are erased from the function. To avoid creating symbols pointing to these fragments, they need to be removed. Reviewed By: rafauler Differential Revision: https://reviews.llvm.org/D130521
1 parent a191ea7 commit 0f74d19

File tree

9 files changed

+121
-89
lines changed

9 files changed

+121
-89
lines changed

‎bolt/include/bolt/Core/BinaryBasicBlock.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -681,11 +681,11 @@ class BinaryBasicBlock {
681681
bool isCold() const {
682682
assert(Fragment.get() < 2 &&
683683
"Function is split into more than two (hot/cold)-fragments");
684-
return Fragment == FragmentNum::cold();
684+
return isSplit();
685685
}
686686

687687
void setIsCold(const bool Flag) {
688-
Fragment = Flag ? FragmentNum::cold() : FragmentNum::hot();
688+
Fragment = Flag ? FragmentNum::cold() : FragmentNum::main();
689689
}
690690

691691
/// Return true if the block can be outlined. At the moment we disallow

‎bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,8 +1078,8 @@ class BinaryFunction {
10781078

10791079
/// Return MC symbol associated with the function.
10801080
/// All references to the function should use this symbol.
1081-
MCSymbol *getSymbol(const FragmentNum Fragment = FragmentNum::hot()) {
1082-
if (Fragment == FragmentNum::hot())
1081+
MCSymbol *getSymbol(const FragmentNum Fragment = FragmentNum::main()) {
1082+
if (Fragment == FragmentNum::main())
10831083
return Symbols[0];
10841084

10851085
size_t ColdSymbolIndex = Fragment.get() - 1;
@@ -1316,35 +1316,29 @@ class BinaryFunction {
13161316
}
13171317

13181318
/// Return internal section name for this function.
1319-
StringRef getCodeSectionName() const { return StringRef(CodeSectionName); }
1319+
SmallString<32>
1320+
getCodeSectionName(const FragmentNum Fragment = FragmentNum::main()) const {
1321+
if (Fragment == FragmentNum::main())
1322+
return SmallString<32>(CodeSectionName);
1323+
if (Fragment == FragmentNum::cold())
1324+
return SmallString<32>(ColdCodeSectionName);
1325+
return formatv("{0}.{1}", ColdCodeSectionName, Fragment.get() - 1);
1326+
}
13201327

13211328
/// Assign a code section name to the function.
1322-
void setCodeSectionName(StringRef Name) {
1323-
CodeSectionName = std::string(Name);
1329+
void setCodeSectionName(const StringRef Name) {
1330+
CodeSectionName = Name.str();
13241331
}
13251332

13261333
/// Get output code section.
1327-
ErrorOr<BinarySection &> getCodeSection() const {
1328-
return BC.getUniqueSectionByName(getCodeSectionName());
1329-
}
1330-
1331-
/// Return cold code section name for the function.
1332-
std::string getColdCodeSectionName(const FragmentNum Fragment) const {
1333-
std::string Result = ColdCodeSectionName;
1334-
if (Fragment != FragmentNum::cold())
1335-
Result.append(".").append(std::to_string(Fragment.get() - 1));
1336-
return Result;
1334+
ErrorOr<BinarySection &>
1335+
getCodeSection(const FragmentNum Fragment = FragmentNum::main()) const {
1336+
return BC.getUniqueSectionByName(getCodeSectionName(Fragment));
13371337
}
13381338

13391339
/// Assign a section name for the cold part of the function.
1340-
void setColdCodeSectionName(StringRef Name) {
1341-
ColdCodeSectionName = std::string(Name);
1342-
}
1343-
1344-
/// Get output code section for cold code of this function.
1345-
ErrorOr<BinarySection &>
1346-
getColdCodeSection(const FragmentNum Fragment) const {
1347-
return BC.getUniqueSectionByName(getColdCodeSectionName(Fragment));
1340+
void setColdCodeSectionName(const StringRef Name) {
1341+
ColdCodeSectionName = Name.str();
13481342
}
13491343

13501344
/// Return true iif the function will halt execution on entry.
@@ -1878,11 +1872,9 @@ class BinaryFunction {
18781872
if (ColdCallSites.empty())
18791873
return nullptr;
18801874

1881-
SmallString<8> SymbolSuffix;
1882-
if (Fragment != FragmentNum::cold())
1883-
SymbolSuffix = formatv(".{0}", Fragment.get());
1884-
ColdLSDASymbol = BC.Ctx->getOrCreateSymbol(formatv(
1885-
"GCC_cold_except_table{0:x-}{1}", getFunctionNumber(), SymbolSuffix));
1875+
ColdLSDASymbol =
1876+
BC.Ctx->getOrCreateSymbol(formatv("GCC_cold_except_table{0:x-}.{1}",
1877+
getFunctionNumber(), Fragment.get()));
18861878

18871879
return ColdLSDASymbol;
18881880
}

‎bolt/include/bolt/Core/FunctionLayout.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ class FragmentNum {
4747
}
4848

4949
static constexpr FragmentNum main() { return FragmentNum(0); }
50-
static constexpr FragmentNum hot() { return FragmentNum(0); }
5150
static constexpr FragmentNum cold() { return FragmentNum(1); }
5251
};
5352

@@ -161,7 +160,7 @@ class FunctionLayout {
161160
/// Get the fragment that contains all entry blocks and other blocks that
162161
/// cannot be split.
163162
FunctionFragment getMainFragment() const {
164-
return getFragment(FragmentNum::hot());
163+
return getFragment(FragmentNum::main());
165164
}
166165

167166
/// Get the fragment that contains all entry blocks and other blocks that
@@ -181,7 +180,8 @@ class FunctionLayout {
181180
void insertBasicBlocks(BinaryBasicBlock *InsertAfter,
182181
ArrayRef<BinaryBasicBlock *> NewBlocks);
183182

184-
/// Erase all blocks from the layout that are in ToErase.
183+
/// Erase all blocks from the layout that are in ToErase. If this method
184+
/// erases all blocks of a fragment, it will be removed as well.
185185
void eraseBasicBlocks(const DenseSet<const BinaryBasicBlock *> ToErase);
186186

187187
/// Make sure fragments' and basic blocks' indices match the current layout.

‎bolt/lib/Core/BinaryContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2205,7 +2205,7 @@ BinaryContext::calculateEmittedSize(BinaryFunction &BF, bool FixBranches) {
22052205
SplitLabels.emplace_back(SplitStartLabel, SplitEndLabel);
22062206

22072207
MCSectionELF *const SplitSection = LocalCtx->getELFSection(
2208-
BF.getColdCodeSectionName(FF.getFragmentNum()), ELF::SHT_PROGBITS,
2208+
BF.getCodeSectionName(FF.getFragmentNum()), ELF::SHT_PROGBITS,
22092209
ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
22102210
SplitSection->setHasInstructions(true);
22112211
Streamer->switchSection(SplitSection);

‎bolt/lib/Core/BinaryEmitter.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,8 @@ bool BinaryEmitter::emitFunction(BinaryFunction &Function,
287287
if (Function.getState() == BinaryFunction::State::Empty)
288288
return false;
289289

290-
MCSection *Section = BC.getCodeSection(
291-
FF.isSplitFragment()
292-
? Function.getColdCodeSectionName(FF.getFragmentNum())
293-
: Function.getCodeSectionName());
290+
MCSection *Section =
291+
BC.getCodeSection(Function.getCodeSectionName(FF.getFragmentNum()));
294292
Streamer.switchSection(Section);
295293
Section->setHasInstructions(true);
296294
BC.Ctx->addGenDwarfSection(Section);
@@ -408,7 +406,7 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF,
408406
const FunctionFragment &FF,
409407
bool EmitCodeOnly) {
410408
if (!EmitCodeOnly && FF.isSplitFragment() && BF.hasConstantIsland()) {
411-
assert(FF.getFragmentNum() == FragmentNum::cold() &&
409+
assert(BF.getLayout().isHotColdSplit() &&
412410
"Constant island support only with hot/cold split");
413411
BF.duplicateConstantIslands();
414412
}
@@ -918,7 +916,7 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, bool EmitColdPart) {
918916

919917
// Corresponding FDE start.
920918
const MCSymbol *StartSymbol =
921-
EmitColdPart ? BF.getSymbol(FragmentNum::cold()) : BF.getSymbol();
919+
BF.getSymbol(EmitColdPart ? FragmentNum::cold() : FragmentNum::main());
922920

923921
// Emit the LSDA header.
924922

‎bolt/lib/Core/BinaryFunction.cpp

Lines changed: 61 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4038,10 +4038,6 @@ void BinaryFunction::updateOutputValues(const MCAsmLayout &Layout) {
40384038
}
40394039

40404040
const uint64_t BaseAddress = getCodeSection()->getOutputAddress();
4041-
ErrorOr<BinarySection &> ColdSection =
4042-
getColdCodeSection(FragmentNum::cold());
4043-
const uint64_t ColdBaseAddress =
4044-
isSplit() ? ColdSection->getOutputAddress() : 0;
40454041
if (BC.HasRelocations || isInjected()) {
40464042
const uint64_t StartOffset = Layout.getSymbolOffset(*getSymbol());
40474043
const uint64_t EndOffset = Layout.getSymbolOffset(*getFunctionEndLabel());
@@ -4053,20 +4049,35 @@ void BinaryFunction::updateOutputValues(const MCAsmLayout &Layout) {
40534049
setOutputDataAddress(BaseAddress + DataOffset);
40544050
}
40554051
if (isSplit()) {
4056-
const MCSymbol *ColdStartSymbol = getSymbol(FragmentNum::cold());
4057-
assert(ColdStartSymbol && ColdStartSymbol->isDefined() &&
4058-
"split function should have defined cold symbol");
4059-
const MCSymbol *ColdEndSymbol = getFunctionEndLabel(FragmentNum::cold());
4060-
assert(ColdEndSymbol && ColdEndSymbol->isDefined() &&
4061-
"split function should have defined cold end symbol");
4062-
const uint64_t ColdStartOffset = Layout.getSymbolOffset(*ColdStartSymbol);
4063-
const uint64_t ColdEndOffset = Layout.getSymbolOffset(*ColdEndSymbol);
4064-
cold().setAddress(ColdBaseAddress + ColdStartOffset);
4065-
cold().setImageSize(ColdEndOffset - ColdStartOffset);
4066-
if (hasConstantIsland()) {
4067-
const uint64_t DataOffset =
4068-
Layout.getSymbolOffset(*getFunctionColdConstantIslandLabel());
4069-
setOutputColdDataAddress(ColdBaseAddress + DataOffset);
4052+
for (const FunctionFragment &FF : getLayout().getSplitFragments()) {
4053+
ErrorOr<BinarySection &> ColdSection =
4054+
getCodeSection(FF.getFragmentNum());
4055+
// If fragment is empty, cold section might not exist
4056+
if (FF.empty() && ColdSection.getError())
4057+
continue;
4058+
const uint64_t ColdBaseAddress = ColdSection->getOutputAddress();
4059+
4060+
const MCSymbol *ColdStartSymbol = getSymbol(FF.getFragmentNum());
4061+
// If fragment is empty, symbol might have not been emitted
4062+
if (FF.empty() && (!ColdStartSymbol || !ColdStartSymbol->isDefined()) &&
4063+
!hasConstantIsland())
4064+
continue;
4065+
assert(ColdStartSymbol && ColdStartSymbol->isDefined() &&
4066+
"split function should have defined cold symbol");
4067+
const MCSymbol *ColdEndSymbol =
4068+
getFunctionEndLabel(FF.getFragmentNum());
4069+
assert(ColdEndSymbol && ColdEndSymbol->isDefined() &&
4070+
"split function should have defined cold end symbol");
4071+
const uint64_t ColdStartOffset =
4072+
Layout.getSymbolOffset(*ColdStartSymbol);
4073+
const uint64_t ColdEndOffset = Layout.getSymbolOffset(*ColdEndSymbol);
4074+
cold().setAddress(ColdBaseAddress + ColdStartOffset);
4075+
cold().setImageSize(ColdEndOffset - ColdStartOffset);
4076+
if (hasConstantIsland()) {
4077+
const uint64_t DataOffset =
4078+
Layout.getSymbolOffset(*getFunctionColdConstantIslandLabel());
4079+
setOutputColdDataAddress(ColdBaseAddress + DataOffset);
4080+
}
40704081
}
40714082
}
40724083
} else {
@@ -4088,32 +4099,42 @@ void BinaryFunction::updateOutputValues(const MCAsmLayout &Layout) {
40884099
if (getLayout().block_empty())
40894100
return;
40904101

4102+
assert((getLayout().isHotColdSplit() ||
4103+
(cold().getAddress() == 0 && cold().getImageSize() == 0 &&
4104+
BC.HasRelocations)) &&
4105+
"Function must be split two ways or cold fragment must have no "
4106+
"address (only in relocation mode)");
4107+
40914108
BinaryBasicBlock *PrevBB = nullptr;
4092-
for (BinaryBasicBlock *BB : this->Layout.blocks()) {
4093-
assert(BB->getLabel()->isDefined() && "symbol should be defined");
4094-
const uint64_t BBBaseAddress = BB->isCold() ? ColdBaseAddress : BaseAddress;
4095-
if (!BC.HasRelocations) {
4096-
if (BB->isCold()) {
4097-
assert(BBBaseAddress == cold().getAddress());
4098-
} else {
4099-
assert(BBBaseAddress == getOutputAddress());
4109+
for (const FunctionFragment &FF : getLayout().fragments()) {
4110+
const uint64_t FragmentBaseAddress =
4111+
getCodeSection(isSimple() ? FF.getFragmentNum() : FragmentNum::main())
4112+
->getOutputAddress();
4113+
for (BinaryBasicBlock *const BB : FF) {
4114+
assert(BB->getLabel()->isDefined() && "symbol should be defined");
4115+
if (!BC.HasRelocations) {
4116+
if (BB->isSplit()) {
4117+
assert(FragmentBaseAddress == cold().getAddress());
4118+
} else {
4119+
assert(FragmentBaseAddress == getOutputAddress());
4120+
}
41004121
}
4101-
}
4102-
const uint64_t BBOffset = Layout.getSymbolOffset(*BB->getLabel());
4103-
const uint64_t BBAddress = BBBaseAddress + BBOffset;
4104-
BB->setOutputStartAddress(BBAddress);
4105-
4106-
if (PrevBB) {
4107-
uint64_t PrevBBEndAddress = BBAddress;
4108-
if (BB->isCold() != PrevBB->isCold())
4109-
PrevBBEndAddress = getOutputAddress() + getOutputSize();
4110-
PrevBB->setOutputEndAddress(PrevBBEndAddress);
4111-
}
4112-
PrevBB = BB;
4122+
const uint64_t BBOffset = Layout.getSymbolOffset(*BB->getLabel());
4123+
const uint64_t BBAddress = FragmentBaseAddress + BBOffset;
4124+
BB->setOutputStartAddress(BBAddress);
4125+
4126+
if (PrevBB) {
4127+
uint64_t PrevBBEndAddress = BBAddress;
4128+
if (BB->isSplit() != PrevBB->isSplit())
4129+
PrevBBEndAddress = getOutputAddress() + getOutputSize();
4130+
PrevBB->setOutputEndAddress(PrevBBEndAddress);
4131+
}
4132+
PrevBB = BB;
41134133

4114-
BB->updateOutputValues(Layout);
4134+
BB->updateOutputValues(Layout);
4135+
}
41154136
}
4116-
PrevBB->setOutputEndAddress(PrevBB->isCold()
4137+
PrevBB->setOutputEndAddress(PrevBB->isSplit()
41174138
? cold().getAddress() + cold().getImageSize()
41184139
: getOutputAddress() + getOutputSize());
41194140
}

‎bolt/lib/Core/FunctionLayout.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "llvm/ADT/STLExtras.h"
44
#include "llvm/ADT/edit_distance.h"
55
#include <algorithm>
6+
#include <cstddef>
67
#include <functional>
78

89
using namespace llvm;
@@ -61,18 +62,35 @@ void FunctionLayout::eraseBasicBlocks(
6162
NewFragments.emplace_back(0);
6263
for (const FunctionFragment FF : fragments()) {
6364
unsigned ErasedBlocks = count_if(FF, IsErased);
65+
// Only add the fragment if it is non-empty after removing blocks.
6466
unsigned NewFragment = NewFragments.back() + FF.size() - ErasedBlocks;
6567
NewFragments.emplace_back(NewFragment);
6668
}
6769
llvm::erase_if(Blocks, IsErased);
6870
Fragments = std::move(NewFragments);
71+
72+
// Remove empty fragments at the end
73+
const_iterator EmptyTailBegin =
74+
llvm::find_if_not(reverse(fragments()), [](const FunctionFragment &FF) {
75+
return FF.empty();
76+
}).base();
77+
if (EmptyTailBegin != fragment_end()) {
78+
// Add +1 for one-past-the-end entry
79+
const FunctionFragment TailBegin = *EmptyTailBegin;
80+
unsigned NewFragmentSize = TailBegin.getFragmentNum().get() + 1;
81+
Fragments.resize(NewFragmentSize);
82+
}
83+
84+
updateLayoutIndices();
6985
}
7086

7187
void FunctionLayout::updateLayoutIndices() const {
7288
unsigned BlockIndex = 0;
7389
for (const FunctionFragment FF : fragments()) {
74-
for (BinaryBasicBlock *const BB : FF)
90+
for (BinaryBasicBlock *const BB : FF) {
7591
BB->setLayoutIndex(BlockIndex++);
92+
BB->setFragmentNum(FF.getFragmentNum());
93+
}
7694
}
7795
}
7896

‎bolt/lib/Passes/SplitFunctions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ void SplitFunctions::splitFunction(BinaryFunction &BF, SplitStrategy Strategy) {
309309
PreSplitLayout = mergeEHTrampolines(BF, PreSplitLayout, Trampolines);
310310

311311
for (BinaryBasicBlock &BB : BF)
312-
BB.setIsCold(false);
312+
BB.setFragmentNum(FragmentNum::main());
313313
BF.getLayout().update(PreSplitLayout);
314314
} else {
315315
SplitBytesHot += HotSize;

‎bolt/lib/Rewrite/RewriteInstance.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3186,11 +3186,11 @@ void RewriteInstance::emitAndLink() {
31863186
for (const FunctionFragment FF :
31873187
Function->getLayout().getSplitFragments()) {
31883188
if (ErrorOr<BinarySection &> ColdSection =
3189-
Function->getColdCodeSection(FF.getFragmentNum()))
3189+
Function->getCodeSection(FF.getFragmentNum()))
31903190
BC->deregisterSection(*ColdSection);
31913191
}
31923192
if (Function->getLayout().isSplit())
3193-
Function->ColdCodeSectionName = getBOLTTextSectionName().str();
3193+
Function->setColdCodeSectionName(getBOLTTextSectionName());
31943194
}
31953195

31963196
if (opts::PrintCacheMetrics) {
@@ -3728,7 +3728,7 @@ void RewriteInstance::mapCodeSections(RuntimeDyld &RTDyld) {
37283728

37293729
for (const FunctionFragment FF : Function.getLayout().getSplitFragments()) {
37303730
ErrorOr<BinarySection &> ColdSection =
3731-
Function.getColdCodeSection(FF.getFragmentNum());
3731+
Function.getCodeSection(FF.getFragmentNum());
37323732
assert(ColdSection && "cannot find section for cold part");
37333733
// Cold fragments are aligned at 16 bytes.
37343734
NextAvailableAddress = alignTo(NextAvailableAddress, 16);
@@ -4521,12 +4521,12 @@ void RewriteInstance::updateELFSymbolTable(
45214521
for (const FunctionFragment FF :
45224522
Function.getLayout().getSplitFragments()) {
45234523
ELFSymTy NewColdSym = FunctionSymbol;
4524-
const SmallString<256> Buf = formatv(
4524+
const SmallString<256> SymbolName = formatv(
45254525
"{0}.cold.{1}", cantFail(FunctionSymbol.getName(StringSection)),
45264526
FF.getFragmentNum().get() - 1);
4527-
NewColdSym.st_name = AddToStrTab(Buf);
4527+
NewColdSym.st_name = AddToStrTab(SymbolName);
45284528
NewColdSym.st_shndx =
4529-
Function.getColdCodeSection(FF.getFragmentNum())->getIndex();
4529+
Function.getCodeSection(FF.getFragmentNum())->getIndex();
45304530
NewColdSym.st_value = Function.cold().getAddress();
45314531
NewColdSym.st_size = Function.cold().getImageSize();
45324532
NewColdSym.setBindingAndType(ELF::STB_LOCAL, ELF::STT_FUNC);
@@ -4658,7 +4658,7 @@ void RewriteInstance::updateELFSymbolTable(
46584658
NewSymbol.st_shndx =
46594659
OutputAddress >= Function->cold().getAddress() &&
46604660
OutputAddress < Function->cold().getImageSize()
4661-
? Function->getColdCodeSection(FragmentNum::cold())->getIndex()
4661+
? Function->getCodeSection(FragmentNum::cold())->getIndex()
46624662
: Function->getCodeSection()->getIndex();
46634663
} else {
46644664
// Check if the symbol belongs to moved data object and update it.
@@ -4756,6 +4756,9 @@ void RewriteInstance::updateELFSymbolTable(
47564756
Symbols.emplace_back(NewSymbol);
47574757

47584758
if (Function->isSplit()) {
4759+
assert(Function->getLayout().isHotColdSplit() &&
4760+
"Adding symbols based on cold fragment when there are more than "
4761+
"2 fragments");
47594762
ELFSymTy NewColdSym = NewSymbol;
47604763
NewColdSym.setType(ELF::STT_NOTYPE);
47614764
SmallVector<char, 256> Buf;

0 commit comments

Comments
 (0)