[DebugInfo][InstrRef] Fix a tombstone-in-DenseMap crash from D117877

[jmorse]

Summary

This is a follow up to D117877: variable assignments of DBG_VALUE $noreg, or DBG_INSTR_REFs where no value can be found, are represented as a DbgValue object with Kind "Undef". This explicitly represents "There is no value for this, and never can be". In D117877 I added a special-case to make some assignment accounting faster, without considering this scenario. It causes variables to be given the value ValueIDNum::EmptyValue, which would fail gracefully and wouldn't produce an illegal location, it'd just be discarded. However, recently that value began being entered into a DenseMap, where it's a tombstone value, causing assertions.

This patch handles that scenario and adds a regression test. The more general fix is to wrap DbgValue's ID field in Optional<>, just like the MachineOperand field is, but I'm aiming for the minimal fix right now :(

[StephenTozer]

Seems reasonable as a fix to me. Is there any particular reason that the undef value test couldn't exist within the original function, or are they separated to look cleaner? YMMV and I don't feel strongly either way, so LGTM.

[jmorse]

In D118715#3289898, @StephenTozer wrote:

Is there any particular reason that the undef value test couldn't exist within the original function, or are they separated to look cleaner? YMMV and I don't feel strongly either way, so LGTM.

I suppose if the DBG_VALUE $noreg was given for a different variable then that would work, I just didn't think of that. It would indeed be slightly simpler, but IMO not simpler enough to revise.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

void InstrRefBasedLDV::placePHIsForSingleVarDefinition(
   //  * Specify there's no variable value in blocks past the frontier.
   // This is a common case, hence it's worth special-casing it.
 
   // Pick out the variables value from the block transfer function.
   VLocTracker &VLocs = AllTheVLocs[AssignMBB->getNumber()];
   auto ValueIt = VLocs.Vars.find(Var);
   const DbgValue &Value = ValueIt->second;
 
+  // If it's an explicit assignment of "undef", that means there is no location
+  // anyway, anywhere.
+  if (Value.Kind == DbgValue::Undef)
+    return;
+
   // Assign the variable value to entry to each dominated block that's in scope.
   // Skip the definition block -- it's assigned the variable value in the middle
   // of the block somewhere.
   for (auto *ScopeBlock : InScopeBlocks) {
     if (!DomTree->properlyDominates(AssignMBB, ScopeBlock))
       continue;
 
     Output[ScopeBlock->getNumber()].push_back({Var, Value});

llvm/test/DebugInfo/MIR/InstrRef/single-assign-propagation.mir

 # RUN: llc %s -march=x86-64 -run-pass=livedebugvalues -o - \
 # RUN:       -experimental-debug-variable-locations=true \
-# RUN:     | FileCheck %s -implicit-check-not=DBG_VALUE
+# RUN:     | FileCheck %s -implicit-check-not=DBG_VALUE \
+# RUN:        --check-prefixes=CHECK,COMMON
 # RUN: llc %s -march=x86-64 -run-pass=livedebugvalues -o - \
 # RUN:       -experimental-debug-variable-locations=false \
-# RUN:     | FileCheck %s --check-prefixes=VARLOC -implicit-check-not=DBG_VALUE
+# RUN:     | FileCheck %s -implicit-check-not=DBG_VALUE \
+# RUN:        --check-prefixes=VARLOC,COMMON
 #
 # This test is designed to stimulate a simplification of variable-value
 # propagation in InstrRefBasedLDV. When we only have a single assignment of
 # a variable, we don't have to completely enumerate all the PHIs the variable
 # has and then value propagate through them: the variable value is available
 # only in those blocks dominated by the assignment. Example in this test:
 #
 #             entry

 #
 # VARLOC-LABEL: bb.1:
 # VARLOC:       DBG_VALUE
 # VARLOC-LABEL: bb.2:
 ## No location here because it's out-of-scope.
 # VARLOC-LABEL: bb.3:
 # VARLOC:       DBG_VALUE
 #
+## Common tail for 'test2' -- this is checking that the assignment of undef or
+## $noreg in single-assignment mode doesn't lead to trouble further down the
+## line, specifically assertion failures.
+#
+# COMMON-LABEL: name: test2
+# COMMON: DBG_VALUE $noreg
 --- |
   define i32 @_Z8bb_to_bb() local_unnamed_addr !dbg !12 {
   entry:
     ret i32 0, !dbg !17
   }
 
+  define i32 @test2() local_unnamed_addr !dbg !112 {
+  entry:
+    ret i32 0, !dbg !117
+  }
+
   !llvm.dbg.cu = !{!0}
   !llvm.module.flags = !{!7, !8, !9, !10}
   !llvm.ident = !{!11}
 
   !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3, debugInfoForProfiling: true, nameTableKind: None)
   !1 = !DIFile(filename: "main.cpp", directory: "F:\")
   !2 = !{}
   !3 = !{!4}
--- |
   !13 = !DISubroutineType(types: !14)
   !14 = !{!6, !6}
   !15 = !{!16}
   !16 = !DILocalVariable(name: "myVar", scope: !18, file: !1, line: 7, type: !6)
   !17 = !DILocation(line: 10, scope: !18)
   !18 = distinct !DILexicalBlock(scope: !12, file: !1, line: 1, column: 1)
   !19 = distinct !DILexicalBlock(scope: !12, file: !1, line: 1, column: 1)
   !20 = !DILocation(line: 10, scope: !19)
+  !112 = distinct !DISubprogram(name: "test2", linkageName: "102", scope: !1, file: !1, line: 6, type: !13, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !115)
+  !115 = !{!116}
+  !116 = !DILocalVariable(name: "myVar", scope: !118, file: !1, line: 7, type: !6)
+  !117 = !DILocation(line: 10, scope: !118)
+  !118 = distinct !DILexicalBlock(scope: !112, file: !1, line: 1, column: 1)
+  !119 = distinct !DILexicalBlock(scope: !112, file: !1, line: 1, column: 1)
+  !120 = !DILocation(line: 10, scope: !119)
 
 ...
 ---
 name: _Z8bb_to_bb
 debugValueSubstitutions:
   - { srcinst: 4, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
 body:  |
   bb.0.entry:
bb.4:
     JCC_1 %bb.5, 1, implicit $eflags, debug-location !17
     JMP_1 %bb.6, debug-location !17
 
   bb.5:
     RET 0, debug-location !17
 
   bb.6:
     RET 0, debug-location !17
+...
+---
+name: test2
+debugValueSubstitutions:
+  - { srcinst: 4, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
+body:  |
+  bb.0.entry:
+    successors: %bb.1, %bb.5, %bb.6
+
+    $rax = MOV64ri 1, debug-instr-number 1, debug-location !117
+    JCC_1 %bb.5, 1, implicit $eflags
+    JCC_1 %bb.6, 2, implicit $eflags
+    JMP_1 %bb.1
+
+  bb.1:
+    successors: %bb.2, %bb.3
+
+    DBG_VALUE $noreg, $noreg, !116, !DIExpression(), debug-location !117
+    JCC_1 %bb.3, 1, implicit $eflags, debug-location !117
+
+  bb.2:
+    successors: %bb.4
+
+    JMP_1 %bb.4, debug-location !120
+
+  bb.3:
+    successors: %bb.4
+
+    JMP_1 %bb.4, debug-location !117
+
+  bb.4:
+    successors: %bb.5, %bb.6
+
+    JCC_1 %bb.5, 1, implicit $eflags, debug-location !117
+    JMP_1 %bb.6, debug-location !117
+
+  bb.5:
+    RET 0, debug-location !117
+
+  bb.6:
+    RET 0, debug-location !117