From b19ff00918008224a937d4711552a849d8ff3f0b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 31 Oct 2024 09:41:37 -0700 Subject: [PATCH] Remove `wasmtime_environ::TableStyle` This is a follow-up to #9530 to remove the `wasmtime_environ::TableStyle` type. This type was added quite a long time ago for future-proofing other styles of tables but at this point it's simpler to remove the type as adding another style of table hasn't been on the "table" (heh) for quite some time. This intends to further the goal of #9530 of plumbing `Tunables` closer to where it's ready to avoid extra layers of abstraction between the configuration options and what is processing them. --- crates/cranelift/src/func_environ.rs | 66 ++++++++-------------- crates/environ/src/module.rs | 20 ------- crates/wasmtime/src/runtime/vm/table.rs | 21 +++---- winch/codegen/src/codegen/mod.rs | 1 + winch/codegen/src/visitor.rs | 73 ++++++++++--------------- 5 files changed, 60 insertions(+), 121 deletions(-) diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs index 87009b538b76..0fbc58774078 100644 --- a/crates/cranelift/src/func_environ.rs +++ b/crates/cranelift/src/func_environ.rs @@ -21,9 +21,9 @@ use wasmparser::{Operator, WasmFeatures}; use wasmtime_environ::{ BuiltinFunctionIndex, DataIndex, ElemIndex, EngineOrModuleTypeIndex, FuncIndex, GlobalIndex, IndexType, Memory, MemoryIndex, MemoryPlan, MemoryStyle, Module, ModuleInternedTypeIndex, - ModuleTranslation, ModuleTypesBuilder, PtrSize, Table, TableIndex, TableStyle, Tunables, - TypeConvert, TypeIndex, VMOffsets, WasmCompositeType, WasmFuncType, WasmHeapTopType, - WasmHeapType, WasmRefType, WasmResult, WasmValType, + ModuleTranslation, ModuleTypesBuilder, PtrSize, Table, TableIndex, Tunables, TypeConvert, + TypeIndex, VMOffsets, WasmCompositeType, WasmFuncType, WasmHeapTopType, WasmHeapType, + WasmRefType, WasmResult, WasmValType, }; use wasmtime_environ::{FUNCREF_INIT_BIT, FUNCREF_MASK}; @@ -869,7 +869,6 @@ impl<'module_environment> FuncEnvironment<'module_environment> { table_index: TableIndex, index: ir::Value, cold_blocks: bool, - lazy_init: bool, ) -> ir::Value { let pointer_type = self.pointer_type(); self.ensure_table_exists(builder.func, table_index); @@ -882,7 +881,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { let (table_entry_addr, flags) = table_data.prepare_table_addr(self, builder, index); let value = builder.ins().load(pointer_type, flags, table_entry_addr, 0); - if !lazy_init { + if !self.tunables.table_lazy_init { return value; } @@ -1325,15 +1324,11 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> { cold_blocks: bool, ) -> WasmResult> { // Get the funcref pointer from the table. - let table = &self.env.module.tables[table_index]; - let TableStyle::CallerChecksSignature { lazy_init } = - TableStyle::for_table(*table, self.env.tunables); let funcref_ptr = self.env.get_or_init_func_ref_table_elem( self.builder, table_index, callee, cold_blocks, - lazy_init, ); // If necessary, check the signature. @@ -1381,11 +1376,6 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> { let sig_id_size = self.env.offsets.size_of_vmshared_type_index(); let sig_id_type = Type::int(u16::from(sig_id_size) * 8).unwrap(); - // Generate a rustc compile error here if more styles are added in - // the future as the following code is tailored to just this style. - let TableStyle::CallerChecksSignature { .. } = - TableStyle::for_table(*table, self.env.tunables); - // Test if a type check is necessary for this table. If this table is a // table of typed functions and that type matches `ty_index`, then // there's no need to perform a typecheck. @@ -1802,16 +1792,9 @@ impl<'module_environment> crate::translate::FuncEnvironment } // Function types. - WasmHeapTopType::Func => match TableStyle::for_table(table, self.tunables) { - TableStyle::CallerChecksSignature { lazy_init } => Ok(self - .get_or_init_func_ref_table_elem( - builder, - table_index, - index, - false, - lazy_init, - )), - }, + WasmHeapTopType::Func => { + Ok(self.get_or_init_func_ref_table_elem(builder, table_index, index, false)) + } } } @@ -1842,26 +1825,21 @@ impl<'module_environment> crate::translate::FuncEnvironment // Function types. WasmHeapTopType::Func => { - match TableStyle::for_table(table, self.tunables) { - TableStyle::CallerChecksSignature { lazy_init } => { - let (elem_addr, flags) = - table_data.prepare_table_addr(self, builder, index); - // Set the "initialized bit". See doc-comment on - // `FUNCREF_INIT_BIT` in - // crates/environ/src/ref_bits.rs for details. - let value_with_init_bit = if lazy_init { - builder - .ins() - .bor_imm(value, Imm64::from(FUNCREF_INIT_BIT as i64)) - } else { - value - }; - builder - .ins() - .store(flags, value_with_init_bit, elem_addr, 0); - Ok(()) - } - } + let (elem_addr, flags) = table_data.prepare_table_addr(self, builder, index); + // Set the "initialized bit". See doc-comment on + // `FUNCREF_INIT_BIT` in + // crates/environ/src/ref_bits.rs for details. + let value_with_init_bit = if self.tunables.table_lazy_init { + builder + .ins() + .bor_imm(value, Imm64::from(FUNCREF_INIT_BIT as i64)) + } else { + value + }; + builder + .ins() + .store(flags, value_with_init_bit, elem_addr, 0); + Ok(()) } } } diff --git a/crates/environ/src/module.rs b/crates/environ/src/module.rs index 52d8f0a3eafd..5fda34efe50f 100644 --- a/crates/environ/src/module.rs +++ b/crates/environ/src/module.rs @@ -310,26 +310,6 @@ pub trait InitMemory { fn write(&mut self, memory_index: MemoryIndex, init: &StaticMemoryInitializer) -> bool; } -/// Implementation styles for WebAssembly tables. -#[derive(Debug, Clone, Hash, Serialize, Deserialize)] -pub enum TableStyle { - /// Signatures are stored in the table and checked in the caller. - CallerChecksSignature { - /// Whether this table is initialized lazily and requires an - /// initialization check on every access. - lazy_init: bool, - }, -} - -impl TableStyle { - /// Decide on an implementation style for the given `Table`. - pub fn for_table(_table: Table, tunables: &Tunables) -> Self { - Self::CallerChecksSignature { - lazy_init: tunables.table_lazy_init, - } - } -} - /// Table initialization data for all tables in the module. #[derive(Debug, Default, Serialize, Deserialize)] pub struct TableInitialization { diff --git a/crates/wasmtime/src/runtime/vm/table.rs b/crates/wasmtime/src/runtime/vm/table.rs index 4e703ce6dc1d..8034d9029418 100644 --- a/crates/wasmtime/src/runtime/vm/table.rs +++ b/crates/wasmtime/src/runtime/vm/table.rs @@ -13,8 +13,7 @@ use core::slice; use core::{cmp, usize}; use sptr::Strict; use wasmtime_environ::{ - IndexType, TableStyle, Trap, Tunables, WasmHeapTopType, WasmRefType, FUNCREF_INIT_BIT, - FUNCREF_MASK, + IndexType, Trap, Tunables, WasmHeapTopType, WasmRefType, FUNCREF_INIT_BIT, FUNCREF_MASK, }; /// An element going into or coming out of a table. @@ -275,15 +274,11 @@ impl Table { ) -> Result { let (minimum, maximum) = Self::limit_new(ty, store)?; match wasm_to_table_type(ty.ref_type) { - TableElementType::Func => { - let TableStyle::CallerChecksSignature { lazy_init } = - TableStyle::for_table(*ty, tunables); - Ok(Self::from(DynamicFuncTable { - elements: vec![None; minimum], - maximum, - lazy_init, - })) - } + TableElementType::Func => Ok(Self::from(DynamicFuncTable { + elements: vec![None; minimum], + maximum, + lazy_init: tunables.table_lazy_init, + })), TableElementType::GcRef => Ok(Self::from(DynamicGcRefTable { elements: (0..minimum).map(|_| None).collect(), maximum, @@ -321,12 +316,10 @@ impl Table { data.as_non_null().cast::(), cmp::min(len, max), )); - let TableStyle::CallerChecksSignature { lazy_init } = - TableStyle::for_table(*ty, tunables); Ok(Self::from(StaticFuncTable { data, size, - lazy_init, + lazy_init: tunables.table_lazy_init, })) } TableElementType::GcRef => { diff --git a/winch/codegen/src/codegen/mod.rs b/winch/codegen/src/codegen/mod.rs index bd074d8ef4d6..8c735557f6e4 100644 --- a/winch/codegen/src/codegen/mod.rs +++ b/winch/codegen/src/codegen/mod.rs @@ -478,6 +478,7 @@ where } pub fn emit_lazy_init_funcref(&mut self, table_index: TableIndex) { + assert!(self.tunables.table_lazy_init, "unsupported eager init"); let table_data = self.env.resolve_table_data(table_index); let ptr_type = self.env.ptr_type(); let builtin = self diff --git a/winch/codegen/src/visitor.rs b/winch/codegen/src/visitor.rs index bee8bc9e0e3d..bd9a97a30042 100644 --- a/winch/codegen/src/visitor.rs +++ b/winch/codegen/src/visitor.rs @@ -17,8 +17,8 @@ use smallvec::SmallVec; use wasmparser::{BlockType, BrTable, Ieee32, Ieee64, MemArg, VisitOperator, V128}; use wasmtime_cranelift::TRAP_INDIRECT_CALL_TO_NULL; use wasmtime_environ::{ - FuncIndex, GlobalIndex, MemoryIndex, TableIndex, TableStyle, TypeIndex, WasmHeapType, - WasmValType, FUNCREF_INIT_BIT, + FuncIndex, GlobalIndex, MemoryIndex, TableIndex, TypeIndex, WasmHeapType, WasmValType, + FUNCREF_INIT_BIT, }; /// A macro to define unsupported WebAssembly operators. @@ -1405,18 +1405,10 @@ where // Perform the indirect call. // This code assumes that [`Self::emit_lazy_init_funcref`] will // push the funcref to the value stack. - match TableStyle::for_table( - self.env.translation.module.tables[table_index], - self.tunables, - ) { - TableStyle::CallerChecksSignature { lazy_init: true } => { - let funcref_ptr = self.context.stack.peek().map(|v| v.unwrap_reg()).unwrap(); - self.masm - .trapz(funcref_ptr.into(), TRAP_INDIRECT_CALL_TO_NULL); - self.emit_typecheck_funcref(funcref_ptr.into(), type_index); - } - _ => unimplemented!("Support for eager table init"), - } + let funcref_ptr = self.context.stack.peek().map(|v| v.unwrap_reg()).unwrap(); + self.masm + .trapz(funcref_ptr.into(), TRAP_INDIRECT_CALL_TO_NULL); + self.emit_typecheck_funcref(funcref_ptr.into(), type_index); let callee = self.env.funcref(type_index); FnCall::emit::(&mut self.env, self.masm, &mut self.context, callee) @@ -1461,12 +1453,7 @@ where let heap_type = table.ref_type.heap_type; match heap_type { - WasmHeapType::Func => match TableStyle::for_table(*table, self.tunables) { - TableStyle::CallerChecksSignature { lazy_init: true } => { - self.emit_lazy_init_funcref(table_index) - } - _ => unimplemented!("Support for eager table init"), - }, + WasmHeapType::Func => self.emit_lazy_init_funcref(table_index), WasmHeapType::Extern => { self.found_unsupported_instruction = Some("unsupported table.get of externref table"); @@ -1543,29 +1530,29 @@ where let table_data = self.env.resolve_table_data(table_index); let table = self.env.table(table_index); match table.ref_type.heap_type { - WasmHeapType::Func => match TableStyle::for_table(*table, self.tunables) { - TableStyle::CallerChecksSignature { lazy_init: true } => { - let value = self.context.pop_to_reg(self.masm, None); - let index = self.context.pop_to_reg(self.masm, None); - let base = self.context.any_gpr(self.masm); - let elem_addr = - self.emit_compute_table_elem_addr(index.into(), base, &table_data); - // Set the initialized bit. - self.masm.or( - writable!(value.into()), - value.into(), - RegImm::i64(FUNCREF_INIT_BIT as i64), - ptr_type.into(), - ); - - self.masm.store_ptr(value.into(), elem_addr); - - self.context.free_reg(value); - self.context.free_reg(index); - self.context.free_reg(base); - } - _ => unimplemented!("Support for eager table init"), - }, + WasmHeapType::Func => { + assert!( + self.tunables.table_lazy_init, + "unsupported table eager init" + ); + let value = self.context.pop_to_reg(self.masm, None); + let index = self.context.pop_to_reg(self.masm, None); + let base = self.context.any_gpr(self.masm); + let elem_addr = self.emit_compute_table_elem_addr(index.into(), base, &table_data); + // Set the initialized bit. + self.masm.or( + writable!(value.into()), + value.into(), + RegImm::i64(FUNCREF_INIT_BIT as i64), + ptr_type.into(), + ); + + self.masm.store_ptr(value.into(), elem_addr); + + self.context.free_reg(value); + self.context.free_reg(index); + self.context.free_reg(base); + } ty => unimplemented!("Support for WasmHeapType: {ty}"), }; }