From bbd52772de76d0f5ab4cf9a0ab8744c4ed59571d Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Tue, 22 Feb 2022 16:48:53 +0100 Subject: [PATCH] Make VMOffset calculation more readable (#3793) * Fix typo * Move vmoffset field size and field name together The previous code was quite confusing about what applied to which field. The new code also makes it easier to move fields around and insert and delete fields. * Move builtin_functions before all variable sized fields This allows the offset to be calculated at compile time * Add cadd and cmul convenience functions * Remove comment * Change fields! syntax as per review * Add implicit u32::from to fields! --- crates/cranelift/src/func_environ.rs | 8 +- crates/environ/src/vmoffsets.rs | 157 +++++++++++---------------- 2 files changed, 65 insertions(+), 100 deletions(-) diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs index ee0b15b76493..b6565bdef447 100644 --- a/crates/cranelift/src/func_environ.rs +++ b/crates/cranelift/src/func_environ.rs @@ -299,7 +299,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { /// reference count. /// /// The new reference count is returned. - fn mutate_extenref_ref_count( + fn mutate_externref_ref_count( &mut self, builder: &mut FunctionBuilder, externref: ir::Value, @@ -1036,7 +1036,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m // * store the reference into the bump table at `*next`, // * and finally increment the `next` bump finger. builder.switch_to_block(no_gc_block); - self.mutate_extenref_ref_count(builder, elem, 1); + self.mutate_externref_ref_count(builder, elem, 1); builder.ins().store(ir::MemFlags::trusted(), elem, next, 0); let new_next = builder @@ -1159,7 +1159,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m .brnz(value_is_null, check_current_elem_block, &[]); builder.ins().jump(inc_ref_count_block, &[]); builder.switch_to_block(inc_ref_count_block); - self.mutate_extenref_ref_count(builder, value, 1); + self.mutate_externref_ref_count(builder, value, 1); builder.ins().jump(check_current_elem_block, &[]); // Grab the current element from the table, and store the new @@ -1193,7 +1193,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m builder.ins().jump(continue_block, &[]); builder.switch_to_block(dec_ref_count_block); - let prev_ref_count = self.mutate_extenref_ref_count(builder, current_elem, -1); + let prev_ref_count = self.mutate_externref_ref_count(builder, current_elem, -1); let one = builder.ins().iconst(pointer_type, 1); builder .ins() diff --git a/crates/environ/src/vmoffsets.rs b/crates/environ/src/vmoffsets.rs index eca046a31582..d30f6b924481 100644 --- a/crates/environ/src/vmoffsets.rs +++ b/crates/environ/src/vmoffsets.rs @@ -7,6 +7,7 @@ // interrupts: *const VMInterrupts, // externref_activations_table: *mut VMExternRefActivationsTable, // store: *mut dyn Store, +// builtins: *mut VMBuiltinFunctionsArray, // signature_ids: *const VMSharedSignatureIndex, // imported_functions: [VMFunctionImport; module.num_imported_functions], // imported_tables: [VMTableImport; module.num_imported_tables], @@ -16,7 +17,6 @@ // memories: [VMMemoryDefinition; module.num_defined_memories], // globals: [VMGlobalDefinition; module.num_defined_globals], // anyfuncs: [VMCallerCheckedAnyfunc; module.num_imported_functions + module.num_defined_functions], -// builtins: *mut VMBuiltinFunctionsArray, // } use crate::{ @@ -74,6 +74,7 @@ pub struct VMOffsets

{ epoch_ptr: u32, externref_activations_table: u32, store: u32, + builtin_functions: u32, signature_ids: u32, imported_functions: u32, imported_tables: u32, @@ -83,7 +84,6 @@ pub struct VMOffsets

{ defined_memories: u32, defined_globals: u32, defined_anyfuncs: u32, - builtin_functions: u32, size: u32, } @@ -172,6 +172,7 @@ impl From> for VMOffsets

{ epoch_ptr: 0, externref_activations_table: 0, store: 0, + builtin_functions: 0, signature_ids: 0, imported_functions: 0, imported_tables: 0, @@ -181,103 +182,67 @@ impl From> for VMOffsets

{ defined_memories: 0, defined_globals: 0, defined_anyfuncs: 0, - builtin_functions: 0, size: 0, }; - ret.interrupts = 0; - ret.epoch_ptr = ret - .interrupts - .checked_add(u32::from(ret.ptr.size())) - .unwrap(); - ret.externref_activations_table = ret - .epoch_ptr - .checked_add(u32::from(ret.ptr.size())) - .unwrap(); - ret.store = ret - .externref_activations_table - .checked_add(u32::from(ret.ptr.size())) - .unwrap(); - ret.signature_ids = ret - .store - .checked_add(u32::from(ret.ptr.size() * 2)) - .unwrap(); - ret.imported_functions = ret - .signature_ids - .checked_add(u32::from(ret.ptr.size())) - .unwrap(); - ret.imported_tables = ret - .imported_functions - .checked_add( - ret.num_imported_functions - .checked_mul(u32::from(ret.size_of_vmfunction_import())) - .unwrap(), - ) - .unwrap(); - ret.imported_memories = ret - .imported_tables - .checked_add( - ret.num_imported_tables - .checked_mul(u32::from(ret.size_of_vmtable_import())) - .unwrap(), - ) - .unwrap(); - ret.imported_globals = ret - .imported_memories - .checked_add( - ret.num_imported_memories - .checked_mul(u32::from(ret.size_of_vmmemory_import())) - .unwrap(), - ) - .unwrap(); - ret.defined_tables = ret - .imported_globals - .checked_add( - ret.num_imported_globals - .checked_mul(u32::from(ret.size_of_vmglobal_import())) - .unwrap(), - ) - .unwrap(); - ret.defined_memories = ret - .defined_tables - .checked_add( - ret.num_defined_tables - .checked_mul(u32::from(ret.size_of_vmtable_definition())) - .unwrap(), - ) - .unwrap(); - ret.defined_globals = align( - ret.defined_memories - .checked_add( - ret.num_defined_memories - .checked_mul(u32::from(ret.size_of_vmmemory_definition())) - .unwrap(), - ) - .unwrap(), - 16, - ); - ret.defined_anyfuncs = ret - .defined_globals - .checked_add( - ret.num_defined_globals - .checked_mul(u32::from(ret.size_of_vmglobal_definition())) - .unwrap(), - ) - .unwrap(); - ret.builtin_functions = ret - .defined_anyfuncs - .checked_add( - ret.num_imported_functions - .checked_add(ret.num_defined_functions) - .unwrap() - .checked_mul(u32::from(ret.size_of_vmcaller_checked_anyfunc())) - .unwrap(), - ) - .unwrap(); - ret.size = ret - .builtin_functions - .checked_add(u32::from(ret.pointer_size())) - .unwrap(); + // Convenience functions for checked addition and multiplication. + // As side effect this reduces binary size by using only a single + // `#[track_caller]` location for each function instead of one for + // each individual invocation. + #[inline] + fn cadd(count: u32, size: u32) -> u32 { + count.checked_add(size).unwrap() + } + + #[inline] + fn cmul(count: u32, size: u8) -> u32 { + count.checked_mul(u32::from(size)).unwrap() + } + + let mut next_field_offset = 0; + + macro_rules! fields { + (size($field:ident) = $size:expr, $($rest:tt)*) => { + ret.$field = next_field_offset; + next_field_offset = cadd(next_field_offset, u32::from($size)); + fields!($($rest)*); + }; + (align($align:literal), $($rest:tt)*) => { + next_field_offset = align(next_field_offset, $align); + fields!($($rest)*); + }; + () => {}; + } + + fields! { + size(interrupts) = ret.ptr.size(), + size(epoch_ptr) = ret.ptr.size(), + size(externref_activations_table) = ret.ptr.size(), + size(store) = ret.ptr.size() * 2, + size(builtin_functions) = ret.pointer_size(), + size(signature_ids) = ret.ptr.size(), + size(imported_functions) + = cmul(ret.num_imported_functions, ret.size_of_vmfunction_import()), + size(imported_tables) + = cmul(ret.num_imported_tables, ret.size_of_vmtable_import()), + size(imported_memories) + = cmul(ret.num_imported_memories, ret.size_of_vmmemory_import()), + size(imported_globals) + = cmul(ret.num_imported_globals, ret.size_of_vmglobal_import()), + size(defined_tables) + = cmul(ret.num_defined_tables, ret.size_of_vmtable_definition()), + size(defined_memories) + = cmul(ret.num_defined_memories, ret.size_of_vmmemory_definition()), + align(16), + size(defined_globals) + = cmul(ret.num_defined_globals, ret.size_of_vmglobal_definition()), + size(defined_anyfuncs) = cmul( + cadd(ret.num_imported_functions, ret.num_defined_functions), + ret.size_of_vmcaller_checked_anyfunc(), + ), + } + + ret.size = next_field_offset; return ret; }