[PATCH] D69178: [DebugInfo] Use DBG_VALUEs IsIndirect field for describing stack spills

Fri Oct 18 09:15:45 PDT 2019

jmorse created this revision.
jmorse added reviewers: aprantl, vsk, wolfgangp.
Herald added subscribers: llvm-commits, steven.zhang, jsji, MaskRay, kristof.beyls, nemanjai, qcolombet.
Herald added a project: LLVM.

This is a patch that significantly changes the meaning of the IsIndirect field of DBG_VALUEs. The objective here is to pass more stack-spill information down from LiveDebugVariables to LiveDebugValues: at the moment, if a DBG_VALUE of a stack slot is created by LiveDebugVariables recording a stack spill, then LiveDebugValues interprets it as a register location, not a spill location (see the added test, dbg-value-when-spilt.ll).

Rather than just using IsIndirect to indicate "memory location please" to DwarfExpression.cpp, with this patch it means:

- Between LiveDebugVariables and PrologEpilog and where operand 0 is a stack slot, $noreg means this is a stack _pointer_ variable, immediate means it's a spill location.
- Between PrologEpilog and LiveDebugValues, for spill locations this patch places the frame-index of the spill into the IsIndirect field, sets it to $noreg for everything else.
- IsIndirect is invalid / must be $noreg everywhere else.

This means that when LiveDebugValues handles spill-location DBG_VALUEs, it can extract the frame offset and slot size from the frame index and create a SpillLocKind VarLoc instead of a register location based on the stack pointer. As a result, we can recognise loads from spills that we couldn't before, and, we can access a DIExpression that doesn't have the stack-offset already baked into it. Finally, LiveDebugValues can now propagate stack-spill locations over instructions that modify the stack pointer (like push) as shown in the new dbg-value-of-spill-with-fp.ll.

(This last improvement might be a bit debatable as the debugger would need to be aware that the stack pointer has changed. Ignoring stack-pointer writes for spills is what we do already for LiveDebugValues detected spills. I couldn't quickly convince gcc to cough up a location list with no frame pointer in a function where the stack pointer was modified, so can't show that anyone else does this).

There's a downside however: in some circumstances we newly recognise a spill restore in the middle of a loop, restore it, and then drop both the spilt and restored location for the whole loop because there isn't a single location on entering the head of the loop. For an example, see test/DebugInfo/X86/op_deref.ll where this now happens, I've marked the test XFail until there's been some discussion about this. Happily I think we can say that this is moving LiveDebugValues into the domain of having to make better choices about which location to pick, instead of just trying to stop it producing bad locations! My feeling is that there's a hierarchy of duration, of {volatile-reg, nonvolatile-reg, stack, entry-value}, and it might be worthwhile building this into LiveDebugValues. (Consider also PR43373 [0] which exhibits problems with mixing "long term" and "short term" locations).

Unfortunately there are negligable differences in location coverage on a clang-3.4 build, probably due to a mixture of the above improvements and downsides. This patch might also be too much churn; IMHO getting this information down to LiveDebugValues is something we should aim to do though. If we're going to be aware of spills/restores in LiveDebugValues then we want to know about what was spilt during LiveDebugVariables. Right now, that would have to involve interpreting a DIExpression, which IMO is undesirable.

I've added three new flavours of tests:

- The whole backend of LLVM-IR to dwarf "spill and restore of an arg" performed by regalloc should be tested for. test/DebugInfo/X86/spill-indirect-nrvo.ll does most of this at the moment, I've added a check that a restore happens.
- A DBG_VALUE that happens in the range where a Value is spilt should be dealt with in the same way. dbg-value-when-spilt.ll covers this.
- For not closing spill locations when we def the stack pointer, dbg-value-of-spill-with-fp.ll

There are a bunch of uninteresting test changes; the "standard" updates to inputs and changes in outputs are:

- I've updated MIR inputs to use frame indexes in the IsIndirect field rather than having offsets in DIExpressions and IsIndirect=0
- Checks for after LiveDebugValues now expect IsIndirect to never be set, and every memory location to explicitly have a DW_OP_deref
- Textual assembly output changes same as D68945 <https://reviews.llvm.org/D68945>

