[lld] r232866 - [ELF] OrderPass : Order atoms only by ordinals.
Shankar Easwaran
shankare at codeaurora.org
Fri Mar 20 17:29:51 PDT 2015
On 3/20/2015 7:22 PM, Rui Ueyama wrote:
> 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.
Thanks for the review, I will check it out and fix.
>
> +};
>> +
>> 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.
No there are two output sections .init_array/.fini_array. So the first
invocation only runs through sections inside the init_array/fini_array
respectively.
>
>
>> + }
>> };
>>
>> 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
>>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation
More information about the llvm-commits
mailing list