From a07c08f74fafcbf196cda4b20f0761538fca3dbe Mon Sep 17 00:00:00 2001
From: Thomas Lively <tlively@google.com>
Date: Thu, 4 Jun 2020 13:25:10 -0700
Subject: [PATCH] [WebAssembly] Lower llvm.debugtrap properly

Summary:
Unlike normal traps, debug traps are allowed to return and can have
additional instructions in the same basic block. Without explicit
backend support for debug traps, they are lowered in ISel as normal
traps. Since normal traps are lowered in the WebAssembly backend to
the UNREACHABLE instruction, which is a terminator, using debug traps
could lead to invalid MBBs when there are additional instructions
after the trap. This patch fixes the issue by lowering debug traps to
a new version of the UNREACHABLE instruction, DEBUG_UNREACHABLE, that
is not a terminator.

An alternative approach would have been to make UNREACHABLE not a
terminator, but that breaks a large number of tests. In particular, it
would require removing the traps inserted after noreturn calls to
@llvm.wasm.throw because otherwise the terminator throw would be
followed by a non-terminator UNREACHABLE and we would be back to
having invalid MBBs. Overall the approach in this patch seems simpler.

Reviewers: aheejin, dschuff

Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D81055
---
 .../WebAssembly/WebAssemblyISelLowering.cpp   |  1 +
 .../WebAssembly/WebAssemblyInstrControl.td    | 19 +++++-
 llvm/test/CodeGen/WebAssembly/debugtrap.ll    | 60 +++++++++++++++++++
 3 files changed, 77 insertions(+), 3 deletions(-)
 create mode 100644 llvm/test/CodeGen/WebAssembly/debugtrap.ll

diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 9cef2025809e..f33a63b105a5 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -253,6 +253,7 @@ WebAssemblyTargetLowering::WebAssemblyTargetLowering(
 
   // Trap lowers to wasm unreachable
   setOperationAction(ISD::TRAP, MVT::Other, Legal);
+  setOperationAction(ISD::DEBUGTRAP, MVT::Other, Legal);
 
   // Exception handling intrinsics
   setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::Other, Custom);
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
index 1afc9a8790dc..03b8004315a0 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
@@ -85,8 +85,8 @@ defm END_FUNCTION : NRI<(outs), (ins), [], "end_function", 0x0b>;
 } // Uses = [VALUE_STACK], Defs = [VALUE_STACK]
 
 
-let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
-
+let hasCtrlDep = 1, isBarrier = 1 in {
+let isTerminator = 1 in {
 let isReturn = 1 in {
 
 defm RETURN : I<(outs), (ins variable_ops), (outs), (ins),
@@ -99,8 +99,21 @@ defm FALLTHROUGH_RETURN : I<(outs), (ins variable_ops), (outs), (ins), []>;
 
 } // isReturn = 1
 
+let isTrap = 1 in
 defm UNREACHABLE : NRI<(outs), (ins), [(trap)], "unreachable", 0x00>;
-} // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1
+
+} // isTerminator = 1
+
+// debugtrap explicitly returns despite trapping because it is supposed to just
+// get the attention of the debugger. Unfortunately, because UNREACHABLE is a
+// terminator, lowering debugtrap to UNREACHABLE can create an invalid
+// MachineBasicBlock when there is additional code after it. Lower it to this
+// non-terminator version instead.
+// TODO: Actually execute the debugger statement when running on the Web
+let isTrap = 1 in
+defm DEBUG_UNREACHABLE : NRI<(outs), (ins), [(debugtrap)], "unreachable", 0x00>;
+
+} // hasCtrlDep = 1, isBarrier = 1
 
 //===----------------------------------------------------------------------===//
 // Exception handling instructions
diff --git a/llvm/test/CodeGen/WebAssembly/debugtrap.ll b/llvm/test/CodeGen/WebAssembly/debugtrap.ll
new file mode 100644
index 000000000000..27504ac6443d
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/debugtrap.ll
@@ -0,0 +1,60 @@
+; RUN: llc < %s -asm-verbose=false -verify-machineinstrs | FileCheck %s
+
+; Test lowering of __builtin_debugtrap in cases where lowering it via
+; the normal UNREACHABLE instruction would yield invalid
+; MachineFunctions.
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32"
+
+declare void @llvm.debugtrap()
+
+; CHECK-LABEL: foo:
+; CHECK-NEXT: .functype       foo (i32) -> ()
+; CHECK-NEXT: .LBB0_1:
+; CHECK-NEXT: loop
+; CHECK-NEXT: unreachable
+; CHECK-NEXT: i32.const       0
+; CHECK-NEXT: br_if           0
+; CHECK-NEXT: end_loop
+; CHECK-NEXT: end_function
+define void @foo(i32 %g) {
+entry:
+  br label %for.body
+
+for.body:
+  call void @llvm.debugtrap()
+  %exitcond = icmp eq i32 undef, %g
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+
+for.cond.cleanup:
+  ret void
+}
+
+; CHECK-LABEL: middle_of_block:
+; CHECK-NEXT: .functype       middle_of_block (i32, i32) -> (i32)
+; CHECK-NEXT: unreachable
+; CHECK-NEXT: local.get       0
+; CHECK-NEXT: local.get       1
+; CHECK-NEXT: i32.add
+; CHECK-NEXT: end_function
+define i32 @middle_of_block(i32 %x, i32 %y) {
+  %r = add i32 %x, %y
+  call void @llvm.debugtrap()
+  ret i32 %r
+}
+
+; CHECK-LABEL: really_middle_of_block:
+; CHECK-NEXT: .functype       really_middle_of_block () -> (i32)
+; CHECK-NEXT: call    bar
+; CHECK-NEXT: drop
+; CHECK-NEXT: unreachable
+; CHECK-NEXT: call    bar
+; CHECK-NEXT: end_function
+declare i32 @bar()
+define i32 @really_middle_of_block() {
+  %x = call i32 @bar()
+  call void @llvm.debugtrap()
+  %r = call i32 @bar()
+  ret i32 %r
+}