Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-07-20 Thread Davide Italiano via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL276242: [CodeGen] Handle recursion in LLVMIRGeneration Timer. (authored by davide). Changed prior to commit: https://reviews.llvm.org/D20748?vs=58833&id=64826#toc Repository: rL LLVM https://reviews

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-07-18 Thread Davide Italiano via cfe-commits
davide added a comment. Richard, can you please take a look at this? The more I look at it the more it seems weird that we can recurse in this case, but I may miss something https://reviews.llvm.org/D20748 ___ cfe-commits mailing list cfe-commits@l

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-06-15 Thread Davide Italiano via cfe-commits
davide added a comment. ping? http://reviews.llvm.org/D20748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-06-08 Thread Davide Italiano via cfe-commits
I agree with Rafael. I ran with -ftime-report for a couple of weeks on a lot of tests and never hit this problem. It actually seems weird we have to handle the timer recursively here as well (I'd like to avoid it at all), but let's see what Richard has to say when he gets to this review. On Wed, J

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-06-08 Thread Rafael Espíndola via cfe-commits
Since we found only one user, I think my preference is to handle it there. Cheers, Rafael On 8 June 2016 at 13:49, Vedant Kumar wrote: > vsk added a comment. > > Ping, any updates on this patch? > > > http://reviews.llvm.org/D20748 > > > ___ cfe-commi

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-06-08 Thread Vedant Kumar via cfe-commits
vsk added a comment. Ping, any updates on this patch? http://reviews.llvm.org/D20748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-05-27 Thread Vedant Kumar via cfe-commits
> On May 27, 2016, at 2:55 PM, Rafael Espíndola > wrote: > > On 27 May 2016 at 17:32, Vedant Kumar wrote: >> vsk added a comment. >> >> Comments on this patch -- The increment and decrement snippets seem like >> they could be pulled into helper methods. That should make it easier to >> upda

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-05-27 Thread Rafael Espíndola via cfe-commits
On 27 May 2016 at 17:32, Vedant Kumar wrote: > vsk added a comment. > > Comments on this patch -- The increment and decrement snippets seem like they > could be pulled into helper methods. That should make it easier to update all > users of LLVMIRGeneration. I wasn't able to come up with a regre

Re: [PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-05-27 Thread Vedant Kumar via cfe-commits
vsk added a comment. Comments on this patch -- The increment and decrement snippets seem like they could be pulled into helper methods. That should make it easier to update all users of LLVMIRGeneration. I wasn't able to come up with a regression test, but I do think this patch needs one. Over

[PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

2016-05-27 Thread Davide Italiano via cfe-commits
davide created this revision. davide added reviewers: rafael, vsk. davide added a subscriber: cfe-commits. See http://reviews.llvm.org/D20735 for more context. Implementing this clang-side is not as terrible as I originally thought. Maybe needs a test case, but I wasn't able to reduce one easily