Skip to content

Commit f66d631

Browse files
aaupovaaupov
committed
Revert "[BOLT] Add BB index to BAT (#86044)"
This reverts commit 3b3de48.
1 parent 6e28ecd commit f66d631

File tree

12 files changed

+77
-92
lines changed

12 files changed

+77
-92
lines changed

‎bolt/docs/BAT.md

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,11 @@ current function.
9090
### Address translation table
9191
Delta encoding means that only the difference with the previous corresponding
9292
entry is encoded. Input offsets implicitly start at zero.
93-
| Entry | Encoding | Description | Branch/BB |
94-
| ------ | ------| ----------- | ------ |
95-
| `OutputOffset` | Continuous, Delta, ULEB128 | Function offset in output binary | Both |
96-
| `InputOffset` | Optional, Delta, SLEB128 | Function offset in input binary with `BRANCHENTRY` LSB bit | Both |
97-
| `BBHash` | Optional, 8b | Basic block hash in input binary | BB |
98-
| `BBIdx` | Optional, Delta, ULEB128 | Basic block index in input binary | BB |
93+
| Entry | Encoding | Description |
94+
| ------ | ------| ----------- |
95+
| `OutputOffset` | Continuous, Delta, ULEB128 | Function offset in output binary |
96+
| `InputOffset` | Optional, Delta, SLEB128 | Function offset in input binary with `BRANCHENTRY` LSB bit |
97+
| `BBHash` | Optional, 8b | Basic block entries only: basic block hash in input binary |
9998

10099
`BRANCHENTRY` bit denotes whether a given offset pair is a control flow source
101100
(branch or call instruction). If not set, it signifies a control flow target

