[WIP][DebugInfo] Distinguish reads/writes of VRegs when creating debug SDNodes

[jmorse]

Summary

This is a work-in-progress for a bug that only becomes visible via PR38754 [0] disabling placeDbgValues, put up for visibility rather than review.

When placeDbgValues is enabled, the dbg.value(%call2...) calls in this test get hoisted to a different BB. However when placeDbgValues is disabled, the DBG_VALUE for inlineinc:a sinks below the "add" it's above, and point at the result of the add rather than the true value of a, thus:

addl    $1, %eax
#DEBUG_VALUE: inlineinc:a <- $eax
#DEBUG_VALUE: inlineinc:b <- $eax

where the original code was "int b = a + 1".

There are two parts to this: first, the logic in resolveDanglingDebugInfo forces DBG_VALUEs to come after the IR-Order number that brings the Value into the DAG, to prevent use-before-def. This forces the dbg.values ahead of the add to sink below it, as seen here:

%3:gr32 = nsw ADD32ri8 %0:gr32(tied-def 0), 1, implicit-def dead $eflags
DBG_VALUE %0:gr32, $noreg, !"a", !DIExpression()
DBG_VALUE %3:gr32, $noreg, !"b", !DIExpression()

However, secondly, the "add" is selected as a two-operand add with a tied-def between %0 and %3. That forces %0 and %3 to be allocated the same phys-reg, and so the "a" DBG_VALUE winds up operating on the result of "add" rather than the input.

IMO the fault is resolveDanglingDebugInfo's shifting of the DBG_VALUE, which in turn is because the current debug-handling code doesn't appear to distinguish between when an SDNode is truly defining something, and when it's actually reading a VReg from outside the current BB. The fix in this patch examines the referenced SDNode: if it's a CopyFromReg we know that that VReg:

• Isn't modified in this BB or we would have emitted this debug node already
• Is live-in, because whatever triggered the dangling-debug-info resolution reads it, and thus
• It isn't a use-before-def to reference it earlier in the BB

And so we can put the DBG_VALUE in its originally desired location, for this test meaning ahead of the add.

Making such node-specific decisions seems hacky, and should probably be designed out, hence this is a WIP.

[0] https://bugs.llvm.org/show_bug.cgi?id=38754

[jmorse]

This problem had PR40427 hiding at the bottom of it, so this WIP isn't required.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (Val.getNode()) {
       if (!EmitFuncArgumentDbgValue(V, Variable, Expr, dl, false, Val)) {
         LLVM_DEBUG(dbgs() << "Resolve dangling debug info [order="
                           << DbgSDNodeOrder << "] for:\n  " << *DI << "\n");
         LLVM_DEBUG(dbgs() << "  By mapping to:\n    "; Val.dump());
         // Increase the SDNodeOrder for the DbgValue here to make sure it is
         // inserted after the definition of Val when emitting the instructions
         // after ISel. An alternative could be to teach
         // ScheduleDAGSDNodes::EmitSchedule to delay the insertion properly.
+
+        auto *Node = Val.getNode();
+        if (Node->getOpcode() == ISD::CopyFromReg) {
+          RegisterSDNode *RN = dyn_cast<RegisterSDNode>(Node->getOperand(1));
+          SDV = DAG.getVRegDbgValue(Variable, Expr, RN->getReg(), false, dl,
+                                    DbgSDNodeOrder);
+          DAG.AddDbgValue(SDV, nullptr, false);
+        } else {
           LLVM_DEBUG(if (ValSDNodeOrder > DbgSDNodeOrder) dbgs()
                      << "changing SDNodeOrder from " << DbgSDNodeOrder << " to "
                      << ValSDNodeOrder << "\n");
           SDV = getDbgValue(Val, Variable, Expr, dl,
                             std::max(DbgSDNodeOrder, ValSDNodeOrder));
           DAG.AddDbgValue(SDV, Val.getNode(), false);
+        }
       } else
         LLVM_DEBUG(dbgs() << "Resolved dangling debug info for " << *DI
                           << "in EmitFuncArgumentDbgValue\n");
     } else
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI << "\n");
   }
   DDIV.clear();
 }

