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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
