[WebAssembly] Omit DBG_VALUE after terminator

[aheejin]

Summary

When a stackified variable has an associated DBG_VALUE instruction,
DebugFixup pass adds a DBG_VALUE instruction after the stackified
value's last use to clear the variable's debug range info. But when the
last use instruction is a terminator, it can cause a verification
failure (when run with -verify-machineinstrs) because there are no
instructions allowed after a terminator.

For example:

%myvar = ...
DBG_VALUE target-index(wasm-operand-stack), $noreg, !"myvar", ...
BR_IF 0, %myvar, ...
DBG_VALUE $noreg, $noreg, !"myvar", ...

In this test, %myvar is stackified, so the first DBG_VALUE
instruction's first operand has changed to wasm-operand-stack to
denote it. And an additional DBG_VALUE instruction is added after its
last use, BR_IF, to signal variable myvar is not in the operand
stack anymore. But because the DBG_VALUE instruction is added after
the BR_IF, a terminator, it fails MachineVerifier.

DBG_VALUE instructions are used in DbgEntityHistoryCalculator to
compute value ranges to emit DWARF info, and it turns out the
DbgEntityHistoryCalculator terminates ranges at the end of a BB, so we
don't need to emit DBG_VALUE after a terminator.

Fixes https://bugs.llvm.org/show_bug.cgi?id=50175.

[aheejin]

This is a second attempt after D101736, which was a hacky take to make MachineVerifier pass. It looks that was not necessary after all; thanks @jmorse.

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

for (auto MII = MBB.begin(); MII != MBB.end(); ++MII) {
       } else {
         // Track stack depth.
         for (MachineOperand &MO : reverse(MI.explicit_uses())) {
           if (MO.isReg() && MFI.isVRegStackified(MO.getReg())) {
             auto Prev = Stack.back();
             Stack.pop_back();
             assert(Prev.Reg == MO.getReg() &&
                    "WebAssemblyDebugFixup: Pop: Register not matched!");
-            if (Prev.DebugValue) {
+            // We should not put a DBG_VALUE after a terminator; debug ranges
+            // are terminated at the end of a BB anyway.
+            if (Prev.DebugValue && !MI.isTerminator()) {
               // This stackified reg is a variable that started life at
               // Prev.DebugValue, so now that we're popping it we must insert
               // a $noreg DBG_VALUE for the variable to end it, right after
               // the current instruction.
               BuildMI(*Prev.DebugValue->getParent(), std::next(MII),
-                      Prev.DebugValue->getDebugLoc(), TII->get(WebAssembly::DBG_VALUE), false,
-                      Register(), Prev.DebugValue->getOperand(2).getMetadata(),
+                      Prev.DebugValue->getDebugLoc(),
+                      TII->get(WebAssembly::DBG_VALUE), false, Register(),
+                      Prev.DebugValue->getOperand(2).getMetadata(),
                       Prev.DebugValue->getOperand(3).getMetadata());
             }
           }
         }
         for (MachineOperand &MO : MI.defs()) {
           if (MO.isReg() && MFI.isVRegStackified(MO.getReg())) {
             Stack.push_back({MO.getReg(), nullptr});
           }

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

-; RUN: llc < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -filetype=obj %s -o - | llvm-dwarfdump - | FileCheck %s --check-prefix DWARF
 
 ; Input C code:
 
 ;  int i = input();  // Nested case
 ;  int j = input();  // Trivial def-use.
 ;  output(i, j);
 
 ; The ll below generates 330 lines of .S, so relevant parts that the

 ; CHECK:  .int32  .Ltmp2-.Lfunc_begin0
 ; CHECK:  .int16  4                       # Loc expr size
 ; CHECK:  .int8 237                       # DW_OP_WASM_location
 ; CHECK:  .int8 2                         # 2
 ; CHECK:  .int8 1                         # 1
 ; CHECK:  .int8 159                       # DW_OP_stack_value
 
 
-
-
 source_filename = "stackified.c"
 target triple = "wasm32-unknown-unknown"
 
 define void @foo() !dbg !12 {
 entry:
   %call = call i32 @input(), !dbg !18
   call void @llvm.dbg.value(metadata i32 %call, metadata !16, metadata !DIExpression()), !dbg !19
   %call1 = call i32 @input(), !dbg !20
   call void @llvm.dbg.value(metadata i32 %call1, metadata !17, metadata !DIExpression()), !dbg !19
   call void @output(i32 %call, i32 %call1), !dbg !21
   ret void, !dbg !22
 }
 
-declare i32 @input()
+; DebugFixup pass should not add a DBG_VALUE after the BR_IF instruction at the
+; end of the 'entry' BB, because it is not allowed to have more instructions
+; after a terminator and debug ranges are terminated at the end of a BB anyway.
+; If this passes 'llc -verify-machineinstrs', that means the DBG_VALUE
+; instruction is correctly omitted.
+
+; DWARF-LABEL: DW_AT_name ("no_dbg_value_after_terminator")
+; DWARF:       DW_TAG_variable
+; DWARF-NEXT:    DW_AT_location
+; DWARF-NEXT:      [
+; DWARF-NEXT:    DW_AT_name ("myvar")
+define void @no_dbg_value_after_terminator(i32 %a, i32 %b) !dbg !23 {
+entry:
+  %cmp = icmp ne i32 %a, %b, !dbg !25
+  call void @llvm.dbg.value(metadata i1 %cmp, metadata !27, metadata !DIExpression(DW_OP_LLVM_convert, 1, DW_ATE_unsigned, DW_OP_LLVM_convert, 8, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !25
+  br i1 %cmp, label %bb.1, label %bb.0, !dbg !25
 
-declare !dbg !4 void @output(i32, i32)
+bb.0:                                             ; preds = %entry
+  unreachable
 
+bb.1:                                             ; preds = %entry
+  ret void
+}
+
+declare i32 @input()
+declare !dbg !4 void @output(i32, i32)
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!8, !9, !10}
 !llvm.ident = !{!11}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git ed7aaf832444411ce93aa0443425ce401f5c7a8e)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
 !1 = !DIFile(filename: "stackified.c", directory: "C:\\stuff\\llvm-project")

 !15 = !{!16, !17}
 !16 = !DILocalVariable(name: "i", scope: !12, file: !1, line: 4, type: !7)
 !17 = !DILocalVariable(name: "j", scope: !12, file: !1, line: 5, type: !7)
 !18 = !DILocation(line: 4, column: 11, scope: !12)
 !19 = !DILocation(line: 0, scope: !12)
 !20 = !DILocation(line: 5, column: 11, scope: !12)
 !21 = !DILocation(line: 6, column: 3, scope: !12)
 !22 = !DILocation(line: 7, column: 1, scope: !12)
+!23 = distinct !DISubprogram(name: "no_dbg_value_after_terminator", scope: null, type: !24, spFlags: DISPFlagDefinition, unit: !0)
+!24 = !DISubroutineType(types: !2)
+!25 = !DILocation(line: 0, scope: !26)
+!26 = distinct !DILexicalBlock(scope: !23)
+!27 = !DILocalVariable(name: "myvar", scope: !26, type: !28)
+!28 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)