[lld] r317406 - [ELF] - Stop using SectionKey for creating output sections.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 4 02:11:27 PDT 2017


Author: grimar
Date: Sat Nov  4 02:11:27 2017
New Revision: 317406

URL: http://llvm.org/viewvc/llvm-project?rev=317406&view=rev
Log:
[ELF] - Stop using SectionKey for creating output sections.

Stop using SectionKey for creating output sections.

Initially SectionKey was designed because we merged section with
use of Flags and Alignment fields. Currently LLD merges them by name only,
except the case when -relocatable output is produced. In that case
we still merge sections only with the same flags and alignment.
There is probably no issue at all to stop using Flags and Alignment for -r and
just disable the merging in that case.

After doing that change we can get rid of using SectionKey. That is not only
simplifies the code, but also gives some perfomance boost.

I tried to link chrome and mozilla, results are next:
* chrome link time goes from 1,666750355s to 1,551585364s, that is about 7%.
* mozilla time changes from 3,210261947 to 3,153782940, or about 2%.

Differential revision: https://reviews.llvm.org/D39594

Modified:
    lld/trunk/ELF/OutputSections.cpp
    lld/trunk/ELF/OutputSections.h

Modified: lld/trunk/ELF/OutputSections.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=317406&r1=317405&r2=317406&view=diff
==============================================================================
--- lld/trunk/ELF/OutputSections.cpp (original)
+++ lld/trunk/ELF/OutputSections.cpp Sat Nov  4 02:11:27 2017
@@ -142,61 +142,6 @@ void OutputSection::addSection(InputSect
   }
 }
 
-static SectionKey createKey(InputSectionBase *IS, StringRef OutsecName) {
-  // When control reaches here, mergeable sections have already been
-  // merged except the -r case. If that's the case, we want to combine
-  // mergeable sections by sh_entsize and sh_flags.
-  if (Config->Relocatable && (IS->Flags & SHF_MERGE)) {
-    uint64_t Flags = IS->Flags & (SHF_MERGE | SHF_STRINGS);
-    uint32_t Alignment = std::max<uint32_t>(IS->Alignment, IS->Entsize);
-    return SectionKey{OutsecName, Flags, Alignment};
-  }
-
-  //  The ELF spec just says
-  // ----------------------------------------------------------------
-  // In the first phase, input sections that match in name, type and
-  // attribute flags should be concatenated into single sections.
-  // ----------------------------------------------------------------
-  //
-  // However, it is clear that at least some flags have to be ignored for
-  // section merging. At the very least SHF_GROUP and SHF_COMPRESSED have to be
-  // ignored. We should not have two output .text sections just because one was
-  // in a group and another was not for example.
-  //
-  // It also seems that that wording was a late addition and didn't get the
-  // necessary scrutiny.
-  //
-  // Merging sections with different flags is expected by some users. One
-  // reason is that if one file has
-  //
-  // int *const bar __attribute__((section(".foo"))) = (int *)0;
-  //
-  // gcc with -fPIC will produce a read only .foo section. But if another
-  // file has
-  //
-  // int zed;
-  // int *const bar __attribute__((section(".foo"))) = (int *)&zed;
-  //
-  // gcc with -fPIC will produce a read write section.
-  //
-  // Last but not least, when using linker script the merge rules are forced by
-  // the script. Unfortunately, linker scripts are name based. This means that
-  // expressions like *(.foo*) can refer to multiple input sections with
-  // different flags. We cannot put them in different output sections or we
-  // would produce wrong results for
-  //
-  // start = .; *(.foo.*) end = .; *(.bar)
-  //
-  // and a mapping of .foo1 and .bar1 to one section and .foo2 and .bar2 to
-  // another. The problem is that there is no way to layout those output
-  // sections such that the .foo sections are the only thing between the start
-  // and end symbols.
-  //
-  // Given the above issues, we instead merge sections by name and error on
-  // incompatible types and flags.
-  return SectionKey{OutsecName, 0, 0};
-}
-
 OutputSectionFactory::OutputSectionFactory() {}
 
 void elf::sortByOrder(MutableArrayRef<InputSection *> In,
@@ -252,8 +197,55 @@ OutputSection *OutputSectionFactory::add
     return Out->RelocationSection;
   }
 
