[PATCH] D144800: [DebugInfo] Remove `dbg.addr` from CodeGen

Mon Feb 27 09:01:40 PST 2023

StephenTozer added inline comments.

================
Comment at: llvm/test/DebugInfo/X86/merge-equivalent-ranges.ll:31
   store i32 %CUID, ptr %CUID.addr, align 4
   call void @llvm.dbg.value(metadata ptr %CUID.addr, metadata !16, metadata !DIExpression(DW_OP_deref)), !dbg !17
   call void @llvm.dbg.value(metadata ptr %CUID.addr, metadata !26, metadata !DIExpression(DW_OP_deref, DW_OP_stack_value)), !dbg !17
----------------
jryans wrote:
> StephenTozer wrote:
> > jryans wrote:
> > > StephenTozer wrote:
> > > > For the purposes of this test, we can also remove this line and the test for "Var1" above - since this test is testing that this range merges with the one that is being deleted, the test isn't meaningful anymore; ideally we'd have a good way to test this otherwise, but don't consider it a blocker on this patch.
> > > Ah hmm, what if instead of removing the `dbg.addr` below, I were to replace it with `dbg.declare`...? Would that preserve the full meaning of this test?
> > If a `dbg.declare` exists for a variable, there should be no other debug intrinsics for that variable; I'm not sure if that would be validated, but it *shouldn't* happen naturally, so best not to test against it imo.
> Ah hmm, perhaps then we could repeat the `dbg.value` + `deref` from line 31 at the removed line 40 (effectively that's what the auto-upgraded `dbg.addr` would turn into anyway)...?
Also sadly not - the difference with dbg.addr is that we have `DBG_VALUE %r, $noreg, !"var1", !DIExpr(DW_OP_deref)` and `DBG_VALUE %r, 0, !"var1", !DIExpr()`; identical meanings, but different representations. That's what's being tested for here - that at some point the compiler recognizes the debug values mean the same thing and merges them. I wouldn't worry about this test blocking the patch - the right move might be to convert this to a MIR test with the instructions being explicit, but that can be done separately.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144800