Skip to content

Commit

Permalink
Remove the wasmtime_environ::TablePlan type
Browse files Browse the repository at this point in the history
In my quest to simplify memory configuration and how things work
internally in Wasmtime one thing I've identified to accomplish is the
removal of the `TablePlan` and `MemoryPlan` types. These introduce an
abstraction layer between table/memory implementations and `Tunables`
and personally I find it simpler to directly reference `Tunables` and
use that instead. The goal of this commit is to plumb `Tunables` closer
to where it's directly read by removing the "indirection" through the
`*Plan` types.

The `TablePlan` and `MemoryPlan` types are pervasively used throughout
Wasmtime so instead of having one large commit delete everything this is
instead a piecemeal approach to incrementally get towards the goal of
removal. Here just `TablePlan` is removed and `Tunables` is plumbed in a
few more places. I plan to also in the future remove `TableStyle` and
`MemoryStyle` in favor of directly reading `Tunables` but that's left
for a future commit. For now `TableStyle` persists and its usage is a
bit odd in isolation but I plan to follow this up with the removal of
`TableStyle`.
  • Loading branch information
alexcrichton committed Oct 31, 2024
1 parent 16eb9fa commit 7ee695c
Show file tree
Hide file tree
Showing 21 changed files with 132 additions and 134 deletions.
32 changes: 16 additions & 16 deletions crates/cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {

/// Get the Table for the given index.
fn table(&self, index: TableIndex) -> Table {
self.module.table_plans[index].table
self.module.tables[index]
}

/// Cast the value to I64 and sign extend if necessary.
Expand Down Expand Up @@ -817,7 +817,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
}
};

let table = &self.module.table_plans[index].table;
let table = &self.module.tables[index];
let element_size = if table.ref_type.is_vmgcref_type() {
// For GC-managed references, tables store `Option<VMGcRef>`s.
ir::types::I32.bytes()
Expand Down Expand Up @@ -1325,8 +1325,9 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
cold_blocks: bool,
) -> WasmResult<Option<(ir::Value, ir::Value)>> {
// Get the funcref pointer from the table.
let table = &self.env.module.table_plans[table_index];
let TableStyle::CallerChecksSignature { lazy_init } = table.style;
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,
Expand Down Expand Up @@ -1376,18 +1377,19 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
ty_index: TypeIndex,
funcref_ptr: ir::Value,
) -> CheckIndirectCallTypeSignature {
let table = &self.env.module.table_plans[table_index];
let table = &self.env.module.tables[table_index];
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 { .. } = table.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.
match table.table.ref_type.heap_type {
match table.ref_type.heap_type {
// Functions do not have a statically known type in the table, a
// typecheck is required. Fall through to below to perform the
// actual typecheck.
Expand All @@ -1403,7 +1405,7 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
let specified_ty = self.env.module.types[ty_index];
if specified_ty == table_ty {
return CheckIndirectCallTypeSignature::StaticMatch {
may_be_null: table.table.ref_type.nullable,
may_be_null: table.ref_type.nullable,
};
}

Expand All @@ -1421,7 +1423,7 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
// a null pointer, then this was a call to null. Otherwise
// if it succeeds then we know it won't match, so trap
// anyway.
if table.table.ref_type.nullable {
if table.ref_type.nullable {
if self.env.signals_based_traps() {
let mem_flags = ir::MemFlags::trusted().with_readonly();
self.builder.ins().load(
Expand All @@ -1446,7 +1448,7 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> {
// Tables of `nofunc` can only be inhabited by null, so go ahead and
// trap with that.
WasmHeapType::NoFunc => {
assert!(table.table.ref_type.nullable);
assert!(table.ref_type.nullable);
self.env
.trap(self.builder, crate::TRAP_INDIRECT_CALL_TO_NULL);
return CheckIndirectCallTypeSignature::StaticTrap;
Expand Down Expand Up @@ -1782,8 +1784,7 @@ impl<'module_environment> crate::translate::FuncEnvironment
table_index: TableIndex,
index: ir::Value,
) -> WasmResult<ir::Value> {
let plan = &self.module.table_plans[table_index];
let table = plan.table;
let table = self.module.tables[table_index];
self.ensure_table_exists(builder.func, table_index);
let table_data = self.tables[table_index].clone().unwrap();
let heap_ty = table.ref_type.heap_type;
Expand All @@ -1801,7 +1802,7 @@ impl<'module_environment> crate::translate::FuncEnvironment
}

// Function types.
WasmHeapTopType::Func => match plan.style {
WasmHeapTopType::Func => match TableStyle::for_table(table, self.tunables) {
TableStyle::CallerChecksSignature { lazy_init } => Ok(self
.get_or_init_func_ref_table_elem(
builder,
Expand All @@ -1821,8 +1822,7 @@ impl<'module_environment> crate::translate::FuncEnvironment
value: ir::Value,
index: ir::Value,
) -> WasmResult<()> {
let plan = &self.module.table_plans[table_index];
let table = plan.table;
let table = self.module.tables[table_index];
self.ensure_table_exists(builder.func, table_index);
let table_data = self.tables[table_index].clone().unwrap();
let heap_ty = table.ref_type.heap_type;
Expand All @@ -1842,7 +1842,7 @@ impl<'module_environment> crate::translate::FuncEnvironment

// Function types.
WasmHeapTopType::Func => {
match plan.style {
match TableStyle::for_table(table, self.tunables) {
TableStyle::CallerChecksSignature { lazy_init } => {
let (elem_addr, flags) =
table_data.prepare_table_addr(self, builder, index);
Expand Down
26 changes: 9 additions & 17 deletions crates/environ/src/compile/module_environ.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::module::{
FuncRefIndex, Initializer, MemoryInitialization, MemoryInitializer, MemoryPlan, Module,
TablePlan, TableSegment, TableSegmentElements,
TableSegment, TableSegmentElements,
};
use crate::prelude::*;
use crate::{
Expand Down Expand Up @@ -343,13 +343,12 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {
Payload::TableSection(tables) => {
self.validator.table_section(&tables)?;
let cnt = usize::try_from(tables.count()).unwrap();
self.result.module.table_plans.reserve_exact(cnt);
self.result.module.tables.reserve_exact(cnt);

for entry in tables {
let wasmparser::Table { ty, init } = entry?;
let table = self.convert_table_type(&ty)?;
let plan = TablePlan::for_table(table, &self.tunables);
self.result.module.table_plans.push(plan);
self.result.module.tables.push(table);
let init = match init {
wasmparser::TableInit::RefNull => TableInitialValue::Null {
precomputed: Vec::new(),
Expand Down Expand Up @@ -767,10 +766,7 @@ and for re-adding support for interface types you can see this issue:
self.flag_func_escaped(func_index);
func_index
}),
EntityType::Table(ty) => {
let plan = TablePlan::for_table(ty, &self.tunables);
EntityIndex::Table(self.result.module.table_plans.push(plan))
}
EntityType::Table(ty) => EntityIndex::Table(self.result.module.tables.push(ty)),
EntityType::Memory(ty) => {
let plan = MemoryPlan::for_memory(ty, &self.tunables);
EntityIndex::Memory(self.result.module.memory_plans.push(plan))
Expand Down Expand Up @@ -1141,19 +1137,19 @@ impl ModuleTranslation<'_> {

// First convert any element-initialized tables to images of just that
// single function if the minimum size of the table allows doing so.
for ((_, init), (_, plan)) in self
for ((_, init), (_, table)) in self
.module
.table_initialization
.initial_values
.iter_mut()
.zip(
self.module
.table_plans
.tables
.iter()
.skip(self.module.num_imported_tables),
)
{
let table_size = plan.table.limits.min;
let table_size = table.limits.min;
if table_size > MAX_FUNC_TABLE_SIZE {
continue;
}
Expand Down Expand Up @@ -1206,16 +1202,12 @@ impl ModuleTranslation<'_> {
Some(top) => top,
None => break,
};
let table_size = self.module.table_plans[segment.table_index]
.table
.limits
.min;
let table_size = self.module.tables[segment.table_index].limits.min;
if top > table_size || top > MAX_FUNC_TABLE_SIZE {
break;
}

match self.module.table_plans[segment.table_index]
.table
match self.module.tables[segment.table_index]
.ref_type
.heap_type
.top()
Expand Down
28 changes: 8 additions & 20 deletions crates/environ/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,24 +330,6 @@ impl TableStyle {
}
}

/// A WebAssembly table description along with our chosen style for
/// implementing it.
#[derive(Debug, Clone, Hash, Serialize, Deserialize)]
pub struct TablePlan {
/// The WebAssembly table description.
pub table: Table,
/// Our chosen implementation style.
pub style: TableStyle,
}

impl TablePlan {
/// Draw up a plan for implementing a `Table`.
pub fn for_table(table: Table, tunables: &Tunables) -> Self {
let style = TableStyle::for_table(table, tunables);
Self { table, style }
}
}

/// Table initialization data for all tables in the module.
#[derive(Debug, Default, Serialize, Deserialize)]
pub struct TableInitialization {
Expand Down Expand Up @@ -481,7 +463,7 @@ pub struct Module {
pub functions: PrimaryMap<FuncIndex, FunctionType>,

/// WebAssembly tables.
pub table_plans: PrimaryMap<TableIndex, TablePlan>,
pub tables: PrimaryMap<TableIndex, Table>,

/// WebAssembly linear memory plans.
pub memory_plans: PrimaryMap<MemoryIndex, MemoryPlan>,
Expand Down Expand Up @@ -651,7 +633,7 @@ impl Module {
pub fn type_of(&self, index: EntityIndex) -> EntityType {
match index {
EntityIndex::Global(i) => EntityType::Global(self.globals[i]),
EntityIndex::Table(i) => EntityType::Table(self.table_plans[i].table),
EntityIndex::Table(i) => EntityType::Table(self.tables[i]),
EntityIndex::Memory(i) => EntityType::Memory(self.memory_plans[i].memory),
EntityIndex::Function(i) => {
EntityType::Function(EngineOrModuleTypeIndex::Module(self.functions[i].signature))
Expand All @@ -674,6 +656,12 @@ impl Module {
pub fn defined_func_indices(&self) -> impl Iterator<Item = DefinedFuncIndex> {
(0..self.functions.len() - self.num_imported_funcs).map(|i| DefinedFuncIndex::new(i))
}

/// Returns the number of tables defined by this module itself: all tables
/// minus imported tables.
pub fn num_defined_tables(&self) -> usize {
self.tables.len() - self.num_imported_tables
}
}

/// Type information about functions in a wasm module.
Expand Down
2 changes: 1 addition & 1 deletion crates/environ/src/vmoffsets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ impl<P: PtrSize> VMOffsets<P> {
num_imported_tables: cast_to_u32(module.num_imported_tables),
num_imported_memories: cast_to_u32(module.num_imported_memories),
num_imported_globals: cast_to_u32(module.num_imported_globals),
num_defined_tables: cast_to_u32(module.table_plans.len() - module.num_imported_tables),
num_defined_tables: cast_to_u32(module.num_defined_tables()),
num_defined_memories: cast_to_u32(
module.memory_plans.len() - module.num_imported_memories,
),
Expand Down
1 change: 1 addition & 0 deletions crates/wasmtime/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ impl Engine {
}
}

#[inline]
pub(crate) fn tunables(&self) -> &Tunables {
&self.inner.tunables
}
Expand Down
5 changes: 2 additions & 3 deletions crates/wasmtime/src/runtime/externals/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl Table {
}

fn _ty(&self, store: &StoreOpaque) -> TableType {
let ty = &store[self.0].table.table;
let ty = &store[self.0].table;
TableType::from_wasmtime_table(store.engine(), ty)
}

Expand Down Expand Up @@ -404,7 +404,6 @@ impl Table {
) -> Table {
// Ensure that the table's type is engine-level canonicalized.
wasmtime_export
.table
.table
.ref_type
.canonicalize_for_runtime_usage(&mut |module_index| {
Expand All @@ -417,7 +416,7 @@ impl Table {
}

pub(crate) fn wasmtime_ty<'a>(&self, data: &'a StoreData) -> &'a wasmtime_environ::Table {
&data[self.0].table.table
&data[self.0].table
}

pub(crate) fn vmimport(&self, store: &StoreOpaque) -> crate::runtime::vm::VMTableImport {
Expand Down
1 change: 1 addition & 0 deletions crates/wasmtime/src/runtime/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ impl Instance {
store: StorePtr::new(store.traitobj()),
wmemcheck: store.engine().config().wmemcheck,
pkey: store.get_pkey(),
tunables: store.engine().tunables(),
})?;

// The instance still has lots of setup, for example
Expand Down
6 changes: 3 additions & 3 deletions crates/wasmtime/src/runtime/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -883,12 +883,12 @@ impl Module {
.skip(em.num_imported_memories)
.map(|plan| plan.memory.limits.min)
.max();
let num_tables = u32::try_from(em.table_plans.len() - em.num_imported_tables).unwrap();
let num_tables = u32::try_from(em.num_defined_tables()).unwrap();
let max_initial_table_size = em
.table_plans
.tables
.values()
.skip(em.num_imported_tables)
.map(|plan| plan.table.limits.min)
.map(|table| table.limits.min)
.max();
ResourcesRequired {
num_memories,
Expand Down
3 changes: 2 additions & 1 deletion crates/wasmtime/src/runtime/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,7 @@ impl<T> Store<T> {
runtime_info: &shim,
wmemcheck: engine.config().wmemcheck,
pkey: None,
tunables: engine.tunables(),
})
.expect("failed to allocate default callee")
};
Expand Down Expand Up @@ -1293,7 +1294,7 @@ impl StoreOpaque {

let module = module.env_module();
let memories = module.memory_plans.len() - module.num_imported_memories;
let tables = module.table_plans.len() - module.num_imported_tables;
let tables = module.num_defined_tables();

bump(&mut self.instance_count, self.instance_limit, 1, "instance")?;
bump(
Expand Down
1 change: 1 addition & 0 deletions crates/wasmtime/src/runtime/trampoline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ fn create_handle(
runtime_info,
wmemcheck: false,
pkey: None,
tunables: store.engine().tunables(),
})?;

Ok(store.add_dummy_instance(handle))
Expand Down
8 changes: 5 additions & 3 deletions crates/wasmtime/src/runtime/trampoline/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use wasmtime_environ::{
#[cfg(feature = "component-model")]
use wasmtime_environ::{
component::{Component, VMComponentOffsets},
StaticModuleIndex,
StaticModuleIndex, Tunables,
};

/// Create a "frankenstein" instance with a single memory.
Expand Down Expand Up @@ -64,6 +64,7 @@ pub fn create_memory(
runtime_info,
wmemcheck: false,
pkey: None,
tunables: store.engine().tunables(),
};

unsafe {
Expand Down Expand Up @@ -234,10 +235,11 @@ unsafe impl InstanceAllocatorImpl for SingleMemoryInstance<'_> {
unsafe fn allocate_table(
&self,
req: &mut InstanceAllocationRequest,
table_plan: &wasmtime_environ::TablePlan,
ty: &wasmtime_environ::Table,
tunables: &Tunables,
table_index: DefinedTableIndex,
) -> Result<(TableAllocationIndex, Table)> {
self.ondemand.allocate_table(req, table_plan, table_index)
self.ondemand.allocate_table(req, ty, tunables, table_index)
}

unsafe fn deallocate_table(
Expand Down
4 changes: 1 addition & 3 deletions crates/wasmtime/src/runtime/trampoline/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@ pub fn create_table(store: &mut StoreOpaque, table: &TableType) -> Result<Instan
let mut module = Module::new();

let wasmtime_table = *table.wasmtime_table();
let tunables = store.engine().tunables();

debug_assert!(
wasmtime_table.ref_type.is_canonicalized_for_runtime_usage(),
"should be canonicalized for runtime usage: {:?}",
wasmtime_table.ref_type
);

let table_plan = wasmtime_environ::TablePlan::for_table(wasmtime_table, tunables);
let table_id = module.table_plans.push(table_plan);
let table_id = module.tables.push(wasmtime_table);

// TODO: can this `exports.insert` get removed?
module
Expand Down
Loading

0 comments on commit 7ee695c

Please sign in to comment.