[PATCH] D68945: [DebugInfo] Don't translate dbg.addr and similar intrinsics into indirect DBG_VALUEs
Reid Kleckner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 14 11:49:08 PDT 2019
rnk added a comment.
This makes sense to me, we should be able to replace isIndirect with DW_OP_deref.
I checked the test case updates, they look good.
One concern that I have about folding everything into the DIExpression is that they are effectively leaked. For every unique variable location description, which now includes offsets and indirection, we make a DIExpression and never clean it up. This is probably not significant in overall compilation, but it feels sloppy to me. As a future alternative, maybe we could use the same representation (sequence of DWARF opcodes), but store them as MachineInstr operands, so that they get cleaned up after emitting the MachineFunction.
================
Comment at: test/DebugInfo/COFF/pieces.ll:208
; OBJ: }
; OBJ: DefRangeRegisterRelSym {
; OBJ: BaseRegister: RCX (0x14A)
----------------
This change looks good to me. The reference usage came about in D36907, which shouldn't apply in this case. The DefRangeRegisterRel record below already implies that the variable `o` lives in memory pointed to by RCX, which is correct, `o` is passed indirectly and it's address lives in RCX.
We started using references sometimes in D36907 as a way to squeeze out one more level of indirection through memory from this format. We'd use the reference if RCX was spilled, for example. In that case, we'd say that `o` was a reference, it lives *in* RCX, and later is spilled to stack slot offset N, and the debugger will display `o` as expected, even though it's the wrong type at the source level.
================
Comment at: test/DebugInfo/X86/dbg-addr-dse.ll:6-8
+; XFAIL: *
+; See PR41992, the third and final dbg.value disappears after
+; LiveDebugVariables.
----------------
I just want to make sure that this gets addressed soon-ish, I suppose before the next release. I see that you said @Orlando has a patch for it.
I don't think it will cause any issues for Chromium, because we still set -instcombine-lower-dbg-declare=0, so things that are escaped are mainly described as stack locations in the side table. But, if we ever want to get off that flag, I want to make sure that LLVM can reliably track escaped variables.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68945/new/
https://reviews.llvm.org/D68945
More information about the llvm-commits
mailing list