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

[Yury Delendik via Phabricator via llvm-commits]

yurydelendik added a comment.

In D58453#1582934 <https://reviews.llvm.org/D58453#1582934>, @jmorse wrote:

> I've added some changes to the patch that makes collectDebugValues behave in the way I described, returning all debug users of a VReg, optionally limited to only the basic block of the source MachineInst. This should be useful for testing; and it's immediately caused the test/DebugInfo/WebAssembly/dbg-value-move-reg-stackify.mir [0] test to fail. The output is:
>
>   %0:i64 = ARGUMENT_i64 0, implicit $arguments
>   dead %1:i32 = I32_WRAP_I64 %0, implicit-def dead $arguments
>   dead %1:i32 = CALL_i32 @bar, implicit-def dead $arguments, implicit $sp32, implicit $sp64
>   %2:i32 = CALL_i32 @bar, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def $value_stack, implicit $value_stack
>   DBG_VALUE %2, $noreg, <0x5490d00>, !DIExpression(), debug-location !DILocation(line: 359, column: 12, scope: <0x548f5f0>)
>   DBG_VALUE %2, $noreg, <0x5490cb0>, !DIExpression(), debug-location !DILocation(line: 358, column: 12, scope: <0x548f5f0>)
>   DBG_VALUE %2, $noreg, <0x5490a50>, !DIExpression(), debug-location !DILocation(line: 357, column: 12, scope: <0x548f5f0>)
>   CALL_VOID @foo, %2, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def $value_stack, implicit $value_stack
>   RETURN_VOID implicit-def dead $arguments
>   
>
> While I don't know webassembly, it seems clear that a) the test post-SSA, and b) the test expects collectDebugValues to only return DBG_VALUEs that refer to the same vreg def that the source instruction does.
>
> Is that a fair assessment, or some kind of test artefact? That behaviour is almost certainly achievable after this patch lands, although it might require more plumbing somewhere.
>
> [0] https://github.com/llvm/llvm-project/blob/a1d97a960e622ee21550d92809512cb0870be499/llvm/test/DebugInfo/WebAssembly/dbg-value-move-reg-stackify.mir


Looks like I misunderstood meaning of *all*. I assumed that means DBG_VALUEs the refer the vreg def of the source instruction, excluding DBG_VALUEs that refer the same vreg def by the above instructions (in the same block). The test result above shows that collectDebugValues locates all DBG_VALUEs for the instruction even if it does not (re)define the value, but there are few instructions in the same block above that do. Is this the intended effect of this patch? If yes, we probably need to create more robust collectDebugValues-like method that will track lifetime of DBG_VALUEs.


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

https://reviews.llvm.org/D58453