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