Skip to content

Commit cc88a5e

Browse files
authored
[lld-macho,NFC] Switch to increasing priorities
--order_file, call graph profile, and BalancedPartitioning currently build the section order vector by decreasing priority (from SIZE_MAX to 0). However, it's conventional to use an increasing key (see OutputSection::inputOrder). Switch to increasing priorities, remove the global variable highestAvailablePriority, and remove the highestAvailablePriority parameter from BPSectionOrderer. Change size_t to int. This improves consistenty with the ELF and COFF ports. The ELF port utilizes negative priorities for --symbol-ordering-file and call graph profile, and non-negative priorities for --shuffle-sections (no Mach-O counterpart yet). Pull Request: #121727
1 parent beba4b0 commit cc88a5e

File tree

7 files changed

+41
-54
lines changed

7 files changed

+41
-54
lines changed

lld/Common/BPSectionOrdererBase.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,10 @@ static SmallVector<std::pair<unsigned, UtilityNodes>> getUnsForCompression(
9696
return sectionUns;
9797
}
9898

99-
llvm::DenseMap<const BPSectionBase *, size_t>
99+
llvm::DenseMap<const BPSectionBase *, int>
100100
BPSectionBase::reorderSectionsByBalancedPartitioning(
101-
size_t &highestAvailablePriority, llvm::StringRef profilePath,
102-
bool forFunctionCompression, bool forDataCompression,
103-
bool compressionSortStartupFunctions, bool verbose,
101+
llvm::StringRef profilePath, bool forFunctionCompression,
102+
bool forDataCompression, bool compressionSortStartupFunctions, bool verbose,
104103
SmallVector<std::unique_ptr<BPSectionBase>> &inputSections) {
105104
TimeTraceScope timeScope("Setup Balanced Partitioning");
106105
SmallVector<const BPSectionBase *> sections;
@@ -364,8 +363,9 @@ BPSectionBase::reorderSectionsByBalancedPartitioning(
364363
}
365364
}
366365

367-
DenseMap<const BPSectionBase *, size_t> sectionPriorities;
366+
DenseMap<const BPSectionBase *, int> sectionPriorities;
367+
int prio = -orderedSections.size();
368368
for (const auto *isec : orderedSections)
369-
sectionPriorities[isec] = --highestAvailablePriority;
369+
sectionPriorities[isec] = prio++;
370370
return sectionPriorities;
371371
}

lld/MachO/BPSectionOrderer.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@
1515
using namespace llvm;
1616
using namespace lld::macho;
1717

18-
DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
19-
size_t &highestAvailablePriority, StringRef profilePath,
20-
bool forFunctionCompression, bool forDataCompression,
18+
DenseMap<const InputSection *, int> lld::macho::runBalancedPartitioning(
19+
StringRef profilePath, bool forFunctionCompression, bool forDataCompression,
2120
bool compressionSortStartupFunctions, bool verbose) {
2221

2322
SmallVector<std::unique_ptr<BPSectionBase>> sections;
@@ -34,10 +33,10 @@ DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
3433
}
3534

3635
auto reorderedSections = BPSectionBase::reorderSectionsByBalancedPartitioning(
37-
highestAvailablePriority, profilePath, forFunctionCompression,
38-
forDataCompression, compressionSortStartupFunctions, verbose, sections);
36+
profilePath, forFunctionCompression, forDataCompression,
37+
compressionSortStartupFunctions, verbose, sections);
3938

