diff --git a/cranelift/filetests/verifier/return_at_end.clif b/cranelift/filetests/verifier/return_at_end.clif deleted file mode 100644 index fcbae57070..0000000000 --- a/cranelift/filetests/verifier/return_at_end.clif +++ /dev/null @@ -1,22 +0,0 @@ -test verifier -set return_at_end - -function %ok(i32) { -ebb0(v0: i32): - brnz v0, ebb1 - trap int_divz - -ebb1: - trapz v0, user5 - return -} - -function %bad(i32) { -ebb0(v0: i32): - brnz v0, ebb1 - return ; error: Internal return not allowed - -ebb1: - trapz v0, user6 - return -} diff --git a/cranelift/src/wasm.rs b/cranelift/src/wasm.rs index 11b859d52b..3e91c3e7b0 100644 --- a/cranelift/src/wasm.rs +++ b/cranelift/src/wasm.rs @@ -11,7 +11,9 @@ use cranelift_codegen::print_errors::{pretty_error, pretty_verifier_error}; use cranelift_codegen::settings::FlagsOrIsa; use cranelift_codegen::Context; use cranelift_entity::EntityRef; -use cranelift_wasm::{translate_module, DummyEnvironment, FuncIndex, ModuleEnvironment}; +use cranelift_wasm::{ + translate_module, DummyEnvironment, FuncIndex, ModuleEnvironment, ReturnMode, +}; use std::error::Error; use std::path::Path; use std::path::PathBuf; @@ -102,8 +104,11 @@ fn handle_module( } }; - let mut dummy_environ = - DummyEnvironment::with_triple_flags(isa.triple().clone(), fisa.flags.clone()); + let mut dummy_environ = DummyEnvironment::with_triple_flags( + isa.triple().clone(), + fisa.flags.clone(), + ReturnMode::NormalReturns, + ); translate_module(&module_binary, &mut dummy_environ).map_err(|e| e.to_string())?; let _ = terminal.fg(term::color::GREEN); diff --git a/cranelift/wasmtests/return_at_end.wat b/cranelift/wasmtests/use_fallthrough_return.wat similarity index 100% rename from cranelift/wasmtests/return_at_end.wat rename to cranelift/wasmtests/use_fallthrough_return.wat diff --git a/lib/codegen/meta-python/base/instructions.py b/lib/codegen/meta-python/base/instructions.py index c22b6b9b77..2993d6de49 100644 --- a/lib/codegen/meta-python/base/instructions.py +++ b/lib/codegen/meta-python/base/instructions.py @@ -234,6 +234,16 @@ x_return = Instruction( """, ins=rvals, is_return=True, is_terminator=True) +fallthrough_return = Instruction( + 'fallthrough_return', r""" + Return from the function by fallthrough. + + This is a specialized instruction for use where one wants to append + a custom epilogue, which will then perform the real return. This + instruction has no encoding. + """, + ins=rvals, is_return=True, is_terminator=True) + FN = Operand( 'FN', entities.func_ref, diff --git a/lib/codegen/meta-python/base/settings.py b/lib/codegen/meta-python/base/settings.py index e43e455693..4eb7541840 100644 --- a/lib/codegen/meta-python/base/settings.py +++ b/lib/codegen/meta-python/base/settings.py @@ -65,16 +65,6 @@ colocated_libcalls = BoolSetting( they can use more efficient addressing. """) -return_at_end = BoolSetting( - """ - Generate functions with at most a single return instruction at the - end of the function. - - This guarantees that functions do not have any internal return - instructions. Either they never return, or they have a single return - instruction at the end. - """) - avoid_div_traps = BoolSetting( """ Generate explicit checks around native division instructions to avoid diff --git a/lib/codegen/src/settings.rs b/lib/codegen/src/settings.rs index 743bd29268..c9374d9e47 100644 --- a/lib/codegen/src/settings.rs +++ b/lib/codegen/src/settings.rs @@ -370,7 +370,6 @@ mod tests { call_conv = \"fast\"\n\ is_pic = false\n\ colocated_libcalls = false\n\ - return_at_end = false\n\ avoid_div_traps = false\n\ enable_float = true\n\ enable_nan_canonicalization = false\n\ diff --git a/lib/codegen/src/verifier/mod.rs b/lib/codegen/src/verifier/mod.rs index 2daa1d5440..ba396a8b10 100644 --- a/lib/codegen/src/verifier/mod.rs +++ b/lib/codegen/src/verifier/mod.rs @@ -307,7 +307,6 @@ struct Verifier<'a> { func: &'a Function, expected_cfg: ControlFlowGraph, expected_domtree: DominatorTree, - flags: &'a Flags, isa: Option<&'a TargetIsa>, } @@ -319,7 +318,6 @@ impl<'a> Verifier<'a> { func, expected_cfg, expected_domtree, - flags: fisa.flags, isa: fisa.isa, } } @@ -1654,9 +1652,9 @@ impl<'a> Verifier<'a> { // Instructions with side effects are not allowed to be ghost instructions. let opcode = self.func.dfg[inst].opcode(); - // The `fallthrough` instruction is marked as a terminator and a branch, but it is not - // required to have an encoding. - if opcode == Opcode::Fallthrough { + // The `fallthrough` and `fallthrough_return` instructions are marked as terminators and + // branches, but they are not required to have an encoding. + if opcode == Opcode::Fallthrough || opcode == Opcode::FallthroughReturn { return Ok(()); } @@ -1696,24 +1694,6 @@ impl<'a> Verifier<'a> { Ok(()) } - /// Verify the `return_at_end` property which requires that there are no internal return - /// instructions. - fn verify_return_at_end(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { - for ebb in self.func.layout.ebbs() { - let inst = self.func.layout.last_inst(ebb).unwrap(); - if self.func.dfg[inst].opcode().is_return() && Some(ebb) != self.func.layout.last_ebb() - { - report!( - errors, - inst, - "Internal return not allowed with return_at_end=1" - ); - } - } - - errors.as_result() - } - pub fn run(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { self.verify_global_values(errors)?; self.verify_heaps(errors)?; @@ -1729,10 +1709,6 @@ impl<'a> Verifier<'a> { } } - if self.flags.return_at_end() { - self.verify_return_at_end(errors)?; - } - verify_flags(self.func, &self.expected_cfg, self.isa, errors)?; Ok(()) diff --git a/lib/wasm/src/code_translator.rs b/lib/wasm/src/code_translator.rs index 3c615568ca..152c20ca20 100644 --- a/lib/wasm/src/code_translator.rs +++ b/lib/wasm/src/code_translator.rs @@ -28,7 +28,7 @@ use cranelift_codegen::ir::{self, InstBuilder, JumpTableData, MemFlags}; use cranelift_codegen::packed_option::ReservedValue; use cranelift_entity::EntityRef; use cranelift_frontend::{FunctionBuilder, Variable}; -use environ::{FuncEnvironment, GlobalVariable, WasmError, WasmResult}; +use environ::{FuncEnvironment, GlobalVariable, ReturnMode, WasmError, WasmResult}; use state::{ControlStackFrame, TranslationState}; use std::collections::{hash_map, HashMap}; use std::vec::Vec; @@ -340,11 +340,10 @@ pub fn translate_operator( }; { let args = state.peekn(return_count); - if environ.flags().return_at_end() { - builder.ins().jump(br_destination, args); - } else { - builder.ins().return_(args); - } + match environ.return_mode() { + ReturnMode::NormalReturns => builder.ins().return_(args), + ReturnMode::FallthroughReturn => builder.ins().jump(br_destination, args), + }; } state.popn(return_count); state.reachable = false; diff --git a/lib/wasm/src/environ/dummy.rs b/lib/wasm/src/environ/dummy.rs index d1e6a8052c..ae0edb96e3 100644 --- a/lib/wasm/src/environ/dummy.rs +++ b/lib/wasm/src/environ/dummy.rs @@ -7,7 +7,7 @@ use cranelift_codegen::ir::types::*; use cranelift_codegen::ir::{self, InstBuilder}; use cranelift_codegen::settings; use cranelift_entity::{EntityRef, PrimaryMap}; -use environ::{FuncEnvironment, GlobalVariable, ModuleEnvironment, WasmResult}; +use environ::{FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, WasmResult}; use func_translator::FuncTranslator; use std::string::String; use std::vec::Vec; @@ -105,38 +105,55 @@ pub struct DummyEnvironment { /// Vector of wasm bytecode size for each function. pub func_bytecode_sizes: Vec, + + /// How to return from functions. + return_mode: ReturnMode, } impl DummyEnvironment { /// Allocates the data structures with default flags. pub fn with_triple(triple: Triple) -> Self { - Self::with_triple_flags(triple, settings::Flags::new(settings::builder())) + Self::with_triple_flags( + triple, + settings::Flags::new(settings::builder()), + ReturnMode::NormalReturns, + ) } - /// Allocates the data structures with the given flags. - pub fn with_triple_flags(triple: Triple, flags: settings::Flags) -> Self { + /// Allocates the data structures with the given triple. + pub fn with_triple_flags( + triple: Triple, + flags: settings::Flags, + return_mode: ReturnMode, + ) -> Self { Self { info: DummyModuleInfo::with_triple_flags(triple, flags), trans: FuncTranslator::new(), func_bytecode_sizes: Vec::new(), + return_mode, } } /// Return a `DummyFuncEnvironment` for translating functions within this /// `DummyEnvironment`. pub fn func_env(&self) -> DummyFuncEnvironment { - DummyFuncEnvironment::new(&self.info) + DummyFuncEnvironment::new(&self.info, self.return_mode) } } /// The `FuncEnvironment` implementation for use by the `DummyEnvironment`. pub struct DummyFuncEnvironment<'dummy_environment> { pub mod_info: &'dummy_environment DummyModuleInfo, + + return_mode: ReturnMode, } impl<'dummy_environment> DummyFuncEnvironment<'dummy_environment> { - pub fn new(mod_info: &'dummy_environment DummyModuleInfo) -> Self { - Self { mod_info } + pub fn new(mod_info: &'dummy_environment DummyModuleInfo, return_mode: ReturnMode) -> Self { + Self { + mod_info, + return_mode, + } } // Create a signature for `sigidx` amended with a `vmctx` argument after the standard wasm @@ -321,6 +338,10 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ ) -> WasmResult { Ok(pos.ins().iconst(I32, -1)) } + + fn return_mode(&self) -> ReturnMode { + self.return_mode + } } impl<'data> ModuleEnvironment<'data> for DummyEnvironment { @@ -433,7 +454,7 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment { fn define_function_body(&mut self, body_bytes: &'data [u8]) -> WasmResult<()> { let func = { - let mut func_environ = DummyFuncEnvironment::new(&self.info); + let mut func_environ = DummyFuncEnvironment::new(&self.info, self.return_mode); let func_index = FuncIndex::new(self.get_num_func_imports() + self.info.function_bodies.len()); let name = get_func_name(func_index); diff --git a/lib/wasm/src/environ/mod.rs b/lib/wasm/src/environ/mod.rs index 0ca34575f7..d44011fd8c 100644 --- a/lib/wasm/src/environ/mod.rs +++ b/lib/wasm/src/environ/mod.rs @@ -5,5 +5,5 @@ mod spec; pub use environ::dummy::DummyEnvironment; pub use environ::spec::{ - FuncEnvironment, GlobalVariable, ModuleEnvironment, WasmError, WasmResult, + FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, WasmError, WasmResult, }; diff --git a/lib/wasm/src/environ/spec.rs b/lib/wasm/src/environ/spec.rs index 31640635b5..872ec28475 100644 --- a/lib/wasm/src/environ/spec.rs +++ b/lib/wasm/src/environ/spec.rs @@ -74,6 +74,15 @@ impl WasmError { /// A convenient alias for a `Result` that uses `WasmError` as the error type. pub type WasmResult = Result; +/// How to return from functions. +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum ReturnMode { + /// Use normal return instructions as needed. + NormalReturns, + /// Use a single fallthrough return at the end of the function. + FallthroughReturn, +} + /// Environment affecting the translation of a single WebAssembly function. /// /// A `FuncEnvironment` trait object is required to translate a WebAssembly function to Cranelift @@ -216,6 +225,11 @@ pub trait FuncEnvironment { fn translate_loop_header(&mut self, _pos: FuncCursor) { // By default, don't emit anything. } + + /// Should the code be structured to use a single `fallthrough_return` instruction at the end + /// of the function body, rather than `return` instructions as needed? This is used by VMs + /// to append custom epilogues. + fn return_mode(&self) -> ReturnMode; } /// An object satisfying the `ModuleEnvironment` trait can be passed as argument to the diff --git a/lib/wasm/src/func_translator.rs b/lib/wasm/src/func_translator.rs index 0f946751db..0d99754c79 100644 --- a/lib/wasm/src/func_translator.rs +++ b/lib/wasm/src/func_translator.rs @@ -9,7 +9,7 @@ use cranelift_codegen::entity::EntityRef; use cranelift_codegen::ir::{self, Ebb, InstBuilder}; use cranelift_codegen::timing; use cranelift_frontend::{FunctionBuilder, FunctionBuilderContext, Variable}; -use environ::{FuncEnvironment, WasmError, WasmResult}; +use environ::{FuncEnvironment, ReturnMode, WasmError, WasmResult}; use state::TranslationState; use wasmparser::{self, BinaryReader}; @@ -209,7 +209,10 @@ fn parse_function_body( if state.reachable { debug_assert!(builder.is_pristine()); if !builder.is_unreachable() { - builder.ins().return_(&state.stack); + match environ.return_mode() { + ReturnMode::NormalReturns => builder.ins().return_(&state.stack), + ReturnMode::FallthroughReturn => builder.ins().fallthrough_return(&state.stack), + }; } } diff --git a/lib/wasm/src/lib.rs b/lib/wasm/src/lib.rs index 95667d870d..21b757f9c8 100644 --- a/lib/wasm/src/lib.rs +++ b/lib/wasm/src/lib.rs @@ -59,7 +59,8 @@ mod state; mod translation_utils; pub use environ::{ - DummyEnvironment, FuncEnvironment, GlobalVariable, ModuleEnvironment, WasmError, WasmResult, + DummyEnvironment, FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, WasmError, + WasmResult, }; pub use func_translator::FuncTranslator; pub use module_translator::translate_module; diff --git a/lib/wasm/tests/wasm_testsuite.rs b/lib/wasm/tests/wasm_testsuite.rs index f7cc0e678e..be9c836463 100644 --- a/lib/wasm/tests/wasm_testsuite.rs +++ b/lib/wasm/tests/wasm_testsuite.rs @@ -6,9 +6,9 @@ extern crate wabt; use cranelift_codegen::isa; use cranelift_codegen::print_errors::pretty_verifier_error; -use cranelift_codegen::settings::{self, Configurable, Flags}; +use cranelift_codegen::settings::{self, Flags}; use cranelift_codegen::verifier; -use cranelift_wasm::{translate_module, DummyEnvironment}; +use cranelift_wasm::{translate_module, DummyEnvironment, ReturnMode}; use std::fs; use std::fs::File; use std::io; @@ -35,16 +35,18 @@ fn testsuite() { let flags = Flags::new(settings::builder()); for path in paths { let path = path.path(); - handle_module(&path, &flags); + handle_module(&path, &flags, ReturnMode::NormalReturns); } } #[test] -fn return_at_end() { - let mut flag_builder = settings::builder(); - flag_builder.enable("return_at_end").unwrap(); - let flags = Flags::new(flag_builder); - handle_module(Path::new("../../wasmtests/return_at_end.wat"), &flags); +fn use_fallthrough_return() { + let flags = Flags::new(settings::builder()); + handle_module( + Path::new("../../wasmtests/use_fallthrough_return.wat"), + &flags, + ReturnMode::FallthroughReturn, + ); } fn read_file(path: &Path) -> io::Result> { @@ -54,7 +56,7 @@ fn read_file(path: &Path) -> io::Result> { Ok(buf) } -fn handle_module(path: &Path, flags: &Flags) { +fn handle_module(path: &Path, flags: &Flags, return_mode: ReturnMode) { let data = match path.extension() { None => { panic!("the file extension is not wasm or wat"); @@ -73,7 +75,9 @@ fn handle_module(path: &Path, flags: &Flags) { None | Some(&_) => panic!("the file extension for {:?} is not wasm or wat", path), }, }; - let mut dummy_environ = DummyEnvironment::with_triple_flags(triple!("riscv64"), flags.clone()); + let mut dummy_environ = + DummyEnvironment::with_triple_flags(triple!("riscv64"), flags.clone(), return_mode); + translate_module(&data, &mut dummy_environ).unwrap(); let isa = isa::lookup(dummy_environ.info.triple)