The uninteresting standard changes are:
test/DebugInfo/MIR/X86/live-debug-values-3preds.mir:
test/DebugInfo/X86/spill-indirect-nrvo.ll:
test/DebugInfo/X86/spill-nontrivial-param.ll:
test/DebugInfo/X86/spill-nospill.ll:
test/DebugInfo/PowerPC/live-debug-vars-subreg-offset.ll:
test/DebugInfo/X86/fission-ranges.ll:
test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir: 
test/DebugInfo/MIR/X86/kill-after-spill.mir:
test/DebugInfo/MIR/X86/live-debug-values-restore-collide.mir
test/DebugInfo/X86/pieces-4.ll:
test/DebugInfo/MIR/X86/live-debug-values-spill.mir: <--- I also set a bunch of _'s to explicitly be $noreg
test/DebugInfo/MIR/X86/prolog-epilog-indirection.mir: <-- this test is mostly checking stack slot pointers, not spilt values
test/DebugInfo/MIR/X86/live-debug-values-restore.mir: <--- and now gets the deref in the right location
test/DebugInfo/MIR/X86/live-debug-values-stack-clobber.mir: <--- I added some more expression testing
test/DebugInfo/X86/prolog-params.mir: <--- contains some negative FrameIndexes that get printed positive

More interesting but sound updates:
test/CodeGen/X86/prologepilog_deref_size.mir:
prologepilog no longer produces completely lowered DBG_VALUEs. Instead, I've just let the test run until after LiveDebugValues, and left the checks the same.

test/DebugInfo/COFF/pieces.ll:
We now recognise a restore to eax prior to the end of the function. Which could be considered better.

test/DebugInfo/X86/dbg-declare-arg.ll:
This dbg.declare gets lowered to a single DBG_VALUE in a stack slot, which is then restored.

test/DebugInfo/X86/pr34545.ll:
This wants to check for spill expression, I've let it run til after LiveDebugValues not LiveDebugVars

test/DebugInfo/X86/parameters.ll:
We pick up a restore here that we didn't before, on account of not recognising when a DBG_VALUE was a spill.

The sketchier test updates:
test/DebugInfo/X86/sret.ll:
We recognise a restore now, but the hash of the split-dwarf file changes. I think this is expected, but know nothing of split-dwarf.

test/DebugInfo/X86/op_deref.ll:
I've left this XFail, because I anticipate people will have opinions on how we should go about deciding when to restore from spill and when to not.

test/CodeGen/MIR/X86/diexpr-win32.mir:
I've updated the DBG_VALUEs here to use stack slots, but one of the output offsets (-4) doesn't correspond to any actual stack slot. I've updated checks for the current output, but it's not really a sound update. Problems with this test:

- It dbg.declares an arbitrary thing in memory
- it doesn't dbg.declare a pointer
- We would now never generate this IR or MIR, I think.

My preference would be to delete the sketchy function in this test; or at the least replace the DBG_VALUE with one that isn't impersonating a stack spill.

Repository:
  rL LLVM

https://reviews.llvm.org/D69178

Files:
  lib/CodeGen/LiveDebugValues.cpp
  lib/CodeGen/LiveDebugVariables.cpp
  lib/CodeGen/PrologEpilogInserter.cpp
  test/CodeGen/MIR/X86/diexpr-win32.mir
  test/CodeGen/X86/prologepilog_deref_size.mir
  test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir
  test/DebugInfo/COFF/pieces.ll
  test/DebugInfo/MIR/X86/kill-after-spill.mir
  test/DebugInfo/MIR/X86/live-debug-values-3preds.mir
  test/DebugInfo/MIR/X86/live-debug-values-restore-collide.mir
  test/DebugInfo/MIR/X86/live-debug-values-restore.mir
  test/DebugInfo/MIR/X86/live-debug-values-spill.mir
  test/DebugInfo/MIR/X86/live-debug-values-stack-clobber.mir
  test/DebugInfo/MIR/X86/prolog-epilog-indirection.mir
  test/DebugInfo/PowerPC/live-debug-vars-subreg-offset.ll
  test/DebugInfo/X86/dbg-declare-arg.ll
  test/DebugInfo/X86/dbg-value-of-spill-with-fp.ll
  test/DebugInfo/X86/dbg-value-when-spilt.ll
  test/DebugInfo/X86/fission-ranges.ll
  test/DebugInfo/X86/op_deref.ll
  test/DebugInfo/X86/parameters.ll
  test/DebugInfo/X86/pieces-4.ll
  test/DebugInfo/X86/pr34545.ll
  test/DebugInfo/X86/prolog-params.mir
  test/DebugInfo/X86/spill-indirect-nrvo.ll
  test/DebugInfo/X86/spill-nontrivial-param.ll
  test/DebugInfo/X86/spill-nospill.ll
  test/DebugInfo/X86/sret.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69178.225644.patch
Type: text/x-patch
Size: 61387 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191018/bcb1c40c/attachment-0001.bin>