[lld] r303787 - Simplify a variable type by using StringRef instead of CachedHashStringRef.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Wed May 24 16:07:06 PDT 2017


I tried to link clang and run "perf stat -r5" a few times. Feel free to
revert if the performance degradation seems real.

On Wed, May 24, 2017 at 3:54 PM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> Hi,
>
> Unfortunately the results I get are the opposite. The largest slowdown
> is in firefox with -O0 (2.5%). That is somewhat expected, as the
> basics take a larger part of the link at -O0. I also don't see any
> case where it is faster.
>
> What testcase have you used? If you can share the reproduce.tar I can
> add it to the set I normally use.
>
> Cheers,
> Rafael
>
>
> On 24 May 2017 at 11:22, Rui Ueyama via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: ruiu
> > Date: Wed May 24 13:22:27 2017
> > New Revision: 303787
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=303787&view=rev
> > Log:
> > Simplify a variable type by using StringRef instead of
> CachedHashStringRef.
> >
> > A variable `ComdatGroup` is not supposed to contain a large number of
> > items. Even when linking clang, it ends up having only 300K strings.
> > It doesn't make sense to use CachedHashStringRef for this hash table.
> > This patch has neutral or slightly positive impact on performance while
> > reducing code complexity.
> >
> > Modified:
> >     lld/trunk/ELF/InputFiles.cpp
> >     lld/trunk/ELF/InputFiles.h
> >     lld/trunk/ELF/SymbolTable.cpp
> >     lld/trunk/ELF/SymbolTable.h
> >
> > Modified: lld/trunk/ELF/InputFiles.cpp
> > URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> InputFiles.cpp?rev=303787&r1=303786&r2=303787&view=diff
> > ============================================================
> ==================
> > --- lld/trunk/ELF/InputFiles.cpp (original)
> > +++ lld/trunk/ELF/InputFiles.cpp Wed May 24 13:22:27 2017
> > @@ -192,7 +192,7 @@ ArrayRef<SymbolBody *> elf::ObjectFile<E
> >  }
> >
> >  template <class ELFT>
> > -void elf::ObjectFile<ELFT>::parse(DenseSet<CachedHashStringRef>
> &ComdatGroups) {
> > +void elf::ObjectFile<ELFT>::parse(DenseSet<StringRef> &ComdatGroups) {
> >    // Read section and symbol tables.
> >    initializeSections(ComdatGroups);
> >    initializeSymbols();
> > @@ -280,7 +280,7 @@ bool elf::ObjectFile<ELFT>::shouldMerge(
> >
> >  template <class ELFT>
> >  void elf::ObjectFile<ELFT>::initializeSections(
> > -    DenseSet<CachedHashStringRef> &ComdatGroups) {
> > +    DenseSet<StringRef> &ComdatGroups) {
> >    ArrayRef<Elf_Shdr> ObjSections =
> >        check(this->getObj().sections(), toString(this));
> >    const ELFFile<ELFT> &Obj = this->getObj();
> > @@ -305,10 +305,7 @@ void elf::ObjectFile<ELFT>::initializeSe
> >      switch (Sec.sh_type) {
> >      case SHT_GROUP:
> >        this->Sections[I] = &InputSection::Discarded;
> > -      if (ComdatGroups
> > -              .insert(
> > -                  CachedHashStringRef(getShtGroupSignature(ObjSections,
> Sec)))
> > -              .second)
> > +      if (ComdatGroups.insert(getShtGroupSignature(ObjSections,
> Sec)).second)
> >          continue;
> >        for (uint32_t SecIndex : getShtGroupEntries(Sec)) {
> >          if (SecIndex >= Size)
> > @@ -876,10 +873,10 @@ static Symbol *createBitcodeSymbol(const
> >  }
> >
> >  template <class ELFT>
> > -void BitcodeFile::parse(DenseSet<CachedHashStringRef> &ComdatGroups) {
> > +void BitcodeFile::parse(DenseSet<StringRef> &ComdatGroups) {
> >    std::vector<bool> KeptComdats;
> >    for (StringRef S : Obj->getComdatTable())
> > -    KeptComdats.push_back(ComdatGroups.insert(CachedHashStringRef(S)).
> second);
> > +    KeptComdats.push_back(ComdatGroups.insert(StringRef(S)).second);
> >
> >    for (const lto::InputFile::Symbol &ObjSym : Obj->symbols())
> >      Symbols.push_back(createBitcodeSymbol<ELFT>(KeptComdats, ObjSym,
> this));
> > @@ -1048,10 +1045,10 @@ template void ArchiveFile::parse<ELF32BE
> >  template void ArchiveFile::parse<ELF64LE>();
> >  template void ArchiveFile::parse<ELF64BE>();
> >
> > -template void BitcodeFile::parse<ELF32LE>(DenseSet<CachedHashStringRef>
> &);
> > -template void BitcodeFile::parse<ELF32BE>(DenseSet<CachedHashStringRef>
> &);
> > -template void BitcodeFile::parse<ELF64LE>(DenseSet<CachedHashStringRef>
> &);
> > -template void BitcodeFile::parse<ELF64BE>(DenseSet<CachedHashStringRef>
> &);
> > +template void BitcodeFile::parse<ELF32LE>(DenseSet<StringRef> &);
> > +template void BitcodeFile::parse<ELF32BE>(DenseSet<StringRef> &);
> > +template void BitcodeFile::parse<ELF64LE>(DenseSet<StringRef> &);
> > +template void BitcodeFile::parse<ELF64BE>(DenseSet<StringRef> &);
> >
> >  template void LazyObjectFile::parse<ELF32LE>();
> >  template void LazyObjectFile::parse<ELF32BE>();
> >
> > Modified: lld/trunk/ELF/InputFiles.h
> > URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> InputFiles.h?rev=303787&r1=303786&r2=303787&view=diff
> > ============================================================
> ==================
> > --- lld/trunk/ELF/InputFiles.h (original)
> > +++ lld/trunk/ELF/InputFiles.h Wed May 24 13:22:27 2017
> > @@ -17,7 +17,6 @@
> >
> >  #include "lld/Core/LLVM.h"
> >  #include "lld/Core/Reproduce.h"
> > -#include "llvm/ADT/CachedHashString.h"
> >  #include "llvm/ADT/DenseSet.h"
> >  #include "llvm/ADT/STLExtras.h"
> >  #include "llvm/IR/Comdat.h"
> > @@ -157,7 +156,7 @@ public:
> >    ArrayRef<SymbolBody *> getLocalSymbols();
> >
> >    ObjectFile(MemoryBufferRef M, StringRef ArchiveName);
> > -  void parse(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
> > +  void parse(llvm::DenseSet<StringRef> &ComdatGroups);
> >
> >    InputSectionBase *getSection(const Elf_Sym &Sym) const;
> >
> > @@ -189,8 +188,7 @@ public:
> >    StringRef SourceFile;
> >
> >  private:
> > -  void
> > -  initializeSections(llvm::DenseSet<llvm::CachedHashStringRef>
> &ComdatGroups);
> > +  void initializeSections(llvm::DenseSet<StringRef> &ComdatGroups);
> >    void initializeSymbols();
> >    void initializeDwarfLine();
> >    InputSectionBase *getRelocTarget(const Elf_Shdr &Sec);
> > @@ -265,8 +263,7 @@ public:
> >    BitcodeFile(MemoryBufferRef M, StringRef ArchiveName,
> >                uint64_t OffsetInArchive);
> >    static bool classof(const InputFile *F) { return F->kind() ==
> BitcodeKind; }
> > -  template <class ELFT>
> > -  void parse(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
> > +  template <class ELFT> void parse(llvm::DenseSet<StringRef>
> &ComdatGroups);
> >    ArrayRef<Symbol *> getSymbols() { return Symbols; }
> >    std::unique_ptr<llvm::lto::InputFile> Obj;
> >
> >
> > Modified: lld/trunk/ELF/SymbolTable.cpp
> > URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> SymbolTable.cpp?rev=303787&r1=303786&r2=303787&view=diff
> > ============================================================
> ==================
> > --- lld/trunk/ELF/SymbolTable.cpp (original)
> > +++ lld/trunk/ELF/SymbolTable.cpp Wed May 24 13:22:27 2017
> > @@ -122,7 +122,7 @@ template <class ELFT> void SymbolTable<E
> >
> >    for (InputFile *File : LTO->compile()) {
> >      ObjectFile<ELFT> *Obj = cast<ObjectFile<ELFT>>(File);
> > -    DenseSet<CachedHashStringRef> DummyGroups;
> > +    DenseSet<StringRef> DummyGroups;
> >      Obj->parse(DummyGroups);
> >      ObjectFiles.push_back(Obj);
> >    }
> >
> > Modified: lld/trunk/ELF/SymbolTable.h
> > URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> SymbolTable.h?rev=303787&r1=303786&r2=303787&view=diff
> > ============================================================
> ==================
> > --- lld/trunk/ELF/SymbolTable.h (original)
> > +++ lld/trunk/ELF/SymbolTable.h Wed May 24 13:22:27 2017
> > @@ -117,7 +117,7 @@ private:
> >    // Comdat groups define "link once" sections. If two comdat groups
> have the
> >    // same name, only one of them is linked, and the other is ignored.
> This set
> >    // is used to uniquify them.
> > -  llvm::DenseSet<llvm::CachedHashStringRef> ComdatGroups;
> > +  llvm::DenseSet<StringRef> ComdatGroups;
> >
> >    std::vector<ObjectFile<ELFT> *> ObjectFiles;
> >    std::vector<SharedFile<ELFT> *> SharedFiles;
> >
> >
> > _______________________________________________
> > 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/20170524/6f958cd2/attachment.html>


More information about the llvm-commits mailing list