[PATCH] D59272: [DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare

Wed Mar 13 11:57:05 PDT 2019

dblaikie added a comment.

In D59272#1426970 <https://reviews.llvm.org/D59272#1426970>, @aprantl wrote:

> In D59272#1426904 <https://reviews.llvm.org/D59272#1426904>, @rnk wrote:
>
> > This makes me nervous. The pair of the DIVariable and DILocation is used to uniquely identify the variable that a dbg.value applies to. In practice, inlining is usually what creates multiple instances of the same variable that get updated by dbg.values. You can see how the location is used in `DwarfDebug::collectVariableInfoFromMFTable`.
>
>
> Last time I checked, the file, line number and column of a dbg.value was completely unused. To uniquely identify and inlined instance of a variable you need to the combination of inlinedAt field of the dbg.value's DILocation and the dbg.value's DILocalVariable.
>
> > I see other places in the code that seem to assume that the scope of the location of a dbg.value will get the scope in which the variable is declared. Your change breaks that invariant. I think we could update such code to get the scope from the DIVariable, but maybe that would be wrong since it would lack inlining information.
>
> Isn't the scope coming from the DILocalVariable though?

The scope, perhaps/probably - but as you say, not the "inlinedAt" part. So, yeah, picking the wrong DebugLoc for dbg.value instruction could still drop/break/mess up inlining info due to the inlinedAt part of the DebugLoc.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59272/new/

https://reviews.llvm.org/D59272