-  SectionKey Key = createKey(IS, OutsecName);
-  OutputSection *&Sec = Map[Key];
+  // When control reaches here, mergeable sections have already been
+  // merged except the -r case. If that's the case, we do not combine them
+  // and let final link to handle this optimization.
+  if (Config->Relocatable && (IS->Flags & SHF_MERGE))
+    return createSection(IS, OutsecName);
+
+  //  The ELF spec just says
+  // ----------------------------------------------------------------
+  // In the first phase, input sections that match in name, type and
+  // attribute flags should be concatenated into single sections.
+  // ----------------------------------------------------------------
+  //
+  // However, it is clear that at least some flags have to be ignored for
+  // section merging. At the very least SHF_GROUP and SHF_COMPRESSED have to be
+  // ignored. We should not have two output .text sections just because one was
+  // in a group and another was not for example.
+  //
+  // It also seems that that wording was a late addition and didn't get the
+  // necessary scrutiny.
+  //
+  // Merging sections with different flags is expected by some users. One
+  // reason is that if one file has
+  //
+  // int *const bar __attribute__((section(".foo"))) = (int *)0;
+  //
+  // gcc with -fPIC will produce a read only .foo section. But if another
+  // file has
+  //
+  // int zed;
+  // int *const bar __attribute__((section(".foo"))) = (int *)&zed;
+  //
+  // gcc with -fPIC will produce a read write section.
+  //
+  // Last but not least, when using linker script the merge rules are forced by
+  // the script. Unfortunately, linker scripts are name based. This means that
+  // expressions like *(.foo*) can refer to multiple input sections with
+  // different flags. We cannot put them in different output sections or we
+  // would produce wrong results for
+  //
+  // start = .; *(.foo.*) end = .; *(.bar)
+  //
+  // and a mapping of .foo1 and .bar1 to one section and .foo2 and .bar2 to
+  // another. The problem is that there is no way to layout those output
+  // sections such that the .foo sections are the only thing between the start
+  // and end symbols.
+  //
+  // Given the above issues, we instead merge sections by name and error on
+  // incompatible types and flags.
+  OutputSection *&Sec = Map[OutsecName];
   if (Sec) {
     Sec->addSection(cast<InputSection>(IS));
     return nullptr;
@@ -265,24 +257,6 @@ OutputSection *OutputSectionFactory::add
 
 OutputSectionFactory::~OutputSectionFactory() {}
 
-SectionKey DenseMapInfo<SectionKey>::getEmptyKey() {
-  return SectionKey{DenseMapInfo<StringRef>::getEmptyKey(), 0, 0};
-}
-
-SectionKey DenseMapInfo<SectionKey>::getTombstoneKey() {
-  return SectionKey{DenseMapInfo<StringRef>::getTombstoneKey(), 0, 0};
-}
-
-unsigned DenseMapInfo<SectionKey>::getHashValue(const SectionKey &Val) {
-  return hash_combine(Val.Name, Val.Flags, Val.Alignment);
-}
-
-bool DenseMapInfo<SectionKey>::isEqual(const SectionKey &LHS,
-                                       const SectionKey &RHS) {
-  return DenseMapInfo<StringRef>::isEqual(LHS.Name, RHS.Name) &&
-         LHS.Flags == RHS.Flags && LHS.Alignment == RHS.Alignment;
-}
-
 uint64_t elf::getHeaderSize() {
   if (Config->OFormatBinary)
     return 0;

Modified: lld/trunk/ELF/OutputSections.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.h?rev=317406&r1=317405&r2=317406&view=diff
==============================================================================
--- lld/trunk/ELF/OutputSections.h (original)
+++ lld/trunk/ELF/OutputSections.h Sat Nov  4 02:11:27 2017
@@ -131,24 +131,9 @@ struct Out {
   static OutputSection *FiniArray;
 };
 
-struct SectionKey {
-  StringRef Name;
-  uint64_t Flags;
-  uint32_t Alignment;
-};
 } // namespace elf
 } // namespace lld
 
-namespace llvm {
-template <> struct DenseMapInfo<lld::elf::SectionKey> {
-  static lld::elf::SectionKey getEmptyKey();
-  static lld::elf::SectionKey getTombstoneKey();
-  static unsigned getHashValue(const lld::elf::SectionKey &Val);
-  static bool isEqual(const lld::elf::SectionKey &LHS,
-                      const lld::elf::SectionKey &RHS);
-};
-} // namespace llvm
-
 namespace lld {
 namespace elf {
 // This class knows how to create an output section for a given
@@ -163,7 +148,7 @@ public:
   OutputSection *addInputSec(InputSectionBase *IS, StringRef OutsecName);
 
 private:
-  llvm::SmallDenseMap<SectionKey, OutputSection *> Map;
+  llvm::StringMap<OutputSection *> Map;
 };
 
 uint64_t getHeaderSize();




More information about the llvm-commits mailing list