Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure NoTrapAfterNoreturn is false for the wasm backend #65876

Merged
merged 16 commits into from
Oct 5, 2023
Merged
2 changes: 1 addition & 1 deletion lld/test/wasm/init-fini.ll
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ entry:
; CHECK-NEXT: Body: 10031004100A100F1012100F10141003100C100F10161001100E0B
; CHECK: - Index: 22
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 02404186808080004100418088808000108780808000450D0000000B0B
; CHECK-NEXT: Body: 02404186808080004100418088808000108780808000450D00000B0B
; CHECK-NEXT: - Type: CUSTOM
; CHECK-NEXT: Name: name
; CHECK-NEXT: FunctionNames:
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/MachineFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
// RegInfo - Information about each register in use in the function.
MachineRegisterInfo *RegInfo;

// Used to keep track of target-specific per-machine function information for
// Used to keep track of target-specific per-machine-function information for
// the target implementation.
MachineFunctionInfo *MFInfo;

Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/MachineInstr.h
Original file line number Diff line number Diff line change
Expand Up @@ -1276,7 +1276,7 @@ class MachineInstr
/// eraseFromBundle() to erase individual bundled instructions.
void eraseFromParent();

/// Unlink 'this' form its basic block and delete it.
/// Unlink 'this' from its basic block and delete it.
///
/// If the instruction is part of a bundle, the other instructions in the
/// bundle remain bundled.
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/CodeGen/LLVMTargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ static cl::opt<bool>
EnableTrapUnreachable("trap-unreachable", cl::Hidden,
cl::desc("Enable generating trap for unreachable"));

static cl::opt<bool>
EnableNoTrapAfterNoreturn("no-trap-after-noreturn", cl::Hidden,
cl::desc("Do not emit a trap instruction for 'unreachable' IR instructions "
"after noreturn calls, even if --trap-unreachable is set."));

