[lld] r328905 - ELF: Place ordered sections in the middle of the unordered section list on targets with limited-range branches.

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 3 12:33:05 PDT 2018


On Mon, Apr 2, 2018 at 1:32 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:

>
>
> On Mon, Apr 2, 2018 at 10:30 AM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>>
>> > +// Sorts the sections in ISD according to the provided section order.
>> > +static void
>> > +sortISDBySectionOrder(InputSectionDescription *ISD,
>> > +                      const DenseMap<const InputSectionBase *, int>
>> &Order) {
>> > +  std::vector<InputSection *> UnorderedSections;
>> > +  std::vector<InputSection *> OrderedSections;
>> > +  uint64_t UnorderedSize = 0;
>> > +
>> > +  for (InputSection *IS : ISD->Sections) {
>> > +    if (!Order.count(IS)) {
>> > +      UnorderedSections.push_back(IS);
>> > +      UnorderedSize += IS->getSize();
>> > +      continue;
>> > +    }
>> > +    OrderedSections.push_back(IS);
>> > +  }
>> > +  std::sort(OrderedSections.begin(), OrderedSections.end(),
>> > +            [&](InputSection *A, InputSection *B) {
>> > +              return Order.lookup(A) < Order.lookup(B);
>> > +            });
>>
>> Unlike the previous code this does the lookpup in the sort
>> callback. Should OrderedSections be an array of std::pair or should
>> sortByOrder be simplified too?
>>
>
> I considered using an array of pairs here but decided to keep things
> simple -- I didn't realise that that was what sortByOrder was already
> doing. I'll see what the perf impact is on Chrome.
>

It turned out to improve performance by 0.5-1%, so I sent
https://reviews.llvm.org/D45222

Peter


> > +  if (Target->ThunkSectionSpacing && !OrderedSections.empty()) {
>>
>> What is the disadvantage of also doing this in the
>> !Target->ThunkSectionSpacing case?
>>
>
> No major disadvantages -- I suppose that there's the minor disadvantage
> that the layout is visible to users in various ways (.symtab, map files,
> ...), and placing the ordered sections in the middle could be confusing to
> users (e.g. a user might suspect a bug if they do not see the sections at
> the start), so we should probably only do it on architectures where there
> is a benefit in doing it.
>
> There's also the cost of scanning the section array again but I wouldn't
> expect that to be large.
>
> Peter
>
>>
>> Cheers,
>> Rafael
>>
>
>
>
> --
> --
> Peter
>



-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180403/baa9435b/attachment.html>


More information about the llvm-commits mailing list