r276517 - [Profile] Use a flag to enable PGO rather than the profraw filename

2016-07-22 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Fri Jul 22 23:28:59 2016 New Revision: 276517 URL: http://llvm.org/viewvc/llvm-project?rev=276517=rev Log: [Profile] Use a flag to enable PGO rather than the profraw filename Patch by Jake VanAdrighem Differential Revision: http://reviews.llvm.org/D22608 Modified:

Re: [PATCH] D22608: [Profile] Use a flag to enable PGO rather than the profraw filename.

2016-07-22 Thread David Li via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL276517: [Profile] Use a flag to enable PGO rather than the profraw filename (authored by davidxl). Changed prior to commit: https://reviews.llvm.org/D22608?vs=64812=65216#toc Repository: rL LLVM

r276484 - [Profile] Enable profile merging with -fprofile-generat[=]

2016-07-22 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Fri Jul 22 17:25:01 2016 New Revision: 276484 URL: http://llvm.org/viewvc/llvm-project?rev=276484=rev Log: [Profile] Enable profile merging with -fprofile-generat[=] This patch enables raw profile merging for this option which is the new intended behavior. Modified:

r276356 - [profile] update test case with interface change.

2016-07-21 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Thu Jul 21 18:19:39 2016 New Revision: 276356 URL: http://llvm.org/viewvc/llvm-project?rev=276356=rev Log: [profile] update test case with interface change. See http://reviews.llvm.org/D22613, http://reviews.llvm.org/D22614 Modified: cfe/trunk/test/Profile/c-generate.c

Re: [PATCH] D22608: [Profile] Use a flag to enable PGO rather than the profraw filename.

2016-07-21 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D22608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread Xinliang David Li via cfe-commits
ok David On Wed, Jul 20, 2016 at 4:32 PM, Sean Silva wrote: > silvas accepted this revision. > silvas added a comment. > This revision is now accepted and ready to land. > > LGTM (also, another small suggestion). > > > > Comment at:

r276207 - [Profile] Document new profile file name modifiers

2016-07-20 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Wed Jul 20 18:32:50 2016 New Revision: 276207 URL: http://llvm.org/viewvc/llvm-project?rev=276207=rev Log: [Profile] Document new profile file name modifiers Differential Revision: http://reviews.llvm.org/D22593 Modified: cfe/trunk/docs/UsersManual.rst Modified:

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL276207: [Profile] Document new profile file name modifiers (authored by davidxl). Changed prior to commit: https://reviews.llvm.org/D22593?vs=64793=64794#toc Repository: rL LLVM

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl updated this revision to Diff 64793. davidxl added a comment. Addressed review comments. I still think %4m etc is an advanced feature that needs more explanation. We can delay that to a later time. https://reviews.llvm.org/D22593 Files: docs/UsersManual.rst Index:

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: docs/UsersManual.rst:1500 @@ +1499,3 @@ + name. When this specifier is used, the profiler runtime will substitute ``%m`` + with a unique integer identifier associated with the instrumented binary. Multiple + profiles dumped from

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl updated this revision to Diff 64759. davidxl added a comment. Fixed a typo https://reviews.llvm.org/D22593 Files: docs/UsersManual.rst Index: docs/UsersManual.rst === --- docs/UsersManual.rst +++ docs/UsersManual.rst @@

[PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl created this revision. davidxl added reviewers: vsk, silvas. davidxl added a subscriber: cfe-commits. Added documentation for %h and %m specifiers. %m specifier which specifies the number of copies is not documented yet (treated as internal for now) https://reviews.llvm.org/D22593

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-19 Thread Xinliang David Li via cfe-commits
On Tue, Jul 19, 2016 at 5:06 PM, Vedant Kumar wrote: > > > On Jul 19, 2016, at 5:01 PM, Xinliang David Li > wrote: > > > > The new behavior works really well. There is one follow up change I > would like to discuss. > > > > The EQ form of the option

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-19 Thread Xinliang David Li via cfe-commits
The new behavior works really well. There is one follow up change I would like to discuss. The EQ form of the option -fprofile-generate= takes a directory name as the argument instead of the filename. Currently the compiler will create a default 'default.profraw' name for it, but this means,

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-15 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. lgtm Repository: rL LLVM https://reviews.llvm.org/D21823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-12 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/Driver/Tools.cpp:3596 @@ -3576,3 +3595,3 @@ Args.hasFlag(options::OPT_fprofile_generate, -options::OPT_fno_profile_instr_generate, false) || +options::OPT_fno_profile_generate, false)

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-12 Thread David Li via cfe-commits
davidxl added a comment. Please also update user manual: docs/UserManual.rst Comment at: test/Driver/clang_f_opts.c:90 @@ -89,3 +89,2 @@ // RUN: %clang -### -S -fprofile-instr-generate=file -fno-profile-instr-generate %s 2>&1 | FileCheck -check-prefix=CHECK-DISABLE-GEN %s

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-09 Thread David Li via cfe-commits
davidxl added a comment. I should have brought it up earlier, but I forgot.I think a better (and simpler) proposal is to make -fprofile-generate and -fprofile-use turn on IR based PGO. -fprofile-generate and -fprofile-use options were introduced by Diego (cc'ed) recently for GCC option

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-08 Thread Xinliang David Li via cfe-commits
On Sun, Jul 3, 2016 at 1:50 PM, Sean Silva wrote: > > > On Sat, Jul 2, 2016 at 7:38 PM, Xinliang David Li > wrote: > >> Sanitizers are different IMO. Different santizers are rather independent, >> and there is no such thing as -fsantize to mean

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-02 Thread Xinliang David Li via cfe-commits
On Fri, Jul 1, 2016 at 4:32 PM, Sean Silva wrote: > silvas added inline comments. > > > Comment at: lib/Driver/Tools.cpp:3560 > @@ +3559,3 @@ > +if (PGOTrainArg->getOption().matches(options::OPT_fpgo_train_EQ)) { > + if

Re: [PATCH] D21823: [Driver] Add flags for enabling both types of PGO Instrumentation

2016-07-01 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: include/clang/Driver/Options.td:507 @@ +506,3 @@ +Group, Flags<[DriverOption]>, MetaVarName<"">, +HelpText<"Set to be the default profile output file (overridden by LLVM_PROFILE_FILE env var)">; +def fpgo_apply_EQ :

r270728 - Use new triple API to check comdat /NFC

2016-05-25 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Wed May 25 12:25:57 2016 New Revision: 270728 URL: http://llvm.org/viewvc/llvm-project?rev=270728=rev Log: Use new triple API to check comdat /NFC Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp URL:

Re: [PATCH] D20287: [Coverage] Ensure that the hash for a used function is non-zero.

2016-05-16 Thread David Li via cfe-commits
davidxl added a comment. Strictly speaking, this patch requires a version bump of the indexed format. The profile reader also needs to adjust the FunctionHash computation (either using 0 or simple function hash) based on the version of the profile data. Check with Justin/vsk to see if it is

Re: [PATCH] D19725: [Coverage] Fix an issue where a coverage region might not be created for a macro containing for or while statements.

2016-05-01 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/CodeGen/CoverageMappingGen.cpp:468 @@ +467,3 @@ +MostRecentLocation == getEndOfFileOrMacro(MostRecentLocation) && +

Re: [PATCH] D19612: Use the new path for coverage related headers and update CMakeLists.txt

2016-04-27 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a reviewer: davidxl. davidxl added a comment. This revision is now accepted and ready to land. lgtm http://reviews.llvm.org/D19612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D18624: [PGO] PGOFuncName meta data if PGOFuncName is different from function's raw name.

2016-04-22 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. lgtm http://reviews.llvm.org/D18624 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D19299: lower __builtin_expect() directly to prof metadata instead of LLVM intrinsic

2016-04-21 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. lgtm http://reviews.llvm.org/D19299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D18624: [PGO] PGOFuncName meta data if PGOFuncName is different from function's raw name.

2016-04-20 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CodeGenPGO.cpp:47 @@ +46,3 @@ + // Create PGOFuncName meta data. + if (!llvm::getPGOFuncNameMetadata(*Fn)) +llvm::createPGOFuncNameMetadata(*Fn); This check be folded into the creator. The creator

Re: [PATCH] D19299: lower __builtin_expect() directly to prof metadata instead of LLVM intrinsic

2016-04-20 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:636 @@ -640,1 +635,3 @@ + case Builtin::BI__builtin_unpredictable: case Builtin::BI__builtin_expect: { +// Always return the first argument. LLVM does not handle these builtins. Can this

Re: [PATCH] D19299: lower __builtin_expect() directly to prof metadata instead of LLVM intrinsic

2016-04-19 Thread David Li via cfe-commits
davidxl added a comment. I like the direction this patch is going. Will look into details soon. http://reviews.llvm.org/D19299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18624: [PGO] PGOFuncName meta data if PGOFuncName is different from function's raw name.

2016-04-19 Thread Xinliang David Li via cfe-commits
Can you also try IR based instrumentation? -fprofile-instr-generate -Xclang=-fprofile-instrument=llvm David On Tue, Apr 19, 2016 at 9:40 AM, Adam Nemet wrote: > anemet added a comment. > > As discussed under http://reviews.llvm.org/D17864, I did a run with this > and I don't

Re: [PATCH] D18624: [PGO] PGOFuncName meta data if PGOFuncName is different from function's raw name.

2016-04-15 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CodeGenPGO.cpp:791 @@ +790,3 @@ +// Create PGOFuncName meta data. +llvm::Function *F = ValueSite->getFunction(); +if (!llvm::getPGOFuncNameMetadata(*F)) This may not be the best place do set the

Re: [PATCH] D18489: [PGO, clang] Comment how function pointers for indirect calls are mapped to function names

2016-03-28 Thread Xinliang David Li via cfe-commits
Sure. thanks, David On Mon, Mar 28, 2016 at 12:41 PM, Adam Nemet wrote: > anemet added a comment. > > In http://reviews.llvm.org/D18489#384851, @davidxl wrote: > > > What I meant is that putting the comments in InstrProfData.inc file, and > > update the one in

Re: [PATCH] D18489: [PGO, clang] Comment how function pointers for indirect calls are mapped to function names

2016-03-28 Thread Xinliang David Li via cfe-commits
What I meant is that putting the comments in InstrProfData.inc file, and update the one in CodeGenPGO.cpp to reference that. David On Mon, Mar 28, 2016 at 12:35 PM, Xinliang David Li wrote: > > > On Mon, Mar 28, 2016 at 12:31 PM, Adam Nemet wrote: > >>

Re: [PATCH] D18489: [PGO, clang] Comment how function pointers for indirect calls are mapped to function names

2016-03-28 Thread Xinliang David Li via cfe-commits
On Mon, Mar 28, 2016 at 12:31 PM, Adam Nemet wrote: > anemet added inline comments. > > > Comment at: lib/CodeGen/CodeGenPGO.cpp:758 > @@ -757,1 +757,3 @@ > > + // During instrumentation, function pointers are collected for the > different > + // indirect

Re: [PATCH] D18489: [PGO, clang] Comment how function pointers for indirect calls are mapped to function names

2016-03-28 Thread Xinliang David Li via cfe-commits
On Mon, Mar 28, 2016 at 10:40 AM, Adam Nemet wrote: > anemet added inline comments. > > > Comment at: lib/CodeGen/CodeGenPGO.cpp:758 > @@ -757,1 +757,3 @@ > > + // During instrumentation, function pointers are collected for the > different > + // indirect

Re: [PATCH] D18489: [PGO, clang] Comment how function pointers for indirect calls are mapped to function names

2016-03-28 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CodeGenPGO.cpp:758 @@ -757,1 +757,3 @@ + // During instrumentation, function pointers are collected for the different + // indirect call targets. Then as part of the conversion from raw to merged anemet

Re: [PATCH] D18489: [PGO, clang] Comment how function pointers for indirect calls are mapped to function names

2016-03-26 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CodeGenPGO.cpp:758 @@ -757,1 +757,3 @@ + // During instrumentation, function pointers are collected for the different + // indirect call targets. Then as part of the conversion from raw to merged

Re: [PATCH] D18289: Attach profile summary information to module

2016-03-24 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a reviewer: davidxl. davidxl added a comment. lgtm http://reviews.llvm.org/D18289 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18289: Attach profile summary information to module

2016-03-22 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:398 @@ -397,2 +397,3 @@ if (PGOReader) { getModule().setMaximumFunctionCount(PGOReader->getMaximumFunctionCount()); +auto *SummaryMD = PGOReader->getSummary().getMD(getModule().getContext());

Re: [PATCH] D17737: [PGO] change profile use cc1 option

2016-03-02 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. lgtm http://reviews.llvm.org/D17737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17737: [PGO] change profile use cc1 option

2016-03-01 Thread David Li via cfe-commits
davidxl added a comment. I think we just need one cc1 option -fprofile-instrument-use-path=<>. An overloaded setPGOInstrumenter method can peak at the profile header and get the Profile flavor. http://reviews.llvm.org/D17737 ___ cfe-commits

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

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

r261047 - Test simplification

2016-02-16 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Tue Feb 16 18:59:01 2016 New Revision: 261047 URL: http://llvm.org/viewvc/llvm-project?rev=261047=rev Log: Test simplification Modified: cfe/trunk/test/Profile/def-assignop.cpp Modified: cfe/trunk/test/Profile/def-assignop.cpp URL:

r261046 - Restrengthen tests relaxed in r259955

2016-02-16 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Tue Feb 16 18:58:13 2016 New Revision: 261046 URL: http://llvm.org/viewvc/llvm-project?rev=261046=rev Log: Restrengthen tests relaxed in r259955 Modified: cfe/trunk/test/CoverageMapping/ir.c cfe/trunk/test/CoverageMapping/unused_names.c Modified:

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread Xinliang David Li via cfe-commits
On Tue, Feb 9, 2016 at 11:30 AM, David Blaikie wrote: > > > On Tue, Feb 9, 2016 at 11:26 AM, Xinliang David Li > wrote: >> >> On Tue, Feb 9, 2016 at 11:14 AM, David Blaikie wrote: >> > >> > >> > On Mon, Feb 8, 2016 at 9:23 PM, Xinliang

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread Xinliang David Li via cfe-commits
On Tue, Feb 9, 2016 at 11:14 AM, David Blaikie wrote: > > > On Mon, Feb 8, 2016 at 9:23 PM, Xinliang David Li > wrote: >> >> Wrong in the sense the the coverage result for the default operators >> (the line where they are declared) is marked as if they are

r260270 - [PGO] Fix issue: explicitly defaulted assignop is not profiled

2016-02-09 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Tue Feb 9 14:02:59 2016 New Revision: 260270 URL: http://llvm.org/viewvc/llvm-project?rev=260270=rev Log: [PGO] Fix issue: explicitly defaulted assignop is not profiled Differential Revision: http://reviews.llvm.org/D16947 Added:

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread David Li via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL260270: [PGO] Fix issue: explicitly defaulted assignop is not profiled (authored by davidxl). Changed prior to commit: http://reviews.llvm.org/D16947?vs=47239=47351#toc Repository: rL LLVM

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread Xinliang David Li via cfe-commits
On Tue, Feb 9, 2016 at 11:44 AM, David Blaikie wrote: > > > On Tue, Feb 9, 2016 at 11:41 AM, Xinliang David Li > wrote: >> >> On Tue, Feb 9, 2016 at 11:30 AM, David Blaikie wrote: >> > >> > >> > On Tue, Feb 9, 2016 at 11:26 AM,

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
Wrong in the sense the the coverage result for the default operators (the line where they are declared) is marked as if they are not called which can be confusing to the user. David On Mon, Feb 8, 2016 at 9:09 PM, David Blaikie wrote: > > > On Mon, Feb 8, 2016 at 9:00 PM,

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
I took a look at the problem. The implicitly defaulted operators should not be instrumented as specified -- I actually I just added the new test case for that (checking profile counter not generated) right after my previous reply and it still passes with this patch. The reason is that those

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
On Mon, Feb 8, 2016 at 8:46 PM, David Blaikie wrote: > > On Mon, Feb 8, 2016 at 7:39 PM, Xinliang David Li > wrote: >> >> I took a look at the problem. The implicitly defaulted operators >> should not be instrumented as specified -- I actually I just added

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Li via cfe-commits
davidxl updated this revision to Diff 47217. davidxl added a comment. Simplified test case suggested by Vedant. http://reviews.llvm.org/D16947 Files: lib/CodeGen/CGClass.cpp test/Profile/def-assignop.cpp Index: test/Profile/def-assignop.cpp

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
On Mon, Feb 8, 2016 at 11:39 AM, David Blaikie wrote: > > > On Mon, Feb 8, 2016 at 9:25 AM, David Li via llvm-commits > wrote: >> >> davidxl updated this revision to Diff 47217. >> davidxl added a comment. >> >> Simplified test case suggested by

r260126 - Simplify test cases

2016-02-08 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Mon Feb 8 13:14:14 2016 New Revision: 260126 URL: http://llvm.org/viewvc/llvm-project?rev=260126=rev Log: Simplify test cases Modified: cfe/trunk/test/Profile/def-ctors.cpp cfe/trunk/test/Profile/def-dtors.cpp Modified: cfe/trunk/test/Profile/def-ctors.cpp URL:

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
Both cfe-commits and llvm-commits are cc'ed. David On Mon, Feb 8, 2016 at 11:29 AM, David Blaikie <dblai...@gmail.com> wrote: > This looks like a change to clang - could you test it in clang (& review it > on cfe-commits instead of llvm-commits)? > > On Sat, Feb 6, 2016 at

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Li via cfe-commits
davidxl updated this revision to Diff 47239. davidxl added a comment. Further simplify tests according to David B's comment. http://reviews.llvm.org/D16947 Files: lib/CodeGen/CGClass.cpp test/Profile/def-assignop.cpp Index: test/Profile/def-assignop.cpp

r260161 - [PGO] Cover more cases of implicitly generated C++ methods

2016-02-08 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Mon Feb 8 16:41:37 2016 New Revision: 260161 URL: http://llvm.org/viewvc/llvm-project?rev=260161=rev Log: [PGO] Cover more cases of implicitly generated C++ methods Modified: cfe/trunk/test/Profile/cxx-implicit.cpp Modified: cfe/trunk/test/Profile/cxx-implicit.cpp

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
On Mon, Feb 8, 2016 at 4:05 PM, David Blaikie wrote: > > > On Mon, Feb 8, 2016 at 3:58 PM, Xinliang David Li > wrote: >> >> To be clear, you are suggesting breaking the test into two (one for >> copy, and one for move) ? I am totally fine with that. > > >

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
On Mon, Feb 8, 2016 at 3:17 PM, David Blaikie wrote: > > > On Mon, Feb 8, 2016 at 12:07 PM, Xinliang David Li > wrote: >> >> On Mon, Feb 8, 2016 at 11:39 AM, David Blaikie wrote: >> > >> > >> > On Mon, Feb 8, 2016 at 9:25 AM, David Li

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
On Mon, Feb 8, 2016 at 3:35 PM, David Blaikie wrote: > > > On Mon, Feb 8, 2016 at 3:21 PM, Xinliang David Li > wrote: >> >> On Mon, Feb 8, 2016 at 3:17 PM, David Blaikie wrote: >> > >> > >> > On Mon, Feb 8, 2016 at 12:07 PM, Xinliang

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
To be clear, you are suggesting breaking the test into two (one for copy, and one for move) ? I am totally fine with that. I thought you suggested removing the testing of move/op case because they might share the same code path (clang's implementation) as the copy/op. thanks, David On Mon, Feb

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread Xinliang David Li via cfe-commits
ha! somehow I kept thinking you are referring to implicit declared ctors. From your test case, it is seems that the implicit copy/move op is also broken and is fixed by this patch too. That means a missing test case to me. Will update the case when verified. thanks, David On Mon, Feb 8,

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-07 Thread David Li via cfe-commits
davidxl updated this revision to Diff 47150. davidxl added a comment. Updated test case. Another test case in profile-rt (pending) is : http://reviews.llvm.org/D16974 http://reviews.llvm.org/D16947 Files: lib/CodeGen/CGClass.cpp test/Profile/def-assignop.cpp Index:

[PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-06 Thread David Li via cfe-commits
davidxl created this revision. davidxl added a reviewer: vsk. davidxl added subscribers: llvm-commits, cfe-commits. For compiler generated assignment operator that is not trivial (calling base class operator=()), Clang FE assign region counters to the function body but does not emit profile

r260022 - Fix test case problem(caused by clang-format

2016-02-06 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Sun Feb 7 01:13:18 2016 New Revision: 260022 URL: http://llvm.org/viewvc/llvm-project?rev=260022=rev Log: Fix test case problem(caused by clang-format Modified: cfe/trunk/test/Profile/def-ctors.cpp cfe/trunk/test/Profile/def-dtors.cpp Modified:

r260021 - [PGO] add profile/coverage test cases for defaulted ctor/ctors

2016-02-06 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Sun Feb 7 00:57:29 2016 New Revision: 260021 URL: http://llvm.org/viewvc/llvm-project?rev=260021=rev Log: [PGO] add profile/coverage test cases for defaulted ctor/ctors Added: cfe/trunk/test/Profile/def-ctors.cpp cfe/trunk/test/Profile/def-dtors.cpp Added:

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

r259955 - [PGO] Test case update

2016-02-05 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Fri Feb 5 17:36:08 2016 New Revision: 259955 URL: http://llvm.org/viewvc/llvm-project?rev=259955=rev Log: [PGO] Test case update Temporarily relax check in test to avoid breakage for format change in LLVM side. Once that is done, the test case will be retightened.

r259819 - [PGO] code simplification: use existing VP annotation API /NFC

2016-02-04 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Thu Feb 4 13:54:17 2016 New Revision: 259819 URL: http://llvm.org/viewvc/llvm-project?rev=259819=rev Log: [PGO] code simplification: use existing VP annotation API /NFC Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp URL:

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 >

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,

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

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

r259067 - [PGO] test case cleanups

2016-01-28 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Thu Jan 28 12:25:53 2016 New Revision: 259067 URL: http://llvm.org/viewvc/llvm-project?rev=259067=rev Log: [PGO] test case cleanups 1. Make test case more focused and robust by focusing on what to be tested (linkage, icall) -- make it easier to validate 2. Testing linkages

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.

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

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

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

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

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

Re: [PATCH] D8940: Clang changes for indirect call target profiling

2016-01-22 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. LGTM. I think we also need a user level option to turn value profiling on/off (in followups). http://reviews.llvm.org/D8940 ___ cfe-commits

Re: [PATCH] D8940: Clang changes for indirect call target profiling

2016-01-20 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: lib/CodeGen/CodeGenPGO.cpp:768 @@ +767,3 @@ + llvm::IndexedInstrProfReader *PGOReader = CGM.getPGOReader(); + if (!InstrumentValueSites && !PGOReader) +return; && --> || Comment at:

r258261 - Reference the updated function name /NFC

2016-01-19 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Tue Jan 19 18:24:52 2016 New Revision: 258261 URL: http://llvm.org/viewvc/llvm-project?rev=258261=rev Log: Reference the updated function name /NFC Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp URL:

Re: [PATCH] D15853: [PGO]: Simplify coverage data lowering code

2016-01-18 Thread David Li via cfe-commits
davidxl added a comment. I missed the review comments earlier. The local var name is fixed. The getCoverageNamesVarName API name will be fixed later. Regarding to your question: the patch does not not change the behavior of the name handling. For all used functions, their names are handled

Re: [PATCH] D15853: [PGO]: Simplify coverage data lowering code

2016-01-06 Thread David Li via cfe-commits
davidxl updated this revision to Diff 44127. davidxl added a comment. Update patch to reduce overhead: Only record names for unused functions. http://reviews.llvm.org/D15853 Files: lib/CodeGen/CodeGenPGO.cpp lib/CodeGen/CoverageMappingGen.cpp lib/CodeGen/CoverageMappingGen.h Index:

[PATCH] D15853: [PGO]: Simplify coverage data lowering code

2016-01-03 Thread David Li via cfe-commits
davidxl created this revision. davidxl added a reviewer: vsk. davidxl added a subscriber: cfe-commits. The names referenced by the coverage data may be associated with functions that are never emitted by Clang. That means those PGO names won't be materialized into the __llvm_prf_names section

r256714 - [PGO] Cleanup: Use covmap header definition in the template file

2016-01-03 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Sun Jan 3 13:25:54 2016 New Revision: 256714 URL: http://llvm.org/viewvc/llvm-project?rev=256714=rev Log: [PGO] Cleanup: Use covmap header definition in the template file This is one last remaining instrumentatation related structure that needs to be migrate to use the

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())

Re: [PATCH] D15726: Remove setting of inlinehint and cold attributes based on profile data

2015-12-30 Thread David Li via cfe-commits
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. lgtm http://reviews.llvm.org/D15726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D15726: Remove setting of inlinehint and cold attributes based on profile data

2015-12-22 Thread David Li via cfe-commits
davidxl added a comment. This looks like a straightforward cleanup (the test cases have been in a previous patch). http://reviews.llvm.org/D15726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D15163: Attach maximum function count to Module when using PGO mode.

2015-12-17 Thread David Li via cfe-commits
davidxl added a comment. LGTM Repository: rL LLVM http://reviews.llvm.org/D15163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15163: Attach maximum function count to Module when using PGO mode.

2015-12-15 Thread David Li via cfe-commits
davidxl added a comment. I prefer using the profile from the original test case where Max count is not 1. Repository: rL LLVM http://reviews.llvm.org/D15163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

r255587 - [PGO] make profile prefix even shorter and more readable

2015-12-14 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Mon Dec 14 18:33:12 2015 New Revision: 255587 URL: http://llvm.org/viewvc/llvm-project?rev=255587=rev Log: [PGO] make profile prefix even shorter and more readable Modified: cfe/trunk/test/CoverageMapping/ir.c cfe/trunk/test/CoverageMapping/unused_names.c

r255576 - [PGO] Shorten profile symbol prefixes

2015-12-14 Thread Xinliang David Li via cfe-commits
Author: davidxl Date: Mon Dec 14 17:26:46 2015 New Revision: 255576 URL: http://llvm.org/viewvc/llvm-project?rev=255576=rev Log: [PGO] Shorten profile symbol prefixes (test case update) Profile symbols have long prefixes which waste space and creating pressure for linker. This patch shortens

Re: [PATCH] D15163: Attach maximum function count to Module when using PGO mode.

2015-12-14 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: cfe/trunk/test/CodeGen/pgo-max-function-count.c:1 @@ +1,2 @@ +// RUN: %clang -fprofile-generate -o %t -O2 %s +// RUN: env LLVM_PROFILE_FILE=%t.profraw %t Use -fprofile-instr-generate option. -fprofile-generate is for

Re: [PATCH] D15163: Attach maximum function count to Module when using PGO mode.

2015-12-14 Thread Xinliang David Li via cfe-commits
On Fri, Dec 11, 2015 at 6:19 PM, Justin Bogner wrote: > Easwaran Raman writes: >> eraman updated this revision to Diff 42549. >> eraman added a comment. >> >> Added a test case. >> >> >> Repository: >> rL LLVM >> >> http://reviews.llvm.org/D15163 >> >>

Re: [PATCH] D15163: Attach maximum function count to Module when using PGO mode.

2015-12-14 Thread Xinliang David Li via cfe-commits
On Fri, Dec 11, 2015 at 6:19 PM, Justin Bogner wrote: > Easwaran Raman writes: >> eraman updated this revision to Diff 42549. >> eraman added a comment. >> >> Added a test case. >> >> >> Repository: >> rL LLVM >> >> http://reviews.llvm.org/D15163 >> >>

<    1   2   3   4   >