[PATCH] D43687: Improve merging of debug locations (fixes PR 36410)

[David Blaikie via llvm-commits]

On Tue, Feb 27, 2018 at 12:48 AM Ulrich Weigand via Phabricator <
reviews at reviews.llvm.org> wrote:

> uweigand added a comment.
>
> In https://reviews.llvm.org/D43687#1020083, @dblaikie wrote:
>
> > Have you considered a test case where there's a common scope within an
> inlinedAt location?
> >
> >   SP1 <- S1 <- IA1 <- L1
> >            \-- IA2 <- L2
> >
> >
> > I think it's reasonable for S1 to be the scope of the merged location -
> as much as it would be if IA1 and IA2 were not present. (SP = Subprogram, S
> = Scope, IA = InlinedAt, L = Location).
>
>
> Well, there's variants of that scenario: https://reviews.llvm.org/L1 and
> L2 might already have the same scope, or they might have different scopes.
>

The same scope but inlined in different places?


> If https://reviews.llvm.org/L1 and L2 already have the same scope, this
> **must** remain the scope of the merged location, or else we break debug
> intrinsics again.


That seems strange - can you explain that constraint & what motivates it in
some detail?


>   If https://reviews.llvm.org/L1 and L2 have different scopes, my current
> code just uses LocA->getInlinedAtScope() as simple fallback scope, which
> actually happens to agree with S in your example.  I guess this could still
> be refined for more complex cases later, if necessary ...
>

Ah - fair point. Well, expand S out to S1, with children S2 and S3 (which
are parents of IA1 and IA2). Then I think the algorithm would pick SP1,
instead of S1?


>
> > I /think/ this should be achievable with a single walk, rather than two
> separate walks (one up scopes, one up inlinedAt's), perhaps?
>
> Well, if https://reviews.llvm.org/L1 and L2 have the same scope, we need
> to keep it (see above).  But then if their inlined-at locations are
> different, we still need to come up with a merged inlined-at location
> without changing that common scope.  That's why I have two loops.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D43687
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180227/f04d563d/attachment.html>