[PATCH] D19351: ELF: Add initial ThinLTO support.

[Rafael Ávila de Espíndola via llvm-commits]

rafael added inline comments.

================
Comment at: ELF/InputFiles.h:224
@@ -223,2 +223,3 @@
   std::unique_ptr<llvm::object::IRObjectFile> Obj;
+  bool ThinLto;
 
----------------
I would make the name a bit more descriptive. How about hasThinLtoSummary?

================
Comment at: ELF/LTO.cpp:27
@@ -25,1 +26,3 @@
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/thread.h"
 #include "llvm/Target/TargetMachine.h"
----------------
Just to make this easier to review, can the first version be single threaded?

================
Comment at: ELF/LTO.cpp:110
@@ +109,3 @@
+
+  Options = InitTargetOptionsFromCodeGenFlags();
+  TheTriple = getBitcodeTargetTriple(F.MB, Driver->Context);
----------------
Add a comment on why thin-lto requires initializing these bits early.

================
Comment at: ELF/LTO.cpp:127
@@ +126,3 @@
+// definition in the combined LTO object will replace it when parsed.
+static void undefine(Symbol *S) {
+  replaceBody<Undefined>(S, S->body()->getName(), STV_DEFAULT, 0);
----------------
Making this a static helper is a nice independent cleanup. Please commit and rebase.

================
Comment at: ELF/LTO.cpp:213
@@ +212,3 @@
+    auto *B = dyn_cast<DefinedBitcode>(S->body());
+    if (!B || B->File != &F)
+      continue;
----------------
This logic is mostly duplicated with the regular lto case. Can you add a helper that return S or null? It should basically return S when we need to call undefine.

================
Comment at: ELF/LTO.cpp:283
@@ +282,3 @@
+  legacy::PassManager CGPasses;
+  if (TM->addPassesToEmitFile(CGPasses, OS, TargetMachine::CGFT_ObjectFile))
+    report_fatal_error("Failed to setup codegen");
----------------
This is duplicated with what splitCodeGen does. We should populate the passes in one place.

================
Comment at: ELF/LTO.h:70
@@ -46,1 +69,3 @@
+  // Regular LTO
+  bool HasLto = false;
   std::unique_ptr<llvm::Module> Combined;
----------------
A more descriptive name would be nice.

================
Comment at: ELF/SymbolTable.cpp:59
@@ +58,3 @@
+template <class ELFT> SymbolTable<ELFT>::SymbolTable() {}
+template <class ELFT> SymbolTable<ELFT>::~SymbolTable() {}
+
----------------
Why?


http://reviews.llvm.org/D19351