40-
DenseMap<const InputSection *, size_t> result;
39+
DenseMap<const InputSection *, int> result;
4140
for (const auto &[sec, priority] : reorderedSections) {
4241
if (auto *machoSection = dyn_cast<BPSectionMacho>(sec)) {
4342
result.try_emplace(

lld/MachO/BPSectionOrderer.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,8 @@ class BPSectionMacho : public BPSectionBase {
145145
///
146146
/// It is important that .subsections_via_symbols is used to ensure functions
147147
/// and data are in their own sections and thus can be reordered.
148-
llvm::DenseMap<const lld::macho::InputSection *, size_t>
149-
runBalancedPartitioning(size_t &highestAvailablePriority,
150-
llvm::StringRef profilePath,
148+
llvm::DenseMap<const lld::macho::InputSection *, int>
149+
runBalancedPartitioning(llvm::StringRef profilePath,
151150
bool forFunctionCompression, bool forDataCompression,
152151
bool compressionSortStartupFunctions, bool verbose);
153152

lld/MachO/SectionPriorities.cpp

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ using namespace lld::macho;
3838
PriorityBuilder macho::priorityBuilder;
3939

4040
namespace {
41-
42-
size_t highestAvailablePriority = std::numeric_limits<size_t>::max();
43-
4441
struct Edge {
4542
int from;
4643
uint64_t weight;
@@ -67,7 +64,7 @@ class CallGraphSort {
6764
public:
6865
CallGraphSort(const MapVector<SectionPair, uint64_t> &profile);
6966

70-
DenseMap<const InputSection *, size_t> run();
67+
DenseMap<const InputSection *, int> run();
7168

7269
private:
7370
std::vector<Cluster> clusters;
@@ -157,7 +154,7 @@ static void mergeClusters(std::vector<Cluster> &cs, Cluster &into, int intoIdx,
157154

158155
// Group InputSections into clusters using the Call-Chain Clustering heuristic
159156
// then sort the clusters by density.
160-
DenseMap<const InputSection *, size_t> CallGraphSort::run() {
157+
DenseMap<const InputSection *, int> CallGraphSort::run() {
161158
const uint64_t maxClusterSize = target->getPageSize();
162159

163160
// Cluster indices sorted by density.
@@ -205,16 +202,14 @@ DenseMap<const InputSection *, size_t> CallGraphSort::run() {
205202
return clusters[a].getDensity() > clusters[b].getDensity();
206203
});
207204

208-
DenseMap<const InputSection *, size_t> orderMap;
205+
DenseMap<const InputSection *, int> orderMap;
209206

210207
// Sections will be sorted by decreasing order. Absent sections will have
211208
// priority 0 and be placed at the end of sections.
212-
// NB: This is opposite from COFF/ELF to be compatible with the existing
213-
// order-file code.
214-
int curOrder = highestAvailablePriority;
209+
int curOrder = -clusters.size();
215210
for (int leader : sorted) {
216211
for (int i = leader;;) {
217-
orderMap[sections[i]] = curOrder--;
212+
orderMap[sections[i]] = curOrder++;
218213
i = clusters[i].next;
219214
if (i == leader)
220215
break;
@@ -250,7 +245,7 @@ DenseMap<const InputSection *, size_t> CallGraphSort::run() {
250245
return orderMap;
251246
}
252247

253-
std::optional<size_t>
248+
std::optional<int>
254249
macho::PriorityBuilder::getSymbolPriority(const Defined *sym) {
255250
if (sym->isAbsolute())
256251
return std::nullopt;
@@ -270,7 +265,7 @@ macho::PriorityBuilder::getSymbolPriority(const Defined *sym) {
270265
else
271266
filename = saver().save(path::filename(f->archiveName) + "(" +
272267
path::filename(f->getName()) + ")");
273-
return std::max(entry.objectFiles.lookup(filename), entry.anyObjectFile);
268+
return std::min(entry.objectFiles.lookup(filename), entry.anyObjectFile);
274269
}
275270

276271
void macho::PriorityBuilder::extractCallGraphProfile() {
@@ -302,6 +297,7 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) {
302297
return;
303298
}
304299

300+
int prio = std::numeric_limits<int>::min();
305301
MemoryBufferRef mbref = *buffer;
306302
for (StringRef line : args::getLines(mbref)) {
307303
StringRef objectFile, symbol;
@@ -339,25 +335,22 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) {
339335
if (!symbol.empty()) {
340336
SymbolPriorityEntry &entry = priorities[symbol];
341337
if (!objectFile.empty())
342-
entry.objectFiles.insert(
343-
std::make_pair(objectFile, highestAvailablePriority));
338+
entry.objectFiles.insert(std::make_pair(objectFile, prio));
344339
else
345-
entry.anyObjectFile =
346-
std::max(entry.anyObjectFile, highestAvailablePriority);
340+
entry.anyObjectFile = std::min(entry.anyObjectFile, prio);
347341
}
348342

349-
--highestAvailablePriority;
343+
++prio;
350344
}
351345
}
352346

353-
DenseMap<const InputSection *, size_t>
347+
DenseMap<const InputSection *, int>
354348
macho::PriorityBuilder::buildInputSectionPriorities() {
355-
DenseMap<const InputSection *, size_t> sectionPriorities;
349+
DenseMap<const InputSection *, int> sectionPriorities;
356350
if (config->bpStartupFunctionSort || config->bpFunctionOrderForCompression ||
357351
config->bpDataOrderForCompression) {
358352
TimeTraceScope timeScope("Balanced Partitioning Section Orderer");
359353
sectionPriorities = runBalancedPartitioning(
360-
highestAvailablePriority,
361354
config->bpStartupFunctionSort ? config->irpgoProfilePath : "",
362355
config->bpFunctionOrderForCompression,
363356
config->bpDataOrderForCompression,
@@ -378,11 +371,11 @@ macho::PriorityBuilder::buildInputSectionPriorities() {
378371
return sectionPriorities;
379372

380373
auto addSym = [&](const Defined *sym) {
381-
std::optional<size_t> symbolPriority = getSymbolPriority(sym);
374+
std::optional<int> symbolPriority = getSymbolPriority(sym);
382375
if (!symbolPriority)
383376
return;
384-
size_t &priority = sectionPriorities[sym->isec()];
385-
priority = std::max(priority, *symbolPriority);
377+
int &priority = sectionPriorities[sym->isec()];
378+
priority = std::min(priority, *symbolPriority);
386379
};
387380

388381
// TODO: Make sure this handles weak symbols correctly.

lld/MachO/SectionPriorities.h

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,24 +53,20 @@ class PriorityBuilder {
5353
//
5454
// Each section gets assigned the priority of the highest-priority symbol it
5555
// contains.
56-
llvm::DenseMap<const InputSection *, size_t> buildInputSectionPriorities();
56+
llvm::DenseMap<const InputSection *, int> buildInputSectionPriorities();
5757

5858
private:
59-
// The symbol with the highest priority should be ordered first in the output
60-
// section (modulo input section contiguity constraints). Using priority
61-
// (highest first) instead of order (lowest first) has the convenient property
62-
// that the default-constructed zero priority -- for symbols/sections without
63-
// a user-defined order -- naturally ends up putting them at the end of the
64-
// output.
59+
// The symbol with the smallest priority should be ordered first in the output
60+
// section (modulo input section contiguity constraints).
6561
struct SymbolPriorityEntry {
6662
// The priority given to a matching symbol, regardless of which object file
6763
// it originated from.
68-
size_t anyObjectFile = 0;
64+
int anyObjectFile = 0;
6965
// The priority given to a matching symbol from a particular object file.
70-
llvm::DenseMap<llvm::StringRef, size_t> objectFiles;
66+
llvm::DenseMap<llvm::StringRef, int> objectFiles;
7167
};
7268

73-
std::optional<size_t> getSymbolPriority(const Defined *sym);
69+
std::optional<int> getSymbolPriority(const Defined *sym);
7470
llvm::DenseMap<llvm::StringRef, SymbolPriorityEntry> priorities;
7571
llvm::MapVector<SectionPair, uint64_t> callGraphProfile;
7672
};

lld/MachO/Writer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,7 @@ static void sortSegmentsAndSections() {
975975
TimeTraceScope timeScope("Sort segments and sections");
976976
sortOutputSegments();
977977

978-
DenseMap<const InputSection *, size_t> isecPriorities =
978+
DenseMap<const InputSection *, int> isecPriorities =
979979
priorityBuilder.buildInputSectionPriorities();
980980

981981
uint32_t sectionIndex = 0;
@@ -1008,7 +1008,7 @@ static void sortSegmentsAndSections() {
10081008
if (auto *merged = dyn_cast<ConcatOutputSection>(osec)) {
10091009
llvm::stable_sort(
10101010
merged->inputs, [&](InputSection *a, InputSection *b) {
1011-
return isecPriorities.lookup(a) > isecPriorities.lookup(b);
1011+
return isecPriorities.lookup(a) < isecPriorities.lookup(b);
10121012
});
10131013
}
10141014
}

lld/include/lld/Common/BPSectionOrdererBase.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ class BPSectionBase {
6666

6767
/// Reorders sections using balanced partitioning algorithm based on profile
6868
/// data.
69-
static llvm::DenseMap<const BPSectionBase *, size_t>
69+
static llvm::DenseMap<const BPSectionBase *, int>
7070
reorderSectionsByBalancedPartitioning(
71-
size_t &highestAvailablePriority, llvm::StringRef profilePath,
72-
bool forFunctionCompression, bool forDataCompression,
73-
bool compressionSortStartupFunctions, bool verbose,
71+
llvm::StringRef profilePath, bool forFunctionCompression,
72+
bool forDataCompression, bool compressionSortStartupFunctions,
73+
bool verbose,
7474
llvm::SmallVector<std::unique_ptr<BPSectionBase>> &inputSections);
7575
};
7676

0 commit comments

Comments
 (0)