<div dir="ltr">Thanks for cleaning this up!<div><br></div><div>-- Sean Silva</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 11, 2016 at 8:11 AM, Rafael Espindola via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Fri Mar 11 10:11:47 2016<br>
New Revision: 263248<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=263248&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=263248&view=rev</a><br>
Log:<br>
Avoid calling getNamedValue.<br>
<br>
In lld we usually avoid hash lookups. In addition to that, IR names are<br>
not fully mangled, so it is best to avoid using them whenever possible.<br>
<br>
Modified:<br>
    lld/trunk/ELF/InputFiles.cpp<br>
    lld/trunk/ELF/InputFiles.h<br>
    lld/trunk/ELF/SymbolTable.cpp<br>
<br>
Modified: lld/trunk/ELF/InputFiles.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=263248&r1=263247&r2=263248&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=263248&r1=263247&r2=263248&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/InputFiles.cpp (original)<br>
+++ lld/trunk/ELF/InputFiles.cpp Fri Mar 11 10:11:47 2016<br>
@@ -436,6 +436,50 @@ static uint8_t getGvVisibility(const Glo<br>
   llvm_unreachable("Unknown visibility");<br>
 }<br>
<br>
+SymbolBody *<br>
+BitcodeFile::createSymbolBody(const DenseSet<const Comdat *> &KeptComdats,<br>
+                              const IRObjectFile &Obj,<br>
+                              const BasicSymbolRef &Sym) {<br>
+  const GlobalValue *GV = Obj.getSymbolGV(Sym.getRawDataRefImpl());<br>
+  assert(GV);<br>
+  if (const Comdat *C = GV->getComdat())<br>
+    if (!KeptComdats.count(C))<br>
+      return nullptr;<br>
+<br>
+  uint8_t Visibility = getGvVisibility(GV);<br>
+<br>
+  SmallString<64> Name;<br>
+  raw_svector_ostream OS(Name);<br>
+  Sym.printName(OS);<br>
+  StringRef NameRef = Saver.save(StringRef(Name));<br>
+<br>
+  const Module &M = Obj.getModule();<br>
+  SymbolBody *Body;<br>
+  uint32_t Flags = Sym.getFlags();<br>
+  bool IsWeak = Flags & BasicSymbolRef::SF_Weak;<br>
+  if (Flags & BasicSymbolRef::SF_Undefined) {<br>
+    Body = new (Alloc) Undefined(NameRef, IsWeak, Visibility, false);<br>
+  } else if (Flags & BasicSymbolRef::SF_Common) {<br>
+    const DataLayout &DL = M.getDataLayout();<br>
+    uint64_t Size = DL.getTypeAllocSize(GV->getValueType());<br>
+    Body = new (Alloc)<br>
+        DefinedCommon(NameRef, Size, GV->getAlignment(), IsWeak, Visibility);<br>
+  } else {<br>
+    Body = new (Alloc) DefinedBitcode(NameRef, IsWeak, Visibility);<br>
+  }<br>
+  Body->IsTls = GV->isThreadLocal();<br>
+  return Body;<br>
+}<br>
+<br>
+bool BitcodeFile::shouldSkip(const BasicSymbolRef &Sym) {<br>
+  uint32_t Flags = Sym.getFlags();<br>
+  if (!(Flags & BasicSymbolRef::SF_Global))<br>
+    return true;<br>
+  if (Flags & BasicSymbolRef::SF_FormatSpecific)<br>
+    return true;<br>
+  return false;<br>
+}<br>
+<br>
 void BitcodeFile::parse(DenseSet<StringRef> &ComdatGroups) {<br>
   LLVMContext Context;<br>
   std::unique_ptr<IRObjectFile> Obj = check(IRObjectFile::create(MB, Context));<br>
@@ -448,43 +492,9 @@ void BitcodeFile::parse(DenseSet<StringR<br>
       KeptComdats.insert(&P.second);<br>
   }<br>
<br>
-  for (const BasicSymbolRef &Sym : Obj->symbols()) {<br>
-    const GlobalValue *GV = Obj->getSymbolGV(Sym.getRawDataRefImpl());<br>
-    assert(GV);<br>
-    uint32_t Flags = Sym.getFlags();<br>
-    if (const Comdat *C = GV->getComdat())<br>
-      if (!KeptComdats.count(C))<br>
-        continue;<br>
-    if (!(Flags & BasicSymbolRef::SF_Global))<br>
-        continue;<br>
-    if (GV->hasAppendingLinkage()) {<br>
-      ExtraKeeps.push_back(GV->getName().copy(Alloc));<br>
-      continue;<br>
-    }<br>
-    if (Flags & BasicSymbolRef::SF_FormatSpecific)<br>
-      continue;<br>
-    uint8_t Visibility = getGvVisibility(GV);<br>
-<br>
-    SmallString<64> Name;<br>
-    raw_svector_ostream OS(Name);<br>
-    Sym.printName(OS);<br>
-    StringRef NameRef = Saver.save(StringRef(Name));<br>
-<br>
-    SymbolBody *Body;<br>
-    bool IsWeak = Flags & BasicSymbolRef::SF_Weak;<br>
-    if (Flags & BasicSymbolRef::SF_Undefined) {<br>
-      Body = new (Alloc) Undefined(NameRef, IsWeak, Visibility, false);<br>
-    } else if (Flags & BasicSymbolRef::SF_Common) {<br>
-      const DataLayout &DL = M.getDataLayout();<br>
-      uint64_t Size = DL.getTypeAllocSize(GV->getValueType());<br>
-      Body = new (Alloc)<br>
-          DefinedCommon(NameRef, Size, GV->getAlignment(), IsWeak, Visibility);<br>
-    } else {<br>
-      Body = new (Alloc) DefinedBitcode(NameRef, IsWeak, Visibility);<br>
-    }<br>
-    Body->IsTls = GV->isThreadLocal();<br>
-    SymbolBodies.push_back(Body);<br>
-  }<br>
+  for (const BasicSymbolRef &Sym : Obj->symbols())<br>
+    if (!shouldSkip(Sym))<br>
+      SymbolBodies.push_back(createSymbolBody(KeptComdats, *Obj, Sym));<br>
 }<br>
