<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Mar 29, 2018 at 2:17 PM Rafael Avila de Espindola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Rui Ueyama via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> writes:<br>
<br>
> -  // By removing all zero-size sections we can simplify the check for overlap to<br>
> -  // just checking whether the section range contains the other section's start<br>
> -  // address. Additionally, it also slightly speeds up the checking since we<br>
> -  // don't bother checking for overlap with sections that can never overlap.<br>
<br>
This comment is missing in the new code.<br></blockquote><div><br></div><div>I'll add them with some simplification. I don't think we need to mention marginal speedup.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +static void checkOverlap(StringRef Name, std::vector<SectionOffset> &Sections) {<br>
<br>
Could this take an ArrayRef<SectionOffset>?<br></blockquote><div><br></div><div>Unfortunately not. We sort Sections, so this must be mutable.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +  // Since the sections are sorted by start address we only need to check<br>
> +  // whether the other sections starts before the end of Sec. If this is<br>
> +  // not the case we can break out of this loop since all following sections<br>
> +  // will also start after the end of Sec.<br>
>    for (size_t I = 0; I < Sections.size(); ++I) {<br>
> -    OutputSection *Sec = Sections[I];<br>
> -    uint64_t Start = GetStart(Sec);<br>
> -    for (auto *Other : ArrayRef<OutputSection *>(Sections).slice(I + 1)) {<br>
> -      // Since the sections are sorted by start address we only need to check<br>
> -      // whether the other sections starts before the end of Sec. If this is<br>
> -      // not the case we can break out of this loop since all following sections<br>
> -      // will also start after the end of Sec.<br>
> -      if (Start + Sec->Size <= GetStart(Other))<br>
> -        break;<br>
> -      errorOrWarn("section " + Sec->Name + " " + Kind +<br>
> +    OutputSection *Sec = Sections[I].Sec;<br>
> +    uint64_t Start = Sections[I].Offset;<br>
> +<br>
> +    for (size_t J = I + 1; J < Sections.size(); ++J) {<br>
> +      OutputSection *Other = Sections[J].Sec;<br>
> +      uint64_t OtherStart = Sections[J].Offset;<br>
> +      if (Start + Sec->Size <= OtherStart)<br>
> +        continue;<br>
<br>
The comment says break, and the old code did break. That is probably<br>
critical for the performance of this loop, no?</blockquote><div><br></div><div>Yes, that's already fixed.</div></div></div>