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

[David Blaikie via llvm-commits]

On Wed, Nov 2, 2016 at 1:38 PM Adrian Prantl <aprantl at apple.com> wrote:

> aprantl added a comment.
>
> In https://reviews.llvm.org/D26256#586228, @rob.lougher wrote:
>
> > In 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.


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