[PATCH] D123279: [lld-macho][nfc] Give non-text ConcatOutputSections order-independent finalization
Vy Nguyen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 7 07:14:57 PDT 2022
oontvoo accepted this revision.
oontvoo added inline comments.
This revision is now accepted and ready to land.
================
Comment at: lld/MachO/ConcatOutputSection.cpp:199-209
+ auto finalizeOne = [&](ConcatInputSection *isec) {
+ isecAddr = alignTo(isecAddr, isec->align);
+ isecFileOff = alignTo(isecFileOff, isec->align);
+ isec->outSecOff = isecAddr;
+ isec->isFinal = true;
+ isecAddr += isec->getSize();
+ isecFileOff += isec->getFileSize();
----------------
Why is the lambda needed here? (It's only got one caller ...)
Why not make it the body of the for loop?
================
Comment at: lld/MachO/ConcatOutputSection.cpp:428-434
+ if (!osec) {
+ if (isec->getSegName() != segment_names::text ||
+ isec->getName() == section_names::gccExceptTab ||
+ isec->getName() == section_names::ehFrame)
+ osec = make<ConcatOutputSection>(names.second);
+ else
+ osec = make<TextOutputSection>(names.second);
----------------
nit: it seems a bit more intuitive to reverse the condition here. ie., "if segment == text", then make text-output-section, otherwise, concat-output-section.
================
Comment at: lld/MachO/ConcatOutputSection.h:65-66
+ explicit TextOutputSection(StringRef name) : ConcatOutputSection(name) {}
+ void finalizeContents() override {}
+ void finalize() override;
+ bool needsThunks() const;
----------------
could we have some comments here on finalizae() vs finalizeContents() pls?
================
Comment at: lld/MachO/Writer.cpp:976
+
+ // We could parallelize this loop, but it seems cheaper to do it all in the
+ // main thread.
----------------
less memory?
(citations needed 😄 )
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123279/new/
https://reviews.llvm.org/D123279
More information about the llvm-commits
mailing list