test/DebugInfo/COFF/register-variables.ll

 ; ASM:         pushq   %rsi
 ; ASM:         subq    $32, %rsp
 ; ASM:         #DEBUG_VALUE: f:p <- $ecx
 ; ASM:         movl    %ecx, %esi
 ; ASM: [[p_ecx_esi:\.Ltmp.*]]:
 ; ASM:         #DEBUG_VALUE: f:p <- $esi
 ; ASM:         callq   getint
 ; ASM: [[after_getint:\.Ltmp.*]]:
-; ASM:         #DEBUG_VALUE: a <- $eax
-; ASM:         #DEBUG_VALUE: inlineinc:a <- $eax
-; ASM:         #DEBUG_VALUE: c <- $eax
 ; ASM:         testl   %esi, %esi
 ; ASM:         je      .LBB0_2
 ; ASM: [[after_je:\.Ltmp.*]]:
 ; ASM: # %bb.1:                                 # %if.then
 ; ASM-DAG:     #DEBUG_VALUE: inlineinc:a <- $eax
 ; ASM-DAG:     #DEBUG_VALUE: a <- $eax
 ; ASM-DAG:     #DEBUG_VALUE: f:p <- $esi
 ; ASM:         addl    $1, %eax
 ; ASM: [[after_inc_eax:\.Ltmp.*]]:
 ; ASM:         #DEBUG_VALUE: inlineinc:b <- $eax
-; ASM:         #DEBUG_VALUE: b <- $eax
 ; ASM:         addl    $1, x(%rip)
 ; ASM: [[after_if:\.Ltmp.*]]:
 ; ASM: .LBB0_2:                                # %if.else
 ; ASM:         #DEBUG_VALUE: f:p <- $esi
+; ASM:         #DEBUG_VALUE: c <- $eax
 ; ASM:         movl    %eax, %ecx
 ; ASM:         addq    $32, %rsp
 ; ASM:         popq    %rsi
 ; ASM: [[func_end:\.Ltmp.*]]:
 ; ASM:         jmp     putint                  # TAILCALL
+; ASM: [[func_finished:\.Ltmp.*]]:
 
 ; ASM:         .short  4414                    # Record kind: S_LOCAL
 ; ASM:         .asciz  "p"
 ; ASM:         .cv_def_range    .Lfunc_begin0 [[p_ecx_esi]], "A\021\022\000\000\000"
 ; ASM:         .cv_def_range    [[p_ecx_esi]] [[func_end]], "A\021\027\000\000\000"
 ; ASM:         .short  4414                    # Record kind: S_LOCAL
 ; ASM:         .asciz  "c"
-; ASM:         .cv_def_range    [[after_getint]] [[after_je]], "A\021\021\000\000\000"
+; ASM:         .cv_def_range    [[after_if]] [[func_finished]], "A\021\021\000\000\000"
 ; ASM:         .short  4414                    # Record kind: S_LOCAL
 ; ASM:         .asciz  "a"
-; ASM:         .cv_def_range    [[after_getint]] [[after_inc_eax]], "A\021\021\000\000\000"
-; ASM:         .short  4414                    # Record kind: S_LOCAL
-; ASM:         .asciz  "b"
-; ASM:         .cv_def_range    [[after_inc_eax]] [[after_if]], "A\021\021\000\000\000"
+; ASM:         .cv_def_range    [[after_je]] [[after_inc_eax]], "A\021\021\000\000\000"
 
 ; ASM:         .short  4429                    # Record kind: S_INLINESITE
 ; ASM:         .short  4414                    # Record kind: S_LOCAL
 ; ASM:         .asciz  "a"
