[PATCH] D67393: [DebugInfo] LiveDebugValues: Defer all DBG_VALUE creation during analysis

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: aprantl, vsk, wolfgangp.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

As stated in the docs here [0], the meaning of a DBG_VALUE instruction changes after LiveDebugValues runs, from corresponding to a source-level assignment, to being a per-block variable location record [1]. Unfortunately, LiveDebugValues seems to mix these up and there's a feedback effect. The problem is this:

- Currently all register/stack transfers have per-block DBG_VALUE instructions created and inserted during the dataflow analysis [2],
- Sometimes variable locations are invalidated during the dataflow analysis [3],
- But the register/stack transfer DBG_VALUEs have already been created by then, and are interpreted as source-level assignments.

This means that a variable location that's propagated once (such as the first iteration where backedges are ignored) but invalidated on later iterations, can "latch" if it experiences a transfer in that time. Observe the test case in this patch, where in the loop block a value is shifted through four different registers (`esi` -> `edi` -> `ecx` -> `eax`). Currently the incoming location in `ecx` is transferred to `ebx` in the loop block, then invalidated on the second LiveDebugValues iteration through the loop because ecx is clobbered. However a DBG_VALUE for `ebx` has already been created by that point, and gets propagated into the exit block.

I've mentally divided this into two issues: ensuring that block-only DBG_VALUEs aren't interpreted as source-assignments, and deleting transfers of locations that are later invalidated. The former is easy to fix by moving the code in [2] out of the location propagation loop, which is what this patch does. Dealing with deleting transfers is in some more patches that are coming.

This patch also removes a condition in transferTerminator that doesn't update OutLocs if there are no locations at the end of processing a block. I think this made sense when we were only taking the union of OutLocs after propagation, but now we're deleting locations too, an empty set of OutLocs is something that needs to be handled. This is covered by the test added too.

[0] https://llvm.org/docs/SourceLevelDebugging.html#livedebugvalues-expansion-of-variable-locations
[1] I'm pretty confident of this, but as I wrote that paragraph of the docs this might be a circular argument.
[2] https://github.com/llvm/llvm-project/blob/5d9cd3b4ca457e55c3c21094cfea5e49dddef36c/llvm/lib/CodeGen/LiveDebugValues.cpp#L1365
[3] https://reviews.llvm.org/D66599


Repository:
  rL LLVM

https://reviews.llvm.org/D67393

Files:
  lib/CodeGen/LiveDebugValues.cpp
  test/DebugInfo/MIR/X86/live-debug-values-bad-transfer.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67393.219514.patch
Type: text/x-patch
Size: 5200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190910/560fe914/attachment.bin>