[lld] r240603 - COFF: Cache raw pointers to relocation tables.

[Rui Ueyama]

On Wed, Jun 24, 2015 at 7:42 PM, Sean Silva <chisophugis at gmail.com> wrote:

> +  mutable const coff_relocation *Reloc = nullptr;
>
> Do we really need `mutable const`? I have a feeling that will leave most
> people scratching their heads (at least it did for me). Also it might be
> useful to put `Cache` or `Cached` somewhere in the variable name.
>

Yeah, this variable was not very good. It's removed in a followup patch.


> -- Sean Silva
>
> On Wed, Jun 24, 2015 at 4:03 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Author: ruiu
>> Date: Wed Jun 24 18:03:17 2015
>> New Revision: 240603
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=240603&view=rev
>> Log:
>> COFF: Cache raw pointers to relocation tables.
>>
>> Getting an iterator to the relocation table is very hot operation
>> in the linker. We do that not only to apply relocations but also
>> to mark live sections and to do ICF.
>>
>> libObject's interface is slow. By caching pointers to the first
>> relocation table entries makes the linker 6% faster to self-link.
>>
>> We probably need to fix libObject as well.
>>
>> Modified:
>>     lld/trunk/COFF/Chunks.cpp
>>     lld/trunk/COFF/Chunks.h
>>
>> Modified: lld/trunk/COFF/Chunks.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.cpp?rev=240603&r1=240602&r2=240603&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/COFF/Chunks.cpp (original)
>> +++ lld/trunk/COFF/Chunks.cpp Wed Jun 24 18:03:17 2015
>> @@ -58,10 +58,8 @@ void SectionChunk::writeTo(uint8_t *Buf)
>>    memcpy(Buf + FileOff, Data.data(), Data.size());
>>
>>    // Apply relocations.
>> -  for (const auto &I : getSectionRef().relocations()) {
>> -    const coff_relocation *Rel =
>> File->getCOFFObj()->getCOFFRelocation(I);
>> +  for (const coff_relocation *Rel = relBegin(), *E = relEnd(); Rel != E;
>> ++Rel)
>>      applyReloc(Buf, Rel);
>> -  }
>>  }
>>
>>  void SectionChunk::mark() {
>> @@ -69,8 +67,8 @@ void SectionChunk::mark() {
>>    Live = true;
>>
>>    // Mark all symbols listed in the relocation table for this section.
>> -  for (const auto &I : getSectionRef().relocations()) {
>> -    const coff_relocation *Rel =
>> File->getCOFFObj()->getCOFFRelocation(I);
>> +  for (const coff_relocation *Rel = relBegin(), *E = relEnd(); Rel != E;
>> +       ++Rel) {
>>      SymbolBody *B = File->getSymbolBody(Rel->SymbolTableIndex);
>>      if (auto *Def = dyn_cast<Defined>(B))
>>        Def->markLive();
>> @@ -120,11 +118,11 @@ void SectionChunk::applyReloc(uint8_t *B
>>  // which need to be fixed by the loader if load-time relocation is
>> needed.
>>  // Only called when base relocation is enabled.
>>  void SectionChunk::getBaserels(std::vector<uint32_t> *Res, Defined
>> *ImageBase) {
>> -  for (const auto &I : getSectionRef().relocations()) {
>> +  for (const coff_relocation *Rel = relBegin(), *E = relEnd(); Rel != E;
>> +       ++Rel) {
>>      // ADDR64 relocations contain absolute addresses.
>>      // Symbol __ImageBase is special -- it's an absolute symbol, but its
>>      // address never changes even if image is relocated.
>> -    const coff_relocation *Rel =
>> File->getCOFFObj()->getCOFFRelocation(I);
>>      if (Rel->Type != IMAGE_REL_AMD64_ADDR64)
>>        continue;
>>      SymbolBody *Body = File->getSymbolBody(Rel->SymbolTableIndex);
>> @@ -156,12 +154,6 @@ void SectionChunk::printDiscardedMessage
>>    }
>>  }
>>
>> -SectionRef SectionChunk::getSectionRef() const {
>> -  DataRefImpl Ref;
>> -  Ref.p = uintptr_t(Header);
>> -  return SectionRef(Ref, File->getCOFFObj());
>> -}
>> -
>>  StringRef SectionChunk::getDebugName() {
>>    return Sym->getName();
>>  }
>> @@ -196,12 +188,10 @@ bool SectionChunk::equals(const SectionC
>>      return false;
>>
>>    // Compare relocations
>> -  auto Range1 = getSectionRef().relocations();
>> -  auto Range2 = X->getSectionRef().relocations();
>> -  auto End = Range1.end();
>> -  for (auto I = Range1.begin(), J = Range2.begin(); I != End; ++I, ++J) {
>> -    const coff_relocation *Rel1 =
>> File->getCOFFObj()->getCOFFRelocation(*I);
>> -    const coff_relocation *Rel2 =
>> X->File->getCOFFObj()->getCOFFRelocation(*J);
>> +  const coff_relocation *Rel1 = relBegin();
>> +  const coff_relocation *End = relEnd();
>> +  const coff_relocation *Rel2 = X->relBegin();
>> +  for (; Rel1 != End; ++Rel1, ++Rel2) {
>>      if (Rel1->Type != Rel2->Type)
>>        return false;
>>      if (Rel1->VirtualAddress != Rel2->VirtualAddress)
>> @@ -230,6 +220,22 @@ void SectionChunk::replaceWith(SectionCh
>>    Live = false;
>>  }
>>
>> +const coff_relocation *SectionChunk::relBegin() const {
>> +  if (!Reloc) {
>> +    DataRefImpl Ref;
>> +    Ref.p = uintptr_t(Header);
>> +    SectionRef Sref(Ref, File->getCOFFObj());
>> +    relocation_iterator It = Sref.relocation_begin();
>> +    Reloc = File->getCOFFObj()->getCOFFRelocation(*It);
>> +  }
>> +  return Reloc;
>> +}
>> +
>> +const coff_relocation *SectionChunk::relEnd() const {
>> +  const coff_relocation *I = relBegin();
>> +  return I + Header->NumberOfRelocations;
>> +}
>> +
>>  CommonChunk::CommonChunk(const COFFSymbolRef S) : Sym(S) {
>>    // Common symbols are aligned on natural boundaries up to 32 bytes.
>>    // This is what MSVC link.exe does.
>>
>> Modified: lld/trunk/COFF/Chunks.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=240603&r1=240602&r2=240603&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/COFF/Chunks.h (original)
>> +++ lld/trunk/COFF/Chunks.h Wed Jun 24 18:03:17 2015
>> @@ -149,12 +149,16 @@ public:
>>
>>  private:
>>    void mark() override;
>> -  SectionRef getSectionRef() const;
>>    void applyReloc(uint8_t *Buf, const coff_relocation *Rel);
>>
>>    // A file this chunk was created from.
>>    ObjectFile *File;
>>
>> +  // A raw pointer to the relocation table.
>> +  mutable const coff_relocation *Reloc = nullptr;
>> +  const coff_relocation *relBegin() const;
>> +  const coff_relocation *relEnd() const;
>> +
>>    // A pointer pointing to a replacement for this chunk.
>>    // Initially it points to "this" object. If this chunk is merged
>>    // with other chunk by ICF, it points to another chunk,
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150624/20393dff/attachment.html>