[PATCH] D26256: [InstCombine] Don't set debug location when folding through a phi node

[Adrian Prantl via llvm-commits]

> On Nov 2, 2016, at 1:44 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Nov 2, 2016 at 1:38 PM Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> aprantl added a comment.
> 
> In https://reviews.llvm.org/D26256#586228 <https://reviews.llvm.org/D26256#586228>, @rob.lougher wrote:
> 
> > In https://reviews.llvm.org/D26256#586214 <https://reviews.llvm.org/D26256#586214>, @aprantl wrote:
> >
> > > Shouldn't it only drop the location if the two locations are distinct (and perhaps add a discriminator)?
> >
> >
> > Sorry, didn't make myself clear in the last comment.  As the two instructions feed into a phi node they are in different basic-blocks, so the two locations must be distinct (they are in different scopes and will have different discriminators).   But in the case where we have an if-then-else all on the same line we could create a new debug location with a different scope/discriminator.
> 
> 
> Yes, that sounds reasonable.
> 
> I think the single-line situation is not just an edge case and we should handle it correctly. In C++11 it is becoming quite common to have a lot of control flow on a single line (think anything from <algorithm> with lambdas or the ternary operator).
> 
> I'm not sure how practical it is to find the right scope to tie this in to... (at least not obvious to me off the cuff) nor how much it'd help the debugging experience to have this info. But I'm not too invested either way.

Hmm, algorithm/lambda is an example for control flow being on one line, but you are right that it does not apply to the situation in this review. The ternary operator is a better example.

-- adrian
>  
> 
> 
> https://reviews.llvm.org/D26256 <https://reviews.llvm.org/D26256>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161102/cc479ab0/attachment.html>