[lld] [lld-macho,NFC] Switch to increasing priorities (PR #121727)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 5 19:04:24 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lld-coff
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
--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 and remove the global variable
highestAvailablePriority.
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).
---
Full diff: https://github.com/llvm/llvm-project/pull/121727.diff
7 Files Affected:
- (modified) lld/Common/BPSectionOrdererBase.cpp (+6-6)
- (modified) lld/MachO/BPSectionOrderer.cpp (+5-6)
- (modified) lld/MachO/BPSectionOrderer.h (+2-3)
- (modified) lld/MachO/SectionPriorities.cpp (+16-23)
- (modified) lld/MachO/SectionPriorities.h (+6-10)
- (modified) lld/MachO/Writer.cpp (+2-2)
- (modified) lld/include/lld/Common/BPSectionOrdererBase.h (+4-4)
``````````diff
diff --git a/lld/Common/BPSectionOrdererBase.cpp b/lld/Common/BPSectionOrdererBase.cpp
index 75be4f6aa9bc09..7d26a5fb844835 100644
--- a/lld/Common/BPSectionOrdererBase.cpp
+++ b/lld/Common/BPSectionOrdererBase.cpp
@@ -96,11 +96,10 @@ static SmallVector<std::pair<unsigned, UtilityNodes>> getUnsForCompression(
return sectionUns;
}
-llvm::DenseMap<const BPSectionBase *, size_t>
+llvm::DenseMap<const BPSectionBase *, int>
BPSectionBase::reorderSectionsByBalancedPartitioning(
- size_t &highestAvailablePriority, llvm::StringRef profilePath,
- bool forFunctionCompression, bool forDataCompression,
- bool compressionSortStartupFunctions, bool verbose,
+ llvm::StringRef profilePath, bool forFunctionCompression,
+ bool forDataCompression, bool compressionSortStartupFunctions, bool verbose,
SmallVector<std::unique_ptr<BPSectionBase>> &inputSections) {
TimeTraceScope timeScope("Setup Balanced Partitioning");
SmallVector<const BPSectionBase *> sections;
@@ -364,8 +363,9 @@ BPSectionBase::reorderSectionsByBalancedPartitioning(
}
}
- DenseMap<const BPSectionBase *, size_t> sectionPriorities;
+ DenseMap<const BPSectionBase *, int> sectionPriorities;
+ int prio = -orderedSections.size();
for (const auto *isec : orderedSections)
- sectionPriorities[isec] = --highestAvailablePriority;
+ sectionPriorities[isec] = prio++;
return sectionPriorities;
}
diff --git a/lld/MachO/BPSectionOrderer.cpp b/lld/MachO/BPSectionOrderer.cpp
index 0ffbf16007fdad..18c8aad58344f2 100644
--- a/lld/MachO/BPSectionOrderer.cpp
+++ b/lld/MachO/BPSectionOrderer.cpp
@@ -15,9 +15,8 @@
using namespace llvm;
using namespace lld::macho;
-DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
- size_t &highestAvailablePriority, StringRef profilePath,
- bool forFunctionCompression, bool forDataCompression,
+DenseMap<const InputSection *, int> lld::macho::runBalancedPartitioning(
+ StringRef profilePath, bool forFunctionCompression, bool forDataCompression,
bool compressionSortStartupFunctions, bool verbose) {
SmallVector<std::unique_ptr<BPSectionBase>> sections;
@@ -34,10 +33,10 @@ DenseMap<const InputSection *, size_t> lld::macho::runBalancedPartitioning(
}
auto reorderedSections = BPSectionBase::reorderSectionsByBalancedPartitioning(
- highestAvailablePriority, profilePath, forFunctionCompression,
- forDataCompression, compressionSortStartupFunctions, verbose, sections);
+ profilePath, forFunctionCompression, forDataCompression,
+ compressionSortStartupFunctions, verbose, sections);
- DenseMap<const InputSection *, size_t> result;
+ DenseMap<const InputSection *, int> result;
for (const auto &[sec, priority] : reorderedSections) {
if (auto *machoSection = dyn_cast<BPSectionMacho>(sec)) {
result.try_emplace(
diff --git a/lld/MachO/BPSectionOrderer.h b/lld/MachO/BPSectionOrderer.h
index 8ba911fcc546bd..4facb652d4c874 100644
--- a/lld/MachO/BPSectionOrderer.h
+++ b/lld/MachO/BPSectionOrderer.h
@@ -145,9 +145,8 @@ class BPSectionMacho : public BPSectionBase {
///
/// It is important that .subsections_via_symbols is used to ensure functions
/// and data are in their own sections and thus can be reordered.
-llvm::DenseMap<const lld::macho::InputSection *, size_t>
-runBalancedPartitioning(size_t &highestAvailablePriority,
- llvm::StringRef profilePath,
+llvm::DenseMap<const lld::macho::InputSection *, int>
+runBalancedPartitioning(llvm::StringRef profilePath,
bool forFunctionCompression, bool forDataCompression,
bool compressionSortStartupFunctions, bool verbose);
diff --git a/lld/MachO/SectionPriorities.cpp b/lld/MachO/SectionPriorities.cpp
index 0a15112c1250de..7a4a5d8465f64d 100644
--- a/lld/MachO/SectionPriorities.cpp
+++ b/lld/MachO/SectionPriorities.cpp
@@ -38,9 +38,6 @@ using namespace lld::macho;
PriorityBuilder macho::priorityBuilder;
namespace {
-
-size_t highestAvailablePriority = std::numeric_limits<size_t>::max();
-
struct Edge {
int from;
uint64_t weight;
@@ -67,7 +64,7 @@ class CallGraphSort {
public:
CallGraphSort(const MapVector<SectionPair, uint64_t> &profile);
- DenseMap<const InputSection *, size_t> run();
+ DenseMap<const InputSection *, int> run();
private:
std::vector<Cluster> clusters;
@@ -157,7 +154,7 @@ static void mergeClusters(std::vector<Cluster> &cs, Cluster &into, int intoIdx,
// Group InputSections into clusters using the Call-Chain Clustering heuristic
// then sort the clusters by density.
-DenseMap<const InputSection *, size_t> CallGraphSort::run() {
+DenseMap<const InputSection *, int> CallGraphSort::run() {
const uint64_t maxClusterSize = target->getPageSize();
// Cluster indices sorted by density.
@@ -205,16 +202,14 @@ DenseMap<const InputSection *, size_t> CallGraphSort::run() {
return clusters[a].getDensity() > clusters[b].getDensity();
});
- DenseMap<const InputSection *, size_t> orderMap;
+ DenseMap<const InputSection *, int> orderMap;
// Sections will be sorted by decreasing order. Absent sections will have
// priority 0 and be placed at the end of sections.
- // NB: This is opposite from COFF/ELF to be compatible with the existing
- // order-file code.
- int curOrder = highestAvailablePriority;
+ int curOrder = -clusters.size();
for (int leader : sorted) {
for (int i = leader;;) {
- orderMap[sections[i]] = curOrder--;
+ orderMap[sections[i]] = curOrder++;
i = clusters[i].next;
if (i == leader)
break;
@@ -250,7 +245,7 @@ DenseMap<const InputSection *, size_t> CallGraphSort::run() {
return orderMap;
}
-std::optional<size_t>
+std::optional<int>
macho::PriorityBuilder::getSymbolPriority(const Defined *sym) {
if (sym->isAbsolute())
return std::nullopt;
@@ -270,7 +265,7 @@ macho::PriorityBuilder::getSymbolPriority(const Defined *sym) {
else
filename = saver().save(path::filename(f->archiveName) + "(" +
path::filename(f->getName()) + ")");
- return std::max(entry.objectFiles.lookup(filename), entry.anyObjectFile);
+ return std::min(entry.objectFiles.lookup(filename), entry.anyObjectFile);
}
void macho::PriorityBuilder::extractCallGraphProfile() {
@@ -302,6 +297,7 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) {
return;
}
+ int prio = std::numeric_limits<int>::min();
MemoryBufferRef mbref = *buffer;
for (StringRef line : args::getLines(mbref)) {
StringRef objectFile, symbol;
@@ -339,25 +335,22 @@ void macho::PriorityBuilder::parseOrderFile(StringRef path) {
if (!symbol.empty()) {
SymbolPriorityEntry &entry = priorities[symbol];
if (!objectFile.empty())
- entry.objectFiles.insert(
- std::make_pair(objectFile, highestAvailablePriority));
+ entry.objectFiles.insert(std::make_pair(objectFile, prio));
else
- entry.anyObjectFile =
- std::max(entry.anyObjectFile, highestAvailablePriority);
+ entry.anyObjectFile = std::min(entry.anyObjectFile, prio);
}
- --highestAvailablePriority;
+ ++prio;
}
}
-DenseMap<const InputSection *, size_t>
+DenseMap<const InputSection *, int>
macho::PriorityBuilder::buildInputSectionPriorities() {
- DenseMap<const InputSection *, size_t> sectionPriorities;
+ DenseMap<const InputSection *, int> sectionPriorities;
if (config->bpStartupFunctionSort || config->bpFunctionOrderForCompression ||
config->bpDataOrderForCompression) {
TimeTraceScope timeScope("Balanced Partitioning Section Orderer");
sectionPriorities = runBalancedPartitioning(
- highestAvailablePriority,
config->bpStartupFunctionSort ? config->irpgoProfilePath : "",
config->bpFunctionOrderForCompression,
config->bpDataOrderForCompression,
@@ -378,11 +371,11 @@ macho::PriorityBuilder::buildInputSectionPriorities() {
return sectionPriorities;
auto addSym = [&](const Defined *sym) {
- std::optional<size_t> symbolPriority = getSymbolPriority(sym);
+ std::optional<int> symbolPriority = getSymbolPriority(sym);
if (!symbolPriority)
return;
- size_t &priority = sectionPriorities[sym->isec()];
- priority = std::max(priority, *symbolPriority);
+ int &priority = sectionPriorities[sym->isec()];
+ priority = std::min(priority, *symbolPriority);
};
// TODO: Make sure this handles weak symbols correctly.
diff --git a/lld/MachO/SectionPriorities.h b/lld/MachO/SectionPriorities.h
index 83cfe354e72638..44fb101990c516 100644
--- a/lld/MachO/SectionPriorities.h
+++ b/lld/MachO/SectionPriorities.h
@@ -53,24 +53,20 @@ class PriorityBuilder {
//
// Each section gets assigned the priority of the highest-priority symbol it
// contains.
- llvm::DenseMap<const InputSection *, size_t> buildInputSectionPriorities();
+ llvm::DenseMap<const InputSection *, int> buildInputSectionPriorities();
private:
- // The symbol with the highest priority should be ordered first in the output
- // section (modulo input section contiguity constraints). Using priority
- // (highest first) instead of order (lowest first) has the convenient property
- // that the default-constructed zero priority -- for symbols/sections without
- // a user-defined order -- naturally ends up putting them at the end of the
- // output.
+ // The symbol with the smallest priority should be ordered first in the output
+ // section (modulo input section contiguity constraints).
struct SymbolPriorityEntry {
// The priority given to a matching symbol, regardless of which object file
// it originated from.
- size_t anyObjectFile = 0;
+ int anyObjectFile = 0;
// The priority given to a matching symbol from a particular object file.
- llvm::DenseMap<llvm::StringRef, size_t> objectFiles;
+ llvm::DenseMap<llvm::StringRef, int> objectFiles;
};
- std::optional<size_t> getSymbolPriority(const Defined *sym);
+ std::optional<int> getSymbolPriority(const Defined *sym);
llvm::DenseMap<llvm::StringRef, SymbolPriorityEntry> priorities;
llvm::MapVector<SectionPair, uint64_t> callGraphProfile;
};
diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 6a1dd0ae7ecaf0..bec980e18e18b4 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -975,7 +975,7 @@ static void sortSegmentsAndSections() {
TimeTraceScope timeScope("Sort segments and sections");
sortOutputSegments();
- DenseMap<const InputSection *, size_t> isecPriorities =
+ DenseMap<const InputSection *, int> isecPriorities =
priorityBuilder.buildInputSectionPriorities();
uint32_t sectionIndex = 0;
@@ -1008,7 +1008,7 @@ static void sortSegmentsAndSections() {
if (auto *merged = dyn_cast<ConcatOutputSection>(osec)) {
llvm::stable_sort(
merged->inputs, [&](InputSection *a, InputSection *b) {
- return isecPriorities.lookup(a) > isecPriorities.lookup(b);
+ return isecPriorities.lookup(a) < isecPriorities.lookup(b);
});
}
}
diff --git a/lld/include/lld/Common/BPSectionOrdererBase.h b/lld/include/lld/Common/BPSectionOrdererBase.h
index e2cb41f69cc684..bd5bd638ccd2ac 100644
--- a/lld/include/lld/Common/BPSectionOrdererBase.h
+++ b/lld/include/lld/Common/BPSectionOrdererBase.h
@@ -66,11 +66,11 @@ class BPSectionBase {
/// Reorders sections using balanced partitioning algorithm based on profile
/// data.
- static llvm::DenseMap<const BPSectionBase *, size_t>
+ static llvm::DenseMap<const BPSectionBase *, int>
reorderSectionsByBalancedPartitioning(
- size_t &highestAvailablePriority, llvm::StringRef profilePath,
- bool forFunctionCompression, bool forDataCompression,
- bool compressionSortStartupFunctions, bool verbose,
+ llvm::StringRef profilePath, bool forFunctionCompression,
+ bool forDataCompression, bool compressionSortStartupFunctions,
+ bool verbose,
llvm::SmallVector<std::unique_ptr<BPSectionBase>> &inputSections);
};
``````````
</details>
https://github.com/llvm/llvm-project/pull/121727
More information about the llvm-commits
mailing list