<br>
 template <typename T><br>
<br>
Modified: lld/trunk/ELF/InputFiles.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.h?rev=263248&r1=263247&r2=263248&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.h?rev=263248&r1=263247&r2=263248&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/InputFiles.h (original)<br>
+++ lld/trunk/ELF/InputFiles.h Fri Mar 11 10:11:47 2016<br>
@@ -18,8 +18,10 @@<br>
 #include "lld/Core/LLVM.h"<br>
 #include "llvm/ADT/DenseSet.h"<br>
 #include "llvm/ADT/STLExtras.h"<br>
+#include "llvm/IR/Comdat.h"<br>
 #include "llvm/Object/Archive.h"<br>
 #include "llvm/Object/ELF.h"<br>
+#include "llvm/Object/IRObjectFile.h"<br>
 #include "llvm/Support/StringSaver.h"<br>
<br>
 namespace lld {<br>
@@ -180,19 +182,16 @@ public:<br>
   static bool classof(const InputFile *F);<br>
   void parse(llvm::DenseSet<StringRef> &ComdatGroups);<br>
   ArrayRef<SymbolBody *> getSymbols() { return SymbolBodies; }<br>
-  ArrayRef<StringRef> getExtraKeeps() { return ExtraKeeps; }<br>
+  static bool shouldSkip(const llvm::object::BasicSymbolRef &Sym);<br>
<br>
 private:<br>
   std::vector<SymbolBody *> SymbolBodies;<br>
-  // Some symbols like llvm.global_ctors are internal to the IR and so<br>
-  // don't show up in SymbolBodies, but must be kept when creating the<br>
-  // combined LTO module. We track them here.<br>
-  // We currently use a different Module for creating SymbolBody's vs when<br>
-  // we are creating the combined LTO module, and so we can't store IR<br>
-  // pointers directly and must rely on the IR names.<br>
-  std::vector<StringRef> ExtraKeeps;<br>
   llvm::BumpPtrAllocator Alloc;<br>
   llvm::StringSaver Saver{Alloc};<br>
+  SymbolBody *<br>
+  createSymbolBody(const llvm::DenseSet<const llvm::Comdat *> &KeptComdats,<br>
+                   const llvm::object::IRObjectFile &Obj,<br>
+                   const llvm::object::BasicSymbolRef &Sym);<br>
 };<br>
