[PATCH] D123279: [lld-macho][nfc] Give non-text ConcatOutputSections order-independent finalization

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
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();
----------------
oontvoo wrote:
> Why is the lambda needed here? (It's only got one caller ...)
> 
> Why not make it the body of the for loop?
because I forgot to clean up my copypasta :p good catch


================
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);
----------------
oontvoo wrote:
> 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.
> 
agreed


================
Comment at: lld/MachO/Writer.cpp:976
+
+  // We could parallelize this loop, but it seems cheaper to do it all in the
+  // main thread.
----------------
oontvoo wrote:
> less memory? 
> (citations needed 😄 )
oh, chromium_framework actually links a bit faster on my machine with the non-parallel setup. I'll make the comment more explicit :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123279/new/

https://reviews.llvm.org/D123279