[lld] r232866 - [ELF] OrderPass : Order atoms only by ordinals.

Rui Ueyama ruiu at google.com
Fri Mar 20 17:22:01 PDT 2015


This is non-trivial change, you should have sent this to pre-commit review.
Please do so next time. This change sacrificed readability a bit. Also, I
believe there's a bug in the code.

On Fri, Mar 20, 2015 at 4:47 PM, Shankar Easwaran <shankare at codeaurora.org>
wrote:

> Author: shankare
> Date: Fri Mar 20 18:47:03 2015
> New Revision: 232866
>
> URL: http://llvm.org/viewvc/llvm-project?rev=232866&view=rev
> Log:
> [ELF] OrderPass : Order atoms only by ordinals.
>
> Move the init array/fini array sorting to the Output ELF writer.
>
> AFAIK, this is only needed by the X86_64/ARM ABI.
>
> This shaves time taken to self host lld by 0.2 seconds.
>
> Before patch
> ----------------
> 4791.062059 task-clock                #    1.634 CPUs utilized
> ( +-  0.28% )
>      61,107 context-switches          #    0.013 M/sec
> ( +-  0.56% )
> 2.932902671 seconds time elapsed
> ( +-  0.84% )
>
> After patch
> -------------
> 4608.417248 task-clock                #    1.669 CPUs utilized
> ( +-  0.30% )
>      61,616 context-switches          #    0.013 M/sec
> ( +-  0.63% )
> 2.761012703 seconds time elapsed
> ( +-  0.63% )
>
> Modified:
>     lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h
>     lld/trunk/lib/ReaderWriter/ELF/OrderPass.h
>     lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64TargetHandler.h
>     lld/trunk/test/elf/init_array-order.test
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h?rev=232866&r1=232865&r2=232866&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/DefaultLayout.h Fri Mar 20 18:47:03 2015
> @@ -316,13 +316,25 @@ public:
>      return _copiedDynSymNames.count(sla->name());
>    }
>
> +  /// \brief Handle SORT_BY_PRIORITY.
> +  void sortOutputSectionByPriority(StringRef outputSectionName,
> +                                   StringRef prefix);
> +
>  protected:
> +  /// \brief TargetLayouts may use these functions to reorder the input
> sections
> +  /// in a order defined by their ABI.
> +  virtual void finalizeOutputSectionLayout() {}
> +
>    /// \brief Allocate a new section.
>    virtual AtomSection<ELFT> *createSection(
>        StringRef name, int32_t contentType,
>        DefinedAtom::ContentPermissions contentPermissions,
>        SectionOrder sectionOrder);
>
> +private:
> +  /// Helper function that returns the priority value from an input
> section.
> +  uint32_t getPriorityFromSectionName(StringRef sectionName) const;
> +
>  protected:
>    llvm::BumpPtrAllocator _allocator;
>    SectionMapT _sectionMap;
> @@ -657,13 +669,56 @@ template <class ELFT> void DefaultLayout
>    }
>  }
>
> +template <class ELFT>
> +uint32_t
> +DefaultLayout<ELFT>::getPriorityFromSectionName(StringRef sectionName)
> const {
> +  StringRef priority = sectionName.drop_front().rsplit('.').second;
> +  uint32_t prio;
> +  if (priority.getAsInteger(10, prio))
> +    return std::numeric_limits<uint32_t>::max();
> +  return prio;
> +}
> +
> +template <class ELFT>
> +void DefaultLayout<ELFT>::sortOutputSectionByPriority(
> +    StringRef outputSectionName, StringRef prefix) {
> +  OutputSection<ELFT> *outputSection =
> findOutputSection(outputSectionName);
> +  if (!outputSection)
> +    return;
> +
> +  auto sections = outputSection->sections();
> +
> +  std::sort(sections.begin(), sections.end(),
> +            [&](Chunk<ELFT> *lhs, Chunk<ELFT> *rhs) {
> +              Section<ELFT> *lhsSection = dyn_cast<Section<ELFT>>(lhs);
> +              Section<ELFT> *rhsSection = dyn_cast<Section<ELFT>>(rhs);
> +              if (!lhsSection || !rhsSection)
> +                return false;
> +              StringRef lhsSectionName = lhsSection->inputSectionName();
> +              StringRef rhsSectionName = rhsSection->inputSectionName();
> +
> +              if (!prefix.empty()) {
> +                if (!lhsSectionName.startswith(prefix) ||
> +                    !rhsSectionName.startswith(prefix))
> +                  return false;
> +              }

+              return getPriorityFromSectionName(lhsSectionName) <
> +                     getPriorityFromSectionName(rhsSectionName);
> +            });
>

This sort does not preserve the original section order if they don't start
with a given prefix.

+};
> +
>  template <class ELFT> void
> DefaultLayout<ELFT>::assignSectionsToSegments() {
>    ScopedTask task(getDefaultDomain(), "assignSectionsToSegments");
>    ELFLinkingContext::OutputMagic outputMagic = _context.getOutputMagic();
>    // sort the sections by their order as defined by the layout
>    sortInputSections();
> +
>    // Create output sections.
>    createOutputSections();
> +
> +  // Finalize output section layout.
> +  finalizeOutputSectionLayout();
> +
>    // Set the ordinal after sorting the sections
>    int ordinal = 1;
>    for (auto osi : _outputSections) {
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/OrderPass.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/OrderPass.h?rev=232866&r1=232865&r2=232866&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/OrderPass.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/OrderPass.h Fri Mar 20 18:47:03 2015
> @@ -17,49 +17,13 @@ namespace lld {
>  namespace elf {
>
>  /// \brief This pass sorts atoms by file and atom ordinals.
> -/// .{init,fini}_array.<priority> sections are handled specially.
>  class OrderPass : public Pass {
>  public:
>    void perform(std::unique_ptr<MutableFile> &file) override {
> -    MutableFile::DefinedAtomRange defined = file->definedAtoms();
> -    auto last = std::partition(defined.begin(), defined.end(),
> isInitFini);
> -    parallel_sort(defined.begin(), last, compareInitFini);
> -    parallel_sort(last, defined.end(), DefinedAtom::compareByPosition);
> -  }
> -
> -private:
> -  static bool isInitFini(const DefinedAtom *atom) {
> -    if (atom->sectionChoice() != DefinedAtom::sectionCustomRequired)
> -      return false;
> -    StringRef name = atom->customSectionName();
> -    return name.startswith(".init_array") ||
> name.startswith(".fini_array");
> -  }
> -
> -  // Parses the number in .{init,fini}_array.<number>.
> -  // Returns UINT32_MAX by default.
> -  static uint32_t getPriority(const DefinedAtom *atom) {
> -    StringRef sec = atom->customSectionName();
> -    StringRef num = sec.drop_front().rsplit('.').second;
> -    uint32_t prio;
> -    if (num.getAsInteger(10, prio))
> -      return std::numeric_limits<uint32_t>::max();
> -    return prio;
> -  }
> -
> -  static bool compareInitFini(const DefinedAtom *lhs, const DefinedAtom
> *rhs) {
> -    // Sort {.init_array, .fini_array}[.<num>] sections
> -    // according to their number. Sections without optional
> -    // numer suffix should go last.
> -    uint32_t lp = getPriority(lhs);
> -    uint32_t rp = getPriority(rhs);
> -    if (lp != rp)
> -      return lp < rp;
> -
> -    // If both atoms have the same priority, fall back to default.
> -    return DefinedAtom::compareByPosition(lhs, rhs);
> +    parallel_sort(file->definedAtoms().begin(),
> file->definedAtoms().end(),
> +                  DefinedAtom::compareByPosition);
>    }
>  };
> -
>  }
>  }
>
>
> Modified: lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64TargetHandler.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64TargetHandler.h?rev=232866&r1=232865&r2=232866&view=diff
>
> ==============================================================================
> --- lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64TargetHandler.h (original)
> +++ lld/trunk/lib/ReaderWriter/ELF/X86_64/X86_64TargetHandler.h Fri Mar 20
> 18:47:03 2015
> @@ -24,6 +24,11 @@ class X86_64TargetLayout : public Target
>  public:
>    X86_64TargetLayout(X86_64LinkingContext &context)
>        : TargetLayout(context) {}
> +
> +  void finalizeOutputSectionLayout() override {
> +    sortOutputSectionByPriority(".init_array", ".init_array");
> +    sortOutputSectionByPriority(".fini_array", ".fini_array");
>

And you call the function twice. The result of the first invocation would
be invalidated by the second invocation.


> +  }
>  };
>
>  class X86_64TargetHandler
>
> Modified: lld/trunk/test/elf/init_array-order.test
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/init_array-order.test?rev=232866&r1=232865&r2=232866&view=diff
>
> ==============================================================================
> --- lld/trunk/test/elf/init_array-order.test (original)
> +++ lld/trunk/test/elf/init_array-order.test Fri Mar 20 18:47:03 2015
> @@ -1,6 +1,7 @@
>  #RUN: yaml2obj -format=elf %s > %t
>  #RUN: lld -flavor gnu -target x86_64-linux %t --noinhibit-exec \
> -#RUN:   --output-filetype=yaml | FileCheck %s
> +#RUN:   -o %t1.out
> +#RUN: llvm-objdump -s %t1.out | FileCheck %s
>
>  !ELF
>  FileHeader:
> @@ -62,12 +63,5 @@ Symbols:
>      Type: STT_SECTION
>      Section: .init_array
>
> -#CHECK: defined-atoms:
> -#CHECK:   content: [ 01,
> -#CHECK:   section-name: .init_array.1
> -#CHECK:   content: [ 02,
> -#CHECK:   section-name: .init_array.2
> -#CHECK:   content: [ 03,
> -#CHECK:   section-name: .init_array.3
> -#CHECK:   content: [ 99,
> -#CHECK:   section-name: .init_array
> +#CHECK:  {{[0xa-f0-9]+}} 01000000 00000000 02000000 00000000
> +#CHECK:  {{[0xa-f0-9]+}} 03000000 00000000 99000000 00000000
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150320/661c180b/attachment.html>


More information about the llvm-commits mailing list