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

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: ELF/Driver.cpp:22
@@ -21,2 +21,3 @@
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
----------------
ruiu wrote:
> Ditto.
Removed.

================
Comment at: ELF/Driver.h:17
@@ -16,2 +16,3 @@
 #include "llvm/ADT/StringRef.h"
+#include "llvm/IR/LLVMContext.h"
 #include "llvm/Option/ArgList.h"
----------------
ruiu wrote:
> Do you need this?
Yes. A definition of `LLVMContext` is required for the field `LinkerDriver::Context`. We were previously getting the definition from `LTO.h` via `SymbolTable.h`, until I removed the `#include` of `LTO.h` to `SymbolTable.cpp`.

================
Comment at: ELF/InputFiles.cpp:504
@@ -500,1 +503,3 @@
+  if (ThinLto)
+    Body->setUsedInRegularObj();
   // FIXME: Expose a thread-local flag for module asm symbols.
----------------
tejohnson wrote:
> The comment sounds like the opposite of what is being done here, but I am probably misunderstanding something...if ThinLto is true then this is a ThinLTO object not a regular LTO object, and it sounds like we are making it visible to regular LTO object files by setting this flag.
The idea with this comment was that if the regular LTO object defines a symbol, we should mark it as used if we see a ThinLTO object with an undefined reference to that symbol. But yes, it works the other way as well. I have moved this comment to `isRegularLtoInputFile` with a slightly better explanation.

================
Comment at: ELF/LTO.cpp:247
@@ -198,2 +246,3 @@
 
-  return runSplitCodegen(CreateTargetMachine);
+  return createObjectFile(MemoryBufferRef(Obj, "LLD-INTERNAL-thinlto-object"));
+}
----------------
tejohnson wrote:
> add SaveTemps handling?
We can most likely add that separately.

================
Comment at: ELF/SymbolTable.cpp:58-62
@@ -56,1 +57,7 @@
 
+template <class ELFT>
+SymbolTable<ELFT>::SymbolTable() {}
+
+template <class ELFT>
+SymbolTable<ELFT>::~SymbolTable() {}
+
----------------
Reformatted.


http://reviews.llvm.org/D19351