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

tejohnson updated this revision to Diff 42069. tejohnson added a comment. - Rename -fthinlto-backend to -fthinlto-index as per review and IRC. http://reviews.llvm.org/D15025 Files: include/clang/CodeGen/BackendUtil.h include/clang/Driver/Options.td include/clang/Driver/Types.h

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

joker.eph accepted this revision. joker.eph added a comment. This revision is now accepted and ready to land. LGTM, thanks! http://reviews.llvm.org/D15025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

tejohnson added a comment. > Talked with Duncan, we're not convinced about -fthinlto-backend=... for the > option name: the word backend does not seems right here. What about > -fthinlto-index=...? I wanted to express that this was invoking a different pipeline, and distinguish from a normal

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

> On Dec 4, 2015, at 9:14 AM, Teresa Johnson wrote: > > tejohnson added inline comments. > > > Comment at: lib/CodeGen/CodeGenAction.cpp:822 > @@ +821,3 @@ > + TheModule = std::move(Combined); > +} > + > > joker.eph wrote: >>

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

tejohnson added inline comments. Comment at: lib/CodeGen/CodeGenAction.cpp:822 @@ +821,3 @@ + TheModule = std::move(Combined); +} + joker.eph wrote: > tejohnson wrote: > > joker.eph wrote: > > > Could we refactor this in a helper in llvm? > > I can

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

joker.eph added inline comments. Comment at: lib/CodeGen/CodeGenAction.cpp:822 @@ +821,3 @@ + TheModule = std::move(Combined); +} + tejohnson wrote: > joker.eph wrote: > > tejohnson wrote: > > > joker.eph wrote: > > > > Could we refactor this in a helper

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

joker.eph added inline comments. Comment at: lib/CodeGen/CodeGenAction.cpp:822 @@ +821,3 @@ + TheModule = std::move(Combined); +} + tejohnson wrote: > joker.eph wrote: > > Could we refactor this in a helper in llvm? > I can create a new Linker interface

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

tejohnson updated this revision to Diff 41926. tejohnson added a comment. - Use new renameModuleForThinLTO helper function from llvm. - Fix tests I have not change the option name yet. Wanted to be sure we have a name that reflects the fact that this is a special compile that changes the

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

tejohnson updated this revision to Diff 41935. tejohnson added a comment. - Move ownership of FunctionInfoIndex onto EmitAssemblyHelper http://reviews.llvm.org/D15025 Files: include/clang/CodeGen/BackendUtil.h include/clang/Driver/Options.td include/clang/Driver/Types.h

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

joker.eph added a comment. Looks good to me but I'd like another opinion on the name of "-fthinlto-backend", Duncan? Comment at: include/clang/Driver/Types.h:68 @@ -66,1 +67,3 @@ + bool isLLVMIR(ID Id); + /// isCuda - Is this a CUDA input. I guess you

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

tejohnson updated this revision to Diff 41787. tejohnson added a comment. - Address more comments. http://reviews.llvm.org/D15025 Files: include/clang/CodeGen/BackendUtil.h include/clang/Driver/Options.td include/clang/Driver/Types.h include/clang/Frontend/CodeGenOptions.h

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

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. +

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

joker.eph added a comment. Talked with Duncan, we're not convinced about //-fthinlto-backend=...// for the option name: the word //backend// does not seems right here. What about //-fthinlto-index=...//? Comment at: lib/CodeGen/CodeGenAction.cpp:822 @@ +821,3 @@ +

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

joker.eph added inline comments. 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

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

tejohnson marked an inline comment as done. tejohnson added a comment. Updated patch coming shortly. Comment at: lib/CodeGen/BackendUtil.cpp:308 @@ +307,3 @@ +return; + } + joker.eph wrote: > It does not seem to be nicely integrated within the context of

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

tejohnson updated this revision to Diff 41542. tejohnson added a comment. - Address Mehdi's comments. http://reviews.llvm.org/D15025 Files: include/clang/CodeGen/CodeGenAction.h include/clang/Driver/Options.td include/clang/Driver/Types.h include/clang/Frontend/CodeGenOptions.h

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

tejohnson added a comment. Thanks for the review, comments below. Teresa Comment at: lib/CodeGen/CodeGenAction.cpp:190 @@ -169,3 +189,3 @@ [=](const DiagnosticInfo ) { - linkerDiagnosticHandler(DI, LinkModule); +

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

tejohnson created this revision. tejohnson added reviewers: joker.eph, dexonsmith. tejohnson added subscribers: cfe-commits, davidxl. tejohnson added a dependency: D15024: [ThinLTO] Support for specifying function index from pass manager. Herald added a subscriber: joker.eph. Adds new option

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

joker.eph added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:308 @@ +307,3 @@ +return; + } + It does not seem to be nicely integrated within the context of this function. What about all the options set just a few line below? It is not clear if