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

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 -fsantize=all >> >> For PGO, in most of the cases,

r270728 - Use new triple API to check comdat /NFC

Author: davidxl Date: Wed May 25 12:25:57 2016 New Revision: 270728 URL: http://llvm.org/viewvc/llvm-project?rev=270728&view=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: http://llvm.org/vi

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

Author: davidxl Date: Mon Feb 8 16:41:37 2016 New Revision: 260161 URL: http://llvm.org/viewvc/llvm-project?rev=260161&view=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

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 via llvm-commits >> > wrote: >> >> >> >> davidxl updated thi

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

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 David Li >> > wrote: >> >> >> >> On Mon, Feb 8, 2016 at 11:39

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

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

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. > > > Nah, no need to split the test case - we

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

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, 2016

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

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 operato

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

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 the >> new test case for that (checking

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

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, Xinliang David Li > wr

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

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 not called >> which can be confusing to

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

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 David Li >> > wrote: >> >> >> >> Wrong in the sense the the

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

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, Xinliang David Li >> > wrote: >> >> >> >> On Tue, Feb 9, 2016 at 11

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

Author: davidxl Date: Tue Feb 9 14:02:59 2016 New Revision: 260270 URL: http://llvm.org/viewvc/llvm-project?rev=260270&view=rev Log: [PGO] Fix issue: explicitly defaulted assignop is not profiled Differential Revision: http://reviews.llvm.org/D16947 Added: cfe/trunk/test/Profile/def-as

r261046 - Restrengthen tests relaxed in r259955

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

r261047 - Test simplification

Author: davidxl Date: Tue Feb 16 18:59:01 2016 New Revision: 261047 URL: http://llvm.org/viewvc/llvm-project?rev=261047&view=rev Log: Test simplification Modified: cfe/trunk/test/Profile/def-assignop.cpp Modified: cfe/trunk/test/Profile/def-assignop.cpp URL: http://llvm.org/viewvc/llvm-proj

r258106 - Fix local variable name /NFC

Author: davidxl Date: Mon Jan 18 18:49:06 2016 New Revision: 258106 URL: http://llvm.org/viewvc/llvm-project?rev=258106&view=rev Log: Fix local variable name /NFC Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp cfe/trunk/lib/CodeGen/CoverageMappingGen.h Modified: cfe/trunk/lib/Cod

r258261 - Reference the updated function name /NFC

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

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

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

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

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

r259067 - [PGO] test case cleanups

Author: davidxl Date: Thu Jan 28 12:25:53 2016 New Revision: 259067 URL: http://llvm.org/viewvc/llvm-project?rev=259067&view=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 link

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

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

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

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

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

Author: davidxl Date: Thu Feb 4 13:54:17 2016 New Revision: 259819 URL: http://llvm.org/viewvc/llvm-project?rev=259819&view=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 U

r259955 - [PGO] Test case update

Author: davidxl Date: Fri Feb 5 17:36:08 2016 New Revision: 259955 URL: http://llvm.org/viewvc/llvm-project?rev=259955&view=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 retighten

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

Author: davidxl Date: Sun Feb 7 00:57:29 2016 New Revision: 260021 URL: http://llvm.org/viewvc/llvm-project?rev=260021&view=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: cfe

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

Author: davidxl Date: Sun Feb 7 01:13:18 2016 New Revision: 260022 URL: http://llvm.org/viewvc/llvm-project?rev=260022&view=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: cfe/trunk/test/

r260126 - Simplify test cases

Author: davidxl Date: Mon Feb 8 13:14:14 2016 New Revision: 260126 URL: http://llvm.org/viewvc/llvm-project?rev=260126&view=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

Both cfe-commits and llvm-commits are cc'ed. David On Mon, Feb 8, 2016 at 11:29 AM, David Blaikie 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 11:57 AM, Dav

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

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 Vedant. >> >> >> http://reviews.llvm.org/D16947 >>

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

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 call targets. Then as

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

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 call targets. Then as

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

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: > >> anemet added inline comments. >> >> =

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

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 CodeGenPGO.cpp to reference that

r266638 - Update InstrProf pass creator API reference

Author: davidxl Date: Mon Apr 18 12:48:12 2016 New Revision: 266638 URL: http://llvm.org/viewvc/llvm-project?rev=266638&view=rev Log: Update InstrProf pass creator API reference Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp URL: http://llvm.

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

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 get the indirect c

r250994 - clang driver toolchain refactoring

Author: davidxl Date: Thu Oct 22 01:15:31 2015 New Revision: 250994 URL: http://llvm.org/viewvc/llvm-project?rev=250994&view=rev Log: clang driver toolchain refactoring In this patch, the file static method addProfileRT is moved to be a virtual member function of base ToolChain class. This allows

r251072 - Use newly introduced interfaces in LLVM (NFC)

Author: davidxl Date: Thu Oct 22 17:25:11 2015 New Revision: 251072 URL: http://llvm.org/viewvc/llvm-project?rev=251072&view=rev Log: Use newly introduced interfaces in LLVM (NFC) Replaced references to raw strings in instrumentation and coverage code. Modified: cfe/trunk/lib/CodeGen/CodeGen

Re: [PATCH] D14030: Use linker option to reference profile runtime hook (Linux)

On Mon, Oct 26, 2015 at 8:44 AM, Vedant Kumar wrote: > vsk added a comment. > > This looks good to me. Does this mean we will get rid of > `getInstrProfRuntimeHookVarUseFuncName`? > not yet -- note that this is only enabled for Linux toolchain only for now. If it works for all platforms, we can

r251385 - Create undef reference to profile hook symbol

Author: davidxl Date: Tue Oct 27 00:15:35 2015 New Revision: 251385 URL: http://llvm.org/viewvc/llvm-project?rev=251385&view=rev Log: Create undef reference to profile hook symbol Create undef reference to profile hook symbol when PGO instrumentation is turned on. This allows LLVM to omit emiss

r251611 - Fix a soon to be invalid test

Author: davidxl Date: Wed Oct 28 23:04:07 2015 New Revision: 251611 URL: http://llvm.org/viewvc/llvm-project?rev=251611&view=rev Log: Fix a soon to be invalid test Remove a check that won't be valid when LLVM stops emitting runtime hook user function. Modified: cfe/trunk/test/Profile/gcc-fla

r252147 - Use profile data template file for covmap func record (NFC)

Author: davidxl Date: Wed Nov 4 23:46:39 2015 New Revision: 252147 URL: http://llvm.org/viewvc/llvm-project?rev=252147&view=rev Log: Use profile data template file for covmap func record (NFC) Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp Modified: cfe/trunk/lib/CodeGen/CoverageMap

r252434 - [PGO] Code cleanup [NFC]

Author: davidxl Date: Sun Nov 8 18:04:16 2015 New Revision: 252434 URL: http://llvm.org/viewvc/llvm-project?rev=252434&view=rev Log: [PGO] Code cleanup [NFC] Use interfaces defined in LLVM to create FuncName and FuncNameVar. Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp Modified: cfe/trun

Re: [PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

On Mon, Jun 14, 2021 at 3:59 PM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri added a subscriber: MaskRay. > lebedev.ri added a comment. > > In D104099#2815531 , @wenlei > wrote: > > > In D104099#2814167

Re: [PATCH] D12247: [libc++] remove possible trailing padding from aligned_storage

On Wed, Aug 26, 2015 at 11:11 AM, Eric Fiselier wrote: > EricWF added a comment. > > In http://reviews.llvm.org/D12247#233323, @davidxl wrote: > >> We certainly need a fix without breaking ABI. Is there a ABI conformance >> test for libcxx? > > > Well this patch definitely breaks the ABI. This wa

Re: [PATCH] D12247: [libc++] remove possible trailing padding from aligned_storage

On Wed, Aug 26, 2015 at 3:38 PM, Eric Fiselier wrote: > EricWF added a comment. > > In http://reviews.llvm.org/D12247#233717, @yiranwang wrote: > >> Hi Eric, >> >> Could you please explain a bit more what is broken specifically? As we can >> see, sizeof(), _Len, and _Align, and alignof() of "alig

Re: [PATCH] D12247: [libc++] remove possible trailing padding from aligned_storage

On Thu, Aug 27, 2015 at 2:57 PM, Yiran Wang wrote: > yiranwang added a comment. > > In http://reviews.llvm.org/D12247#234533, @EricWF wrote: > >> So this patch LGTM. Sorry for being slow to understand it. However I want to >> see two things before it lands. >> >> 1. I would like to see a test of

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

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, onlin

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

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 -fprofile-generate= takes a directory name as

r276207 - [Profile] Document new profile file name modifiers

Author: davidxl Date: Wed Jul 20 18:32:50 2016 New Revision: 276207 URL: http://llvm.org/viewvc/llvm-project?rev=276207&view=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

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: docs/UsersManual.rst:1502 > @@ +1501,3 @@ > +

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

Author: davidxl Date: Thu Jul 21 18:19:39 2016 New Revision: 276356 URL: http://llvm.org/viewvc/llvm-project?rev=276356&view=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-genera

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

Author: davidxl Date: Fri Jul 22 17:25:01 2016 New Revision: 276484 URL: http://llvm.org/viewvc/llvm-project?rev=276484&view=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:

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

Author: davidxl Date: Fri Jul 22 23:28:59 2016 New Revision: 276517 URL: http://llvm.org/viewvc/llvm-project?rev=276517&view=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:

<    1   2   3   4