cranelift-wasm: support multi-value Wasm (#1049)

This commit introduces initial support for multi-value Wasm. Wasm blocks and
calls can now take and return an arbitrary number of values.

The encoding for multi-value blocks means that we need to keep the contents of
the "Types" section around when translating function bodies. To do this, we
introduce a `WasmTypesMap` type that maps the type indices to their parameters
and returns, construct it when parsing the "Types" section, and shepherd it
through a bunch of functions and methods when translating function bodies.

cranelift/src/clif-util.rs

@@ -113,6 +113,12 @@ fn add_enable_simd_flag<'a>() -> clap::Arg<'a, 'a> {
         .help("Enable WASM's SIMD operations")
 }
 
+fn add_enable_multi_value<'a>() -> clap::Arg<'a, 'a> {
+    Arg::with_name("enable-multi-value")
+        .long("enable-multi-value")
+        .help("Enable WASM's multi-value support")
+}
+
 /// Returns a vector of clap value options and changes these options into a vector of strings
 fn get_vec(argument_vec: Option<clap::Values>) -> Vec<String> {
     let mut ret_vec: Vec<String> = Vec::new();
@@ -144,6 +150,7 @@ fn add_wasm_or_compile<'a>(cmd: &str) -> clap::App<'a, 'a> {
         .arg(add_input_file_arg())
         .arg(add_debug_flag())
         .arg(add_enable_simd_flag())
+        .arg(add_enable_multi_value())
 }
 
 fn handle_debug_flag(debug: bool) {
@@ -304,6 +311,7 @@ fn main() {
                     rest_cmd.is_present("time-passes"),
                     rest_cmd.is_present("value-ranges"),
                     rest_cmd.is_present("enable-simd"),
+                    rest_cmd.is_present("enable-multi-value"),
                 )
             };
 

cranelift/src/wasm.rs

