[WebAssembly] Nullify dangling register DBG_VALUEs

[aheejin]

Summary

Register-based DBG_VALUE should have been
converted into either local-based or stack-operand-based, so at this
point they don't contain any information.

This CL nullifies them, i.e., turning them info
DBG_VALUE $noreg, $noreg, ..., which makes the variable appear as
"optimized out". It is not safe to simply remove these instruction
because at this point, because the fact that there is a DBG_VALUE here
means the location this variable resides has changed to somewhere else;
we've lost that information where that was.

---

The below is not really about the CL itself, but a pre-existing bug in
llvm-locstats variable coverage we are going to be affected after this
CL. Feel free to skip if you don't have time.

This CL has an unexpected side effect of increasing variable coverage
reported by llvm-locstats, due to a bug mentioned in
https://lists.llvm.org/pipermail/llvm-dev/2021-January/148139.html.
Currently inlined functions' formal parameters that don't have any
coverage, including those who only have DBG_VALUE $noreg, $noregs
associated with them, don't generate DW_TAG_formal_parameter DIEs, and
they don't get into consideration in llvm-dwarf --statistics and
llvm-locstats. This is a known bug mentioned in
https://lists.llvm.org/pipermail/llvm-dev/2021-January/148139.html. For
example, this is a snippet of llvm-dwarfdump output of wc, where
isword is inlined into another function. isword has a formal
parameter c, which doesn't have any coverage in this inlined area, so
its DW_TAG_formal_parameter was not generated:

0x0000018d:     DW_TAG_inlined_subroutine
                  DW_AT_abstract_origin  (0x0000012c "isword")
                  DW_AT_low_pc  (0x00000166)
                  DW_AT_high_pc  (0x0000016d)
                  DW_AT_call_file  ("/usr/local/google/home/aheejin/test/dwarf-verify/wc/wc.c")
                  DW_AT_call_line  (100)
                  DW_AT_call_column  (0x0a)

But our dangling-register-based formal parameters currently generate
DW_TAG_formal_parameter DIEs, even though they don't have any
meaningful coverage, and this happened to generate correct statistics,
because it correctly sees this c doesn't have any coverage in this
inlined area. So our current llvm-dwarfdump (before this CL) is like:

0x0000018d:     DW_TAG_inlined_subroutine
                  DW_AT_abstract_origin  (0x0000012c "isword")
                  DW_AT_low_pc  (0x00000166)
                  DW_AT_high_pc  (0x0000016d)
                  DW_AT_call_file  ("/usr/local/google/home/aheejin/test/dwarf-verify/wc/wc.c")
                  DW_AT_call_line  (100)
                  DW_AT_call_column  (0x0a)

0x0000019a:       DW_TAG_formal_parameter
                    DW_AT_abstract_origin  (0x00000134 "c")

Note that this DW_TAG_formal_parameter doesn't have any
DW_AT_location attribute under it, meaning it doesn't have any
coverage.

On the other hand, if the DW_TAG_formal_parameter is not generated,
llvm-dwarfdump --statistics wouldn't even know about the parameter's
existence, and doesn't include it in the coverage computation, making
the overall coverage (incorrectly) go up.

DBG_VALUE $noreg, $noreg used to generate this empty
DW_TAG_formal_parameter, but it changed for consistency in D95617.

It looks this bug in llvm-dwarf --statistics (and llvm-locstats,
which uses llvm-dwarf --statistics) has not been fixed so far. So we
get the coverage that's little incorrectly higher than our actual
coverage. But this bug apparently affects every target in LLVM.

[dschuff]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

68

If we made this function take the MBB as an arg instead of the MF, we could call it at the end of the main for-loop in runOnMachineFunction. This would mean that we would iterate over each MBB right after having previously iterated over it, (instead of making 2 passes over the whole function), and result in improved locality for functions which are larger than the CPU data cache.

[aheejin]

Make nullifyDanglingDebugValues run on MBBs

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

INITIALIZE_PASS(
     WebAssemblyDebugFixup, DEBUG_TYPE,
     "Ensures debug_value's that have been stackified become stack relative",
     false, false)
 
 FunctionPass *llvm::createWebAssemblyDebugFixup() {
   return new WebAssemblyDebugFixup();
 }
 
