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

@@ -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()

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