void LLVMTargetMachine::initAsmInfo() {
MRI.reset(TheTarget.createMCRegInfo(getTargetTriple().str()));
assert(MRI && "Unable to create reg info");
Expand Down Expand Up @@ -95,6 +100,8 @@ LLVMTargetMachine::LLVMTargetMachine(const Target &T,

if (EnableTrapUnreachable)
this->Options.TrapUnreachable = true;
if (EnableNoTrapAfterNoreturn)
this->Options.NoTrapAfterNoreturn = true;
}

TargetTransformInfo
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ void WebAssemblyCFGStackify::removeUnnecessaryInstrs(MachineFunction &MF) {

// When there is an unconditional branch right before a catch instruction and
// it branches to the end of end_try marker, we don't need the branch, because
// it there is no exception, the control flow transfers to that point anyway.
// if there is no exception, the control flow transfers to that point anyway.
// bb0:
// try
// ...
Expand Down
11 changes: 11 additions & 0 deletions llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,20 @@ bool WebAssemblyDebugFixup::runOnMachineFunction(MachineFunction &MF) {
// it will be culled later.
}
} else {

// WebAssembly Peephole optimisation can remove instructions around wasm unreachable.
// This is valid for wasm, as unreachable is operand stack polymorphic. But this is not modeled
// in llvm at the moment, and so the stack may not seem to pop all that it pushes.
// Clear the stack so we don't violate the assert(Stack.empty()) later on.
if (MI.getOpcode() == WebAssembly::UNREACHABLE) {
Stack.clear();
break;
}

// Track stack depth.
for (MachineOperand &MO : reverse(MI.explicit_uses())) {
if (MO.isReg() && MFI.isVRegStackified(MO.getReg())) {
assert(Stack.size() != 0 && "WebAssemblyDebugFixup: Pop: Operand stack empty!");
auto Prev = Stack.back();
Stack.pop_back();
assert(Prev.Reg == MO.getReg() &&
Expand Down
51 changes: 51 additions & 0 deletions llvm/lib/Target/WebAssembly/WebAssemblyPeephole.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include <iterator>
using namespace llvm;

#define DEBUG_TYPE "wasm-peephole"
Expand Down Expand Up @@ -109,6 +110,53 @@ static bool maybeRewriteToFallthrough(MachineInstr &MI, MachineBasicBlock &MBB,
return true;
}

static bool eraseDeadCodeAroundUnreachable(MachineInstr &UnreachbleMI, MachineBasicBlock &MBB) {
SmallVector<MachineInstr*, 16> ToDelete;

// Because wasm unreachable is stack polymorphic and unconditionally ends control,
// all instructions after it can be removed until the end of this block.
// We remove the common case of double unreachable.
auto ForwardsIterator = UnreachbleMI.getIterator();
for (ForwardsIterator++; !ForwardsIterator.isEnd(); ForwardsIterator++) {
MachineInstr& MI = *ForwardsIterator;
if (MI.getOpcode() == WebAssembly::UNREACHABLE) {
ToDelete.push_back(&MI);
} else {
break;
}
}

[&]() {
majaha marked this conversation as resolved.
Show resolved Hide resolved
// For the same reasons as above, previous instructions that only affect
// local function state can be removed (e.g. local.set, drop, various reads).
// We remove the common case of "drop unreachable".
auto BackwardsIterator = UnreachbleMI.getReverseIterator();
for (BackwardsIterator++; !BackwardsIterator.isEnd(); BackwardsIterator++) {
MachineInstr& MI = *BackwardsIterator;
switch(MI.getOpcode()) {
case WebAssembly::DROP_I32:
case WebAssembly::DROP_I64:
case WebAssembly::DROP_F32:
case WebAssembly::DROP_F64:
case WebAssembly::DROP_EXTERNREF:
case WebAssembly::DROP_FUNCREF:
case WebAssembly::DROP_V128:
ToDelete.push_back(&MI);
continue;
default:
return;
}
}
}();

bool Changed = false;
for (MachineInstr* MI : ToDelete) {
MI->eraseFromParent();
Changed = true;
}
return Changed;
}

bool WebAssemblyPeephole::runOnMachineFunction(MachineFunction &MF) {
LLVM_DEBUG({
dbgs() << "********** Peephole **********\n"
Expand Down Expand Up @@ -159,6 +207,9 @@ bool WebAssemblyPeephole::runOnMachineFunction(MachineFunction &MF) {
case WebAssembly::RETURN:
Changed |= maybeRewriteToFallthrough(MI, MBB, MF, MFI, MRI, TII);
break;
case WebAssembly::UNREACHABLE:
Changed |= eraseDeadCodeAroundUnreachable(MI, MBB);
majaha marked this conversation as resolved.
Show resolved Hide resolved
break;
}

return Changed;
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ WebAssemblyTargetMachine::WebAssemblyTargetMachine(
// LLVM 'unreachable' to ISD::TRAP and then lower that to WebAssembly's
// 'unreachable' instructions which is meant for that case.
this->Options.TrapUnreachable = true;
this->Options.NoTrapAfterNoreturn = false;
majaha marked this conversation as resolved.
Show resolved Hide resolved

// WebAssembly treats each function as an independent unit. Force
// -ffunction-sections, effectively, so that we can emit them independently.
Expand Down
137 changes: 116 additions & 21 deletions llvm/test/CodeGen/WebAssembly/unreachable.ll
Original file line number Diff line number Diff line change
@@ -1,33 +1,128 @@
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs | FileCheck %s
; RUN: llc < %s -asm-verbose=false -fast-isel -fast-isel-abort=1 -verify-machineinstrs | FileCheck %s

; Test that LLVM unreachable instruction and trap intrinsic are lowered to
; wasm unreachable
; RUN: llc < %s -verify-machineinstrs | FileCheck %s
majaha marked this conversation as resolved.
Show resolved Hide resolved
majaha marked this conversation as resolved.
Show resolved Hide resolved
; RUN: llc < %s -fast-isel -fast-isel-abort=1 -verify-machineinstrs | FileCheck %s
; RUN: llc < %s -verify-machineinstrs --trap-unreachable | FileCheck %s
; RUN: llc < %s -fast-isel -fast-isel-abort=1 -verify-machineinstrs --trap-unreachable | FileCheck %s
; RUN: llc < %s -verify-machineinstrs --trap-unreachable --no-trap-after-noreturn | FileCheck %s
; RUN: llc < %s -fast-isel -fast-isel-abort=1 -verify-machineinstrs --trap-unreachable --no-trap-after-noreturn | FileCheck %s

target triple = "wasm32-unknown-unknown"

declare void @llvm.trap()
declare void @llvm.debugtrap()
declare void @abort()

; CHECK-LABEL: f1:
; CHECK: call abort{{$}}
; CHECK: unreachable
define i32 @f1() {
call void @abort()
unreachable
}
; Test that the LLVM trap and debug trap intrinsics are lowered to wasm unreachable.
majaha marked this conversation as resolved.
Show resolved Hide resolved

declare void @llvm.trap() cold noreturn nounwind
declare void @llvm.debugtrap() nounwind

; CHECK-LABEL: f2:
; CHECK: unreachable
define void @f2() {
define void @trap_ret_void() {
; CHECK-LABEL: trap_ret_void:
; CHECK: .functype trap_ret_void () -> ()
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: unreachable
; CHECK-NEXT: # fallthrough-return
; CHECK-NEXT: end_function
call void @llvm.trap()
ret void
}

; CHECK-LABEL: f3:
; CHECK: unreachable
define void @f3() {
define void @dtrap_ret_void() {
; CHECK-LABEL: dtrap_ret_void:
; CHECK: .functype dtrap_ret_void () -> ()
majaha marked this conversation as resolved.
Show resolved Hide resolved
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: unreachable
; CHECK-NEXT: # fallthrough-return
; CHECK-NEXT: end_function
call void @llvm.debugtrap()
ret void
}

; Test that LLVM trap followed by LLVM unreachable becomes exactly one wasm unreachable.
majaha marked this conversation as resolved.
Show resolved Hide resolved
define void @trap_unreach() {
; CHECK-LABEL: trap_unreach:
; CHECK: .functype trap_unreach () -> ()
majaha marked this conversation as resolved.
Show resolved Hide resolved
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function
call void @llvm.trap()
unreachable
}


; Test that LLVM unreachable instruction is lowered to wasm unreachable when necessary
; to fulfill the wasm operand stack requirements.

declare void @ext_func()
declare i32 @ext_func_i32()
declare void @ext_never_return() noreturn

; This test emits wasm unreachable to fill in for the missing i32 return value.
majaha marked this conversation as resolved.
Show resolved Hide resolved
define i32 @missing_ret_unreach() {
; CHECK-LABEL: missing_ret_unreach:
; CHECK: .functype missing_ret_unreach () -> (i32)
majaha marked this conversation as resolved.
Show resolved Hide resolved
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: call ext_func
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function
call void @ext_func()
unreachable
}

; This is similar to the above test, but ensures wasm unreachable is emitted even
; after a noreturn call.
define i32 @missing_ret_noreturn_unreach() {
; CHECK-LABEL: missing_ret_noreturn_unreach:
; CHECK: .functype missing_ret_noreturn_unreach () -> (i32)
majaha marked this conversation as resolved.
Show resolved Hide resolved
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: call ext_never_return
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function
call void @ext_never_return()
unreachable
}

; We could emit no instructions at all for the llvm unreachables in these next three tests, as the signatures match
; and reaching llvm unreachable is undefined behaviour. But wasm unreachable is emitted for the time being.

define void @void_sig_match_unreach() {
; CHECK-LABEL: void_sig_match_unreach:
; CHECK: .functype void_sig_match_unreach () -> ()
majaha marked this conversation as resolved.
Show resolved Hide resolved
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: call ext_func
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function
call void @ext_func()
unreachable
}

define i32 @i32_sig_match_unreach() {
; CHECK-LABEL: i32_sig_match_unreach:
; CHECK: .functype i32_sig_match_unreach () -> (i32)
majaha marked this conversation as resolved.
Show resolved Hide resolved
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: call ext_func_i32
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function
call i32 @ext_func_i32()
unreachable
}

define void @void_sig_match_noreturn_unreach() {
; CHECK-LABEL: void_sig_match_noreturn_unreach:
; CHECK: .functype void_sig_match_noreturn_unreach () -> ()
majaha marked this conversation as resolved.
Show resolved Hide resolved
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: call ext_never_return
; CHECK-NEXT: unreachable
; CHECK-NEXT: end_function
call void @ext_never_return()
unreachable
}

; This function currently doesn't emit unreachable.
majaha marked this conversation as resolved.
Show resolved Hide resolved
define void @void_sig_match_noreturn_ret() {
; CHECK-LABEL: void_sig_match_noreturn_ret:
; CHECK: .functype void_sig_match_noreturn_ret () -> ()
; CHECK-NEXT: # %bb.0:
; CHECK-NEXT: call ext_never_return
; CHECK-NEXT: # fallthrough-return
; CHECK-NEXT: end_function
call void @ext_never_return()
ret void
}
12 changes: 6 additions & 6 deletions llvm/test/MC/WebAssembly/global-ctor-dtor.ll
Original file line number Diff line number Diff line change
Expand Up @@ -80,29 +80,29 @@ declare void @func3()
; CHECK-NEXT: Offset: 0x1D
; CHECK-NEXT: - Type: R_WASM_FUNCTION_INDEX_LEB
; CHECK-NEXT: Index: 6
; CHECK-NEXT: Offset: 0x2C
; CHECK-NEXT: Offset: 0x2B
; CHECK-NEXT: - Type: R_WASM_TABLE_INDEX_SLEB
; CHECK-NEXT: Index: 5
; CHECK-NEXT: Offset: 0x37
; CHECK-NEXT: Offset: 0x36
; CHECK-NEXT: - Type: R_WASM_MEMORY_ADDR_SLEB
; CHECK-NEXT: Index: 3
; CHECK-NEXT: Offset: 0x3F
; CHECK-NEXT: Offset: 0x3E
; CHECK-NEXT: - Type: R_WASM_FUNCTION_INDEX_LEB
; CHECK-NEXT: Index: 4
; CHECK-NEXT: Offset: 0x45
; CHECK-NEXT: Offset: 0x44
; CHECK-NEXT: Functions:
; CHECK-NEXT: - Index: 5
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 1080808080000B
; CHECK-NEXT: - Index: 6
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 02404181808080004100418080808000108180808000450D0000000B0B
; CHECK-NEXT: Body: 02404181808080004100418080808000108180808000450D00000B0B
; CHECK-NEXT: - Index: 7
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 1082808080000B
; CHECK-NEXT: - Index: 8
; CHECK-NEXT: Locals:
; CHECK-NEXT: Body: 02404182808080004100418080808000108180808000450D0000000B0B
; CHECK-NEXT: Body: 02404182808080004100418080808000108180808000450D00000B0B
; CHECK-NEXT: - Type: DATA
; CHECK-NEXT: Segments:
; CHECK-NEXT: - SectionOffset: 6
Expand Down