[lld] r232866 - [ELF] OrderPass : Order atoms only by ordinals.
Rui Ueyama
ruiu at google.com
Mon Mar 23 14:28:12 PDT 2015
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150323/586836a2/attachment.html>
More information about the llvm-commits
mailing list