[PATCH] D114362: [DebugInfo][4/N] Adjust x86 location-list tests for instruction referencing

Wed Nov 24 04:09:29 PST 2021

jmorse added a comment.

Orlando wrote:

> wouldn't we want to preserve the -experimental-debug-variable-locations=false coverage that these tests are currently (pre-patch) providing

Yes and no; I think it's a question of what parts of the codegen pipeline are being tested. Most of these tests are checking parts of the DWARF emitter so that, for example, basic block sections have location lists divided on block boundaries (I think that's what it's doing?). Which isn't something affected by instruction referencing at all. In an ideal world these things would be MIR tests feeding into the DWARF emitter, however I suspect they were written before MIR existed / was good enough.

sdag-combine.ll is testing something that's likely to diverge between instr-ref and DBG_VALUEs, I guess that part does need duplicate test coverage.

================
Comment at: llvm/test/DebugInfo/COFF/register-variables.ll:48
 ; ASM:         #DEBUG_VALUE: f:p <- $esi
-; ASM:         #DEBUG_VALUE: c <- $eax
+; ASM:         #DEBUG_VALUE: c <- undef
 ; ASM:         movl    %eax, %ecx
----------------
Orlando wrote:
> Non-blocking question: why do we get `c <- undef` here? The non-instr-ref location `c <- $eax` looks correct AFAICT.
Looking at the IR, there's some tail-folding happening, where two instances of:

  dbg.value
  tail call void @putint(

on either side of a branch get merged together. In DBG_VALUE mode, I believe this works by coincidence: the tails get merged, debug instructions ignored, and the variable location preserved because one code sequence is spliced into the common tail. If there was only a dbg.value on one side, we'd end up with a false location.

With instruction referencing there's still one DBG_INSTR_REF preserved, but LiveDebugValues can tell that it isn't dominated by the definition it's referring to, so drops the location.

IMO this is a good demonstration that instruction referencing soundly drops questionable locations; exactly how to fix it is a wider question, and probably needs a little more instrumentation. I'll fold a note in remarking on the limitation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114362