[PATCH] D43956: [DebugInfo] Discard invalid DBG_VALUE instructions in LiveDebugVariables

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

**What I see now in test/DebugInfo/X86/dbg-value-inlined-parameter.ll**

Input to ISel is OK as far as I can see:

  define void @foobar() #0 !dbg !28 {
  entry:
    tail call void @llvm.dbg.value(metadata %struct.S1* @p, metadata !19, metadata !DIExpression()) #3, !dbg !31
    tail call void @llvm.dbg.value(metadata i32 1, metadata !21, metadata !DIExpression()) #3, !dbg !34
    store i32 1, i32* getelementptr inbounds (%struct.S1, %struct.S1* @p, i64 0, i32 1), align 8, !dbg !35
    %call.i = tail call float* @bar(i32 1) #4, !dbg !36
    store float* %call.i, float** getelementptr inbounds (%struct.S1, %struct.S1* @p, i64 0, i32 0), align 8, !dbg !36
    ret void, !dbg !37
  }

but then after ISel we get

  *** MachineFunction at end of ISel ***
  # Machine code for function foobar: IsSSA, TracksLiveness
  
  bb.0.entry:
    DBG_VALUE 1, debug-use $noreg, !"nums", !DIExpression(), debug-location !34; line no:7
    DBG_VALUE debug-use %0:gr64, debug-use $noreg, !"sp", !DIExpression(), debug-location !31; line no:7
    %0:gr64 = MOV64rm $rip, 1, $noreg, target-flags(x86-gotpcrel) @p, $noreg, debug-location !35; mem:LD8[GOT]
    MOV32mi %0:gr64, 1, $noreg, 8, $noreg, 1, debug-location !35; mem:ST4[getelementptr inbounds (%struct.S1, %struct.S1* @p, i64 0, i32 1)](align=8)
    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !36
    %1:gr32 = MOV32ri 1, debug-location !35
    $edi = COPY %1:gr32, debug-location !36
    CALL64pcrel32 @bar, <regmask $bh $bl $bp $bpl $bx $ebp $ebx $rbp $rbx $r12 $r13 $r14 $r15 $r12b $r13b $r14b $r15b $r12d $r13d $r14d $r15d $r12w $r13w $r14w $r15w>, implicit $rsp, implicit $ssp, implic
  it $edi, implicit-def $rsp, implicit-def $ssp, implicit-def $rax, debug-location !36
    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !36
    %2:gr64 = COPY $rax, debug-location !36
    MOV64mr %0:gr64, 1, $noreg, 0, $noreg, %2:gr64, debug-location !36; mem:ST8[getelementptr inbounds (%struct.S1, %struct.S1* @p, i64 0, i32 0)]
    RET 0, debug-location !37

So for some reason the

  DBG_VALUE debug-use %0:gr64

ends up before

  %0:gr64 = MOV64rm $rip, 1, $noreg, target-flags(x86-gotpcrel) @p, $noreg, debug-location !35; mem:LD8[GOT]

As far as I can see this is a ISel problem. I haven't tracked down exactly how this happens.

**Historical debugging**

I was actually hunting similar problems to this, with ISel inserting DBG_VALUE in the wrong location, a few months ago (unfortunately haven't had time to finalize the work...).
If I remember correctly there are some problems in ScheduleDAGSDNodes.cpp (methods ScheduleDAGSDNodes::EmitSchedule, ProcessSourceNode, ProcessSDDbgValues).

For example ProcessSourceNode is selecting between three different choices:
a) process SDDbgValue even if the node does not have any order (use IROrder 0)
b) save it in Orders and associate with nullptr
c) save it in Orders and associate with prev, and process SDDbgValue (using given IROrder)

I think the conditions for the (b) choice could cause problems sometimes, as it looks for PHI-nodes, but does not take any already inserted DBG_VALUES into consideration (it should skip past debug values somehow).
And I think processing the SDDbgValue also in situation (b) could improve things.

The ProcessSDDbgValues is having a condition like this

  if (!Order || DVOrder == Order)

but we could get there also with DVOrder < Order (don't remember the exact situation, but I suspected that

  if (!Order || DVOrder <= Order)

would be better.

I think that some of the problems might originate from situations where SelectionDAGBuilder::visitIntrinsicCall creates DanglingDebugInfo and then the IROrder is weird or missing (don't remember exactly).

**Summary**

Code like the one in test/DebugInfo/X86/dbg-value-inlined-parameter.ll (I actually made a C-reproducer ending up in such IR as well) seems to be lowered incorrectly by ISel.
I was analysing problem were ISel lowering caused similar problems a few month ago.
And the RUST-based code in https://bugs.llvm.org/show_bug.cgi?id=36417 also suffered from ISel creating "use before def" situations when inserting DBG_VALUE nodes.

So I guess I need to dig up that old patch I was working on in ScheduleDAGSDNodes.cpp to see if it helps...


Repository:
  rL LLVM

https://reviews.llvm.org/D43956