ELF: Move section merging before ICF. NFCI.

[ruiu]

lld/ELF/Driver.cpp

1007–1008 ↗

(On Diff #102141)

I first read this comment as markLive does all these things. I guess this comment describes all the code until writeResult. This is a bit confusing.

1020–1021 ↗

(On Diff #102141)

Should we move this to mergeSections?

[pcc]

lld/ELF/Driver.cpp

1020–1021 ↗

(On Diff #102141)

Seems reasonable. It'll need to be called something like decompressAndMergeSections, but that's probably fine.

[pcc]

• Address review comments

[pcc]

lld/ELF/Driver.cpp

1007–1008 ↗

(On Diff #102141)

Okay, now all the size optimizations are in one block like before.

[ruiu]

LGTM

lld/ELF/Driver.cpp

1003 ↗

(On Diff #102143)

Can you add a comment saying that this adds a .comment section containing a version string, and we have to do that before mergeSections because the .comment section is a mergeable section?

lld/trunk/ELF/Driver.cpp

 #include "InputFiles.h"
 #include "InputSection.h"
 #include "LinkerScript.h"
 #include "Memory.h"
 #include "OutputSections.h"
 #include "ScriptParser.h"
 #include "Strings.h"
 #include "SymbolTable.h"
+#include "SyntheticSections.h"
 #include "Target.h"
 #include "Threads.h"
 #include "Writer.h"
 #include "lld/Config/Version.h"
 #include "lld/Driver/Driver.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Object/Decompressor.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TarWriter.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstdlib>
 #include <utility>
template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   for (elf::ObjectFile<ELFT> *F : Symtab.getObjectFiles())
     for (InputSectionBase *S : F->getSections())
       if (S && S != &InputSection::Discarded)
         InputSections.push_back(S);
   for (BinaryFile *F : Symtab.getBinaryFiles())
     for (InputSectionBase *S : F->getSections())
       InputSections.push_back(cast<InputSection>(S));
 
-  // Do size optimizations: garbage collection and identical code folding.
+  // This adds a .comment section containing a version string. We have to add it
+  // before decompressAndMergeSections because the .comment section is a
+  // mergeable section.
+  if (!Config->Relocatable)
+    InputSections.push_back(createCommentSection<ELFT>());
+
+  // Do size optimizations: garbage collection, merging of SHF_MERGE sections
+  // and identical code folding.
   if (Config->GcSections)
     markLive<ELFT>();
+  decompressAndMergeSections();
   if (Config->ICF)
     doIcf<ELFT>();
 
-  // MergeInputSection::splitIntoPieces needs to be called before
-  // any call of MergeInputSection::getOffset. Do that.
-  parallelForEach(InputSections.begin(), InputSections.end(),
-                  [](InputSectionBase *S) {
-                    if (!S->Live)
-                      return;
-                    if (Decompressor::isCompressedELFSection(S->Flags, S->Name))
-                      S->uncompress();
-                    if (auto *MS = dyn_cast<MergeInputSection>(S))
-                      MS->splitIntoPieces();
-                  });
-
   // Write the result to the file.
   writeResult<ELFT>();
 }

lld/trunk/ELF/SyntheticSections.h

public:
   void finalizeContents() override;
   bool shouldTailMerge() const;
   size_t getSize() const override;
 
 private:
   void finalizeTailMerge();
   void finalizeNoTailMerge();
 
-  bool Finalized = false;
   llvm::StringTableBuilder Builder;
   std::vector<MergeInputSection *> Sections;
 };
 
 // .MIPS.abiflags section.
 template <class ELFT>
 class MipsAbiFlagsSection final : public SyntheticSection {
   typedef llvm::object::Elf_Mips_ABIFlags<ELFT> Elf_Mips_ABIFlags;

 private:
   std::vector<const Thunk *> Thunks;
   size_t Size = 0;
 };
 
 template <class ELFT> InputSection *createCommonSection();
 InputSection *createInterpSection();
 template <class ELFT> MergeInputSection *createCommentSection();
+void decompressAndMergeSections();
 
 SymbolBody *addSyntheticLocal(StringRef Name, uint8_t Type, uint64_t Value,
                               uint64_t Size, InputSectionBase *Section);
 
 // Linker generated sections which can be used as inputs.
 struct InX {
   static InputSection *ARMAttributes;
   static BssSection *Bss;

lld/trunk/ELF/SyntheticSections.cpp

 #include "Strings.h"
 #include "SymbolTable.h"
 #include "Target.h"
 #include "Threads.h"
 #include "Writer.h"
 #include "lld/Config/Version.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugPubTable.h"
+#include "llvm/Object/Decompressor.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/RandomNumberGenerator.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/xxhash.h"
 #include <cstdlib>
 

 }
 
 MergeSyntheticSection::MergeSyntheticSection(StringRef Name, uint32_t Type,
                                              uint64_t Flags, uint32_t Alignment)
     : SyntheticSection(Flags, Type, Alignment, Name),
       Builder(StringTableBuilder::RAW, Alignment) {}
 
 void MergeSyntheticSection::addSection(MergeInputSection *MS) {
-  assert(!Finalized);
   MS->Parent = this;
   Sections.push_back(MS);
 }
 
 void MergeSyntheticSection::writeTo(uint8_t *Buf) { Builder.write(Buf); }
 
 bool MergeSyntheticSection::shouldTailMerge() const {
   return (this->Flags & SHF_STRINGS) && Config->Optimize >= 2;
for (MergeInputSection *Sec : Sections)
     for (size_t I = 0, E = Sec->Pieces.size(); I != E; ++I)
       if (Sec->Pieces[I].Live)
         Sec->Pieces[I].OutputOff = Builder.add(Sec->getData(I));
 
   Builder.finalizeInOrder();
 }
 
 void MergeSyntheticSection::finalizeContents() {
-  if (Finalized)
-    return;
-  Finalized = true;
   if (shouldTailMerge())
     finalizeTailMerge();
   else
     finalizeNoTailMerge();
 }
 
 size_t MergeSyntheticSection::getSize() const {
-  // We should finalize string builder to know the size.
-  const_cast<MergeSyntheticSection *>(this)->finalizeContents();
   return Builder.getSize();
 }
 
+// This function decompresses compressed sections and scans over the input
+// sections to create mergeable synthetic sections. It removes
+// MergeInputSections from the input section array and adds new synthetic
+// sections at the location of the first input section that it replaces. It then
+// finalizes each synthetic section in order to compute an output offset for
+// each piece of each input section.
+void elf::decompressAndMergeSections() {
+  // splitIntoPieces needs to be called on each MergeInputSection before calling
+  // finalizeContents(). Do that first.
+  parallelForEach(InputSections.begin(), InputSections.end(),
+                  [](InputSectionBase *S) {
+                    if (!S->Live)
+                      return;
+                    if (Decompressor::isCompressedELFSection(S->Flags, S->Name))
+                      S->uncompress();
+                    if (auto *MS = dyn_cast<MergeInputSection>(S))
+                      MS->splitIntoPieces();
+                  });
+
+  std::vector<MergeSyntheticSection *> MergeSections;
+  for (InputSectionBase *&S : InputSections) {
+    MergeInputSection *MS = dyn_cast<MergeInputSection>(S);
+    if (!MS)
+      continue;
+
+    // We do not want to handle sections that are not alive, so just remove
+    // them instead of trying to merge.
+    if (!MS->Live)
+      continue;
+
+    StringRef OutsecName = getOutputSectionName(MS->Name);
+    uint64_t Flags = MS->Flags & ~(uint64_t)SHF_GROUP;
+    uint32_t Alignment = std::max<uint32_t>(MS->Alignment, MS->Entsize);
+
+    auto I = llvm::find_if(MergeSections, [=](MergeSyntheticSection *Sec) {
+      return Sec->Name == OutsecName && Sec->Flags == Flags &&
+             Sec->Alignment == Alignment;
+    });
+    if (I == MergeSections.end()) {
+      MergeSyntheticSection *Syn =
+          make<MergeSyntheticSection>(OutsecName, MS->Type, Flags, Alignment);
+      MergeSections.push_back(Syn);
+      I = std::prev(MergeSections.end());
+      S = Syn;
+    } else {
+      S = nullptr;
+    }
+    (*I)->addSection(MS);
+  }
+  for (auto *MS : MergeSections)
+    MS->finalizeContents();
+
+  std::vector<InputSectionBase *> &V = InputSections;
+  V.erase(std::remove(V.begin(), V.end(), nullptr), V.end());
+}
+
 MipsRldMapSection::MipsRldMapSection()
     : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_PROGBITS, Config->Wordsize,
                        ".rld_map") {}
 
 ARMExidxSentinelSection::ARMExidxSentinelSection()
     : SyntheticSection(SHF_ALLOC | SHF_LINK_ORDER, SHT_ARM_EXIDX,
                        Config->Wordsize, ".ARM.exidx") {}
 

lld/trunk/ELF/Writer.cpp

auto I = std::remove_if(Phdrs.begin(), Phdrs.end(), [&](const PhdrEntry &P) {
     if (!P.First)
       return true;
     uint64_t Size = P.Last->Addr + P.Last->Size - P.First->Addr;
     return Size == 0;
   });
   Phdrs.erase(I, Phdrs.end());
 }
 
-// This function scans over the input sections and creates mergeable
-// synthetic sections. It removes MergeInputSections from array and
-// adds new synthetic ones. Each synthetic section is added to the
-// location of the first input section it replaces.
-static void combineMergableSections() {
-  std::vector<MergeSyntheticSection *> MergeSections;
-  for (InputSectionBase *&S : InputSections) {
-    MergeInputSection *MS = dyn_cast<MergeInputSection>(S);
-    if (!MS)
-      continue;
-
-    // We do not want to handle sections that are not alive, so just remove
-    // them instead of trying to merge.
-    if (!MS->Live)
-      continue;
-
-    StringRef OutsecName = getOutputSectionName(MS->Name);
-    uint64_t Flags = MS->Flags & ~(uint64_t)SHF_GROUP;
-    uint32_t Alignment = std::max<uint32_t>(MS->Alignment, MS->Entsize);
-
-    auto I = llvm::find_if(MergeSections, [=](MergeSyntheticSection *Sec) {
-      return Sec->Name == OutsecName && Sec->Flags == Flags &&
-             Sec->Alignment == Alignment;
-    });
-    if (I == MergeSections.end()) {
-      MergeSyntheticSection *Syn =
-          make<MergeSyntheticSection>(OutsecName, MS->Type, Flags, Alignment);
-      MergeSections.push_back(Syn);
-      I = std::prev(MergeSections.end());
-      S = Syn;
-    } else {
-      S = nullptr;
-    }
-    (*I)->addSection(MS);
-  }
-
-  std::vector<InputSectionBase *> &V = InputSections;
-  V.erase(std::remove(V.begin(), V.end(), nullptr), V.end());
-}
-
 template <class ELFT> static void combineEhFrameSections() {
   for (InputSectionBase *&S : InputSections) {
     EhInputSection *ES = dyn_cast<EhInputSection>(S);
     if (!ES || !ES->Live)
       continue;
 
     In<ELFT>::EhFrame->addSection(ES);
     S = nullptr;
template <class ELFT> void Writer<ELFT>::clearOutputSections() {
   OutputSections.clear();
 }
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
   // Create linker-synthesized sections such as .got or .plt.
   // Such sections are of type input section.
   createSyntheticSections();
-  combineMergableSections();
 
   if (!Config->Relocatable)
     combineEhFrameSections<ELFT>();
 
   // We need to create some reserved symbols such as _end. Create them.
   if (!Config->Relocatable)
     addReservedSymbols();
 
template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
 
   if (needsInterpSection<ELFT>()) {
     InX::Interp = createInterpSection();
     Add(InX::Interp);
   } else {
     InX::Interp = nullptr;
   }
 
-  if (!Config->Relocatable)
-    Add(createCommentSection<ELFT>());
-
   if (Config->Strip != StripPolicy::All) {
     InX::StrTab = make<StringTableSection>(".strtab", false);
     InX::SymTab = make<SymbolTableSection<ELFT>>(*InX::StrTab);
   }
 
   if (Config->BuildId != BuildIdKind::None) {
     InX::BuildId = make<BuildIdSection>();
     Add(InX::BuildId);