<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 2, 2018 at 1:32 PM, Peter Collingbourne <span dir="ltr"><<a href="mailto:peter@pcc.me.uk" target="_blank">peter@pcc.me.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="m_3206934069381704092gmail-">On Mon, Apr 2, 2018 at 10:30 AM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span><br>
> +// Sorts the sections in ISD according to the provided section order.<br>
> +static void<br>
> +sortISDBySectionOrder(InputSe<wbr>ctionDescription *ISD,<br>
> + const DenseMap<const InputSectionBase *, int> &Order) {<br>
> + std::vector<InputSection *> UnorderedSections;<br>
> + std::vector<InputSection *> OrderedSections;<br>
> + uint64_t UnorderedSize = 0;<br>
> +<br>
> + for (InputSection *IS : ISD->Sections) {<br>
> + if (!Order.count(IS)) {<br>
> + UnorderedSections.push_back(IS<wbr>);<br>
> + UnorderedSize += IS->getSize();<br>
> + continue;<br>
> + }<br>
> + OrderedSections.push_back(IS);<br>
> + }<br>
> + std::sort(OrderedSections.begi<wbr>n(), OrderedSections.end(),<br>
> + [&](InputSection *A, InputSection *B) {<br>
> + return Order.lookup(A) < Order.lookup(B);<br>
> + });<br>
<br>
</span>Unlike the previous code this does the lookpup in the sort<br>
callback. Should OrderedSections be an array of std::pair or should<br>
sortByOrder be simplified too?<br></blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div><div>It turned out to improve performance by 0.5-1%, so I sent <a href="https://reviews.llvm.org/D45222" target="_blank">https://reviews.llvm.org/<wbr>D45222</a></div><div><br></div><div>Peter</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="m_3206934069381704092gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>
> + if (Target->ThunkSectionSpacing && !OrderedSections.empty()) {<br>
<br>
</span>What is the disadvantage of also doing this in the<br>
!Target->ThunkSectionSpacing case?<br></blockquote><div><br></div></span><div>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.</div><div><br></div><div>There's also the cost of scanning the section array again but I wouldn't expect that to be large.</div><div><br></div><div>Peter</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Cheers,<br>
Rafael<span class="m_3206934069381704092gmail-HOEnZb"><font color="#888888"><br>
</font></span></blockquote></div><span class="m_3206934069381704092gmail-HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div class="m_3206934069381704092gmail-m_-3019995506074115976gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</font></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="m_3206934069381704092gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</div></div>