Do not reorder/pack struct fields

We were not considering that when a type T is a subtype of type U, that all of
the fields that T and U share must be at the same offsets. Our
reordering/packing algorithm did not consider this. So we stop reordering struct
fields for now.

This also means that we cannot guarantee that all GC refs in an object are
contiguously packed anymore. Instead, we maintain a map keyed by
`VMSharedTypeIndex`, whose value is all the offsets of outgoing GC
references for instances of that type. Tracing looks up the current object's
type in this map, and then extracts each of the GC edges.

This new tracing scheme means that we no longer need to store the number of
outgoing edges in the GC object header, and can put the object size in there
again, reducing the DRC header from 24 to 16 bytes.

Fixes bytecodealliance#10459
Fixes bytecodealliance#10464

Author: fitzgen

crates/cranelift/src/gc/enabled/drc.rs

@@ -260,14 +260,10 @@ impl DrcCompiler {
 }
 
 /// Emit CLIF to call the `gc_raw_alloc` libcall.
-///
-/// It is the caller's responsibility to ensure that `num_gc_refs` fits within
-/// the `VMGcKind`'s unused bits.
 fn emit_gc_raw_alloc(
     func_env: &mut FuncEnvironment<'_>,
     builder: &mut FunctionBuilder<'_>,
     kind: VMGcKind,
-    num_gc_refs: ir::Value,
     ty: ModuleInternedTypeIndex,
     size: ir::Value,
     align: u32,
@@ -278,17 +274,15 @@ fn emit_gc_raw_alloc(
     let kind = builder
         .ins()
         .iconst(ir::types::I32, i64::from(kind.as_u32()));
-    let kind_and_reserved = builder.ins().bor(kind, num_gc_refs);
 
     let ty = builder.ins().iconst(ir::types::I32, i64::from(ty.as_u32()));
 
     assert!(align.is_power_of_two());
     let align = builder.ins().iconst(ir::types::I32, i64::from(align));
 
-    let call_inst = builder.ins().call(
-        gc_alloc_raw_builtin,
-        &[vmctx, kind_and_reserved, ty, size, align],
-    );
+    let call_inst = builder
+        .ins()
+        .call(gc_alloc_raw_builtin, &[vmctx, kind, ty, size, align]);
 
     let gc_ref = builder.func.dfg.first_result(call_inst);
     builder.declare_value_needs_stack_map(gc_ref);
@@ -321,19 +315,13 @@ impl GcCompiler for DrcCompiler {
         // size, and length.
         let len = init.len(&mut builder.cursor());
         let size = emit_array_size(func_env, builder, &array_layout, len);
-        let num_gc_refs = if array_layout.elems_are_gc_refs {
-            len
-        } else {
-            builder.ins().iconst(ir::types::I32, 0)
-        };
 
         // Second, now that we have the array object's total size, call the
         // `gc_alloc_raw` builtin libcall to allocate the array.
         let array_ref = emit_gc_raw_alloc(
             func_env,
             builder,
             VMGcKind::ArrayRef,
-            num_gc_refs,
             interned_type_index,
             size,
             align,
@@ -394,15 +382,10 @@ impl GcCompiler for DrcCompiler {
         assert_eq!(VMGcKind::UNUSED_MASK & struct_size, struct_size);
         let struct_size_val = builder.ins().iconst(ir::types::I32, i64::from(struct_size));
 
-        let num_gc_refs = struct_layout.fields.iter().filter(|f| f.is_gc_ref).count();
-        let num_gc_refs = u32::try_from(num_gc_refs).unwrap();
-        let num_gc_refs = builder.ins().iconst(ir::types::I32, i64::from(num_gc_refs));
-
         let struct_ref = emit_gc_raw_alloc(
             func_env,
             builder,
             VMGcKind::StructRef,
-            num_gc_refs,
             interned_type_index,
             struct_size_val,
             struct_align,

crates/environ/src/gc.rs

@@ -116,7 +116,7 @@ fn common_array_layout(
 
 /// Common code to define a GC struct's layout, given the size and alignment of
 /// the collector's GC header and its expected offset of the array length field.
-#[cfg(feature = "gc-null")]
+#[cfg(any(feature = "gc-null", feature = "gc-drc"))]
 fn common_struct_layout(
     ty: &WasmStructType,
     header_size: u32,
@@ -127,11 +127,12 @@ fn common_struct_layout(
 
     // Process each field, aligning it to its natural alignment.
     //
-    // We don't try and do any fancy field reordering to minimize padding
-    // (yet?) because (a) the toolchain probably already did that and (b)
-    // we're just doing the simple thing first. We can come back and improve
-    // things here if we find that (a) isn't actually holding true in
-    // practice.
+    // We don't try and do any fancy field reordering to minimize padding (yet?)
+    // because (a) the toolchain probably already did that and (b) we're just
+    // doing the simple thing first, and (c) this is tricky in the presence of
+    // subtyping where we need a subtype's fields to be assigned the same
+    // offsets as its supertype's fields. We can come back and improve things
+    // here if we find that (a) isn't actually holding true in practice.
     let mut size = header_size;
     let mut align = header_align;
 

crates/environ/src/gc/drc.rs

@@ -1,10 +1,9 @@
 //! Layout of Wasm GC objects in the deferred reference-counting collector.
 
 use super::*;
-use core::cmp;
 
 /// The size of the `VMDrcHeader` header for GC objects.
-pub const HEADER_SIZE: u32 = 24;
+pub const HEADER_SIZE: u32 = 16;
 
 /// The align of the `VMDrcHeader` header for GC objects.
 pub const HEADER_ALIGN: u32 = 8;
@@ -26,54 +25,6 @@ impl GcTypeLayouts for DrcTypeLayouts {
     }
 
     fn struct_layout(&self, ty: &WasmStructType) -> GcStructLayout {
-        // Sort the struct fields into the order in which we will lay them out.
-        //
-        // NB: we put all GC refs first so that we can simply store the number
-        // of GC refs in any object in the `VMDrcHeader` and then uniformly
-        // trace all structs types.
-        let mut fields: Vec<_> = ty.fields.iter().enumerate().collect();
-        fields.sort_by_key(|(i, f)| {
-            let is_gc_ref = f.element_type.is_vmgcref_type_and_not_i31();
-            let size = byte_size_of_wasm_ty_in_gc_heap(&f.element_type);
-            (cmp::Reverse(is_gc_ref), cmp::Reverse(size), *i)
-        });
-
-        // Compute the offset of each field as well as the size and alignment of
-        // the whole struct.
-        let mut size = HEADER_SIZE;
-        let mut align = HEADER_ALIGN;
-        let mut fields: Vec<_> = fields
-            .into_iter()
-            .map(|(i, f)| {
-                let field_size = byte_size_of_wasm_ty_in_gc_heap(&f.element_type);
-                let offset = field(&mut size, &mut align, field_size);
-                let is_gc_ref = f.element_type.is_vmgcref_type_and_not_i31();
-                (i, GcStructLayoutField { offset, is_gc_ref })
-            })
-            .collect();
-        if let Some((_i, f)) = fields.get(0) {
-            if f.is_gc_ref {
-                debug_assert_eq!(
-                    f.offset, HEADER_SIZE,
-                    "GC refs should come directly after the header, without any padding",
-                );
-            }
-        }
-
-        // Re-sort the fields into their definition (rather than layout) order
-        // and throw away the definition index.
-        fields.sort_by_key(|(i, _f)| *i);
-        let fields: Vec<_> = fields.into_iter().map(|(_i, f)| f).collect();
-
-        // Ensure that the final size is a multiple of the alignment, for
-        // simplicity.
-        let align_size_to = align;
-        align_up(&mut size, &mut align, align_size_to);
-
-        GcStructLayout {
-            size,
-            align,
-            fields,
-        }
+        common_struct_layout(ty, HEADER_SIZE, HEADER_ALIGN)
     }
 }

crates/wasmtime/src/runtime/store.rs

@@ -1423,7 +1423,7 @@ impl StoreOpaque {
             );
             let (index, heap) = engine
                 .allocator()
-                .allocate_gc_heap(&**engine.gc_runtime()?)?;
+                .allocate_gc_heap(engine, &**engine.gc_runtime()?)?;
             Ok(GcStore::new(index, heap))
         }
 

crates/wasmtime/src/runtime/trampoline/memory.rs

@@ -244,6 +244,7 @@ unsafe impl InstanceAllocatorImpl for SingleMemoryInstance<'_> {
     #[cfg(feature = "gc")]
     fn allocate_gc_heap(
         &self,
+        _engine: &crate::Engine,
         _gc_runtime: &dyn crate::runtime::vm::GcRuntime,
     ) -> Result<(
         crate::runtime::vm::GcHeapAllocationIndex,