[PATCH] D54715: [WIP][DebugInfo] Distinguish reads/writes of VRegs when creating debug SDNodes

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
Herald added a subscriber: llvm-commits.

This is a work-in-progress for a bug that only becomes visible via PR38754 [0] disabling placeDbgValues, put up for visibility rather than review.

When placeDbgValues is enabled, the dbg.value(%call2...) calls in this test get hoisted to a different BB. However when placeDbgValues is disabled, the DBG_VALUE for inlineinc:a sinks below the "add" it's above, and point at the result of the add rather than the true value of a, thus:

  addl    $1, %eax
  #DEBUG_VALUE: inlineinc:a <- $eax
  #DEBUG_VALUE: inlineinc:b <- $eax

where the original code was "int b = a + 1".

There are two parts to this: first, the logic in resolveDanglingDebugInfo forces DBG_VALUEs to come after the IR-Order number that brings the Value into the DAG, to prevent use-before-def. This forces the dbg.values ahead of the add to sink below it, as seen here:

  %3:gr32 = nsw ADD32ri8 %0:gr32(tied-def 0), 1, implicit-def dead $eflags
  DBG_VALUE %0:gr32, $noreg, !"a", !DIExpression()
  DBG_VALUE %3:gr32, $noreg, !"b", !DIExpression()

However, secondly, the "add" is selected as a two-operand add with a tied-def between %0 and %3. That forces %0 and %3 to be allocated the same phys-reg, and so the "a" DBG_VALUE winds up operating on the result of "add" rather than the input.

IMO the fault is resolveDanglingDebugInfo's shifting of the DBG_VALUE, which in turn is because the current debug-handling code doesn't appear to distinguish between when an SDNode is truly defining something, and when it's actually reading a VReg from outside the current BB. The fix in this patch examines the referenced SDNode: if it's a CopyFromReg we know that that VReg:

- Isn't modified in this BB or we would have emitted this debug node already
- Is live-in, because whatever triggered the dangling-debug-info resolution reads it, and thus
- It isn't a use-before-def to reference it earlier in the BB

And so we can put the DBG_VALUE in its originally desired location, for this test meaning ahead of the add.

Making such node-specific decisions seems hacky, and should probably be designed out, hence this is a WIP.

[0] https://bugs.llvm.org/show_bug.cgi?id=38754


Repository:
  rL LLVM

https://reviews.llvm.org/D54715

Files:
  lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  test/DebugInfo/COFF/register-variables.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54715.174642.patch
Type: text/x-patch
Size: 5802 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181119/f510e444/attachment.bin>