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

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 29 14:17:05 PDT 2018


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.

> +static void checkOverlap(StringRef Name, std::vector<SectionOffset> &Sections) {

Could this take an ArrayRef<SectionOffset>?

> +  // 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?

Cheers,
Rafael


More information about the llvm-commits mailing list