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

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

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

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 the user. > Presumably a user would have the

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

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

dblaikie added inline comments. Comment at: cfe/trunk/test/Profile/def-assignop.cpp:27 @@ +26,3 @@ + +int main() { + A a1, a2; This doesn't need to be main or have an int return. Just make it a void function (with some generic name) & drop the "return 0" to

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

On Tue, Feb 9, 2016 at 12:07 PM, David Li via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > 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). > In

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

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 coverage

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

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:14 AM,

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,

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

On Mon, Feb 8, 2016 at 9:00 PM, Xinliang David Li wrote: > 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

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,

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

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

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

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

vsk added a comment. Lgtm. http://reviews.llvm.org/D16947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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, David Li via cfe-commits < cfe-commits@lists.llvm.org> wrote: > davidxl created this revision. > davidxl added a reviewer: vsk. > davidxl added

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

On Mon, Feb 8, 2016 at 9:25 AM, David Li via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > 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 >

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

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, David Li via

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

ah, right, sorry about that - gmail didn't render cfe-commits on the to line in the first email... weird. Anyway, no need to include llvm-commits on clang-only changes. On Mon, Feb 8, 2016 at 11:30 AM, Xinliang David Li wrote: > Both cfe-commits and llvm-commits are cc'ed.

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

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

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

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

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 this

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

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

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 AM, David

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

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

On Mon, Feb 8, 2016 at 3:46 PM, Xinliang David Li wrote: > 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

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 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 try to keep the number of test files down (&

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

On Mon, Feb 8, 2016 at 4:31 PM, Xinliang David Li wrote: > 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

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,

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

On Mon, Feb 8, 2016 at 5:05 PM, Xinliang David Li wrote: > ha! somehow I kept thinking you are referring to implicit declared ctors. > Ah, glad we figured out the disconnect - thanks for bearing with me! > > From your test case, it is seems that the implicit copy/move op

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

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: