Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-25 Thread Rong Xu via cfe-commits
Create a new review here: http://reviews.llvm.org/D17622 Thanks, -Rong On Wed, Feb 24, 2016 at 9:22 PM, Sean Silva wrote: > silvas added a comment. > > In http://reviews.llvm.org/D15829#360006, @xur wrote: > >> Here is the new patch that removes the auto detection of profile kind. >> >> In this

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-25 Thread Rong Xu via cfe-commits
xur added a comment. Create a new review here: http://reviews.llvm.org/D17622 Thanks, -Rong http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-24 Thread Sean Silva via cfe-commits
silvas added a comment. In http://reviews.llvm.org/D15829#360006, @xur wrote: > Here is the new patch that removes the auto detection of profile kind. > > In this patch, I replace the CC1 option of -fprofile-instr-use=<> with > -fprofile-instrument={llvm-use|clang-use}. For the use compilation,

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-24 Thread David Li via cfe-commits
davidxl added a comment. Looks good to me -- and it makes the profile-gen and profile-use's cc1 option handling consistent. Please check with Sean or Justin just in case before proceeding. http://reviews.llvm.org/D15829 ___ cfe-commits mailing li

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-23 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 48859. xur marked 2 inline comments as done. xur added a comment. Integrated with David's comments. Thanks, -Rong http://reviews.llvm.org/D15829 Files: include/clang/Driver/CC1Options.td include/clang/Driver/Options.td include/clang/Frontend/CodeGenOpti

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-23 Thread Rong Xu via cfe-commits
xur marked 4 inline comments as done. Comment at: lib/CodeGen/CodeGenModule.cpp:151 @@ -151,1 +150,3 @@ + if (CodeGenOpts.hasProfileClangUse() && + !CodeGenOpts.InstrProfileInput.empty()) { auto ReaderOrErr = davidxl wrote: > Is the second condition sti

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-23 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.h:83 @@ -82,2 +82,3 @@ enum ProfileInstrKind { ProfileNoInstr,// No instrumentation. +ProfileClangInstr, // Clang instrumentation to generate execution counts Maybe Pr

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-23 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 48846. xur marked an inline comment as done. xur added a comment. Here is the new patch that removes the auto detection of profile kind. In this patch, I replace the CC1 option of -fprofile-instr-use=<> with -fprofile-instrument={llvm-use|clang-use}. For the use

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-22 Thread Rong Xu via cfe-commits
xur marked an inline comment as done. xur added a comment. I'll send a revised patch soon. Comment at: lib/CodeGen/CodeGenModule.cpp:150 @@ -149,2 +149,3 @@ - if (!CodeGenOpts.InstrProfileInput.empty()) { + if (!CodeGenOpts.hasProfileIRInstr() && + !CodeGenOpts.InstrPro

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-18 Thread Sean Silva via cfe-commits
silvas added a comment. Apologies for the delay. At this point, I think that this patch has evolved enough that it is best to start a new patch. I think the steps forward are: - Have cc1 accept -fprofile-instrument=llvm (requires no driver changes, but is enough for developers to test by being

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-18 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:150 @@ -149,2 +149,3 @@ - if (!CodeGenOpts.InstrProfileInput.empty()) { + if (!CodeGenOpts.hasProfileIRInstr() && + !CodeGenOpts.InstrProfileInput.empty()) { Better to use if (CodeGe

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-09 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 47331. xur added a comment. Ping (also added the test that missed from last upload). Thanks, -Rong http://reviews.llvm.org/D15829 Files: include/clang/Driver/CC1Options.td include/clang/Frontend/CodeGenOptions.h lib/CodeGen/BackendUtil.cpp lib/CodeGe

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-08 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 47251. xur marked an inline comment as done. xur added a comment. fixed the typo in comments http://reviews.llvm.org/D15829 Files: include/clang/Driver/CC1Options.td include/clang/Frontend/CodeGenOptions.h lib/CodeGen/BackendUtil.cpp lib/CodeGen/CodeGen

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-05 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.h:234 @@ +233,3 @@ + + /// \brief Check if IR profile instrumenation is on. + bool hasProfileIRInstr() const { typo: instrumentation http://reviews.llvm.org/D15829

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-04 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 46960. xur added a comment. Now http://reviews.llvm.org/D16730 has been committed (as r259811) Here is the patch that adds cc1 option -fprofile-instrument=llvm to enable IR level PGO generate and use. The pgo use part of the patch depends http://reviews.llvm.or

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-03 Thread Xinliang David Li via cfe-commits
On Wed, Feb 3, 2016 at 12:40 PM, Bob Wilson wrote: > > On Feb 3, 2016, at 12:23 PM, Xinliang David Li wrote: > > On Tue, Feb 2, 2016 at 1:31 PM, Bob Wilson wrote: > > > On Jan 22, 2016, at 1:43 PM, Sean Silva via cfe-commits > wrote: > > silvas added a comment. > > In http://reviews.llvm.org/D1

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-03 Thread Bob Wilson via cfe-commits
> On Feb 3, 2016, at 12:23 PM, Xinliang David Li wrote: > > On Tue, Feb 2, 2016 at 1:31 PM, Bob Wilson > wrote: >> >>> On Jan 22, 2016, at 1:43 PM, Sean Silva via cfe-commits >>> wrote: >>> >>> silvas added a comment. >>> >>> In http://reviews.llvm.org/D15829#3

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-03 Thread Xinliang David Li via cfe-commits
On Tue, Feb 2, 2016 at 1:31 PM, Bob Wilson wrote: > >> On Jan 22, 2016, at 1:43 PM, Sean Silva via cfe-commits >> wrote: >> >> silvas added a comment. >> >> In http://reviews.llvm.org/D15829#333902, @davidxl wrote: >> >>> For the longer term, one possible solution is to make FE based >>> instrum

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-02-02 Thread Bob Wilson via cfe-commits
> On Jan 22, 2016, at 1:43 PM, Sean Silva via cfe-commits > wrote: > > silvas added a comment. > > In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > >> For the longer term, one possible solution is to make FE based >> instrumentation only used for coverage testing which can be turne

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-29 Thread Rong Xu via cfe-commits
xur added a comment. To make the review easier, I split the cc1 names change part into a new NFC patch here. http://reviews.llvm.org/D16730 http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-29 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 46389. xur added a comment. This new patch adds the change the clang test cases (cc1 option -fprofile-instr-generate to -fprofile-instrument=Clang). It also replaces cc1 option -fprofile-instr-generate= to -fprofile-instrument-path=, as suggested by David. Tha

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-28 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/Driver/Tools.cpp:3232 @@ -3231,2 +3231,3 @@ CmdArgs.push_back( Args.MakeArgString(Twine("-fprofile-instr-generate=") + Path)); +} I also suggest changing CC1 option -fprofile-instr-generate= to b

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-28 Thread Xinliang David Li via cfe-commits
On Thu, Jan 28, 2016 at 2:34 PM, Rong Xu wrote: > The previous patch was not good as it failed 58 tests that use > -fprofile-instr-generate as a cc1 option. > > I can see two methods to solve this: > (1) we change all the 58 tests to use -fprofile-instrument=Clang option. My vote is on (1) -- get

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-28 Thread Rong Xu via cfe-commits
xur added a comment. The previous patch was not good as it failed 58 tests that use -fprofile-instr-generate as a cc1 option. I can see two methods to solve this: (1) we change all the 58 tests to use -fprofile-instrument=Clang option. (2) Fall back to my previous patch: keep -fprofile-instr-gene

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-28 Thread Rong Xu via cfe-commits
The previous patch was not good as it failed 58 tests that use -fprofile-instr-generate as a cc1 option. I can see two methods to solve this: (1) we change all the 58 tests to use -fprofile-instrument=Clang option. (2) Fall back to my previous patch: keep -fprofile-instr-generate as the CC1 option

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-28 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 46303. xur added a comment. Integrated most recently comments/suggestions from David and Sean. Thanks, -Rong http://reviews.llvm.org/D15829 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Driver/CC1Options.td include/clang/Driver/Optio

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-28 Thread Rong Xu via cfe-commits
xur added a comment. I'll send an updated patch shortly. Comment at: include/clang/Frontend/CodeGenOptions.def:106 @@ -105,3 +105,3 @@ -CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate -///< execution counts to us

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread Sean Silva via cfe-commits
silvas added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.def:106 @@ -105,3 +105,3 @@ -CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate -///< execution counts to use with PGO. +CODEGENOPT(Profi

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread Sean Silva via cfe-commits
silvas added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:483 @@ +482,3 @@ +Opts.ProfileIRInstr = true; + else +// Opts.ClangProfInstrGen will be used for Clang instrumentation only. davidxl wrote: > silvas wrote: > > This still isn't

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.def:106 @@ -105,3 +105,3 @@ -CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate -///< execution counts to use with PGO. +CODEGENOPT(Prof

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 46199. xur added a comment. This new version implemented the usage model David proposed. We now have a cc1 option -fprofile-instrumentor={llvm | clang} to choose which instrumentation to use. The slight difference from David's proposal is that we can use this op

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread Xinliang David Li via cfe-commits
We first need to nail the use model of the option, and then talk about implementation. To clarify, I think the following should be the way: (assuming the current clang instrumetnation is the default): 1) To turn on clang based instrumentation: -fprofile-instr-generate[=] 2) To turn on clang bas

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread Rong Xu via cfe-commits
On Wed, Jan 27, 2016 at 11:31 AM, David Li wrote: > davidxl added inline comments. > > > Comment at: lib/Driver/Tools.cpp:5520 > @@ +5519,3 @@ > +A->claim(); > +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && > +(Args.hasArg(options::OPT_fprofile_instr_g

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/Driver/Tools.cpp:5520 @@ +5519,3 @@ +A->claim(); +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && +(Args.hasArg(options::OPT_fprofile_instr_generate) || silvas wrote: > This is definitely no

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread Rong Xu via cfe-commits
xur added a comment. Sean: Adding a new CC1 option ProfileClangInstr will make things cleaner. But this won't solve the problem. The root of all the mess is there is no driver level option for IR instrumentation. I need to either "hijack" the -Xclang option, or move the logic to CompilerInvocation

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-27 Thread Rong Xu via cfe-commits
Sean: Adding a new CC1 option ProfileClangInstr will make things cleaner. But this won't solve the problem. The root of all the mess is there is no driver level option for IR instrumentation. I need to either "hijack" the -Xclang option, or move the logic to CompilerInvocation.cpp, which both you d

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-26 Thread Sean Silva via cfe-commits
silvas added inline comments. Comment at: lib/Driver/Tools.cpp:5520 @@ +5519,3 @@ +A->claim(); +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && +(Args.hasArg(options::OPT_fprofile_instr_generate) || This is definitely not the right thing

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 45929. xur marked an inline comment as done. xur added a comment. Here is the new patch. Changes from previous patch are: (1) rename codegen option ProfileInstrGenerate to ClangProfInstrGen to avoid the name confusion (Davidxl). This option is for clang instrumen

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread Sean Silva via cfe-commits
On Mon, Jan 25, 2016 at 11:00 AM, Xinliang David Li wrote: > On Fri, Jan 22, 2016 at 1:43 PM, Sean Silva wrote: > > silvas added a comment. > > > > In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > > > >> For the longer term, one possible solution is to make FE based > >> instrumentat

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread Sean Silva via cfe-commits
On Mon, Jan 25, 2016 at 12:52 PM, Rong Xu wrote: > OK. Now I understand what you meant. I was trying to avoid driver changes. > It seems you are saying I can change the driver to support cc1 options more > cleanly. This can be done. > Great! Sorry for the confusion. -- Sean Silva > Thanks, > >

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread Rong Xu via cfe-commits
xur added a subscriber: xur. xur added a comment. OK. Now I understand what you meant. I was trying to avoid driver changes. It seems you are saying I can change the driver to support cc1 options more cleanly. This can be done. Thanks, Rong http://reviews.llvm.org/D15829 ___

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread Rong Xu via cfe-commits
OK. Now I understand what you meant. I was trying to avoid driver changes. It seems you are saying I can change the driver to support cc1 options more cleanly. This can be done. Thanks, Rong On Jan 25, 2016 12:43, "Sean Silva" wrote: > silvas added inline comments. > > > Commen

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread Sean Silva via cfe-commits
silvas added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:487 @@ +486,3 @@ + // Opts.ProfileInstrGenerate will be used for Clang instrumentation only. + if (!Opts.ProfileIRInstr) +Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) || ---

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:440 @@ +439,3 @@ + if (CodeGenOpts.ProfileIRInstr) { +// Should not have ProfileInstrGenerate set -- it is for clang +// instrumentation only. xur wrote: > silvas wrote: > > Then chang

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-25 Thread Xinliang David Li via cfe-commits
On Fri, Jan 22, 2016 at 1:43 PM, Sean Silva wrote: > silvas added a comment. > > In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > >> For the longer term, one possible solution is to make FE based >> instrumentation only used for coverage testing which can be turned on >> with -fcovera

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Rong Xu via cfe-commits
xur added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:440 @@ +439,3 @@ + if (CodeGenOpts.ProfileIRInstr) { +// Should not have ProfileInstrGenerate set -- it is for clang +// instrumentation only. silvas wrote: > Then change the existing refe

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Sean Silva via cfe-commits
silvas added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:440 @@ +439,3 @@ + if (CodeGenOpts.ProfileIRInstr) { +// Should not have ProfileInstrGenerate set -- it is for clang +// instrumentation only. Then change the existing references in cla

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Sean Silva via cfe-commits
silvas added a comment. In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > For the longer term, one possible solution is to make FE based > instrumentation only used for coverage testing which can be turned on > with -fcoverage-mapping option (currently, -fcoverage-mapping can not > b

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Chad Rosier via cfe-commits
mcrosier added inline comments. Comment at: lib/Driver/Tools.cpp:3279 @@ -3278,1 +3278,3 @@ + + Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr); } silvas wrote: > xur wrote: > > mcrosier wrote: > > > I don't think AddAllArgs is what you really want. Wh

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Sean Silva via cfe-commits
silvas added inline comments. Comment at: lib/Driver/Tools.cpp:3279 @@ -3278,1 +3278,3 @@ + + Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr); } xur wrote: > mcrosier wrote: > > I don't think AddAllArgs is what you really want. What if the user > > sp

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread David Li via cfe-commits
davidxl added a comment. For the longer term, one possible solution is to make FE based instrumentation only used for coverage testing which can be turned on with -fcoverage-mapping option (currently, -fcoverage-mapping can not be used alone and must be used together with -fprofile-instr-generate)

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Xinliang David Li via cfe-commits
For the longer term, one possible solution is to make FE based instrumentation only used for coverage testing which can be turned on with -fcoverage-mapping option (currently, -fcoverage-mapping can not be used alone and must be used together with -fprofile-instr-generate). To summarize: A. Curren

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Rong Xu via cfe-commits
xur added inline comments. Comment at: lib/Driver/Tools.cpp:3279 @@ -3278,1 +3278,3 @@ + + Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr); } mcrosier wrote: > I don't think AddAllArgs is what you really want. What if the user specifies > the option t

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Chad Rosier via cfe-commits
mcrosier added a subscriber: mcrosier. mcrosier added a comment. Would it make sense to include an additional test (in test/Driver) that shows the -fprofile-ir-instr option being passed from the driver to the frontend? Such a test case would land in clang_f_opt.c, which has many examples.

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 45708. xur added a comment. This new patches integrates Sean review comments: (1) make -fprofile-ir-instr a cc1 option instead of driver option. (2) add one cc1 option test, and test the pass is indeed invoked. (3) remove the driver test (runtime library). (4) fix

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread David Li via cfe-commits
davidxl added a comment. This option is not needed for -fprofile-instr-use compilation as the compiler can detect the flavor of the profile (patch pending review), and decide what to do. The question here is what the option should look like to tell the compiler which phase to do the instrumen

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-22 Thread Rong Xu via cfe-commits
On Thu, Jan 21, 2016 at 7:32 PM, Sean Silva wrote: > silvas added a comment. > > @slingn and I had a discussion offline about the potential names and came up > with some ideas, but none is a clear winner. > > Overall, my feeling is that from a user's perspective, the frontend stuff is > probably

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-21 Thread Sean Silva via cfe-commits
silvas added a comment. @bogner btw, did you say that at Apple you guys have a requirement of supporting existing profdata? I.e. users can pass older profdata to a newer compiler? Realistically, it would be nice if our PGO offering defaulted to the IR stuff (since it seems like it is going to

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-21 Thread Sean Silva via cfe-commits
silvas added a comment. @slingn and I had a discussion offline about the potential names and came up with some ideas, but none is a clear winner. Overall, my feeling is that from a user's perspective, the frontend stuff is probably best referred to as "coverage-based". It's not as clear for the

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-21 Thread Sean Silva via cfe-commits
silvas added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.def:108 @@ -107,2 +107,3 @@ ///< execution counts to use with PGO. +CODEGENOPT(ProfileIRInstr, 1, 0) ///< IR level code PGO instrumentation and use. CODEGENOP

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-21 Thread Sean Silva via cfe-commits
silvas added a comment. This needs tests showing that the IR gen/use passes get run. Maybe use -debug-pass=Structure like test/CodeGen/thinlto_backend.c? My biggest concern is the naming and user visible parts. I can't come up with anything better than `-fprofile-ir-instr` TBH. Overall, from a

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-21 Thread Rong Xu via cfe-commits
xur added a comment. Ping. Passmanagerbuilder change has been committed. Could you take a look at this command line option patch? Thanks, -Rong http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-13 Thread Rong Xu via cfe-commits
xur updated this revision to Diff 44776. xur added a comment. added new tests and comments based on the review comments. http://reviews.llvm.org/D15829 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp lib/CodeGen/CodeGenModule.

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2016-01-13 Thread Rong Xu via cfe-commits
xur marked an inline comment as done. xur added a comment. I'll sent the revised patch based on the comments shortly. Comment at: lib/CodeGen/BackendUtil.cpp:435 @@ +434,3 @@ + if (CodeGenOpts.ProfileIRInstr) { +assert (!CodeGenOpts.ProfileInstrGenerate); +if (!CodeGenO

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2015-12-30 Thread David Li via cfe-commits
davidxl added a comment. Should add a test case in test/Driver/instrprof-ld.c. Comment at: lib/CodeGen/BackendUtil.cpp:435 @@ +434,3 @@ + if (CodeGenOpts.ProfileIRInstr) { +assert (!CodeGenOpts.ProfileInstrGenerate); +if (!CodeGenOpts.InstrProfileOutput.empty()) ---

[PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

2015-12-30 Thread Rong Xu via cfe-commits
xur created this revision. xur added reviewers: davidxl, silvas, bogner. xur added a subscriber: cfe-commits. This patch introduce a new toggle option -fprofile-ir-instr that enables IR level instrumentation. It needs to be used in combination with current PGO options. Without an existing PGO op