+// At this very end of the compilation pipeline, if any DBG_VALUEs with
+// registers remain, it means they are dangling info which we failed to update
+// when their corresponding def instruction was transformed/moved/splitted etc.
+// Because Wasm cannot access values in LLVM virtual registers in the debugger,
+// these dangling DBG_VALUEs in effect kill the effect of any previous DBG_VALUE
+// associated with the variable, which will appear as "optimized out".
+static void nullifyDanglingDebugValues(MachineBasicBlock &MBB,
+                                       const TargetInstrInfo *TII) {
+  for (auto &MI : llvm::make_early_inc_range(MBB)) {
+    if (MI.isDebugValue() && MI.getDebugOperand(0).isReg() &&
+        !MI.isUndefDebugValue()) {
+      LLVM_DEBUG(dbgs() << "Warning: dangling DBG_VALUE nullified: " << MI
+                        << "\n");
+      MI.getDebugOperand(0).setReg(Register());
+    }
+  }
+}
+
 bool WebAssemblyDebugFixup::runOnMachineFunction(MachineFunction &MF) {
   LLVM_DEBUG(dbgs() << "********** Debug Fixup **********\n"
                        "********** Function: "
                     << MF.getName() << '\n');
 
   WebAssemblyFunctionInfo &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
 
for (auto MII = MBB.begin(); MII != MBB.end(); ++MII) {
           if (MO.isReg() && MFI.isVRegStackified(MO.getReg())) {
             Stack.push_back({MO.getReg(), nullptr});
           }
         }
       }
     }
     assert(Stack.empty() &&
            "WebAssemblyDebugFixup: Stack not empty at end of basic block!");
+
+    nullifyDanglingDebugValues(MBB, TII);
   }
 
   return true;
 }

llvm/test/DebugInfo/WebAssembly/remove-reg-dbg-value.mir

+# RUN: llc -run-pass wasm-debug-fixup %s -o - | FileCheck %s
+
+# Test if '#DEBUG_VALUE' comments for target indices are printed correctly.
+
+--- |
+  target triple = "wasm32-unknown-unknown"
+
+  define void @test_remove_dangling_reg_dbg_value() !dbg !5 {
+    call void @llvm.dbg.value(metadata i32 0, metadata !9, metadata !DIExpression()), !dbg !10
+    call void @llvm.dbg.value(metadata i32 0, metadata !11, metadata !DIExpression()), !dbg !10
+    ret void
+  }
+
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!2, !3, !4}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, emissionKind: FullDebug)
+  !1 = !DIFile(filename: "test.c", directory: "")
+  !2 = !{i32 7, !"Dwarf Version", i32 5}
+  !3 = !{i32 2, !"Debug Info Version", i32 3}
+  !4 = !{i32 1, !"wchar_size", i32 4}
+  !5 = distinct !DISubprogram(name: "test_dbg_value_comment", scope: !1, file: !1, line: 1, type: !6, scopeLine: 1, unit: !0)
+  !6 = !DISubroutineType(types: !7)
+  !7 = !{!8}
+  !8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !9 = !DILocalVariable(name: "var0", scope: !5, file: !1, line: 2, type: !8)
+  !10 = !DILocation(line: 0, scope: !5)
+  !11 = !DILocalVariable(name: "var1", scope: !5, file: !1, line: 2, type: !8)
+...
+
+---
+# CHECK-LABEL: name: test_remove_dangling_reg_dbg_value
+name: test_remove_dangling_reg_dbg_value
+liveins:
+  - { reg: '$arguments' }
+body: |
+  ; CHECK: bb.0:
+  ; CHECK: DBG_VALUE $noreg, $noreg, !9, !DIExpression(), debug-location !10
+  ; CHECK-NEXT: DBG_VALUE target-index(wasm-local) + 1, $noreg, !11, !DIExpression(), debug-location !10
+  ; CHECK-NEXT: RETURN
+  bb.0:
+    liveins: $arguments
+    ; This %3 is a danling register and will turn to $noreg
+    DBG_VALUE %3:i32, $noreg, !9, !DIExpression(), debug-location !10
+    ; This debug info will remain the same, because it's in a local
+    DBG_VALUE target-index(wasm-local) + 1, $noreg, !11, !DIExpression(), debug-location !10
+    RETURN implicit-def dead $arguments
+...