[PATCH] D15025: [ThinLTO] Option to invoke ThinLTO backend passes and importing

[Teresa Johnson via cfe-commits]

tejohnson marked 4 inline comments as done.
tejohnson added a comment.

New patch coming I think addresses all your comments.
Thanks, Teresa


================
Comment at: lib/CodeGen/BackendUtil.cpp:290
@@ +289,3 @@
+  // setup for LTO compiles invoked via the gold plugin and the llvm-lto tool.
+  if (!CodeGenOpts.ThinLTOIndexFile.empty() && !CodeGenOpts.DisableLLVMOpts) {
+    legacy::PassManager *MPM = getPerModulePasses();
----------------
joker.eph wrote:
> The `!CodeGenOpts.DisableLLVMOpts` part is unclear to me. This may lead to unexpected user experience?
I'm not sure what the effect of DisableLLVMOpts should be on importing. With the changes to the latest patch I will upload shortly that moves this down and uses the same PM setup as the below code, we will still perform importing, but the optimization level is turned down to 0. That is probably reasonable, I think.

================
Comment at: lib/CodeGen/CodeGenAction.cpp:824
@@ +823,3 @@
+
+      TheModule = std::move(Combined);
+      // Stash on the module object so it can be used by the function
----------------
joker.eph wrote:
> Side note: I found terrible that we can't do the renaming/linkage upgrade in-place :(
> 
This is an unfortunate effect of the currently module linker code, assuming that I want to use the same renaming/promotion support. I suppose I can add some support for doing this in place on the module, it was convenient to just reuse the support I put in the module linker initially.


http://reviews.llvm.org/D15025