Use inline GC barriers for accessing Wasm globals (#9244)

* Use inline GC barriers for accessing Wasm globals Previously, we would call out to the runtime via libcalls to get and set Wasm globals of types that are managed by the GC. This change introduces inline barriers for these operations, which should improve performance, and removes unnecessary libcall implementations. Co-Authored-By: Trevor Elliott <[email protected]> * Update other disas tests for new vmctx offsets * really update disas tests for new vmctx offsets * Remove unused functions when GC is disabled * remove unused import --------- Co-authored-by: Trevor Elliott <[email protected]>
bytecodealliance · Sep 13, 2024 · 3ea8807 · 3ea8807
1 parent 6046762
commit 3ea8807
Show file tree

Hide file tree

Showing 9 changed files with 399 additions and 150 deletions.
diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs
@@ -1912,21 +1912,25 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
             ty.is_vmgcref_type(),
             "We only use GlobalVariable::Custom for VMGcRef types"
         );
+        let cranelift_wasm::WasmValType::Ref(ty) = ty else {
+            unreachable!()
+        };
 
-        // TODO: use `GcCompiler::translate_read_gc_reference` for GC-reference
-        // globals instead of a libcall.
-        let libcall = gc::gc_ref_global_get_builtin(ty, self, &mut builder.func)?;
-
-        let vmctx = self.vmctx_val(&mut builder.cursor());
-
-        let global_index_arg = builder.ins().iconst(I32, index.as_u32() as i64);
-        let call_inst = builder.ins().call(libcall, &[vmctx, global_index_arg]);
+        let (gv, offset) = self.get_global_location(builder.func, index);
+        let gv = builder.ins().global_value(self.pointer_type(), gv);
+        let src = builder.ins().iadd_imm(gv, i64::from(offset));
 
-        let val = builder.func.dfg.first_result(call_inst);
-        if ty.is_vmgcref_type_and_not_i31() {
-            builder.declare_value_needs_stack_map(val);
+        if let WasmHeapType::I31 = ty.heap_type {
+            gc::unbarriered_load_gc_ref(self, builder, ty.heap_type, src, ir::MemFlags::trusted())
+        } else {
+            gc::gc_compiler(self).translate_read_gc_reference(
+                self,
+                builder,
+                ty,
+                src,
+                ir::MemFlags::trusted(),
+            )
         }
-        Ok(val)
     }
 
     fn translate_custom_global_set(
@@ -1940,19 +1944,33 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
             ty.is_vmgcref_type(),
             "We only use GlobalVariable::Custom for VMGcRef types"
         );
+        let cranelift_wasm::WasmValType::Ref(ty) = ty else {
+            unreachable!()
+        };
 
-        // TODO: use `GcCompiler::translate_write_gc_reference` for GC-reference
-        // globals instead of a libcall.
-        let libcall = gc::gc_ref_global_set_builtin(ty, self, &mut builder.func)?;
-
-        let vmctx = self.vmctx_val(&mut builder.cursor());
-
-        let global_index_arg = builder.ins().iconst(I32, index.as_u32() as i64);
-        builder
-            .ins()
-            .call(libcall, &[vmctx, global_index_arg, value]);
+        let (gv, offset) = self.get_global_location(builder.func, index);
+        let gv = builder.ins().global_value(self.pointer_type(), gv);
+        let src = builder.ins().iadd_imm(gv, i64::from(offset));
 
