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

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

Thanks for working on this. Saw the follow up discussion about whether it is too early, but I had a couple comments that I thought I would share now regardless. Overall looks good and pretty straightforward.


================
Comment at: ELF/InputFiles.cpp:504
@@ -500,1 +503,3 @@
+  if (ThinLto)
+    Body->setUsedInRegularObj();
   // FIXME: Expose a thread-local flag for module asm symbols.
----------------
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.

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

================
Comment at: ELF/LTO.cpp:251
@@ +250,3 @@
+std::vector<std::unique_ptr<InputFile>> BitcodeCompiler::compileThinLto() {
+  llvm::ThreadPool Pool(Config->Threads ? std::thread::hardware_concurrency()
+                                        : 1);
----------------
Maybe exit early above here if there are no ThinModules, so that the ThreadPool setup isn't done unnecessarily.


http://reviews.llvm.org/D19351