From 71fbfb499aaaefbb2b24e5652df5525684766bfc Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 11 May 2021 22:46:52 -0700 Subject: [PATCH] [WebAssembly] Omit DBG_VALUE after terminator 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. Reviewed By: dschuff Differential Revision: https://reviews.llvm.org/D102309 --- .../WebAssembly/WebAssemblyDebugFixup.cpp | 9 +++-- .../CodeGen/WebAssembly/stackified-debug.ll | 37 ++++++++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp index 2ec9dda5b09a..e06e359fc592 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp @@ -111,14 +111,17 @@ bool WebAssemblyDebugFixup::runOnMachineFunction(MachineFunction &MF) { 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()); } } diff --git a/llvm/test/CodeGen/WebAssembly/stackified-debug.ll b/llvm/test/CodeGen/WebAssembly/stackified-debug.ll index 310cea338e48..705e8a3bb960 100644 --- a/llvm/test/CodeGen/WebAssembly/stackified-debug.ll +++ b/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 @@ entry: ret void, !dbg !22 } +; 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 + +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 @@ declare void @llvm.dbg.value(metadata, metadata, metadata) !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)