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

[Ulrich Weigand via llvm-commits]

David Blaikie <dblaikie at gmail.com> wrote on 27.02.2018 18:47:22:

This didn't show up in Phabricator ... was this deliberate?

> On Tue, Feb 27, 2018 at 12:48 AM Ulrich Weigand via Phabricator <
> reviews at reviews.llvm.org> wrote:
>> 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?

I guess this can happen if one function is inlined twice at two
different places into the same caller ...

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

Well, this is only reason why I'm working on this in the first place;
to fix this bug https://bugs.llvm.org/show_bug.cgi?id=36410

If the instruction for which locations are being merged is a debug
intrinsics, we have no choice of scopes -- the scope of a debug
intrinsic must match the scope of the variable the intrinsic
refers to, otherwise the module verifier will abort.

If you read the bugzilla, we had been discussing back and forth
whether to ensure we get the correct scope in this case by either:
- special-casing debug intrinsics in the caller; or
- ensuring that the getMergedLocation algorithm has the property
  that if the two inputs have the same scope, then the result
  will as well -- then it will automatically respect the special
  requirement for debug intrinsics

This patch is an attempt to implement the second option.  If we
now decide that we *don't* want that property to hold, then we
need to go back and fix the original bug in another way.

Bye,
Ulrich
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180227/df45724a/attachment.html>