[PATCH] D68945: [DebugInfo] Don't translate dbg.addr and similar intrinsics into indirect DBG_VALUEs

Mon Oct 14 08:24:52 PDT 2019

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

This patch kills-off a significant user of the "IsIndirect" field of DBG_VALUE machine insts. Brought up in in this bug [0], I suggested that IsIndirect was redundant, and @dblaikie broadly agreed. IMHO IsIndirect is a bit of a liability because its current useage can mean two things:

- The IR input (dbg.declare/dbg.addr) wants the operand plus expression dereferenced,
- The register allocator spilt the operand, and wants a dereference between the spill-address-calculation and whatever the original expression was.

Better IMO to just eliminate the first use case by appending a DW_OP_deref to the expression when the DBG_VALUE is created -- there are no circumstances AFAIUI where the machine-lowering transforms needs to know whether the location a DBG_VALUE refers to will be interpreted as an address or not. In the future we can use the IsIndirect field for Better Things (TM), more on that in a future patch [1]. This patch isn't quite NFC as it exposes another bug (see below).

This patch has all creators of DBG_VALUEs append a DW_OP_deref to all dbg.declare/dbg.addr's they handle, which makes the DWARF expression backend emit a memory location, the same as the IsIndirect field. Some producers were also setting IsIndirect for constant-valued DBG_VALUEs, I've switched those to set IsIndirect to false.

SelectionDAGBuilder::EmitFuncArgumentDbgValue has code that theoretically could lower a dbg.declare onto multiple registers. I don't think this ever happens, and I've added an assertion; no test hits it, a clang-3.4 build for x86_64 doesn't hit it. I don't *think* a scenario where LLVM specifies something is in memory with dbg.declare but then the backend reckons it arrives in registers would make sense, but I'm no ABI expert.

I've stripped out the "IsIndirect" field throughout LiveDebugVariables as its use case is gone, and added an assertion that LiveDebugVariables doesn't see any incoming DBG_VALUEs with IsIndirect set.

There are a lot of test changes, they fall into four categories:

- Textual output change: a bunch of tests check what the textual assembly printer produces. If the IsIndirect field is set, the assembly printer writes "[$reg+0]" to represent indirection, wheras if there's a DW_OP_deref in the expression then it prints "[DW_OP_deref] $reg". Both produce the same DWARF. Tests that change in this way are:
  - test/CodeGen/ARM/debug-info-arg.ll
  - test/DebugInfo/ARM/PR16736.ll
  - test/DebugInfo/X86/dbg-addr-dse.ll
  - test/DebugInfo/X86/dbg-addr.ll
  - test/DebugInfo/X86/live-debug-vars-dse.mir
  - test/DebugInfo/X86/spill-indirect-nrvo.ll
  - test/DebugInfo/X86/spill-nontrivial-param.ll
- Internal representation changes (IsIndirect -> DW_OP_deref), which is just a different way of expressing the same thing.
  - test/CodeGen/AArch64/GlobalISel/debug-cpp.ll
  - test/CodeGen/AArch64/GlobalISel/debug-insts.ll
  - test/CodeGen/PowerPC/debuginfo-stackarg.ll
  - test/DebugInfo/ARM/float-stack-arg.ll
- There were a few tests where the input IR had a 'DW_OP_deref' in a dbg.declare, which doesn't play well with this implementation. I don't think it makes sense for dbg.declare to have a deref in its expression, so in these tests I've deleted it.
  - test/DebugInfo/X86/op_deref.ll
  - test/DebugInfo/X86/parameters.ll
  - test/DebugInfo/X86/safestack-byval.ll
  - test/DebugInfo/X86/vla.ll <--- I also added a llvm-dwarfdump check that the DWARF output is right

One COFF/CodeView test changes. This too had an excess DW_OP_deref in the input, but, fixing that causes the type name in the CodeView output to change (see diff). The new type appears correct (the function argument is a struct, not a reference), but I know nothing about CodeView (paging @rnk)

- test/DebugInfo/COFF/pieces.ll

I've added XFails to two tests -- this is because of PR41992 [2], which @Orlando has a patch for somewhere.

I haven't added any new test, this is an internal representation change so I don't believe one is necessary.

[0] https://bugs.llvm.org/show_bug.cgi?id=41675#c9
[1] Signalling more information about spill locations between LiveDebugVariables and LiveDebugValues.
[2] https://bugs.llvm.org/show_bug.cgi?id=41992

Repository:
  rL LLVM

https://reviews.llvm.org/D68945

Files:
  lib/CodeGen/GlobalISel/IRTranslator.cpp
  lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
  lib/CodeGen/LiveDebugVariables.cpp
  lib/CodeGen/SelectionDAG/FastISel.cpp
  lib/CodeGen/SelectionDAG/InstrEmitter.cpp
  lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  test/CodeGen/AArch64/GlobalISel/debug-cpp.ll
  test/CodeGen/AArch64/GlobalISel/debug-insts.ll
  test/CodeGen/ARM/debug-info-arg.ll
  test/CodeGen/PowerPC/debuginfo-stackarg.ll
  test/DebugInfo/ARM/PR16736.ll
  test/DebugInfo/ARM/float-stack-arg.ll
  test/DebugInfo/COFF/pieces.ll
  test/DebugInfo/X86/dbg-addr-dse.ll
  test/DebugInfo/X86/dbg-addr.ll
  test/DebugInfo/X86/live-debug-vars-dse.mir
  test/DebugInfo/X86/op_deref.ll
  test/DebugInfo/X86/parameters.ll
  test/DebugInfo/X86/safestack-byval.ll
  test/DebugInfo/X86/spill-indirect-nrvo.ll
  test/DebugInfo/X86/spill-nontrivial-param.ll
  test/DebugInfo/X86/vla.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68945.224846.patch
Type: text/x-patch
Size: 31039 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191014/0b6f98dd/attachment.bin>