[PATCH] D43687: Improve merging of debug locations (fixes PR 36410)
Ulrich Weigand via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 27 10:25:01 PST 2018
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
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits