Add initial support for the multi-memory proposal

This commit adds initial (gated) support for the multi-memory wasm proposal. This was actually quite easy since almost all of wasmtime already expected multi-memory to be implemented one day. The only real substantive change is the `memory.copy` intrinsic changes, which now accounts for the source/destination memories possibly being different.
bytecodealliance · Oct 5, 2020 · 81633b0 · 81633b0
1 parent 2c68410
commit 81633b0
Show file tree

Hide file tree

Showing 13 changed files with 244 additions and 107 deletions.
diff --git a/build.rs b/build.rs
@@ -30,6 +30,7 @@ fn main() -> anyhow::Result<()> {
             test_directory(out, "tests/misc_testsuite", strategy)?;
             test_directory_module(out, "tests/misc_testsuite/bulk-memory-operations", strategy)?;
             test_directory_module(out, "tests/misc_testsuite/reference-types", strategy)?;
+            test_directory_module(out, "tests/misc_testsuite/multi-memory", strategy)?;
             Ok(())
         })?;
 

diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs
@@ -992,7 +992,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             let index = FuncIndex::from_u32(*function_index);
             state.push1(environ.translate_ref_func(builder.cursor(), index)?);
         }
-        Operator::MemoryAtomicWait32 { .. } | Operator::MemoryAtomicWait64 { .. } => {
+        Operator::MemoryAtomicWait32 { memarg } | Operator::MemoryAtomicWait64 { memarg } => {
             // The WebAssembly MVP only supports one linear memory and
             // wasmparser will ensure that the memory indices specified are
             // zero.
@@ -1001,8 +1001,8 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
                 Operator::MemoryAtomicWait32 { .. } => I32,
                 _ => unreachable!(),
             };
-            let heap_index = MemoryIndex::from_u32(0);
-            let heap = state.get_heap(builder.func, 0, environ)?;
+            let heap_index = MemoryIndex::from_u32(memarg.memory);
+            let heap = state.get_heap(builder.func, memarg.memory, environ)?;
             let timeout = state.pop1(); // 64 (fixed)
             let expected = state.pop1(); // 32 or 64 (per the `Ixx` in `IxxAtomicWait`)
             let addr = state.pop1(); // 32 (fixed)
@@ -1019,12 +1019,9 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             )?;
             state.push1(res);
         }