-        Ok(())
+        if let WasmHeapType::I31 = ty.heap_type {
+            gc::unbarriered_store_gc_ref(
+                self,
+                builder,
+                ty.heap_type,
+                src,
+                value,
+                ir::MemFlags::trusted(),
+            )
+        } else {
+            gc::gc_compiler(self).translate_write_gc_reference(
+                self,
+                builder,
+                ty,
+                src,
+                value,
+                ir::MemFlags::trusted(),
+            )
+        }
     }
 
     fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> WasmResult<Heap> {

diff --git a/crates/cranelift/src/gc.rs b/crates/cranelift/src/gc.rs
@@ -8,7 +8,7 @@
 use crate::func_environ::FuncEnvironment;
 use cranelift_codegen::ir;
 use cranelift_frontend::FunctionBuilder;
-use cranelift_wasm::{WasmHeapType, WasmRefType, WasmResult, WasmValType};
+use cranelift_wasm::{WasmHeapType, WasmRefType, WasmResult};
 
 #[cfg(feature = "gc")]
 mod enabled;
@@ -80,28 +80,6 @@ pub fn gc_ref_table_fill_builtin(
     imp::gc_ref_table_fill_builtin(ty, func_env, func)
 }
 
-/// Get the index and signature of the built-in function for doing `global.get`
-/// on a GC reference global.
-pub fn gc_ref_global_get_builtin(
-    ty: WasmValType,
-    func_env: &mut FuncEnvironment<'_>,
-    func: &mut ir::Function,
-) -> WasmResult<ir::FuncRef> {
-    debug_assert!(ty.is_vmgcref_type());
-    imp::gc_ref_global_get_builtin(ty, func_env, func)
-}
-
-/// Get the index and signature of the built-in function for doing `global.set`
-/// on a GC reference global.
-pub fn gc_ref_global_set_builtin(
-    ty: WasmValType,
-    func_env: &mut FuncEnvironment<'_>,
-    func: &mut ir::Function,
-) -> WasmResult<ir::FuncRef> {
-    debug_assert!(ty.is_vmgcref_type());
-    imp::gc_ref_global_set_builtin(ty, func_env, func)
-}
-
 /// A trait for different collectors to emit any GC barriers they might require.
 pub trait GcCompiler {
     /// Emit a read barrier for when we are cloning a GC reference onto the Wasm

diff --git a/crates/cranelift/src/gc/disabled.rs b/crates/cranelift/src/gc/disabled.rs
@@ -4,7 +4,7 @@ use super::GcCompiler;
 use crate::func_environ::FuncEnvironment;
 use cranelift_codegen::ir;
 use cranelift_frontend::FunctionBuilder;
-use cranelift_wasm::{wasm_unsupported, WasmHeapType, WasmRefType, WasmResult, WasmValType};
+use cranelift_wasm::{wasm_unsupported, WasmHeapType, WasmRefType, WasmResult};
 
 /// Get the default GC compiler.
 pub fn gc_compiler(_: &FuncEnvironment<'_>) -> Box<dyn GcCompiler> {
@@ -60,28 +60,6 @@ pub fn gc_ref_table_fill_builtin(
     ))
 }
 
-pub fn gc_ref_global_get_builtin(
-    ty: WasmValType,
-    _func_env: &mut FuncEnvironment<'_>,
-    _func: &mut ir::Function,
-) -> WasmResult<ir::FuncRef> {
-    Err(wasm_unsupported!(
-        "support for `{ty}` disabled at compile time because the `gc` cargo \
-         feature was not enabled"
-    ))
-}
-
-pub fn gc_ref_global_set_builtin(
-    ty: WasmValType,
-    _func_env: &mut FuncEnvironment<'_>,
-    _func: &mut ir::Function,
-) -> WasmResult<ir::FuncRef> {
-    Err(wasm_unsupported!(
-        "support for `{ty}` disabled at compile time because the `gc` cargo \
-         feature was not enabled"
-    ))
-}
-
 struct DisabledGcCompiler;
 
 impl GcCompiler for DisabledGcCompiler {

diff --git a/crates/cranelift/src/gc/enabled.rs b/crates/cranelift/src/gc/enabled.rs
@@ -2,9 +2,7 @@ use super::GcCompiler;
 use crate::func_environ::FuncEnvironment;
 use cranelift_codegen::ir::{self, condcodes::IntCC, InstBuilder};
 use cranelift_frontend::FunctionBuilder;
-use cranelift_wasm::{
-    TargetEnvironment, WasmHeapTopType, WasmHeapType, WasmRefType, WasmResult, WasmValType,
-};
+use cranelift_wasm::{TargetEnvironment, WasmHeapTopType, WasmHeapType, WasmRefType, WasmResult};
 use wasmtime_environ::{PtrSize, I31_DISCRIMINANT, NON_NULL_NON_I31_MASK};
 
 /// Get the default GC compiler.
@@ -62,24 +60,6 @@ pub fn gc_ref_table_fill_builtin(
     Ok(func_env.builtin_functions.table_fill_gc_ref(func))
 }
 
-pub fn gc_ref_global_get_builtin(
-    ty: WasmValType,
-    func_env: &mut FuncEnvironment<'_>,
-    func: &mut ir::Function,
-) -> WasmResult<ir::FuncRef> {
-    debug_assert!(ty.is_vmgcref_type());
-    Ok(func_env.builtin_functions.gc_ref_global_get(func))
-}
-
-pub fn gc_ref_global_set_builtin(
-    ty: WasmValType,
-    func_env: &mut FuncEnvironment<'_>,
-    func: &mut ir::Function,
-) -> WasmResult<ir::FuncRef> {
-    debug_assert!(ty.is_vmgcref_type());
-    Ok(func_env.builtin_functions.gc_ref_global_set(func))
-}
-
 impl FuncEnvironment<'_> {
     /// Get the GC heap's base pointer and bound.
     fn get_gc_heap_base_bound(&mut self, builder: &mut FunctionBuilder) -> (ir::Value, ir::Value) {

diff --git a/crates/environ/src/builtin.rs b/crates/environ/src/builtin.rs
@@ -79,16 +79,6 @@ macro_rules! foreach_builtin_function {
             #[cfg(feature = "gc")]
             gc(vmctx: vmctx, root: reference) -> reference;
 
-            // Implementation of Wasm's `global.get` instruction for globals
-            // containing GC references.
-            #[cfg(feature = "gc")]
-            gc_ref_global_get(vmctx: vmctx, global: i32) -> reference;
-
-            // Implementation of Wasm's `global.set` instruction for globals
-            // containing GC references.
-            #[cfg(feature = "gc")]
-            gc_ref_global_set(vmctx: vmctx, global: i32, val: reference);
-
             // Returns an index for Wasm's `table.grow` instruction for GC references.
             #[cfg(feature = "gc")]
             table_grow_gc_ref(vmctx: vmctx, table: i32, delta: i64, init: reference) -> pointer;

diff --git a/crates/wasmtime/src/runtime/vm/libcalls.rs b/crates/wasmtime/src/runtime/vm/libcalls.rs
@@ -437,43 +437,6 @@ unsafe fn gc(instance: &mut Instance, gc_ref: u32) -> Result<u32> {
     }
 }
 
-/// Perform a Wasm `global.get` for GC reference globals.
-#[cfg(feature = "gc")]
-unsafe fn gc_ref_global_get(instance: &mut Instance, index: u32) -> Result<u32> {
-    use core::num::NonZeroUsize;
-
-    let index = wasmtime_environ::GlobalIndex::from_u32(index);
-    let global = instance.defined_or_imported_global_ptr(index);
-    let gc_store = (*instance.store()).gc_store();
-
-    if gc_store
-        .gc_heap
-        .need_gc_before_entering_wasm(NonZeroUsize::new(1).unwrap())
-    {
-        (*instance.store()).gc(None)?;
-    }
-
-    match (*global).as_gc_ref() {
-        None => Ok(0),
-        Some(gc_ref) => {
-            let gc_ref = gc_store.clone_gc_ref(gc_ref);
-            let ret = gc_ref.as_raw_u32();
-            gc_store.expose_gc_ref_to_wasm(gc_ref);
-            Ok(ret)
-        }
-    }
-}
-
-/// Perform a Wasm `global.set` for GC reference globals.
-#[cfg(feature = "gc")]
-unsafe fn gc_ref_global_set(instance: &mut Instance, index: u32, gc_ref: u32) {
-    let index = wasmtime_environ::GlobalIndex::from_u32(index);
-    let global = instance.defined_or_imported_global_ptr(index);
-    let gc_ref = VMGcRef::from_raw_u32(gc_ref);
-    let gc_store = (*instance.store()).gc_store();
-    (*global).write_gc_ref(gc_store, gc_ref.as_ref());
-}
-
 // Implementation of `memory.atomic.notify` for locally defined memories.
 #[cfg(feature = "threads")]
 fn memory_atomic_notify(