Merge pull request #1923 from fitzgen/table-get-and-table-set

wasmtime: Implement `table.get` and `table.set`

build.rs

@@ -205,18 +205,20 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool {
             ("simd", "simd_i16x8_arith2") => return true,
             ("simd", "simd_i8x16_arith2") => return true,
 
-            ("reference_types", "table_copy_on_imported_tables")
-            | ("reference_types", "externref_id_function")
-            | ("reference_types", "table_size")
-            | ("reference_types", "simple_ref_is_null")
-            | ("reference_types", "table_grow_with_funcref") => {
-                // TODO(#1886): Ignore if this isn't x64, because Cranelift only
-                // supports reference types on x64.
-                return env::var("CARGO_CFG_TARGET_ARCH").unwrap() != "x86_64";
+            // Still working on implementing these. See #929.
+            ("reference_types", "global")
+            | ("reference_types", "linking")
+            | ("reference_types", "ref_func")
+            | ("reference_types", "ref_null")
+            | ("reference_types", "table_fill") => {
+                return true;
             }
 
-            // Still working on implementing these. See #929.
-            ("reference_types", _) => return true,
+            // TODO(#1886): Ignore reference types tests if this isn't x64,
+            // because Cranelift only supports reference types on x64.
+            ("reference_types", _) => {
+                return env::var("CARGO_CFG_TARGET_ARCH").unwrap() != "x86_64";
+            }
 
             _ => {}
         },

cranelift/codegen/meta/src/isa/x86/encodings.rs

@@ -231,6 +231,32 @@ impl PerCpuModeEncodings {
         });
     }
 