-; ASM:         .cv_def_range    [[after_getint]] [[after_inc_eax]], "A\021\021\000\000\000"
+; ASM:         .cv_def_range    [[after_je]] [[after_inc_eax]], "A\021\021\000\000\000"
 ; ASM:         .short  4414                    # Record kind: S_LOCAL
 ; ASM:         .asciz  "b"
 ; ASM:         .cv_def_range    [[after_inc_eax]] [[after_if]], "A\021\021\000\000\000"
 ; ASM:         .short  4430                    # Record kind: S_INLINESITE_END
 
 ; OBJ: Subsection [
 ; OBJ:   SubSectionType: Symbols (0xF1)
 ; OBJ:   {{.*}}Proc{{.*}}Sym {

 ; OBJ:     Type: int (0x74)
 ; OBJ:     Flags [ (0x0)
 ; OBJ:     ]
 ; OBJ:     VarName: c
 ; OBJ:   }
 ; OBJ:   DefRangeRegisterSym {
 ; OBJ:     Register: EAX (0x11)
 ; OBJ:     LocalVariableAddrRange {
-; OBJ:       OffsetStart: .text+0xC
+; OBJ:       OffsetStart: .text+0x1A
 ; OBJ:       ISectStart: 0x0
-; OBJ:       Range: 0x4
+; OBJ:       Range: 0xC
 ; OBJ:     }
 ; OBJ:   }
 ; OBJ:   LocalSym {
 ; OBJ:     Type: int (0x74)
 ; OBJ:     Flags [ (0x0)
 ; OBJ:     ]
 ; OBJ:     VarName: a
 ; OBJ:   }
 ; OBJ:   DefRangeRegisterSym {
 ; OBJ:     Register: EAX (0x11)
 ; OBJ:     LocalVariableAddrRange {
-; OBJ:       OffsetStart: .text+0xC
-; OBJ:       ISectStart: 0x0
-; OBJ:       Range: 0x7
-; OBJ:     }
-; OBJ:   }
-; OBJ:   LocalSym {
-; OBJ:     Type: int (0x74)
-; OBJ:     Flags [ (0x0)
-; OBJ:     ]
-; OBJ:     VarName: b
-; OBJ:   }
-; OBJ:   DefRangeRegisterSym {
-; OBJ:     Register: EAX (0x11)
-; OBJ:     MayHaveNoName: 0
-; OBJ:       OffsetStart: .text+0x13
+; OBJ:       OffsetStart: .text+0x10
 ; OBJ:       ISectStart: 0x0
-; OBJ:       Range: 0x7
+; OBJ:       Range: 0x3
 ; OBJ:     }
 ; OBJ:   }
 ; OBJ:   InlineSiteSym {
 ; OBJ:     PtrParent: 0x0
 ; OBJ:     PtrEnd: 0x0
 ; OBJ:     Inlinee: inlineinc (0x1002)
 ; OBJ:   }
 ; OBJ:   LocalSym {
 ; OBJ:     Type: int (0x74)
 ; OBJ:     Flags [ (0x1)
 ; OBJ:       IsParameter (0x1)
 ; OBJ:     ]
 ; OBJ:     VarName: a
 ; OBJ:   }
 ; OBJ:   DefRangeRegisterSym {
 ; OBJ:     Register: EAX (0x11)
 ; OBJ:     LocalVariableAddrRange {
-; OBJ:       OffsetStart: .text+0xC
+; OBJ:       OffsetStart: .text+0x10
 ; OBJ:       ISectStart: 0x0
-; OBJ:       Range: 0x7
+; OBJ:       Range: 0x3
 ; OBJ:     }
 ; OBJ:   }
 ; OBJ:   LocalSym {
 ; OBJ:     Type: int (0x74)
 ; OBJ:     Flags [ (0x0)
 ; OBJ:     ]
 ; OBJ:     VarName: b
 ; OBJ:   }