[lld] r328810 - Refactor Writer::checkNoOverlappingSections. NFC.

[Rui Ueyama via llvm-commits]

On Thu, Mar 29, 2018 at 2:17 PM Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Rui Ueyama via llvm-commits <llvm-commits at lists.llvm.org> writes:
>
> > -  // By removing all zero-size sections we can simplify the check for
> overlap to
> > -  // just checking whether the section range contains the other
> section's start
> > -  // address. Additionally, it also slightly speeds up the checking
> since we
> > -  // don't bother checking for overlap with sections that can never
> overlap.
>
> This comment is missing in the new code.
>

I'll add them with some simplification. I don't think we need to mention
marginal speedup.


> > +static void checkOverlap(StringRef Name, std::vector<SectionOffset>
> &Sections) {
>
> Could this take an ArrayRef<SectionOffset>?
>

Unfortunately not. We sort Sections, so this must be mutable.


> > +  // Since the sections are sorted by start address we only need to
> check
> > +  // whether the other sections starts before the end of Sec. If this is
> > +  // not the case we can break out of this loop since all following
> sections
> > +  // will also start after the end of Sec.
> >    for (size_t I = 0; I < Sections.size(); ++I) {
> > -    OutputSection *Sec = Sections[I];
> > -    uint64_t Start = GetStart(Sec);
> > -    for (auto *Other : ArrayRef<OutputSection *>(Sections).slice(I +
> 1)) {
> > -      // Since the sections are sorted by start address we only need to
> check
> > -      // whether the other sections starts before the end of Sec. If
> this is
> > -      // not the case we can break out of this loop since all following
> sections
> > -      // will also start after the end of Sec.
> > -      if (Start + Sec->Size <= GetStart(Other))
> > -        break;
> > -      errorOrWarn("section " + Sec->Name + " " + Kind +
> > +    OutputSection *Sec = Sections[I].Sec;
> > +    uint64_t Start = Sections[I].Offset;
> > +
> > +    for (size_t J = I + 1; J < Sections.size(); ++J) {
> > +      OutputSection *Other = Sections[J].Sec;
> > +      uint64_t OtherStart = Sections[J].Offset;
> > +      if (Start + Sec->Size <= OtherStart)
> > +        continue;
>
> The comment says break, and the old code did break. That is probably
> critical for the performance of this loop, no?


Yes, that's already fixed.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180329/5121532c/attachment-0001.html>