[PATCH] D102309: [WebAssembly] Omit DBG_VALUE after terminator

Tue May 11 23:27:32 PDT 2021

aheejin created this revision.
aheejin added reviewers: dschuff, aardappel, jmorse.
Herald added subscribers: wingo, ecnelises, sunfish, hiraditya, jgravelle-google, sbc100.
aheejin requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102309

Files:
  llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp
  llvm/test/CodeGen/WebAssembly/stackified-debug.ll


Index: llvm/test/CodeGen/WebAssembly/stackified-debug.ll
===================================================================

--- llvm/test/CodeGen/WebAssembly/stackified-debug.ll
+++ llvm/test/CodeGen/WebAssembly/stackified-debug.ll
@@ -1,4 +1,5 @@
-; 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:
 
@@ -42,8 +43,6 @@
 ; CHECK: 	.int8	159                       # DW_OP_stack_value
 
 
-
-
 source_filename = "stackified.c"
 target triple = "wasm32-unknown-unknown"
 
@@ -57,10 +56,32 @@
   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}
@@ -90,3 +111,9 @@
 !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)
Index: llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp
===================================================================
--- llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp
@@ -111,14 +111,17 @@
             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());
             }
           }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D102309.344671.patch
Type: text/x-patch
Size: 3962 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210512/84cddc82/attachment.bin>