@@ -50,6 +50,7 @@ pub fn run(
     flag_report_times: bool,
     flag_calc_value_ranges: bool,
     flag_enable_simd: bool,
+    flag_enable_multi_value: bool,
 ) -> Result<(), String> {
     let parsed = parse_sets_and_triple(flag_set, flag_triple)?;
 
@@ -66,6 +67,7 @@ pub fn run(
             flag_report_times,
             flag_calc_value_ranges,
             flag_enable_simd,
+            flag_enable_multi_value,
             &path.to_path_buf(),
             &name,
             parsed.as_fisa(),
@@ -84,6 +86,7 @@ fn handle_module(
     flag_report_times: bool,
     flag_calc_value_ranges: bool,
     flag_enable_simd: bool,
+    flag_enable_multi_value: bool,
     path: &PathBuf,
     name: &str,
     fisa: FlagsOrIsa,
@@ -104,6 +107,10 @@ fn handle_module(
         if flag_enable_simd {
             features.enable_simd();
         }
+        if flag_enable_multi_value {
+            features.enable_multi_value();
+        }
+
         module_binary = match wat2wasm_with_features(&module_binary, features) {
             Ok(data) => data,
             Err(e) => return Err(e.to_string()),

cranelift/wasm/src/code_translator.rs

@@ -23,10 +23,10 @@
 //! That is why `translate_function_body` takes an object having the `WasmRuntime` trait as
 //! argument.
 use super::{hash_map, HashMap};
-use crate::environ::{FuncEnvironment, GlobalVariable, ReturnMode, WasmResult};
+use crate::environ::{FuncEnvironment, GlobalVariable, ReturnMode, WasmResult, WasmTypesMap};
-use crate::state::{ControlStackFrame, TranslationState};
+use crate::state::{ControlStackFrame, ElseData, TranslationState};
 use crate::translation_utils::{
-    blocktype_to_type, f32_translation, f64_translation, num_return_values,
+    blocktype_params_results, ebb_with_params, f32_translation, f64_translation,
 };
 use crate::translation_utils::{FuncIndex, MemoryIndex, SignatureIndex, TableIndex};
 use crate::wasm_unsupported;
@@ -43,13 +43,14 @@ use wasmparser::{MemoryImmediate, Operator};
 /// Translates wasm operators into Cranelift IR instructions. Returns `true` if it inserted
 /// a return.
 pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
+    wasm_types: &WasmTypesMap,
     op: &Operator,
     builder: &mut FunctionBuilder,
     state: &mut TranslationState,
     environ: &mut FE,
 ) -> WasmResult<()> {
     if !state.reachable {
-        translate_unreachable_operator(&op, builder, state);
+        translate_unreachable_operator(wasm_types, &op, builder, state)?;
         return Ok(());
     }
 
@@ -132,27 +133,52 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
          *  possible `Ebb`'s arguments values.
          ***********************************************************************************/
         Operator::Block { ty } => {
-            let next = builder.create_ebb();
+            let (params, results) = blocktype_params_results(wasm_types, *ty)?;
-            if let Some(ty_cre) = blocktype_to_type(*ty)? {
+            let next = ebb_with_params(builder, results)?;
-                builder.append_ebb_param(next, ty_cre);
+            state.push_block(next, params.len(), results.len());
-            }
-            state.push_block(next, num_return_values(*ty)?);
         }
         Operator::Loop { ty } => {
-            let loop_body = builder.create_ebb();
+            let (params, results) = blocktype_params_results(wasm_types, *ty)?;
-            let next = builder.create_ebb();
+            let loop_body = ebb_with_params(builder, params)?;
-            if let Some(ty_cre) = blocktype_to_type(*ty)? {
+            let next = ebb_with_params(builder, results)?;
-                builder.append_ebb_param(next, ty_cre);
+            builder.ins().jump(loop_body, state.peekn(params.len()));
-            }
+            state.push_loop(loop_body, next, params.len(), results.len());
-            builder.ins().jump(loop_body, &[]);
+
-            state.push_loop(loop_body, next, num_return_values(*ty)?);
+            // Pop the initial `Ebb` actuals and replace them with the `Ebb`'s
+            // params since control flow joins at the top of the loop.
+            state.popn(params.len());
+            state.stack.extend_from_slice(builder.ebb_params(loop_body));
+
             builder.switch_to_block(loop_body);
             environ.translate_loop_header(builder.cursor())?;
         }
         Operator::If { ty } => {
             let val = state.pop1();
-            let if_not = builder.create_ebb();
+
-            let jump_inst = builder.ins().brz(val, if_not, &[]);
+            let (params, results) = blocktype_params_results(wasm_types, *ty)?;
+            let (destination, else_data) = if params == results {
+                // It is possible there is no `else` block, so we will only
+                // allocate an ebb for it if/when we find the `else`. For now,
+                // we if the condition isn't true, then we jump directly to the
+                // destination ebb following the whole `if...end`. If we do end
+                // up discovering an `else`, then we will allocate an ebb for it
+                // and go back and patch the jump.
+                let destination = ebb_with_params(builder, results)?;
+                let branch_inst = builder
+                    .ins()
+                    .brz(val, destination, state.peekn(params.len()));
+                (destination, ElseData::NoElse { branch_inst })
+            } else {
+                // The `if` type signature is not valid without an `else` block,
+                // so we eagerly allocate the `else` block here.
+                let destination = ebb_with_params(builder, results)?;
+                let else_block = ebb_with_params(builder, params)?;
+                builder
+                    .ins()
+                    .brz(val, else_block, state.peekn(params.len()));
+                builder.seal_block(else_block);
+                (destination, ElseData::WithElse { else_block })
+            };
 
             #[cfg(feature = "basic-blocks")]
             {
@@ -168,41 +194,63 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             //   and we add nothing;
             // - either the If have an Else clause, in that case the destination of this jump
             //   instruction will be changed later when we translate the Else operator.
-            if let Some(ty_cre) = blocktype_to_type(*ty)? {
+            state.push_if(destination, else_data, params.len(), results.len(), *ty);
-                builder.append_ebb_param(if_not, ty_cre);
-            }
-            state.push_if(jump_inst, if_not, num_return_values(*ty)?);
         }
         Operator::Else => {
-            // We take the control frame pushed by the if, use its ebb as the else body
-            // and push a new control frame with a new ebb for the code after the if/then/else
-            // At the end of the then clause we jump to the destination
             let i = state.control_stack.len() - 1;
-            let (destination, return_count, branch_inst, ref mut reachable_from_top) =
+            match state.control_stack[i] {
-                match state.control_stack[i] {
+                ControlStackFrame::If {
-                    ControlStackFrame::If {
+                    else_data: ElseData::NoElse { branch_inst },
-                        destination,
+                    ref mut reachable_from_top,
-                        num_return_values,
+                    blocktype,
-                        branch_inst,
+                    ..
-                        reachable_from_top,
+                } => {
-                        ..
+                    // We take the control frame pushed by the if, use its ebb
-                    } => (
+                    // as the else body and push a new control frame with a new
-                        destination,
+                    // ebb for the code after the if/then/else. At the end of the
-                        num_return_values,
+                    // then clause we jump to the destination.
-                        branch_inst,
+
-                        reachable_from_top,
+                    // The `if` has an `else`, so there's no branch to the end from the top.
-                    ),
+                    *reachable_from_top = false;
-                    _ => panic!("should not happen"),
+
-                };
+                    let (params, _results) = blocktype_params_results(wasm_types, blocktype)?;
-            // The if has an else, so there's no branch to the end from the top.
+                    let else_ebb = ebb_with_params(builder, params)?;
-            *reachable_from_top = false;
+                    builder.ins().jump(else_ebb, state.peekn(params.len()));
-            builder.ins().jump(destination, state.peekn(return_count));
+                    state.popn(params.len());
-            state.popn(return_count);
+
-            // We change the target of the branch instruction
+                    // You might be expecting that we push the parameters for this
-            let else_ebb = builder.create_ebb();
+                    // `else` block here, something like this:
-            builder.change_jump_destination(branch_inst, else_ebb);
+                    //
-            builder.seal_block(else_ebb);
+                    //     state.pushn(&control_stack_frame.params);
-            builder.switch_to_block(else_ebb);
+                    //
+                    // We don't do that because they are already on the top of the stack
+                    // for us: we pushed the parameters twice when we saw the initial
+                    // `if` so that we wouldn't have to save the parameters in the
+                    // `ControlStackFrame` as another `Vec` allocation.
+
+                    builder.change_jump_destination(branch_inst, else_ebb);
+                    builder.seal_block(else_ebb);
+                    builder.switch_to_block(else_ebb);
+
+                    // NB: we don't bother updating the control frame's
+                    // `ElseData` because nothing else will read it.
+                }
+                ControlStackFrame::If {
+                    destination,
+                    num_return_values,
+                    else_data: ElseData::WithElse { else_block, .. },
+                    reachable_from_top,
+                    ..
+                } => {
+                    debug_assert!(!reachable_from_top);
+                    builder
+                        .ins()
+                        .jump(destination, state.peekn(num_return_values));
+                    state.popn(num_return_values);
+                    builder.switch_to_block(else_block);
+                }
+                _ => unreachable!(),
+            }
         }
         Operator::End => {
             let frame = state.control_stack.pop().unwrap();
@@ -211,6 +259,12 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
                 builder
                     .ins()
                     .jump(frame.following_code(), state.peekn(return_count));
+                // You might expect that if we just finished an `if` block that
+                // didn't have a corresponding `else` block, then we would clean
+                // up our duplicate set of parameters that we pushed earlier
+                // right here. However, we don't have to explicitly do that,
+                // since we truncate the stack back to the original height
+                // below.
             }
             builder.switch_to_block(frame.following_code());
             builder.seal_block(frame.following_code());
@@ -1139,40 +1193,71 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
 /// are dropped but special ones like `End` or `Else` signal the potential end of the unreachable
 /// portion so the translation state must be updated accordingly.
 fn translate_unreachable_operator(
+    wasm_types: &WasmTypesMap,
     op: &Operator,
     builder: &mut FunctionBuilder,
     state: &mut TranslationState,
-) {
+) -> WasmResult<()> {
     match *op {
-        Operator::If { ty: _ } => {
+        Operator::If { ty } => {
             // Push a placeholder control stack entry. The if isn't reachable,
             // so we don't have any branches anywhere.
-            state.push_if(ir::Inst::reserved_value(), ir::Ebb::reserved_value(), 0);
+            state.push_if(
+                ir::Ebb::reserved_value(),
+                ElseData::NoElse {
+                    branch_inst: ir::Inst::reserved_value(),
+                },
+                0,
+                0,
+                ty,
+            );
         }
         Operator::Loop { ty: _ } | Operator::Block { ty: _ } => {
-            state.push_block(ir::Ebb::reserved_value(), 0);
+            state.push_block(ir::Ebb::reserved_value(), 0, 0);
         }
         Operator::Else => {
             let i = state.control_stack.len() - 1;
-            if let ControlStackFrame::If {
+            match state.control_stack[i] {
-                branch_inst,
+                ControlStackFrame::If {
-                ref mut reachable_from_top,
+                    else_data: ElseData::NoElse { branch_inst },
-                ..
+                    ref mut reachable_from_top,
-            } = state.control_stack[i]
+                    blocktype,
-            {
+                    ..
-                if *reachable_from_top {
+                } => {
-                    // We have a branch from the top of the if to the else.
+                    if *reachable_from_top {
-                    state.reachable = true;
+                        // We have a branch from the top of the if to the else.
-                    // And because there's an else, there can no longer be a
+                        state.reachable = true;
-                    // branch from the top directly to the end.
+                        // And because there's an else, there can no longer be a
-                    *reachable_from_top = false;
+                        // branch from the top directly to the end.
+                        *reachable_from_top = false;
 
-                    // We change the target of the branch instruction
+                        let (params, _results) = blocktype_params_results(wasm_types, blocktype)?;
-                    let else_ebb = builder.create_ebb();
+                        let else_ebb = ebb_with_params(builder, params)?;
-                    builder.change_jump_destination(branch_inst, else_ebb);
+
-                    builder.seal_block(else_ebb);
+                        // We change the target of the branch instruction.
-                    builder.switch_to_block(else_ebb);
+                        builder.change_jump_destination(branch_inst, else_ebb);
+                        builder.seal_block(else_ebb);
+                        builder.switch_to_block(else_ebb);
+
+                        // Again, no need to push the parameters for the `else`,
+                        // since we already did when we saw the original `if`. See
+                        // the comment for translating `Operator::Else` in
+                        // `translate_operator` for details.
+                    }
                 }
+                ControlStackFrame::If {
+                    else_data: ElseData::WithElse { else_block, .. },
+                    reachable_from_top,
+                    ..
+                } => {
+                    debug_assert!(
+                        !reachable_from_top,
+                        "should not be reachable from top if we have an else block"
+                    );
+                    builder.switch_to_block(else_block);
+                    state.reachable = true;
+                }
+                _ => unreachable!(),
             }
         }
         Operator::End => {
@@ -1192,7 +1277,17 @@ fn translate_unreachable_operator(
                     false
                 }
                 ControlStackFrame::If {
-                    reachable_from_top, ..
+                    else_data: ElseData::WithElse { .. },
+                    reachable_from_top,
+                    ..
+                } => {
+                    debug_assert!(!reachable_from_top);
+                    true
+                }
+                ControlStackFrame::If {
+                    else_data: ElseData::NoElse { .. },
+                    reachable_from_top,
+                    ..
                 } => {
                     // A reachable if without an else has a branch from the top
                     // directly to the bottom.
@@ -1216,6 +1311,8 @@ fn translate_unreachable_operator(
             // We don't translate because this is unreachable code
         }
     }
+
+    Ok(())
 }
 
 /// Get the address+offset to use for a heap access.
@@ -1342,7 +1439,7 @@ fn translate_br_if_args(
         // code that comes after it
         frame.set_branched_to_exit();
         let return_count = if frame.is_loop() {
-            0
+            frame.num_param_values()
         } else {
             frame.num_return_values()
         };

cranelift/wasm/src/environ/dummy.rs

@@ -5,7 +5,9 @@
 //! [wasmtime-environ]: https://crates.io/crates/wasmtime-environ
 //! [Wasmtime]: https://github.com/CraneStation/wasmtime
 
-use crate::environ::{FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, WasmResult};
+use crate::environ::{
+    FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, WasmResult, WasmTypesMap,
+};
 use crate::func_translator::FuncTranslator;
 use crate::translation_utils::{
     DefinedFuncIndex, FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table,
@@ -529,6 +531,7 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
 
     fn define_function_body(
         &mut self,
+        wasm_types: &WasmTypesMap,
         body_bytes: &'data [u8],
         body_offset: usize,
     ) -> WasmResult<()> {
@@ -542,8 +545,13 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
             if self.debug_info {
                 func.collect_debug_info();
             }
-            self.trans
+            self.trans.translate(
-                .translate(body_bytes, body_offset, &mut func, &mut func_environ)?;
+                wasm_types,
+                body_bytes,
+                body_offset,
+                &mut func,
+                &mut func_environ,
+            )?;
             func
         };
         self.func_bytecode_sizes.push(body_bytes.len());

cranelift/wasm/src/environ/mod.rs

@@ -7,4 +7,5 @@ mod spec;
 pub use crate::environ::dummy::DummyEnvironment;
 pub use crate::environ::spec::{
     FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, WasmError, WasmResult,
+    WasmTypesMap,
 };

cranelift/wasm/src/environ/spec.rs

@@ -15,6 +15,7 @@ use cranelift_codegen::cursor::FuncCursor;
 use cranelift_codegen::ir::immediates::Offset32;
 use cranelift_codegen::ir::{self, InstBuilder};
 use cranelift_codegen::isa::TargetFrontendConfig;
+use cranelift_entity::PrimaryMap;
 use cranelift_frontend::FunctionBuilder;
 use failure_derive::Fail;
 use std::boxed::Box;
@@ -103,6 +104,24 @@ pub enum ReturnMode {
     FallthroughReturn,
 }
 
+/// A map containing a Wasm module's original, raw signatures.
+///
+/// This is used for translating multi-value Wasm blocks inside functions, which
+/// are encoded to refer to their type signature via index.
+#[derive(Debug)]
+pub struct WasmTypesMap {
+    pub(crate) inner:
+        PrimaryMap<SignatureIndex, (Box<[wasmparser::Type]>, Box<[wasmparser::Type]>)>,
+}
+
+impl WasmTypesMap {
+    pub(crate) fn new() -> Self {
+        WasmTypesMap {
+            inner: PrimaryMap::new(),
+        }
+    }
+}
+
 /// Environment affecting the translation of a single WebAssembly function.
 ///
 /// A `FuncEnvironment` trait object is required to translate a WebAssembly function to Cranelift
@@ -449,6 +468,7 @@ pub trait ModuleEnvironment<'data> {
     /// functions is already provided by `reserve_func_types`.
     fn define_function_body(
         &mut self,
+        wasm_types: &WasmTypesMap,
         body_bytes: &'data [u8],
         body_offset: usize,
     ) -> WasmResult<()>;

cranelift/wasm/src/func_translator.rs

@@ -5,7 +5,7 @@
 //! WebAssembly module and the runtime environment.
 
 use crate::code_translator::translate_operator;
-use crate::environ::{FuncEnvironment, ReturnMode, WasmResult};
+use crate::environ::{FuncEnvironment, ReturnMode, WasmResult, WasmTypesMap};
 use crate::state::TranslationState;
 use crate::translation_utils::get_vmctx_value_label;
 use crate::wasm_unsupported;
@@ -55,12 +55,14 @@ impl FuncTranslator {
     ///
     pub fn translate<FE: FuncEnvironment + ?Sized>(
         &mut self,
+        wasm_types: &WasmTypesMap,
         code: &[u8],
         code_offset: usize,
         func: &mut ir::Function,
         environ: &mut FE,
     ) -> WasmResult<()> {
         self.translate_from_reader(
+            wasm_types,
             BinaryReader::new_with_offset(code, code_offset),
             func,
             environ,
@@ -70,6 +72,7 @@ impl FuncTranslator {
     /// Translate a binary WebAssembly function from a `BinaryReader`.
     pub fn translate_from_reader<FE: FuncEnvironment + ?Sized>(
         &mut self,
+        wasm_types: &WasmTypesMap,
         mut reader: BinaryReader,
         func: &mut ir::Function,
         environ: &mut FE,
@@ -105,7 +108,7 @@ impl FuncTranslator {
         self.state.initialize(&builder.func.signature, exit_block);
 
         parse_local_decls(&mut reader, &mut builder, num_params, environ)?;
-        parse_function_body(reader, &mut builder, &mut self.state, environ)?;
+        parse_function_body(wasm_types, reader, &mut builder, &mut self.state, environ)?;
 
         builder.finalize();
         Ok(())
@@ -203,6 +206,7 @@ fn declare_locals<FE: FuncEnvironment + ?Sized>(
 /// This assumes that the local variable declarations have already been parsed and function
 /// arguments and locals are declared in the builder.
 fn parse_function_body<FE: FuncEnvironment + ?Sized>(
+    wasm_types: &WasmTypesMap,
     mut reader: BinaryReader,
     builder: &mut FunctionBuilder,
     state: &mut TranslationState,
@@ -216,7 +220,7 @@ fn parse_function_body<FE: FuncEnvironment + ?Sized>(
         builder.set_srcloc(cur_srcloc(&reader));
         let op = reader.read_operator()?;
         environ.before_translate_operator(&op, builder, state)?;
-        translate_operator(&op, builder, state, environ)?;
+        translate_operator(wasm_types, &op, builder, state, environ)?;
         environ.after_translate_operator(&op, builder, state)?;
     }
 
@@ -254,7 +258,7 @@ fn cur_srcloc(reader: &BinaryReader) -> ir::SourceLoc {
 #[cfg(test)]
 mod tests {
     use super::{FuncTranslator, ReturnMode};
-    use crate::environ::DummyEnvironment;
+    use crate::environ::{DummyEnvironment, WasmTypesMap};
     use cranelift_codegen::ir::types::I32;
     use cranelift_codegen::{ir, isa, settings, Context};
     use log::debug;
@@ -286,6 +290,7 @@ mod tests {
             false,
         );
 
+        let wasm_types = WasmTypesMap::new();
         let mut ctx = Context::new();
 
         ctx.func.name = ir::ExternalName::testcase("small1");
@@ -293,7 +298,13 @@ mod tests {
         ctx.func.signature.returns.push(ir::AbiParam::new(I32));
 
         trans
-            .translate(&BODY, 0, &mut ctx.func, &mut runtime.func_env())
+            .translate(
+                &wasm_types,
+                &BODY,
+                0,
+                &mut ctx.func,
+                &mut runtime.func_env(),
+            )
             .unwrap();
         debug!("{}", ctx.func.display(None));
         ctx.verify(&flags).unwrap();
@@ -325,6 +336,8 @@ mod tests {
             ReturnMode::NormalReturns,
             false,
         );
+
+        let wasm_types = WasmTypesMap::new();
         let mut ctx = Context::new();
 
         ctx.func.name = ir::ExternalName::testcase("small2");
@@ -332,7 +345,13 @@ mod tests {
         ctx.func.signature.returns.push(ir::AbiParam::new(I32));
 
         trans
-            .translate(&BODY, 0, &mut ctx.func, &mut runtime.func_env())
+            .translate(
+                &wasm_types,
+                &BODY,
+                0,
+                &mut ctx.func,
+                &mut runtime.func_env(),
+            )
             .unwrap();
         debug!("{}", ctx.func.display(None));
         ctx.verify(&flags).unwrap();
@@ -373,13 +392,21 @@ mod tests {
             ReturnMode::NormalReturns,
             false,
         );
+
+        let wasm_types = WasmTypesMap::new();
         let mut ctx = Context::new();
 
         ctx.func.name = ir::ExternalName::testcase("infloop");
         ctx.func.signature.returns.push(ir::AbiParam::new(I32));
 
         trans
-            .translate(&BODY, 0, &mut ctx.func, &mut runtime.func_env())
+            .translate(
+                &wasm_types,
+                &BODY,
+                0,
+                &mut ctx.func,
+                &mut runtime.func_env(),
+            )
             .unwrap();
         debug!("{}", ctx.func.display(None));
         ctx.verify(&flags).unwrap();

cranelift/wasm/src/lib.rs

@@ -59,7 +59,7 @@ mod translation_utils;
 
 pub use crate::environ::{
     DummyEnvironment, FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, WasmError,
-    WasmResult,
+    WasmResult, WasmTypesMap,
 };
 pub use crate::func_translator::FuncTranslator;
 pub use crate::module_translator::translate_module;

cranelift/wasm/src/module_translator.rs

@@ -1,6 +1,6 @@
 //! Translation skeleton that traverses the whole WebAssembly module and call helper functions
 //! to deal with each part of it.
-use crate::environ::{ModuleEnvironment, WasmError, WasmResult};
+use crate::environ::{ModuleEnvironment, WasmError, WasmResult, WasmTypesMap};
 use crate::sections_translator::{
     parse_code_section, parse_data_section, parse_element_section, parse_export_section,
     parse_function_section, parse_global_section, parse_import_section, parse_memory_section,
@@ -17,12 +17,13 @@ pub fn translate_module<'data>(
 ) -> WasmResult<()> {
     let _tt = timing::wasm_translate_module();
     let mut reader = ModuleReader::new(data)?;
+    let mut wasm_types = WasmTypesMap::new();
 
     while !reader.eof() {
         let section = reader.read()?;
         match section.content()? {
             SectionContent::Type(types) => {
-                parse_type_section(types, environ)?;
+                parse_type_section(types, &mut wasm_types, environ)?;
             }
 
             SectionContent::Import(imports) => {
@@ -58,7 +59,7 @@ pub fn translate_module<'data>(
             }
 
             SectionContent::Code(code) => {
-                parse_code_section(code, environ)?;
+                parse_code_section(code, &wasm_types, environ)?;
             }
 
             SectionContent::Data(data) => {

cranelift/wasm/src/sections_translator.rs

@@ -7,7 +7,7 @@
 //! The special case of the initialize expressions for table elements offsets or global variables
 //! is handled, according to the semantics of WebAssembly, to only specific expressions that are
 //! interpreted on the fly.
-use crate::environ::{ModuleEnvironment, WasmResult};
+use crate::environ::{ModuleEnvironment, WasmResult, WasmTypesMap};
 use crate::translation_utils::{
     tabletype_to_type, type_to_type, FuncIndex, Global, GlobalIndex, GlobalInit, Memory,
     MemoryIndex, SignatureIndex, Table, TableElementType, TableIndex,
@@ -29,16 +29,19 @@ use wasmparser::{
 /// Parses the Type section of the wasm module.
 pub fn parse_type_section(
     types: TypeSectionReader,
+    wasm_types: &mut WasmTypesMap,
     environ: &mut dyn ModuleEnvironment,
 ) -> WasmResult<()> {
-    environ.reserve_signatures(types.get_count())?;
+    let count = types.get_count();
+    wasm_types.inner.reserve(count as usize);
+    environ.reserve_signatures(count)?;
 
     for entry in types {
         match entry? {
             FuncType {
                 form: wasmparser::Type::Func,
-                ref params,
+                params,
-                ref returns,
+                returns,
             } => {
                 let mut sig = Signature::new(environ.target_config().default_call_conv);
                 sig.params.extend(params.iter().map(|ty| {
@@ -52,6 +55,7 @@ pub fn parse_type_section(
                     AbiParam::new(cret_arg)
                 }));
                 environ.declare_signature(sig)?;
+                wasm_types.inner.push((params, returns));
             }
             ty => {
                 return Err(wasm_unsupported!(
@@ -323,13 +327,14 @@ pub fn parse_element_section<'data>(
 /// Parses the Code section of the wasm module.
 pub fn parse_code_section<'data>(
     code: CodeSectionReader<'data>,
+    wasm_types: &WasmTypesMap,
     environ: &mut dyn ModuleEnvironment<'data>,
 ) -> WasmResult<()> {
     for body in code {
         let mut reader = body?.get_binary_reader();
         let size = reader.bytes_remaining();
         let offset = reader.original_position();
-        environ.define_function_body(reader.read_bytes(size)?, offset)?;
+        environ.define_function_body(wasm_types, reader.read_bytes(size)?, offset)?;
     }
     Ok(())
 }

cranelift/wasm/src/state.rs

@@ -9,6 +9,24 @@ use crate::translation_utils::{FuncIndex, GlobalIndex, MemoryIndex, SignatureInd
 use cranelift_codegen::ir::{self, Ebb, Inst, Value};
 use std::vec::Vec;
 
+/// Information about the presence of an associated `else` for an `if`, or the
+/// lack thereof.
+#[derive(Debug)]
+pub enum ElseData {
+    /// The `if` does not already have an `else` block.
+    NoElse {
+        /// If we discover that we need an `else` block, this is the jump
+        /// instruction that needs to be fixed up to point to the new `else`
+        /// block rather than the destination block after the `if...end`.
+        branch_inst: Inst,
+    },
+    /// We have already allocated an `else` block.
+    WithElse {
+        /// This is the `else` block.
+        else_block: Ebb,
+    },
+}
+
 /// A control stack frame can be an `if`, a `block` or a `loop`, each one having the following
 /// fields:
 ///
@@ -23,14 +41,17 @@ use std::vec::Vec;
 pub enum ControlStackFrame {
     If {
         destination: Ebb,
-        branch_inst: Inst,
+        else_data: ElseData,
+        num_param_values: usize,
         num_return_values: usize,
         original_stack_size: usize,
         exit_is_branched_to: bool,
         reachable_from_top: bool,
+        blocktype: wasmparser::TypeOrFuncType,
     },
     Block {
         destination: Ebb,
+        num_param_values: usize,
         num_return_values: usize,
         original_stack_size: usize,
         exit_is_branched_to: bool,
@@ -38,6 +59,7 @@ pub enum ControlStackFrame {
     Loop {
         destination: Ebb,
         header: Ebb,
+        num_param_values: usize,
         num_return_values: usize,
         original_stack_size: usize,
     },
@@ -58,6 +80,19 @@ impl ControlStackFrame {
             } => num_return_values,
         }
     }
+    pub fn num_param_values(&self) -> usize {
+        match *self {
+            ControlStackFrame::If {
+                num_param_values, ..
+            }
+            | ControlStackFrame::Block {
+                num_param_values, ..
+            }
+            | ControlStackFrame::Loop {
+                num_param_values, ..
+            } => num_param_values,
+        }
+    }
     pub fn following_code(&self) -> Ebb {
         match *self {
             ControlStackFrame::If { destination, .. }
@@ -202,6 +237,7 @@ impl TranslationState {
         self.clear();
         self.push_block(
             exit_block,
+            0,
             sig.returns
                 .iter()
                 .filter(|arg| arg.purpose == ir::ArgumentPurpose::Normal)
@@ -221,12 +257,17 @@ impl TranslationState {
 
     /// Pop one value.
     pub(crate) fn pop1(&mut self) -> Value {
-        self.stack.pop().unwrap()
+        self.stack
+            .pop()
+            .expect("attempted to pop a value from an empty stack")
     }
 
     /// Peek at the top of the stack without popping it.
     pub(crate) fn peek1(&self) -> Value {
-        *self.stack.last().unwrap()
+        *self
+            .stack
+            .last()
+            .expect("attempted to peek at a value on an empty stack")
     }
 
     /// Pop two values. Return them in the order they were pushed.
@@ -248,31 +289,58 @@ impl TranslationState {
     ///
     /// The popped values are not returned. Use `peekn` to look at them before popping.
     pub(crate) fn popn(&mut self, n: usize) {
+        debug_assert!(
+            n <= self.stack.len(),
+            "popn({}) but stack only has {} values",
+            n,
+            self.stack.len()
+        );
         let new_len = self.stack.len() - n;
         self.stack.truncate(new_len);
     }
 
     /// Peek at the top `n` values on the stack in the order they were pushed.
     pub(crate) fn peekn(&self, n: usize) -> &[Value] {
+        debug_assert!(
+            n <= self.stack.len(),
+            "peekn({}) but stack only has {} values",
+            n,
+            self.stack.len()
+        );
         &self.stack[self.stack.len() - n..]
     }
 
     /// Push a block on the control stack.
-    pub(crate) fn push_block(&mut self, following_code: Ebb, num_result_types: usize) {
+    pub(crate) fn push_block(
+        &mut self,
+        following_code: Ebb,
+        num_param_types: usize,
+        num_result_types: usize,
+    ) {
+        debug_assert!(num_param_types <= self.stack.len());
         self.control_stack.push(ControlStackFrame::Block {
             destination: following_code,
-            original_stack_size: self.stack.len(),
+            original_stack_size: self.stack.len() - num_param_types,
+            num_param_values: num_param_types,
             num_return_values: num_result_types,
             exit_is_branched_to: false,
         });
     }
 
     /// Push a loop on the control stack.
-    pub(crate) fn push_loop(&mut self, header: Ebb, following_code: Ebb, num_result_types: usize) {
+    pub(crate) fn push_loop(
+        &mut self,
+        header: Ebb,
+        following_code: Ebb,
+        num_param_types: usize,
+        num_result_types: usize,
+    ) {
+        debug_assert!(num_param_types <= self.stack.len());
         self.control_stack.push(ControlStackFrame::Loop {
             header,
             destination: following_code,
-            original_stack_size: self.stack.len(),
+            original_stack_size: self.stack.len() - num_param_types,
+            num_param_values: num_param_types,
             num_return_values: num_result_types,
         });
     }
@@ -280,17 +348,39 @@ impl TranslationState {
     /// Push an if on the control stack.
     pub(crate) fn push_if(
         &mut self,
-        branch_inst: Inst,
+        destination: Ebb,
-        following_code: Ebb,
+        else_data: ElseData,
+        num_param_types: usize,
         num_result_types: usize,
+        blocktype: wasmparser::TypeOrFuncType,
     ) {
+        debug_assert!(num_param_types <= self.stack.len());
+
+        // Push a second copy of our `if`'s parameters on the stack. This lets
+        // us avoid saving them on the side in the `ControlStackFrame` for our
+        // `else` block (if it exists), which would require a second heap
+        // allocation. See also the comment in `translate_operator` for
+        // `Operator::Else`.
+        self.stack.reserve(num_param_types);
+        for i in (self.stack.len() - num_param_types)..self.stack.len() {
+            let val = self.stack[i];
+            self.stack.push(val);
+        }
+
+        let has_else = match else_data {
+            ElseData::NoElse { .. } => false,
+            ElseData::WithElse { .. } => true,
+        };
+
         self.control_stack.push(ControlStackFrame::If {
-            branch_inst,
+            destination,
-            destination: following_code,
+            else_data,
-            original_stack_size: self.stack.len(),
+            original_stack_size: self.stack.len() - num_param_types,
+            num_param_values: num_param_types,
             num_return_values: num_result_types,
             exit_is_branched_to: false,
-            reachable_from_top: self.reachable,
+            reachable_from_top: self.reachable && !has_else,
+            blocktype,
         });
     }
 }

cranelift/wasm/src/translation_utils.rs

@@ -1,10 +1,11 @@
 //! Helper functions and structures for the translation.
-use crate::environ::WasmResult;
+use crate::environ::{WasmResult, WasmTypesMap};
 use crate::wasm_unsupported;
 use core::u32;
 use cranelift_codegen::entity::entity_impl;
 use cranelift_codegen::ir;
 use cranelift_codegen::ir::immediates::V128Imm;
+use cranelift_frontend::FunctionBuilder;
 #[cfg(feature = "enable-serde")]
 use serde::{Deserialize, Serialize};
 use wasmparser;
@@ -145,23 +146,61 @@ pub fn tabletype_to_type(ty: wasmparser::Type) -> WasmResult<Option<ir::Type>> {
     }
 }
 
-/// Helper function translating wasmparser block signatures to Cranelift types when possible.
+/// Get the parameter and result types for the given Wasm blocktype.
-pub fn blocktype_to_type(ty_or_ft: wasmparser::TypeOrFuncType) -> WasmResult<Option<ir::Type>> {
+pub fn blocktype_params_results(
-    match ty_or_ft {
+    wasm_types: &WasmTypesMap,
+    ty_or_ft: wasmparser::TypeOrFuncType,
+) -> WasmResult<(&[wasmparser::Type], &[wasmparser::Type])> {
+    Ok(match ty_or_ft {
         wasmparser::TypeOrFuncType::Type(ty) => match ty {
-            wasmparser::Type::I32 => Ok(Some(ir::types::I32)),
+            wasmparser::Type::I32 => (&[], &[wasmparser::Type::I32]),
-            wasmparser::Type::I64 => Ok(Some(ir::types::I64)),
+            wasmparser::Type::I64 => (&[], &[wasmparser::Type::I64]),
-            wasmparser::Type::F32 => Ok(Some(ir::types::F32)),
+            wasmparser::Type::F32 => (&[], &[wasmparser::Type::F32]),
-            wasmparser::Type::F64 => Ok(Some(ir::types::F64)),
+            wasmparser::Type::F64 => (&[], &[wasmparser::Type::F64]),
-            wasmparser::Type::V128 => Ok(Some(ir::types::I8X16)),
+            wasmparser::Type::V128 => (&[], &[wasmparser::Type::V128]),
-            wasmparser::Type::EmptyBlockType => Ok(None),
+            wasmparser::Type::EmptyBlockType => (&[], &[]),
-            ty => Err(wasm_unsupported!("blocktype_to_type: type {:?}", ty)),
+            ty => return Err(wasm_unsupported!("blocktype_params_results: type {:?}", ty)),
         },
-        wasmparser::TypeOrFuncType::FuncType(_) => Err(wasm_unsupported!(
+        wasmparser::TypeOrFuncType::FuncType(ty_index) => {
-            "blocktype_to_type: multi-value block signature {:?}",
+            let sig_idx = SignatureIndex::from_u32(ty_index);
-            ty_or_ft
+            let (ref params, ref returns) = wasm_types.inner[sig_idx];
-        )),
+            (&*params, &*returns)
+        }
+    })
+}
+
+/// Create an `Ebb` with the given Wasm parameters.
+pub fn ebb_with_params(
+    builder: &mut FunctionBuilder,
+    params: &[wasmparser::Type],
+) -> WasmResult<ir::Ebb> {
+    let ebb = builder.create_ebb();
+    for ty in params.iter() {
+        match ty {
+            wasmparser::Type::I32 => {
+                builder.append_ebb_param(ebb, ir::types::I32);
+            }
+            wasmparser::Type::I64 => {
+                builder.append_ebb_param(ebb, ir::types::I64);
+            }
+            wasmparser::Type::F32 => {
+                builder.append_ebb_param(ebb, ir::types::F32);
+            }
+            wasmparser::Type::F64 => {
+                builder.append_ebb_param(ebb, ir::types::F64);
+            }
+            wasmparser::Type::V128 => {
+                builder.append_ebb_param(ebb, ir::types::I8X16);
+            }
+            ty => {
+                return Err(wasm_unsupported!(
+                    "ebb_with_params: type {:?} in multi-value block's signature",
+                    ty
+                ))
+            }
+        }
     }
+    Ok(ebb)
 }
 
 /// Turns a `wasmparser` `f32` into a `Cranelift` one.
@@ -174,24 +213,6 @@ pub fn f64_translation(x: wasmparser::Ieee64) -> ir::immediates::Ieee64 {
     ir::immediates::Ieee64::with_bits(x.bits())
 }
 
-/// Translate a `wasmparser` type into its `Cranelift` equivalent, when possible
-pub fn num_return_values(ty: wasmparser::TypeOrFuncType) -> WasmResult<usize> {
-    match ty {
-        wasmparser::TypeOrFuncType::Type(ty) => match ty {
-            wasmparser::Type::EmptyBlockType => Ok(0),
-            wasmparser::Type::I32
-            | wasmparser::Type::F32
-            | wasmparser::Type::I64
-            | wasmparser::Type::F64
-            | wasmparser::Type::V128 => Ok(1),
-            ty => Err(wasm_unsupported!("unsupported return value type {:?}", ty)),
-        },
-        wasmparser::TypeOrFuncType::FuncType(_) => {
-            Err(wasm_unsupported!("multi-value block signature {:?}", ty))
-        }
-    }
-}
-
 /// Special VMContext value label. It is tracked as 0xffff_fffe label.
 pub fn get_vmctx_value_label() -> ir::ValueLabel {
     const VMCTX_LABEL: u32 = 0xffff_fffe;

cranelift/wasm/tests/wasm_testsuite.rs

@@ -31,6 +31,7 @@ fn testsuite() {
     let flags = Flags::new(settings::builder());
     for path in paths {
         let path = path.path();
+        println!("=== {} ===", path.display());
         let data = read_module(&path);
         handle_module(data, &flags, ReturnMode::NormalReturns);
     }

cranelift/wasmtests/fac-multi-value.wat

@@ -0,0 +1,19 @@
+(module
+  ;; Iterative factorial without locals.
+  (func $pick0 (param i64) (result i64 i64)
+    (get_local 0) (get_local 0)
+  )
+  (func $pick1 (param i64 i64) (result i64 i64 i64)
+    (get_local 0) (get_local 1) (get_local 0)
+  )
+  (func (export "fac-ssa") (param i64) (result i64)
+    (i64.const 1) (get_local 0)
+    (loop $l (param i64 i64) (result i64)
+      (call $pick1) (call $pick1) (i64.mul)
+      (call $pick1) (i64.const 1) (i64.sub)
+      (call $pick0) (i64.const 0) (i64.gt_u)
+      (br_if $l)
+      (drop) (return)
+    )
+  )
+)

cranelift/wasmtests/multi-0.wat

@@ -0,0 +1,3 @@
+(module
+  (func (export "i64.dup") (param i64) (result i64 i64)
+    (get_local 0) (get_local 0)))

cranelift/wasmtests/multi-1.wat

@@ -0,0 +1,6 @@
+(module
+  (func (export "multiBlock") (param i64 i32) (result i32 i64 f64)
+    (local.get 1)
+    (local.get 0)
+    (block (param i32 i64) (result i32 i64 f64)
+      (f64.const 1234.5))))

cranelift/wasmtests/multi-10.wat

@@ -0,0 +1,10 @@
+(module
+  (func (export "f") (param i64 i32) (result i64 i64)
+    (local.get 0)
+    (local.get 1)
+    ;; If with else. Fewer params than results.
+    (if (param i64) (result i64 i64)
+      (then
+        (i64.const -1))
+      (else
+        (i64.const -2)))))

cranelift/wasmtests/multi-11.wat

@@ -0,0 +1,7 @@
+(module
+  (func (export "multiLoop") (param i64) (result i64 i64)
+    (local.get 0)
+    ;; Fewer params than results.
+    (loop (param i64) (result i64 i64)
+      i64.const 42
+      return)))

cranelift/wasmtests/multi-12.wat

@@ -0,0 +1,9 @@
+(module
+  (func (export "multiLoop") (param i64 i64 i64) (result i64 i64)
+    (local.get 2)
+    (local.get 1)
+    (local.get 0)
+    ;; More params than results.
+    (loop (param i64 i64 i64) (result i64 i64)
+      drop
+      return)))

cranelift/wasmtests/multi-13.wat

@@ -0,0 +1,10 @@
+(module
+  (func (export "as-if-then") (param i32 i32) (result i32)
+    (block (result i32)
+      (if (result i32) (local.get 0)
+        (then (br 1 (i32.const 3)))
+        (else (local.get 1))
+      )
+    )
+  )
+)

cranelift/wasmtests/multi-14.wat

@@ -0,0 +1,10 @@
+(module
+  (func (export "as-if-else") (param i32 i32) (result i32)
+    (block (result i32)
+      (if (result i32) (local.get 0)
+        (then (local.get 1))
+        (else (br 1 (i32.const 4)))
+      )
+    )
+  )
+)

cranelift/wasmtests/multi-15.wat

@@ -0,0 +1,22 @@
+(module
+  (func (export "large-sig")
+    (param i32 i64 f32 f32 i32 f64 f32 i32 i32 i32 f32 f64 f64 f64 i32 i32 f32)
+    (result f64 f32 i32 i32 i32 i64 f32 i32 i32 f32 f64 f64 i32 f32 i32 f64)
+    (local.get 5)
+    (local.get 2)
+    (local.get 0)
+    (local.get 8)
+    (local.get 7)
+    (local.get 1)
+    (local.get 3)
+    (local.get 9)
+    (local.get 4)
+    (local.get 6)
+    (local.get 13)
+    (local.get 11)
+    (local.get 15)
+    (local.get 16)
+    (local.get 14)
+    (local.get 12)
+  )
+)

cranelift/wasmtests/multi-2.wat

@@ -0,0 +1,6 @@
+(module
+  (func (export "multiLoop") (param i64 i64) (result i64 i64)
+    (local.get 1)
+    (local.get 0)
+    (loop (param i64 i64) (result i64 i64)
+       return)))

cranelift/wasmtests/multi-3.wat

@@ -0,0 +1,13 @@
+(module
+  (func (export "multiIf") (param i32 i64 i64) (result i64 i64)
+    (local.get 2)
+    (local.get 1)
+    (local.get 0)
+    (if (param i64 i64) (result i64 i64)
+      (then return)
+      ;; Hits the code path for an `else` after a block that ends unreachable.
+      (else
+        (drop)
+        (drop)
+        (i64.const 0)
+        (i64.const 0)))))

cranelift/wasmtests/multi-4.wat

@@ -0,0 +1,13 @@
+(module
+  (func (export "multiIf2") (param i32 i64 i64) (result i64 i64)
+    (local.get 2)
+    (local.get 1)
+    (local.get 0)
+    (if (param i64 i64) (result i64 i64)
+      (then
+        i64.add
+        i64.const 1)
+      ;; Hits the code path for an `else` after a block that does not end unreachable.
+      (else
+        i64.sub
+        i64.const 2))))

cranelift/wasmtests/multi-5.wat

@@ -0,0 +1,11 @@
+(module
+  (func (export "foo")
+    i32.const 1
+    i64.const 2
+    ;; More params than results.
+    (block (param i32 i64) (result i32)
+      drop
+    )
+    drop
+  )
+)

cranelift/wasmtests/multi-6.wat

@@ -0,0 +1,11 @@
+(module
+  (func (export "foo")
+    i32.const 1
+    ;; Fewer params than results.
+    (block (param i32) (result i32 i64)
+      i64.const 2
+    )
+    drop
+    drop
+  )
+)

cranelift/wasmtests/multi-7.wat

@@ -0,0 +1,9 @@
+(module
+  (func (export "f") (param i64 i32) (result i64)
+    (local.get 0)
+    (local.get 1)
+    ;; If with no else. Same number of params and results.
+    (if (param i64) (result i64)
+      (then
+        (drop)
+        (i64.const -1)))))

cranelift/wasmtests/multi-8.wat

@@ -0,0 +1,12 @@
+(module
+  (func (export "f") (param i64 i32) (result i64)
+    (local.get 0)
+    (local.get 1)
+    ;; If with else. Same number of params and results.
+    (if (param i64) (result i64)
+      (then
+        (drop)
+        (i64.const -1))
+      (else
+        (drop)
+        (i64.const -2)))))

cranelift/wasmtests/multi-9.wat

@@ -0,0 +1,15 @@
+(module
+  (func (export "f") (param i64 i32) (result i64)
+    (local.get 0)
+    (local.get 1)
+    (local.get 1)
+    ;; If with else. More params than results.
+    (if (param i64 i32) (result i64)
+      (then
+        (drop)
+        (drop)
+        (i64.const -1))
+      (else
+        (drop)
+        (drop)
+        (i64.const -2)))))