-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[lld-macho][NFC] Refactor insertions into inputSections #85692
Conversation
06d973c
to
d08195b
Compare
@llvm/pr-subscribers-lld-macho @llvm/pr-subscribers-lld Author: None (alx32) ChangesBefore this change, after Also, multiple other locations also insert directly into For an upcoming feature (relative method lists), we need to route We can achieve the above either by duplicating some of the logic or refactoring the routing and The refactoring & code sharing approach seems the correct way to go - as such this diff performs the refactoring while not introducing any functional changes. Later on we can just call Full diff: https://github.com/llvm/llvm-project/pull/85692.diff 5 Files Affected:
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 36248925d65ad2..7a9e4072bcf489 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -612,7 +612,7 @@ static void replaceCommonSymbols() {
if (!osec)
osec = ConcatOutputSection::getOrCreateForInput(isec);
isec->parent = osec;
- inputSections.push_back(isec);
+ addInputSection(isec);
// FIXME: CommonSymbol should store isReferencedDynamically, noDeadStrip
// and pass them on here.
@@ -1220,53 +1220,18 @@ static void createFiles(const InputArgList &args) {
static void gatherInputSections() {
TimeTraceScope timeScope("Gathering input sections");
- int inputOrder = 0;
for (const InputFile *file : inputFiles) {
for (const Section *section : file->sections) {
// Compact unwind entries require special handling elsewhere. (In
// contrast, EH frames are handled like regular ConcatInputSections.)
if (section->name == section_names::compactUnwind)
continue;
- ConcatOutputSection *osec = nullptr;
- for (const Subsection &subsection : section->subsections) {
- if (auto *isec = dyn_cast<ConcatInputSection>(subsection.isec)) {
- if (isec->isCoalescedWeak())
- continue;
- if (config->emitInitOffsets &&
- sectionType(isec->getFlags()) == S_MOD_INIT_FUNC_POINTERS) {
- in.initOffsets->addInput(isec);
- continue;
- }
- isec->outSecOff = inputOrder++;
- if (!osec)
- osec = ConcatOutputSection::getOrCreateForInput(isec);
- isec->parent = osec;
- inputSections.push_back(isec);
- } else if (auto *isec =
- dyn_cast<CStringInputSection>(subsection.isec)) {
- if (isec->getName() == section_names::objcMethname) {
- if (in.objcMethnameSection->inputOrder == UnspecifiedInputOrder)
- in.objcMethnameSection->inputOrder = inputOrder++;
- in.objcMethnameSection->addInput(isec);
- } else {
- if (in.cStringSection->inputOrder == UnspecifiedInputOrder)
- in.cStringSection->inputOrder = inputOrder++;
- in.cStringSection->addInput(isec);
- }
- } else if (auto *isec =
- dyn_cast<WordLiteralInputSection>(subsection.isec)) {
- if (in.wordLiteralSection->inputOrder == UnspecifiedInputOrder)
- in.wordLiteralSection->inputOrder = inputOrder++;
- in.wordLiteralSection->addInput(isec);
- } else {
- llvm_unreachable("unexpected input section kind");
- }
- }
+ for (const Subsection &subsection : section->subsections)
+ addInputSection(subsection.isec);
}
if (!file->objCImageInfo.empty())
in.objCImageInfo->addFile(file);
}
- assert(inputOrder <= UnspecifiedInputOrder);
}
static void foldIdenticalLiterals() {
diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index 8f5affb1dc21d8..376e426e7c4f71 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -38,6 +38,43 @@ static_assert(sizeof(void *) != 8 ||
std::vector<ConcatInputSection *> macho::inputSections;
+void lld::macho::addInputSection(InputSection *inputSection) {
+ // Used across function calls to impose section ordering
+ static uint64_t inputOrder = 0;
+
+ if (auto *isec = dyn_cast<ConcatInputSection>(inputSection)) {
+ if (isec->isCoalescedWeak())
+ return;
+ if (config->emitInitOffsets &&
+ sectionType(isec->getFlags()) == S_MOD_INIT_FUNC_POINTERS) {
+ in.initOffsets->addInput(isec);
+ return;
+ }
+ isec->outSecOff = inputOrder++;
+ auto *osec = ConcatOutputSection::getOrCreateForInput(isec);
+ isec->parent = osec;
+ inputSections.push_back(isec);
+ } else if (auto *isec = dyn_cast<CStringInputSection>(inputSection)) {
+ if (isec->getName() == section_names::objcMethname) {
+ if (in.objcMethnameSection->inputOrder == UnspecifiedInputOrder)
+ in.objcMethnameSection->inputOrder = inputOrder++;
+ in.objcMethnameSection->addInput(isec);
+ } else {
+ if (in.cStringSection->inputOrder == UnspecifiedInputOrder)
+ in.cStringSection->inputOrder = inputOrder++;
+ in.cStringSection->addInput(isec);
+ }
+ } else if (auto *isec = dyn_cast<WordLiteralInputSection>(inputSection)) {
+ if (in.wordLiteralSection->inputOrder == UnspecifiedInputOrder)
+ in.wordLiteralSection->inputOrder = inputOrder++;
+ in.wordLiteralSection->addInput(isec);
+ } else {
+ llvm_unreachable("unexpected input section kind");
+ }
+
+ assert(inputOrder <= UnspecifiedInputOrder);
+}
+
uint64_t InputSection::getFileSize() const {
return isZeroFill(getFlags()) ? 0 : getSize();
}
diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index b25f0638f4c6cb..d1b60e7a7ebae9 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -369,6 +369,7 @@ constexpr const char addrSig[] = "__llvm_addrsig";
} // namespace section_names
+void addInputSection(InputSection *inputSection);
} // namespace macho
std::string toString(const macho::InputSection *);
diff --git a/lld/MachO/ObjC.cpp b/lld/MachO/ObjC.cpp
index 9b2446360e4f7f..695da8ab728b6f 100644
--- a/lld/MachO/ObjC.cpp
+++ b/lld/MachO/ObjC.cpp
@@ -785,7 +785,7 @@ void ObjcCategoryMerger::emitAndLinkProtocolList(
infoCategoryWriter.catPtrListInfo.align);
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
listSec->live = true;
- allInputSections.push_back(listSec);
+ addInputSection(listSec);
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
@@ -843,7 +843,7 @@ void ObjcCategoryMerger::emitAndLinkPointerList(
infoCategoryWriter.catPtrListInfo.align);
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
listSec->live = true;
- allInputSections.push_back(listSec);
+ addInputSection(listSec);
listSec->parent = infoCategoryWriter.catPtrListInfo.outputSection;
@@ -884,7 +884,7 @@ ObjcCategoryMerger::emitCatListEntrySec(const std::string &forCateogryName,
bodyData, infoCategoryWriter.catListInfo.align);
newCatList->parent = infoCategoryWriter.catListInfo.outputSection;
newCatList->live = true;
- allInputSections.push_back(newCatList);
+ addInputSection(newCatList);
newCatList->parent = infoCategoryWriter.catListInfo.outputSection;
@@ -922,7 +922,7 @@ Defined *ObjcCategoryMerger::emitCategoryBody(const std::string &name,
bodyData, infoCategoryWriter.catBodyInfo.align);
newBodySec->parent = infoCategoryWriter.catBodyInfo.outputSection;
newBodySec->live = true;
- allInputSections.push_back(newBodySec);
+ addInputSection(newBodySec);
std::string symName =
objc::symbol_names::category + baseClassName + "_$_(" + name + ")";
@@ -1124,7 +1124,7 @@ void ObjcCategoryMerger::generateCatListForNonErasedCategories(
infoCategoryWriter.catListInfo.align);
listSec->parent = infoCategoryWriter.catListInfo.outputSection;
listSec->live = true;
- allInputSections.push_back(listSec);
+ addInputSection(listSec);
std::string slotSymName = "<__objc_catlist slot for category ";
slotSymName += nonErasedCatBody->getName();
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 7ee3261ce3075f..1b3694528de1dd 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -793,7 +793,7 @@ void StubHelperSection::setUp() {
in.imageLoaderCache->parent =
ConcatOutputSection::getOrCreateForInput(in.imageLoaderCache);
- inputSections.push_back(in.imageLoaderCache);
+ addInputSection(in.imageLoaderCache);
// Since this isn't in the symbol table or in any input file, the noDeadStrip
// argument doesn't matter.
dyldPrivate =
@@ -855,7 +855,7 @@ ConcatInputSection *ObjCSelRefsSection::makeSelRef(StringRef methname) {
/*addend=*/static_cast<int64_t>(methnameOffset),
/*referent=*/in.objcMethnameSection->isec});
objcSelref->parent = ConcatOutputSection::getOrCreateForInput(objcSelref);
- inputSections.push_back(objcSelref);
+ addInputSection(objcSelref);
objcSelref->isFinal = true;
methnameToSelref[CachedHashStringRef(methname)] = objcSelref;
return objcSelref;
|
d08195b
to
6ec7849
Compare
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.
lgtm
Before this change, after `InputSection` objects are created, they need to be added to the appropriate container for tracking. The logic for selecting the appropriate container lives in `Driver.cpp` / `gatherInputSections`, where the `InputSection` is added to the matching container depending on the input config and the type of `InputSection`. Also, multiple other locations also insert directly into `inputSections` array - assuming that that is the appropriate container for the `InputSection`'s they create. Currently this is the correct assumption, however an upcoming feature will change this. For an upcoming feature (relative method lists), we need to route `InputSection`'s either to `inputSections` array or to a synthetic section, depending on weather the relative method list optimization is enabled or not. We can achieve the above either by duplicating some of the logic or refactoring the routing and `InputSection`'s and reusing that. The refactoring & code sharing approach seems the correct way to go - as such this diff performs the refactoring while not introducing any functional changes. Later on we can just call `addInputSection` and not have to worry about routing logic. ---------
Before this change, after
InputSection
objects are created, they need to be added to the appropriate container for tracking.The logic for selecting the appropriate container lives in
Driver.cpp
/gatherInputSections
, where theInputSection
is added to the matching container depending on the input config and the type ofInputSection
.Also, multiple other locations also insert directly into
inputSections
array - assuming that that is the appropriate container for theInputSection
's they create. Currently this is the correct assumption, however an upcoming feature will change this.For an upcoming feature (relative method lists), we need to route
InputSection
's either toinputSections
array or to a synthetic section, depending on weather the relative method list optimization is enabled or not.We can achieve the above either by duplicating some of the logic or refactoring the routing and
InputSection
's and reusing that.The refactoring & code sharing approach seems the correct way to go - as such this diff performs the refactoring while not introducing any functional changes. Later on we can just call
addInputSection
and not have to worry about routing logic.