[lld] r247963 - COFF: Optimize ICF by not creating temporary vectors.

[David Blaikie via llvm-commits]

On Thu, Sep 17, 2015 at 7:04 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Sep 17, 2015 at 6:51 PM, Rui Ueyama via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: ruiu
>> Date: Thu Sep 17 20:51:37 2015
>> New Revision: 247963
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=247963&view=rev
>> Log:
>> COFF: Optimize ICF by not creating temporary vectors.
>>
>> Previously, ICF created a vector for each SectionChunk. The vector
>> contained pointers to successors, which are namely associative sections
>> and COMDAT relocation targets. The reason I created vectors is because
>> I thought that that would make section comparison faster.
>>
>> It did make the comparison faster. When self-linking, for example, it
>> saved about 10 ms on each iteration.
>
>
> (haven't looked at the algorithm, but just wondering) is there any
> possibility of keeping them up-to-date cheaply, rather than constructing
> them on every iteration?
>

Oh, nevermind - I misunderstood the numbers. The vectors are already a
one-time-cost, not a cost-per-iteration. My question didn't make any sense.


>
>
>> The time we spent on constructing
>> the vectors was 124 ms. If we iterate more than 12 times, return from
>> the investment exceeds the initial cost.
>>
>> In reality, it usually needs 5 iterations. So we shouldn't construct
>> the vectors.
>>
>> Modified:
>>     lld/trunk/COFF/Chunks.h
>>     lld/trunk/COFF/ICF.cpp
>>
>> Modified: lld/trunk/COFF/Chunks.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=247963&r1=247962&r2=247963&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/COFF/Chunks.h (original)
>> +++ lld/trunk/COFF/Chunks.h Thu Sep 17 20:51:37 2015
>> @@ -203,7 +203,6 @@ private:
>>
>>    // Used for ICF (Identical COMDAT Folding)
>>    void replaceWith(SectionChunk *Other);
>> -  std::vector<SectionChunk *> Successors;
>>    uint64_t GroupID;
>>
>>    // Chunks are basically unnamed chunks of bytes.
>>
>> Modified: lld/trunk/COFF/ICF.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/ICF.cpp?rev=247963&r1=247962&r2=247963&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/COFF/ICF.cpp (original)
>> +++ lld/trunk/COFF/ICF.cpp Thu Sep 17 20:51:37 2015
>> @@ -128,12 +128,22 @@ bool ICF::equalsConstant(const SectionCh
>>  }
>>
>>  bool ICF::equalsVariable(const SectionChunk *A, const SectionChunk *B) {
>> -  assert(A->Successors.size() == B->Successors.size());
>> -  return std::equal(A->Successors.begin(), A->Successors.end(),
>> -                    B->Successors.begin(),
>> -                    [](const SectionChunk *X, const SectionChunk *Y) {
>> -                      return X->GroupID == Y->GroupID;
>> -                    });
>> +  // Compare associative sections.
>> +  for (size_t I = 0, E = A->AssocChildren.size(); I != E; ++I)
>> +    if (A->AssocChildren[I]->GroupID != B->AssocChildren[I]->GroupID)
>> +      return false;
>> +
>> +  // Compare relocations.
>> +  auto Eq = [&](const coff_relocation &R1, const coff_relocation &R2) {
>> +    SymbolBody *B1 = A->File->getSymbolBody(R1.SymbolTableIndex)->repl();
>> +    SymbolBody *B2 = B->File->getSymbolBody(R2.SymbolTableIndex)->repl();
>> +    if (B1 == B2)
>> +      return true;
>> +    auto *D1 = dyn_cast<DefinedRegular>(B1);
>> +    auto *D2 = dyn_cast<DefinedRegular>(B2);
>> +    return D1 && D2 && D1->getChunk()->GroupID ==
>> D2->getChunk()->GroupID;
>> +  };
>> +  return std::equal(A->Relocs.begin(), A->Relocs.end(),
>> B->Relocs.begin(), Eq);
>>  }
>>
>>  bool ICF::partition(ChunkIterator Begin, ChunkIterator End, Comparator
>> Eq) {
>> @@ -191,16 +201,6 @@ void ICF::run() {
>>                return A->GroupID < B->GroupID;
>>              });
>>
>> -  // Initialize chunk successors for equalsVariable.
>> -  for (SectionChunk *SC : SChunks) {
>> -    SC->Successors = SC->AssocChildren;
>> -    for (const coff_relocation &R : SC->Relocs) {
>> -      SymbolBody *B =
>> SC->File->getSymbolBody(R.SymbolTableIndex)->repl();
>> -      if (auto D = dyn_cast<DefinedRegular>(B))
>> -        SC->Successors.push_back(D->getChunk());
>> -    }
>> -  }
>> -
>>    // Split groups until we get a convergence.
>>    int Cnt = 1;
>>    forEachGroup(SChunks, equalsConstant);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150917/dfd12bbf/attachment.html>