‎bolt/include/bolt/Profile/BoltAddressTranslation.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,6 @@ class BoltAddressTranslation {
122122
/// Returns BF hash by function output address (after BOLT).
123123
size_t getBFHash(uint64_t OutputAddress) const;
124124

125-
/// Returns BB index by function output address (after BOLT) and basic block
126-
/// input offset.
127-
unsigned getBBIndex(uint64_t FuncOutputAddress, uint32_t BBInputOffset) const;
128-
129125
/// True if a given \p Address is a function with translation table entry.
130126
bool isBATFunction(uint64_t Address) const { return Maps.count(Address); }
131127

@@ -158,8 +154,7 @@ class BoltAddressTranslation {
158154

159155
std::map<uint64_t, MapTy> Maps;
160156

161-
/// Map basic block input offset to a basic block index and hash pair.
162-
using BBHashMap = std::unordered_map<uint32_t, std::pair<unsigned, size_t>>;
157+
using BBHashMap = std::unordered_map<uint32_t, size_t>;
163158
std::unordered_map<uint64_t, std::pair<size_t, BBHashMap>> FuncHashes;
164159

165160
/// Links outlined cold bocks to their original function

‎bolt/lib/Profile/BoltAddressTranslation.cpp

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ void BoltAddressTranslation::writeEntriesForBB(MapTy &Map,
4545
LLVM_DEBUG(dbgs() << formatv(" Hash: {0:x}\n",
4646
getBBHash(HotFuncAddress, BBInputOffset)));
4747
(void)HotFuncAddress;
48-
LLVM_DEBUG(dbgs() << formatv(" Index: {0}\n",
49-
getBBIndex(HotFuncAddress, BBInputOffset)));
5048
// In case of conflicts (same Key mapping to different Vals), the last
5149
// update takes precedence. Of course it is not ideal to have conflicts and
5250
// those happen when we have an empty BB that either contained only
@@ -219,7 +217,6 @@ void BoltAddressTranslation::writeMaps(std::map<uint64_t, MapTy> &Maps,
219217
}
220218
size_t Index = 0;
221219
uint64_t InOffset = 0;
222-
size_t PrevBBIndex = 0;
223220
// Output and Input addresses and delta-encoded
224221
for (std::pair<const uint32_t, uint32_t> &KeyVal : Map) {
225222
const uint64_t OutputAddress = KeyVal.first + Address;
@@ -229,15 +226,11 @@ void BoltAddressTranslation::writeMaps(std::map<uint64_t, MapTy> &Maps,
229226
encodeSLEB128(KeyVal.second - InOffset, OS);
230227
InOffset = KeyVal.second; // Keeping InOffset as if BRANCHENTRY is encoded
231228
if ((InOffset & BRANCHENTRY) == 0) {
232-
unsigned BBIndex;
233-
size_t BBHash;
234-
std::tie(BBIndex, BBHash) = FuncHashPair.second[InOffset >> 1];
229+
// Basic block hash
230+
size_t BBHash = FuncHashPair.second[InOffset >> 1];
235231
OS.write(reinterpret_cast<char *>(&BBHash), 8);
236-
// Basic block index in the input binary
237-
encodeULEB128(BBIndex - PrevBBIndex, OS);
238-
PrevBBIndex = BBIndex;
239-
LLVM_DEBUG(dbgs() << formatv("{0:x} -> {1:x} {2:x} {3}\n", KeyVal.first,
240-
InOffset >> 1, BBHash, BBIndex));
232+
LLVM_DEBUG(dbgs() << formatv("{0:x} -> {1:x} {2:x}\n", KeyVal.first,
233+
InOffset >> 1, BBHash));
241234
}
242235
}
243236
}
@@ -323,7 +316,6 @@ void BoltAddressTranslation::parseMaps(std::vector<uint64_t> &HotFuncs,
323316
LLVM_DEBUG(dbgs() << "Parsing " << NumEntries << " entries for 0x"
324317
<< Twine::utohexstr(Address) << "\n");
325318
uint64_t InputOffset = 0;
326-
size_t BBIndex = 0;
327319
for (uint32_t J = 0; J < NumEntries; ++J) {
328320
const uint64_t OutputDelta = DE.getULEB128(&Offset, &Err);
329321
const uint64_t OutputAddress = PrevAddress + OutputDelta;
@@ -338,25 +330,19 @@ void BoltAddressTranslation::parseMaps(std::vector<uint64_t> &HotFuncs,
338330
}
339331
Map.insert(std::pair<uint32_t, uint32_t>(OutputOffset, InputOffset));
340332
size_t BBHash = 0;
341-
size_t BBIndexDelta = 0;
342333
const bool IsBranchEntry = InputOffset & BRANCHENTRY;
343334
if (!IsBranchEntry) {
344335
BBHash = DE.getU64(&Offset, &Err);
345-
BBIndexDelta = DE.getULEB128(&Offset, &Err);
346-
BBIndex += BBIndexDelta;
347336
// Map basic block hash to hot fragment by input offset
348-
FuncHashes[HotAddress].second.emplace(InputOffset >> 1,
349-
std::pair(BBIndex, BBHash));
337+
FuncHashes[HotAddress].second.emplace(InputOffset >> 1, BBHash);
350338
}
351339
LLVM_DEBUG({
352340
dbgs() << formatv(
353341
"{0:x} -> {1:x} ({2}/{3}b -> {4}/{5}b), {6:x}", OutputOffset,
354342
InputOffset, OutputDelta, getULEB128Size(OutputDelta), InputDelta,
355343
(J < EqualElems) ? 0 : getSLEB128Size(InputDelta), OutputAddress);
356-
if (!IsBranchEntry) {
357-
dbgs() << formatv(" {0:x} {1}/{2}b", BBHash, BBIndex,
358-
getULEB128Size(BBIndexDelta));
359-
}
344+
if (BBHash)
345+
dbgs() << formatv(" {0:x}", BBHash);
360346
dbgs() << '\n';
361347
});
362348
}
@@ -508,19 +494,14 @@ void BoltAddressTranslation::saveMetadata(BinaryContext &BC) {
508494
FuncHashes[BF.getAddress()].first = BF.computeHash();
509495
BF.computeBlockHashes();
510496
for (const BinaryBasicBlock &BB : BF)
511-
FuncHashes[BF.getAddress()].second.emplace(
512-
BB.getInputOffset(), std::pair(BB.getIndex(), BB.getHash()));
497+
FuncHashes[BF.getAddress()].second.emplace(BB.getInputOffset(),
498+
BB.getHash());
513499
}
514500
}
515501

516-
unsigned BoltAddressTranslation::getBBIndex(uint64_t FuncOutputAddress,
517-
uint32_t BBInputOffset) const {
518-
return FuncHashes.at(FuncOutputAddress).second.at(BBInputOffset).first;
519-
}
520-
521502
size_t BoltAddressTranslation::getBBHash(uint64_t FuncOutputAddress,
522503
uint32_t BBInputOffset) const {
523-
return FuncHashes.at(FuncOutputAddress).second.at(BBInputOffset).second;
504+
return FuncHashes.at(FuncOutputAddress).second.at(BBInputOffset);
524505
}
525506

526507
size_t BoltAddressTranslation::getBFHash(uint64_t OutputAddress) const {

‎bolt/test/X86/bolt-address-translation-yaml.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ RUN: | FileCheck --check-prefix CHECK-BOLT-YAML %s
1818

1919
WRITE-BAT-CHECK: BOLT-INFO: Wrote 5 BAT maps
2020
WRITE-BAT-CHECK: BOLT-INFO: Wrote 4 function and 22 basic block hashes
21-
WRITE-BAT-CHECK: BOLT-INFO: BAT section size (bytes): 376
21+
WRITE-BAT-CHECK: BOLT-INFO: BAT section size (bytes): 344
2222

2323
READ-BAT-CHECK-NOT: BOLT-ERROR: unable to save profile in YAML format for input file processed by BOLT
2424
READ-BAT-CHECK: BOLT-INFO: Parsed 5 BAT entries

‎bolt/test/X86/bolt-address-translation.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
# CHECK: BOLT: 3 out of 7 functions were overwritten.
3838
# CHECK: BOLT-INFO: Wrote 6 BAT maps
3939
# CHECK: BOLT-INFO: Wrote 3 function and 58 basic block hashes
40-
# CHECK: BOLT-INFO: BAT section size (bytes): 920
40+
# CHECK: BOLT-INFO: BAT section size (bytes): 816
4141
#
4242
# usqrt mappings (hot part). We match against any key (left side containing
4343
# the bolted binary offsets) because BOLT may change where it puts instructions

‎clang/lib/Driver/ToolChains/Clang.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5863,8 +5863,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
58635863
} else if (Triple.getArch() == llvm::Triple::x86_64) {
58645864
Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"},
58655865
CM);
5866-
} else if (Triple.isNVPTX() || Triple.isAMDGPU()) {
5867-
// NVPTX/AMDGPU does not care about the code model and will accept
5866+
} else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) {
5867+
// NVPTX/AMDGPU/SPIRV does not care about the code model and will accept
58685868
// whatever works for the host.
58695869
Ok = true;
58705870
} else if (Triple.isSPARC64()) {

‎clang/test/Driver/unsupported-option-gpu.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22
// DEFINE: %{check} = %clang -### --target=x86_64-linux-gnu -c -mcmodel=medium
33

44
// RUN: %{check} -x cuda %s --cuda-path=%S/Inputs/CUDA/usr/local/cuda --offload-arch=sm_60 --no-cuda-version-check -fbasic-block-sections=all
5+
// RUN: %{check} -x hip %s --offload=spirv64 -nogpulib -nogpuinc
56
// RUN: %{check} -x hip %s --rocm-path=%S/Inputs/rocm -nogpulib -nogpuinc

‎lld/MachO/Driver.cpp

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ static void replaceCommonSymbols() {
612612
if (!osec)
613613
osec = ConcatOutputSection::getOrCreateForInput(isec);
614614
isec->parent = osec;
615-
inputSections.push_back(isec);
615+
addInputSection(isec);
616616

617617
// FIXME: CommonSymbol should store isReferencedDynamically, noDeadStrip
618618
// and pass them on here.
@@ -1220,53 +1220,18 @@ static void createFiles(const InputArgList &args) {
12201220

12211221
static void gatherInputSections() {
12221222
TimeTraceScope timeScope("Gathering input sections");
1223-
int inputOrder = 0;
12241223
for (const InputFile *file : inputFiles) {
12251224
for (const Section *section : file->sections) {
12261225
// Compact unwind entries require special handling elsewhere. (In
12271226
// contrast, EH frames are handled like regular ConcatInputSections.)
12281227
if (section->name == section_names::compactUnwind)
12291228
continue;
1230-
ConcatOutputSection *osec = nullptr;
1231-
for (const Subsection &subsection : section->subsections) {
1232-
if (auto *isec = dyn_cast<ConcatInputSection>(subsection.isec)) {
1233-
if (isec->isCoalescedWeak())
1234-
continue;
1235-
if (config->emitInitOffsets &&
1236-
sectionType(isec->getFlags()) == S_MOD_INIT_FUNC_POINTERS) {
1237-
in.initOffsets->addInput(isec);
1238-
continue;
1239-
}
1240-
isec->outSecOff = inputOrder++;
1241-
if (!osec)
1242-
osec = ConcatOutputSection::getOrCreateForInput(isec);
1243-
isec->parent = osec;
1244-
inputSections.push_back(isec);
1245-
} else if (auto *isec =
1246-
dyn_cast<CStringInputSection>(subsection.isec)) {
1247-
if (isec->getName() == section_names::objcMethname) {
1248-
if (in.objcMethnameSection->inputOrder == UnspecifiedInputOrder)
1249-
in.objcMethnameSection->inputOrder = inputOrder++;
1250-
in.objcMethnameSection->addInput(isec);
1251-
} else {
1252-
if (in.cStringSection->inputOrder == UnspecifiedInputOrder)
1253-
in.cStringSection->inputOrder = inputOrder++;
1254-
in.cStringSection->addInput(isec);
1255-
}
1256-
} else if (auto *isec =
1257-
dyn_cast<WordLiteralInputSection>(subsection.isec)) {
1258-
if (in.wordLiteralSection->inputOrder == UnspecifiedInputOrder)
1259-
in.wordLiteralSection->inputOrder = inputOrder++;
1260-
in.wordLiteralSection->addInput(isec);
1261-
} else {
1262-
llvm_unreachable("unexpected input section kind");
1263-
}
1264-
}
1229+
for (const Subsection &subsection : section->subsections)
1230+
addInputSection(subsection.isec);
12651231
}
12661232
if (!file->objCImageInfo.empty())
12671233
in.objCImageInfo->addFile(file);
12681234
}
1269-
assert(inputOrder <= UnspecifiedInputOrder);
12701235
}
12711236

12721237
static void foldIdenticalLiterals() {
@@ -1422,6 +1387,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
14221387
concatOutputSections.clear();
14231388
inputFiles.clear();
14241389
inputSections.clear();
1390+
inputSectionsOrder = 0;
14251391
loadedArchives.clear();
14261392
loadedObjectFrameworks.clear();
14271393
missingAutolinkWarnings.clear();

‎lld/MachO/InputSection.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,44 @@ static_assert(sizeof(void *) != 8 ||
3737
"instances of it");
3838

3939
std::vector<ConcatInputSection *> macho::inputSections;
40+
int macho::inputSectionsOrder = 0;
41+
42+
// Call this function to add a new InputSection and have it routed to the
43+
// appropriate container. Depending on its type and current config, it will
44+
// either be added to 'inputSections' vector or to a synthetic section.
45+
void lld::macho::addInputSection(InputSection *inputSection) {
46+
if (auto *isec = dyn_cast<ConcatInputSection>(inputSection)) {
47+
if (isec->isCoalescedWeak())
48+
return;
49+
if (config->emitInitOffsets &&
50+
sectionType(isec->getFlags()) == S_MOD_INIT_FUNC_POINTERS) {
51+
in.initOffsets->addInput(isec);
52+
return;
53+
}
54+
isec->outSecOff = inputSectionsOrder++;
55+
auto *osec = ConcatOutputSection::getOrCreateForInput(isec);
56+
isec->parent = osec;
57+
inputSections.push_back(isec);
58+
} else if (auto *isec = dyn_cast<CStringInputSection>(inputSection)) {
59+
if (isec->getName() == section_names::objcMethname) {
60+
if (in.objcMethnameSection->inputOrder == UnspecifiedInputOrder)
61+
in.objcMethnameSection->inputOrder = inputSectionsOrder++;
62+
in.objcMethnameSection->addInput(isec);
63+
} else {
64+
if (in.cStringSection->inputOrder == UnspecifiedInputOrder)
65+
in.cStringSection->inputOrder = inputSectionsOrder++;
66+
in.cStringSection->addInput(isec);
67+
}
68+
} else if (auto *isec = dyn_cast<WordLiteralInputSection>(inputSection)) {
69+
if (in.wordLiteralSection->inputOrder == UnspecifiedInputOrder)
70+
in.wordLiteralSection->inputOrder = inputSectionsOrder++;
71+
in.wordLiteralSection->addInput(isec);
72+
} else {
73+
llvm_unreachable("unexpected input section kind");
74+
}
75+
76+
assert(inputSectionsOrder <= UnspecifiedInputOrder);
77+
}
4078

4179
uint64_t InputSection::getFileSize() const {
4280
return isZeroFill(getFlags()) ? 0 : getSize();

‎lld/MachO/InputSection.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,8 @@ bool isEhFrameSection(const InputSection *);
302302
bool isGccExceptTabSection(const InputSection *);
303303

304304
extern std::vector<ConcatInputSection *> inputSections;
305+
// This is used as a counter for specyfing input order for input sections
306+
extern int inputSectionsOrder;
305307

306308
namespace section_names {
307309

@@ -369,6 +371,7 @@ constexpr const char addrSig[] = "__llvm_addrsig";
369371

370372
} // namespace section_names
371373

374+
void addInputSection(InputSection *inputSection);
372375
} // namespace macho
373376

374377
std::string toString(const macho::InputSection *);

‎lld/MachO/ObjC.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,7 @@ void ObjcCategoryMerger::emitAndLinkProtocolList(
790790
infoCategoryWriter.catPtrListInfo.align);
791791
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
792792
listSec->live = true;
793-
allInputSections.push_back(listSec);
793+
addInputSection(listSec);
794794

795795
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
796796

@@ -848,7 +848,7 @@ void ObjcCategoryMerger::emitAndLinkPointerList(
848848
infoCategoryWriter.catPtrListInfo.align);
849849
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
850850
listSec->live = true;
851-
allInputSections.push_back(listSec);
851+
addInputSection(listSec);
852852

853853
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
854854

@@ -889,7 +889,7 @@ ObjcCategoryMerger::emitCatListEntrySec(const std::string &forCateogryName,
889889
bodyData, infoCategoryWriter.catListInfo.align);
890890
newCatList->parent = infoCategoryWriter.catListInfo.outputSection;
891891
newCatList->live = true;
892-
allInputSections.push_back(newCatList);
892+
addInputSection(newCatList);
893893

894894
newCatList->parent = infoCategoryWriter.catListInfo.outputSection;
895895

@@ -927,7 +927,7 @@ Defined *ObjcCategoryMerger::emitCategoryBody(const std::string &name,
927927
bodyData, infoCategoryWriter.catBodyInfo.align);
928928
newBodySec->parent = infoCategoryWriter.catBodyInfo.outputSection;
929929
newBodySec->live = true;
930-
allInputSections.push_back(newBodySec);
930+
addInputSection(newBodySec);
931931

932932
std::string symName =
933933
objc::symbol_names::category + baseClassName + "_$_(" + name + ")";
@@ -1132,7 +1132,7 @@ void ObjcCategoryMerger::generateCatListForNonErasedCategories(
11321132
infoCategoryWriter.catListInfo.align);
11331133
listSec->parent = infoCategoryWriter.catListInfo.outputSection;
11341134
listSec->live = true;
1135-
allInputSections.push_back(listSec);
1135+
addInputSection(listSec);
11361136

11371137
std::string slotSymName = "<__objc_catlist slot for category ";
11381138
slotSymName += nonErasedCatBody->getName();
@@ -1221,9 +1221,11 @@ void ObjcCategoryMerger::doCleanup() { generatedSectionData.clear(); }
12211221

12221222
StringRef ObjcCategoryMerger::newStringData(const char *str) {
12231223
uint32_t len = strlen(str);
1224-
auto &data = newSectionData(len + 1);
1224+
uint32_t bufSize = len + 1;
1225+
auto &data = newSectionData(bufSize);
12251226
char *strData = reinterpret_cast<char *>(data.data());
1226-
strncpy(strData, str, len);
1227+
// Copy the string chars and null-terminator
1228+
memcpy(strData, str, bufSize);
12271229
return StringRef(strData, len);
12281230
}
12291231

‎lld/MachO/SyntheticSections.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ void StubHelperSection::setUp() {
793793

794794
in.imageLoaderCache->parent =
795795
ConcatOutputSection::getOrCreateForInput(in.imageLoaderCache);
796-
inputSections.push_back(in.imageLoaderCache);
796+
addInputSection(in.imageLoaderCache);
797797
// Since this isn't in the symbol table or in any input file, the noDeadStrip
798798
// argument doesn't matter.
799799
dyldPrivate =
@@ -855,7 +855,7 @@ ConcatInputSection *ObjCSelRefsSection::makeSelRef(StringRef methname) {
855855
/*addend=*/static_cast<int64_t>(methnameOffset),
856856
/*referent=*/in.objcMethnameSection->isec});
857857
objcSelref->parent = ConcatOutputSection::getOrCreateForInput(objcSelref);
858-
inputSections.push_back(objcSelref);
858+
addInputSection(objcSelref);
859859
objcSelref->isFinal = true;
860860
methnameToSelref[CachedHashStringRef(methname)] = objcSelref;
861861
return objcSelref;

0 commit comments

Comments
 (0)