<br>
 // .so file.<br>
<br>
Modified: lld/trunk/ELF/SymbolTable.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=263248&r1=263247&r2=263248&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=263248&r1=263247&r2=263248&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/SymbolTable.cpp (original)<br>
+++ lld/trunk/ELF/SymbolTable.cpp Fri Mar 11 10:11:47 2016<br>
@@ -84,7 +84,8 @@ void SymbolTable<ELFT>::addFile(std::uni<br>
     BitcodeFiles.emplace_back(cast<BitcodeFile>(File.release()));<br>
     F->parse(ComdatGroups);<br>
     for (SymbolBody *B : F->getSymbols())<br>
-      resolve(B);<br>
+      if (B)<br>
+        resolve(B);<br>
     return;<br>
   }<br>
<br>
@@ -139,28 +140,33 @@ std::unique_ptr<InputFile> SymbolTable<E<br>
<br>
 static void addBitcodeFile(IRMover &Mover, BitcodeFile &F,<br>
                            LLVMContext &Context) {<br>
-  std::unique_ptr<MemoryBuffer> Buffer =<br>
-      MemoryBuffer::getMemBuffer(F.MB, false);<br>
-  std::unique_ptr<Module> M =<br>
-      check(getLazyBitcodeModule(std::move(Buffer), Context,<br>
-                                 /*ShouldLazyLoadMetadata*/ false));<br>
+<br>
+  std::unique_ptr<IRObjectFile> Obj =<br>
+      check(IRObjectFile::create(F.MB, Context));<br>
   std::vector<GlobalValue *> Keep;<br>
-  for (SymbolBody *B : F.getSymbols()) {<br>
-    if (&B->repl() != B)<br>
+  unsigned BodyIndex = 0;<br>
+  ArrayRef<SymbolBody *> Bodies = F.getSymbols();<br>
+<br>
+  for (const BasicSymbolRef &Sym : Obj->symbols()) {<br>
+    GlobalValue *GV = Obj->getSymbolGV(Sym.getRawDataRefImpl());<br>
+    assert(GV);<br>
+    if (GV->hasAppendingLinkage()) {<br>
+      Keep.push_back(GV);<br>
+      continue;<br>
+    }<br>
+    if (BitcodeFile::shouldSkip(Sym))<br>
+      continue;<br>
+    SymbolBody *B = Bodies[BodyIndex++];<br>
+    if (!B || &B->repl() != B)<br>
       continue;<br>
     auto *DB = dyn_cast<DefinedBitcode>(B);<br>
     if (!DB)<br>
       continue;<br>
-    GlobalValue *GV = M->getNamedValue(B->getName());<br>
-    assert(GV);<br>
     Keep.push_back(GV);<br>
   }<br>
-  for (StringRef S : F.getExtraKeeps()) {<br>
-    GlobalValue *GV = M->getNamedValue(S);<br>
-    assert(GV);<br>
-    Keep.push_back(GV);<br>
-  }<br>
-  Mover.move(std::move(M), Keep, [](GlobalValue &, IRMover::ValueAdder) {});<br>
+<br>
+  Mover.move(Obj->takeModule(), Keep,<br>
+             [](GlobalValue &, IRMover::ValueAdder) {});<br>
 }<br>
<br>
 // This is for use when debugging LTO.<br>
<br>
<br>
_______________________________________________<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/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>