+    /// Add encodings for `inst.r32` to X86_32.
+    /// Add encodings for `inst.r32` to X86_64 with and without REX.
+    /// Add encodings for `inst.r64` to X86_64 with a REX.W prefix.
+    fn enc_r32_r64_instp(
+        &mut self,
+        inst: &Instruction,
+        template: Template,
+        instp: InstructionPredicateNode,
+    ) {
+        self.enc32_func(inst.bind(R32), template.nonrex(), |builder| {
+            builder.inst_predicate(instp.clone())
+        });
+
+        // REX-less encoding must come after REX encoding so we don't use it by default. Otherwise
+        // reg-alloc would never use r8 and up.
+        self.enc64_func(inst.bind(R32), template.rex(), |builder| {
+            builder.inst_predicate(instp.clone())
+        });
+        self.enc64_func(inst.bind(R32), template.nonrex(), |builder| {
+            builder.inst_predicate(instp.clone())
+        });
+        self.enc64_func(inst.bind(R64), template.rex().w(), |builder| {
+            builder.inst_predicate(instp)
+        });
+    }
+
     /// Add encodings for `inst.r32` to X86_32.
     /// Add encodings for `inst.r64` to X86_64 with a REX.W prefix.
     fn enc_r32_r64_rex_only(&mut self, inst: impl Into<InstSpec>, template: Template) {
@@ -810,6 +836,11 @@ fn define_memory(
             recipe.opcodes(&MOV_LOAD),
             is_load_complex_length_two.clone(),
         );
+        e.enc_r32_r64_instp(
+            load_complex,
+            recipe.opcodes(&MOV_LOAD),
+            is_load_complex_length_two.clone(),
+        );
         e.enc_x86_64_instp(
             uload32_complex,
             recipe.opcodes(&MOV_LOAD),
@@ -855,6 +886,11 @@ fn define_memory(
             recipe.opcodes(&MOV_STORE),
             is_store_complex_length_three.clone(),
         );
+        e.enc_r32_r64_instp(
+            store_complex,
+            recipe.opcodes(&MOV_STORE),
+            is_store_complex_length_three.clone(),
+        );
         e.enc_x86_64_instp(
             istore32_complex,
             recipe.opcodes(&MOV_STORE),
@@ -948,6 +984,10 @@ fn define_memory(
         e.enc64_rec(fill_nop.bind(ty), rec_ffillnull, 0);
         e.enc32_rec(fill_nop.bind(ty), rec_ffillnull, 0);
     }
+    for &ty in &[R64, R32] {
+        e.enc64_rec(fill_nop.bind(ty), rec_fillnull, 0);
+        e.enc32_rec(fill_nop.bind(ty), rec_fillnull, 0);
+    }
 
     // Load 32 bits from `b1`, `i8` and `i16` spill slots. See `spill.b1` above.
 

cranelift/codegen/meta/src/shared/instructions.rs

@@ -211,7 +211,7 @@ fn define_control_flow(
     let iAddr = &TypeVar::new(
         "iAddr",
         "An integer address type",
-        TypeSetBuilder::new().ints(32..64).build(),
+        TypeSetBuilder::new().ints(32..64).refs(32..64).build(),
     );
 
     {
@@ -744,7 +744,7 @@ pub(crate) fn define(
     let iAddr = &TypeVar::new(
         "iAddr",
         "An integer address type",
-        TypeSetBuilder::new().ints(32..64).build(),
+        TypeSetBuilder::new().ints(32..64).refs(32..64).build(),
     );
 
     let Ref = &TypeVar::new(

cranelift/codegen/src/binemit/relaxation.rs

@@ -302,7 +302,11 @@ fn fallthroughs(func: &mut Function) {
                 Opcode::Fallthrough => {
                     // Somebody used a fall-through instruction before the branch relaxation pass.
                     // Make sure it is correct, i.e. the destination is the layout successor.
-                    debug_assert_eq!(destination, succ, "Illegal fall-through in {}", block)
+                    debug_assert_eq!(
+                        destination, succ,
+                        "Illegal fallthrough from {} to {}, but {}'s successor is {}",
+                        block, destination, block, succ
+                    )
                 }
                 Opcode::Jump => {
                     // If this is a jump to the successor block, change it to a fall-through.

cranelift/codegen/src/ir/valueloc.rs

@@ -41,15 +41,15 @@ impl ValueLoc {
     pub fn unwrap_reg(self) -> RegUnit {
         match self {
             Self::Reg(ru) => ru,
-            _ => panic!("Expected register: {:?}", self),
+            _ => panic!("unwrap_reg expected register, found {:?}", self),
         }
     }
 
     /// Get the stack slot of this location, or panic.
     pub fn unwrap_stack(self) -> StackSlot {
         match self {
             Self::Stack(ss) => ss,
-            _ => panic!("Expected stack slot: {:?}", self),
+            _ => panic!("unwrap_stack expected stack slot, found {:?}", self),
         }
     }
 

cranelift/codegen/src/postopt.rs

@@ -386,7 +386,11 @@ fn optimize_complex_addresses(pos: &mut EncCursor, inst: Inst, isa: &dyn TargetI
     }
 
     let ok = pos.func.update_encoding(inst, isa).is_ok();
-    debug_assert!(ok);
+    debug_assert!(
+        ok,
+        "failed to update encoding for `{}`",
+        pos.func.dfg.display_inst(inst, isa)
+    );
 }
 
 //----------------------------------------------------------------------

cranelift/codegen/src/redundant_reload_remover.rs

@@ -635,7 +635,11 @@ impl RedundantReloadRemover {
                 // Load is completely redundant.  Convert it to a no-op.
                 dfg.replace(inst).fill_nop(arg);
                 let ok = func.update_encoding(inst, isa).is_ok();
-                debug_assert!(ok, "fill_nop encoding missing for this type");
+                debug_assert!(
+                    ok,
+                    "fill_nop encoding missing for this type: `{}`",
+                    func.dfg.display_inst(inst, isa)
+                );
             }
             Transform::ChangeToCopyToSSA(ty, reg) => {
                 // We already have the relevant value in some other register.  Convert the

cranelift/frontend/src/frontend.rs

@@ -206,6 +206,11 @@ impl<'a> FunctionBuilder<'a> {
         }
     }
 
+    /// Get the block that this builder is currently at.
+    pub fn current_block(&self) -> Option<Block> {
+        self.position.expand()
+    }
+
     /// Set the source location that should be assigned to all new instructions.
     pub fn set_srcloc(&mut self, srcloc: ir::SourceLoc) {
         self.srcloc = srcloc;
@@ -223,6 +228,11 @@ impl<'a> FunctionBuilder<'a> {
         block
     }
 
+    /// Insert `block` in the layout *after* the existing block `after`.
+    pub fn insert_block_after(&mut self, block: Block, after: Block) {
+        self.func.layout.insert_block_after(block, after);
+    }
+
     /// After the call to this function, new instructions will be inserted into the designated
     /// block, in the order they are declared. You must declare the types of the Block arguments
     /// you will use here.

cranelift/wasm/src/code_translator.rs

@@ -1186,19 +1186,14 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             let table_index = TableIndex::from_u32(*index);
             let table = state.get_or_create_table(builder.func, *index, environ)?;
             let index = state.pop1();
-            state.push1(environ.translate_table_get(
-                builder.cursor(),
-                table_index,
-                table,
-                index,
-            )?);
+            state.push1(environ.translate_table_get(builder, table_index, table, index)?);
         }
         Operator::TableSet { table: index } => {
             let table_index = TableIndex::from_u32(*index);
             let table = state.get_or_create_table(builder.func, *index, environ)?;
             let value = state.pop1();
             let index = state.pop1();
-            environ.translate_table_set(builder.cursor(), table_index, table, value, index)?;
+            environ.translate_table_set(builder, table_index, table, value, index)?;
         }
         Operator::TableCopy {
             dst_table: dst_table_index,

cranelift/wasm/src/environ/dummy.rs

@@ -22,6 +22,7 @@ use cranelift_codegen::ir::types::*;
 use cranelift_codegen::ir::{self, InstBuilder};
 use cranelift_codegen::isa::TargetFrontendConfig;
 use cranelift_entity::{EntityRef, PrimaryMap, SecondaryMap};
+use cranelift_frontend::FunctionBuilder;
 use std::boxed::Box;
 use std::string::String;
 use std::vec::Vec;
@@ -452,17 +453,17 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
 
     fn translate_table_get(
         &mut self,
-        mut pos: FuncCursor,
+        builder: &mut FunctionBuilder,
         _table_index: TableIndex,
         _table: ir::Table,
         _index: ir::Value,
     ) -> WasmResult<ir::Value> {
-        Ok(pos.ins().null(self.reference_type()))
+        Ok(builder.ins().null(self.reference_type()))
     }
 
     fn translate_table_set(
         &mut self,
-        _pos: FuncCursor,
+        _builder: &mut FunctionBuilder,
         _table_index: TableIndex,
         _table: ir::Table,
         _value: ir::Value,

cranelift/wasm/src/environ/spec.rs

@@ -368,7 +368,7 @@ pub trait FuncEnvironment: TargetEnvironment {
     /// Translate a `table.get` WebAssembly instruction.
     fn translate_table_get(
         &mut self,
-        pos: FuncCursor,
+        builder: &mut FunctionBuilder,
         table_index: TableIndex,
         table: ir::Table,
         index: ir::Value,
@@ -377,7 +377,7 @@ pub trait FuncEnvironment: TargetEnvironment {
     /// Translate a `table.set` WebAssembly instruction.
     fn translate_table_set(
         &mut self,
-        pos: FuncCursor,
+        builder: &mut FunctionBuilder,
         table_index: TableIndex,
         table: ir::Table,
         value: ir::Value,

cranelift/wasm/src/sections_translator.rs

@@ -349,7 +349,9 @@ pub fn parse_element_section<'data>(
                 let index = ElemIndex::from_u32(index as u32);
                 environ.declare_passive_element(index, segments)?;
             }
-            ElementKind::Declared => return Err(wasm_unsupported!("element kind declared")),
+            ElementKind::Declared => {
+                // Nothing to do here.
+            }
         }
     }
     Ok(())

crates/environ/Cargo.toml

@@ -15,6 +15,7 @@ edition = "2018"
 anyhow = "1.0"
 cranelift-codegen = { path = "../../cranelift/codegen", version = "0.65.0", features = ["enable-serde"] }
 cranelift-entity = { path = "../../cranelift/entity", version = "0.65.0", features = ["enable-serde"] }
+cranelift-frontend = { path = "../../cranelift/frontend", version = "0.65.0" }
 cranelift-wasm = { path = "../../cranelift/wasm", version = "0.65.0", features = ["enable-serde"] }
 wasmparser = "0.58.0"
 lightbeam = { path = "../lightbeam", optional = true, version = "0.18.0" }