[PATCH] D88896: [DebugInfo][InstrRef][3/4] Produce DBG_INSTR_REFs for all* variable locations

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added a reviewer: debug-info.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
jmorse requested review of this revision.

This patch emits DBG_INSTR_REFs for the final two flavours of variable locations that couldn't be handled before: copies, and inter-block VRegs.  There are still some variable locations that are represented by DBG_VALUE instructions, which are:

- Constants,
- Frame-index references,
- Physical registers.

The first two are independent of any optimisations that occur, and don't really need to change. DBG_VALUEs of physregs are still emitted for function parameters, and I haven't been brave enough to go anywhere near that situation. Again, such DBG_VALUEs aren't actually affected by optimisations due to their placement and their physreg operands never being optimised.

For variable locations that refer to values defined in different blocks, it's slightly tricky: such values are given cross-block virtual registers before instruction selection begins, but there isn't necessarily an instruction defining them available: I don't believe there's a guarantee on the order in which blocks are instruction-selected and emitted. To get around this: if we can't immediately identify the defining instruction of a VReg, emit a "half done" DBG_INSTR_REF, where the first operand identifies the VReg it's associated with. Then at a later point in ISel, after all blocks are emitted to MIR, find the defining instruction and mutate the DBG_INSTR_REF to refer to it. This is a hack; but either an ordering guarantee has to be put on block emission (which I imagine could involve every CodeGen test needing updating) or some kind of intermediate form is needed until all the information is available.

Copies: this is one of the things in the original RFC that I complained about: having the same value describable in multiple locations is unpleasent. I would like this instruction referencing work to put us in a position where we never refer to copy instructions at all. They wouldn't work well with the InstrRefBased LiveDebugValues implementation anyway, as copies don't define any values, they only move them.

The solution to this is the salvageCopySSA method added in MachineFunction.  This interprets:

- COPYs
- SUBREG_TO_REG
- Anything identifiable by the targets isCopyInstr method

And follows chains of copies back to the defining instruction that they read from. This relies on an assumption: that any copies from physical registers originate from a preceeding instruction in the same block, or is a parameter. Take a look at the tests added in instr-ref-selectiondag.ll: due to the way SelectionDAG operates, whenever an instruction defs a physical register we copy it to a VReg; salvageCopySSA walks backwards from the COPY-from-rax to find the defining instruction. The exception is arguments: those never have a defining instruction, so we have to drop a DBG_PHI at the entry block. (Right now we drop one DBG_PHI for every variable that reads an Argument; I'll revise this with a cache in a future revision of  this patch).

We might end up having to have a similar "Salvage copy" operation for after regalloc, but we'll cross that bridge later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88896

Files:
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
  llvm/lib/CodeGen/SelectionDAG/InstrEmitter.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/test/DebugInfo/X86/instr-ref-selectiondag.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88896.296440.patch
Type: text/x-patch
Size: 27456 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201006/8ec6d0d4/attachment-0001.bin>