-        Operator::MemoryAtomicNotify { .. } => {
-            // The WebAssembly MVP only supports one linear memory and
-            // wasmparser will ensure that the memory indices specified are
-            // zero.
-            let heap_index = MemoryIndex::from_u32(0);
-            let heap = state.get_heap(builder.func, 0, environ)?;
+        Operator::MemoryAtomicNotify { memarg } => {
+            let heap_index = MemoryIndex::from_u32(memarg.memory);
+            let heap = state.get_heap(builder.func, memarg.memory, environ)?;
             let count = state.pop1(); // 32 (fixed)
             let addr = state.pop1(); // 32 (fixed)
             let res =
@@ -1233,16 +1230,23 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             builder.ins().fence();
         }
         Operator::MemoryCopy { src, dst } => {
-            // The WebAssembly MVP only supports one linear memory and
-            // wasmparser will ensure that the memory indices specified are
-            // zero.
-            assert_eq!(src, dst, "unimplemented between-memories copy");
-            let heap_index = MemoryIndex::from_u32(*src);
-            let heap = state.get_heap(builder.func, *src, environ)?;
+            let src_index = MemoryIndex::from_u32(*src);
+            let dst_index = MemoryIndex::from_u32(*dst);
+            let src_heap = state.get_heap(builder.func, *src, environ)?;
+            let dst_heap = state.get_heap(builder.func, *dst, environ)?;
             let len = state.pop1();
-            let src = state.pop1();
-            let dest = state.pop1();
-            environ.translate_memory_copy(builder.cursor(), heap_index, heap, dest, src, len)?;
+            let src_pos = state.pop1();
+            let dst_pos = state.pop1();
+            environ.translate_memory_copy(
+                builder.cursor(),
+                src_index,
+                src_heap,
+                dst_index,
+                dst_heap,
+                dst_pos,
+                src_pos,
+                len,
+            )?;
         }
         Operator::MemoryFill { mem } => {
             let heap_index = MemoryIndex::from_u32(*mem);

diff --git a/cranelift/wasm/src/environ/dummy.rs b/cranelift/wasm/src/environ/dummy.rs
@@ -393,8 +393,10 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
     fn translate_memory_copy(
         &mut self,
         _pos: FuncCursor,
-        _index: MemoryIndex,
-        _heap: ir::Heap,
+        _src_index: MemoryIndex,
+        _src_heap: ir::Heap,
+        _dst_index: MemoryIndex,
+        _dst_heap: ir::Heap,
         _dst: ir::Value,
         _src: ir::Value,
         _len: ir::Value,

diff --git a/cranelift/wasm/src/environ/spec.rs b/cranelift/wasm/src/environ/spec.rs
@@ -387,8 +387,10 @@ pub trait FuncEnvironment: TargetEnvironment {
     fn translate_memory_copy(
         &mut self,
         pos: FuncCursor,
-        index: MemoryIndex,
-        heap: ir::Heap,
+        src_index: MemoryIndex,
+        src_heap: ir::Heap,
+        dst_index: MemoryIndex,
+        dst_heap: ir::Heap,
         dst: ir::Value,
         src: ir::Value,
         len: ir::Value,

diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs
@@ -224,26 +224,6 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
         (sig, BuiltinFunctionIndex::elem_drop())
     }
 
-    fn get_memory_copy_func(
-        &mut self,
-        func: &mut Function,
-        memory_index: MemoryIndex,
-    ) -> (ir::SigRef, usize, BuiltinFunctionIndex) {
-        if let Some(defined_memory_index) = self.module.defined_memory_index(memory_index) {
-            (
-                self.builtin_function_signatures.defined_memory_copy(func),
-                defined_memory_index.index(),
-                BuiltinFunctionIndex::defined_memory_copy(),
-            )
-        } else {
-            (
-                self.builtin_function_signatures.imported_memory_copy(func),
-                memory_index.index(),
-                BuiltinFunctionIndex::imported_memory_copy(),
-            )
-        }
-    }
-
     fn get_memory_fill_func(
         &mut self,
         func: &mut Function,
@@ -1199,23 +1179,25 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
     fn translate_memory_copy(
         &mut self,
         mut pos: FuncCursor,
-        memory_index: MemoryIndex,
-        _heap: ir::Heap,
+        src_index: MemoryIndex,
+        _src_heap: ir::Heap,
+        dst_index: MemoryIndex,
+        _dst_heap: ir::Heap,
         dst: ir::Value,
         src: ir::Value,
         len: ir::Value,
     ) -> WasmResult<()> {
-        let (func_sig, memory_index, func_idx) =
-            self.get_memory_copy_func(&mut pos.func, memory_index);
+        let src_index = pos.ins().iconst(I32, i64::from(src_index.as_u32()));
+        let dst_index = pos.ins().iconst(I32, i64::from(dst_index.as_u32()));
 
-        let memory_index_arg = pos.ins().iconst(I32, memory_index as i64);
-
-        let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx);
+        let (vmctx, func_addr) = self
+            .translate_load_builtin_function_address(&mut pos, BuiltinFunctionIndex::memory_copy());
 
+        let func_sig = self.builtin_function_signatures.memory_copy(&mut pos.func);
         pos.ins().call_indirect(
             func_sig,
             func_addr,
-            &[vmctx, memory_index_arg, dst, src, len],
+            &[vmctx, dst_index, dst, src_index, src, len],
         );
 
         Ok(())

diff --git a/crates/environ/src/builtin.rs b/crates/environ/src/builtin.rs
@@ -18,10 +18,8 @@ macro_rules! foreach_builtin_function {
             table_init(vmctx, i32, i32, i32, i32, i32) -> ();
             /// Returns an index for wasm's `elem.drop`.
             elem_drop(vmctx, i32) -> ();
-            /// Returns an index for wasm's `memory.copy` for locally defined memories.
-            defined_memory_copy(vmctx, i32, i32, i32, i32) -> ();
-            /// Returns an index for wasm's `memory.copy` for imported memories.
-            imported_memory_copy(vmctx, i32, i32, i32, i32) -> ();
+            /// Returns an index for wasm's `memory.copy`
+            memory_copy(vmctx, i32, i32, i32, i32, i32) -> ();
             /// Returns an index for wasm's `memory.fill` for locally defined memories.
             memory_fill(vmctx, i32, i32, i32, i32) -> ();
             /// Returns an index for wasm's `memory.fill` for imported memories.

diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs
@@ -604,29 +604,31 @@ impl Instance {
         // dropping a non-passive element is a no-op (not a trap).
     }
 
-    /// Do a `memory.copy` for a locally defined memory.
+    /// Do a `memory.copy`
     ///
     /// # Errors
     ///
     /// Returns a `Trap` error when the source or destination ranges are out of
     /// bounds.
-    pub(crate) fn defined_memory_copy(
+    pub(crate) fn memory_copy(
         &self,
-        memory_index: DefinedMemoryIndex,
+        dst_index: DefinedMemoryIndex,
         dst: u32,
+        src_index: DefinedMemoryIndex,
         src: u32,
         len: u32,
     ) -> Result<(), Trap> {
         // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-memory-copy
 
-        let memory = self.memory(memory_index);
+        let src_mem = self.memory(src_index);
+        let dst_mem = self.memory(dst_index);
 
         if src
             .checked_add(len)
-            .map_or(true, |n| n as usize > memory.current_length)
+            .map_or(true, |n| n as usize > src_mem.current_length)
             || dst
                 .checked_add(len)
-                .map_or(true, |m| m as usize > memory.current_length)
+                .map_or(true, |m| m as usize > dst_mem.current_length)
         {
             return Err(Trap::wasm(ir::TrapCode::HeapOutOfBounds));
         }
@@ -637,31 +639,14 @@ impl Instance {
         // Bounds and casts are checked above, by this point we know that
         // everything is safe.
         unsafe {
-            let dst = memory.base.add(dst);
-            let src = memory.base.add(src);
+            let dst = dst_mem.base.add(dst);
+            let src = src_mem.base.add(src);
             ptr::copy(src, dst, len as usize);
         }
 
         Ok(())
     }
 
-    /// Perform a `memory.copy` on an imported memory.
-    pub(crate) fn imported_memory_copy(
-        &self,
-        memory_index: MemoryIndex,
-        dst: u32,
-        src: u32,
-        len: u32,
-    ) -> Result<(), Trap> {
-        let import = self.imported_memory(memory_index);
-        unsafe {
-            let foreign_instance = (&*import.vmctx).instance();
-            let foreign_memory = &*import.from;
-            let foreign_index = foreign_instance.memory_index(foreign_memory);
-            foreign_instance.defined_memory_copy(foreign_index, dst, src, len)
-        }
-    }
-
     /// Perform the `memory.fill` operation on a locally defined memory.
     ///
     /// # Errors

diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs
@@ -351,35 +351,19 @@ pub unsafe extern "C" fn wasmtime_elem_drop(vmctx: *mut VMContext, elem_index: u
 }
 
 /// Implementation of `memory.copy` for locally defined memories.
-pub unsafe extern "C" fn wasmtime_defined_memory_copy(
+pub unsafe extern "C" fn wasmtime_memory_copy(
     vmctx: *mut VMContext,
-    memory_index: u32,
+    dst_index: u32,
     dst: u32,
+    src_index: u32,
     src: u32,
     len: u32,
 ) {
     let result = {
-        let memory_index = DefinedMemoryIndex::from_u32(memory_index);
-        let instance = (&mut *vmctx).instance();
-        instance.defined_memory_copy(memory_index, dst, src, len)
-    };
-    if let Err(trap) = result {
-        raise_lib_trap(trap);
-    }
-}
-
-/// Implementation of `memory.copy` for imported memories.
-pub unsafe extern "C" fn wasmtime_imported_memory_copy(
-    vmctx: *mut VMContext,
-    memory_index: u32,
-    dst: u32,
-    src: u32,
-    len: u32,
-) {
-    let result = {
-        let memory_index = MemoryIndex::from_u32(memory_index);
+        let src_index = DefinedMemoryIndex::from_u32(src_index);
+        let dst_index = DefinedMemoryIndex::from_u32(dst_index);
         let instance = (&mut *vmctx).instance();
-        instance.imported_memory_copy(memory_index, dst, src, len)
+        instance.memory_copy(dst_index, dst, src_index, src, len)
     };
     if let Err(trap) = result {
         raise_lib_trap(trap);

diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs
@@ -577,10 +577,7 @@ impl VMBuiltinFunctionsArray {
             wasmtime_table_grow as usize;
         ptrs[BuiltinFunctionIndex::table_init().index() as usize] = wasmtime_table_init as usize;
         ptrs[BuiltinFunctionIndex::elem_drop().index() as usize] = wasmtime_elem_drop as usize;
-        ptrs[BuiltinFunctionIndex::defined_memory_copy().index() as usize] =
-            wasmtime_defined_memory_copy as usize;
-        ptrs[BuiltinFunctionIndex::imported_memory_copy().index() as usize] =
-            wasmtime_imported_memory_copy as usize;
+        ptrs[BuiltinFunctionIndex::memory_copy().index() as usize] = wasmtime_memory_copy as usize;
         ptrs[BuiltinFunctionIndex::memory_fill().index() as usize] = wasmtime_memory_fill as usize;
         ptrs[BuiltinFunctionIndex::imported_memory_fill().index() as usize] =
             wasmtime_imported_memory_fill as usize;

diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs
@@ -238,7 +238,7 @@ impl Config {
         self
     }
 
-    /// Configures whether the WebAssembly multi-value proposal will
+    /// Configures whether the WebAssembly multi-value [proposal] will
     /// be enabled for compilation.
     ///
     /// This feature gates functions and blocks returning multiple values in a
@@ -252,6 +252,20 @@ impl Config {
         self
     }
 
+    /// Configures whether the WebAssembly multi-memory [proposal] will
+    /// be enabled for compilation.
+    ///
+    /// This feature gates modules having more than one linear memory
+    /// declaration or import.
+    ///
+    /// This is `false` by default.
+    ///
+    /// [proposal]: https://github.com/webassembly/multi-memory
+    pub fn wasm_multi_memory(&mut self, enable: bool) -> &mut Self {
+        self.features.multi_memory = enable;
+        self
+    }
+
     /// Configures which compilation strategy will be used for wasm modules.
     ///
     /// This method can be used to configure which compiler is used for wasm

diff --git a/src/lib.rs b/src/lib.rs
@@ -124,6 +124,10 @@ struct CommonOptions {
     #[structopt(long)]
     enable_bulk_memory: Option<bool>,
 
+    /// Enable support for the multi-memory proposal
+    #[structopt(long)]
+    enable_multi_memory: bool,
+
     /// Enable all experimental Wasm features
     #[structopt(long)]
     enable_all: bool,
@@ -194,6 +198,7 @@ impl CommonOptions {
             )
             .wasm_multi_value(self.enable_multi_value.unwrap_or(true) || self.enable_all)
             .wasm_threads(self.enable_threads || self.enable_all)
+            .wasm_multi_memory(self.enable_multi_memory || self.enable_all)
             .cranelift_opt_level(self.opt_level())
             .strategy(pick_compilation_strategy(self.cranelift, self.lightbeam)?)?
             .profiler(pick_profiling_strategy(self.jitdump, self.vtune)?)?

diff --git a/tests/all/wast.rs b/tests/all/wast.rs
@@ -12,7 +12,8 @@ fn run_wast(wast: &str, strategy: Strategy) -> anyhow::Result<()> {
 
     let simd = wast.iter().any(|s| s == "simd");
 
-    let bulk_mem = wast.iter().any(|s| s == "bulk-memory-operations");
+    let multi_memory = wast.iter().any(|s| s == "multi-memory");
+    let bulk_mem = multi_memory || wast.iter().any(|s| s == "bulk-memory-operations");
 
     // Some simd tests assume support for multiple tables, which are introduced
     // by reference types.
@@ -22,6 +23,7 @@ fn run_wast(wast: &str, strategy: Strategy) -> anyhow::Result<()> {
     cfg.wasm_simd(simd)
         .wasm_bulk_memory(bulk_mem)
         .wasm_reference_types(reftypes)
+        .wasm_multi_memory(multi_memory)
         .strategy(strategy)?
         .cranelift_debug_verifier(true);