[PATCH] D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked 2 inline comments as done.
jmorse added a comment.

In D58453#1406020 <https://reviews.llvm.org/D58453#1406020>, @probinson wrote:

> In my view, the real problem as stated in the PR appears to be that the value later gets *lost*, sadly before the instructions directly related to the switch-case where the variable is used; the real problem is *not* that the value's description starts too soon.
>
> So given all that, how does this patch help address the real problem?


Ah, to untangle the True (TM) problem I should illustrate some IR that corresponds to the problem in PR38754:

  someblock:
    %foo = add i32 %bar, %bar
    br i1 %somecond, label %qux, label %quux
  qux:
    %xyzzy = sub i32 %foo, 12
    call void @llvm.dbg.value(metadata i32 %xyzzy, [...])
    br label %quux

In this code, if %xyzzy is optimised out then the 'sub' instruction is deleted, and its operation folded into the dbg.value intrinsics DIExpression, something like:

  call void @llvm.dbg.value(metadata i32 %foo, metadata !1, metadata !DIExpression(DW_OP_constu, 12, DW_OP_minus, DW_OP_stack_value))

Which is good. However the dbg.values operand is now %foo rather than %xyzzy, and placeDbgValues moves the dbg.value call to immediately after its operands definition, moving it up past a conditional branch to the 'someblock' block, giving us:

  someblock:
    %foo = add i32 %bar, %bar
    call void @llvm.dbg.value(metadata i32 %foo, [...])
    br i1 %somecond, label %qux, label %quux

I'm not sure what the correct nomenclature is at this point, but the variable location that the dbg.value is defining now appears on paths through the program where it did not before, which in PR38754 makes a variable appear in the debugger to take on a value that isn't actually computed. Many different code hoisting / sinking / CSE passes can trigger similar behaviour.

> So, what instructions "make sense" in a location list? Well, the ones where the variable has an identifiable machine location or constant value.  If optimization causes a value to be defined "before" the variable is declared, that does not really worry me.  The goal here is to produce a correct description of the instructions-actually-emitted.
>  "One example problem is that conditional variable assignments can be hoisted and appear to debuggers unconditionally." Are the values actually defined unconditionally? Then it is correct to describe them that way to the debugger.  I assume this is referring to PR38754.  If the instructions to compute the value are hoisted, my assertion is that the location-list should describe the variable with that value when it actually occurs.

I think the key phrase here is "when it actually occurs": if in a program:

- Down two code paths the same value is computed but assigned to different variables, and
- The computation is hoisted to a parent block,

Should both variables appear to hold that computed value once it's defined in the parent block?

(NB, I'm aware that many people approach this with a different language to me, i.e. in terms of variable locations rather than values, so it's entirely possible that there's some higher level concept I'm missing).


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

https://reviews.llvm.org/D58453