[PATCH] D65368: [DebugInfo] LiveDebugValues: Don't drop fragment information when restoring spills

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

> Naive question: Why doesn't the restore recognizer insert the exact same expression that was found by the spill recongizer, regardless of what expression is used to describe the spill? Or: what happens on this example with the +1 expression without this patch?

Great question; each "open"/valid variable location is maintained as a VarLoc object [0], which stores the machine location and variable identity of the DBG_VALUE being tracked. That doesn't include the expression, instead that resides in the tracked DBG_VALUE inst [1]. After a spill, the DBG_VALUE being tracked and corresponding VarLoc object describe the spill location, and there's no connection back to the unspilt DBG_VALUE, illustrated here:

  DBG_VALUE $rax, !1, !DIExpression(DW_OP_LLVM_fragment, 0, 32)
  MOV64rm $rsp(-8), $rax
  DBG_VALUE $rsp, !1, !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_LLVM_fragment, 0, 32) ; <--- VarLoc and its MI field point here, no link to earlier DBG_VALUE
  call @somesymbol
  MOV64mr $rax, $rsp(-8)
  DBG_VALUE $rax, !1, !DIExpression()

In the +1 example without this patch, an empty expression is created on restore, as there's no attempt to recover the original expression.

This could be fixed, possibly easily, by adding a DIExpression field to VarLoc and using that expression on restore. I've shied away from this as it could reduce performance, and the risk that adding new features/facilities won't be acceptable to merge into the LLVM-9 branch to fix the upcoming release. (I've no experience with what is or isn't seen as acceptable).

I suppose another alternative is, rather than having the VarLoc for a spill hold a reference to the DBG_VALUE for the spilt location, we could instead point at the unspilt DBG_VALUE, i.e. instead of the earlier illustration we would instead have

  DBG_VALUE $rax, !1, !DIExpression(DW_OP_LLVM_fragment, 0, 32)                               ; <--- VarLoc.MI points here
  MOV64rm $rsp(-8), $rax
  DBG_VALUE $rsp, !1, !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_LLVM_fragment, 0, 32) ; <--- VarLocs other fields record this variable location
  call @somesymbol
  MOV64mr $rax, $rsp(-8)
  DBG_VALUE $rax, !1, !DIExpression()

which would give enough information to recover the original expression. Taking this option would require other bits of LiveDebugValues to recognise when they're manipulating a spilt location, and manually add the spill offset to the expression. The code the propagates locations between blocks [2] would need that, probably nothing else. There's also the risk that there's some other subtle detail that I haven't accounted for -- but it does mean no crash, no reduction in locations, and no digging through DIExpressions. I'll take a shot at prototyping this.

[0] https://github.com/llvm/llvm-project/blob/2bea69bf6503ffc9f3cde9a52b5dac1a25e94e1c/llvm/lib/CodeGen/LiveDebugValues.cpp#L171
[1] https://github.com/llvm/llvm-project/blob/2bea69bf6503ffc9f3cde9a52b5dac1a25e94e1c/llvm/lib/CodeGen/LiveDebugValues.cpp#L656 <--- line 629 produces the DebugInstr pointer from a VarLoc object
[2] https://github.com/llvm/llvm-project/blob/2bea69bf6503ffc9f3cde9a52b5dac1a25e94e1c/llvm/lib/CodeGen/LiveDebugValues.cpp#L1111


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

https://reviews.llvm.org/D65368