[lld] r232866 - [ELF] OrderPass : Order atoms only by ordinals.
Shankar Easwaran
shankare at codeaurora.org
Fri Mar 20 17:31:54 PDT 2015
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
More information about the llvm-commits
mailing list