[lld] r232866 - [ELF] OrderPass : Order atoms only by ordinals.
Shankar Easwaran
shankare at codeaurora.org
Mon Mar 23 14:51:32 PDT 2015
Partly Agree. I am not sure of certain architectures that dont need
sorting constructors by priority and the linker tries to sort them though.
Shankar Easwaran
On 3/23/2015 4:28 PM, Rui Ueyama wrote:
> This might be useful for other purposes, but that's not what I meant.
>
> What I meant is that, doing something over a list of sections is probably
> negligible from the performance point of view, so we should prefer source
> code simplicity than marginal optimization.
>
> On Fri, Mar 20, 2015 at 5:50 PM, Shankar Easwaran <shankare at codeaurora.org>
> wrote:
>
>> I have another change to support --ctors-in-init-array, which will allow
>> sections by name .ctors to be placed in .init_array sections too.
>>
>> This would also be used by linker scripts to support
>> SORT_BY_INIT_PRIORITY(*(.blah.*)) too.
>>
>> Shankar Easwaran
>>
>>
>> On 3/20/2015 7:40 PM, Rui Ueyama wrote:
>>
>>> OK. Maybe we should sort .init_array and fini_array section members
>>> unconditionally. Unlike atoms, the number of output sections is pretty
>>> small. This change seems a bit too clever than it needs to be. No need to
>>> parameterize this code for x86/x86-64.
>>>
>>> On Fri, Mar 20, 2015 at 5:31 PM, Shankar Easwaran <
>>> shankare at codeaurora.org>
>>> wrote:
>>>
>>> The sorting is done by using the input section name. Sorting still holds
>>>> good.
>>>>
>>>>
>>>> On 3/20/2015 7:26 PM, Rui Ueyama wrote:
>>>>
>>>> Oh, I'm not sure if this change is correct at all.
>>>>> Instead of sorting atoms, you sort output sections. Output sections are
>>>>> created, and its name is given by getOutputSection function. That
>>>>> function
>>>>> returns ".fini_array" for any input sections start with ".fini_array".
>>>>> Thus, all .fini_array input sections are binned to the same section
>>>>> *before* you sort them. So, sorting it is too late, no?
>>>>>
>>>>> I really don't like to see this kind of patch is submitted without
>>>>> review.
>>>>>
>>>>> On Fri, Mar 20, 2015 at 5:22 PM, Rui Ueyama <ruiu at google.com> 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.
>>>>>>
>>>>>> +};
>>>>>>
>>>>>> +
>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>>> --
>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
>>>> by the Linux Foundation
>>>>
>>>>
>>>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
>> by the Linux Foundation
>>
>>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation
More information about the llvm-commits
mailing list