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

[Ulrich Weigand via Phabricator via llvm-commits]

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.

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

> 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