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

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=64826#toc Repository: rL LLVM

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

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

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

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

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,

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

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

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

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

> 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

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

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

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

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.

[PATCH] D20748: Handle recursion in LLVMIRGeneration Timer

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