Stream: git-wasmtime

Topic: wasmtime / PR #14081 winch: drc barriers

Wasmtime GitHub notifications bot (Aug 05 2026 at 17:39):

macovedj opened PR #14081 from macovedj:winch-drc-barriers to bytecodealliance:main:

Continues the work tracked in https://github.com/bytecodealliance/wasmtime/issues/14057 by adding DRC barriers for GC references in Winch. This unblocks exception-handling support work in Winch.
Closes https://github.com/bytecodealliance/wasmtime/issues/14057.

Wasmtime GitHub notifications bot (Aug 05 2026 at 17:39):

macovedj requested alexcrichton for a review on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 17:39):

macovedj requested wasmtime-compiler-reviewers for a review on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 17:39):

macovedj requested wasmtime-core-reviewers for a review on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 17:39):

macovedj requested wasmtime-default-reviewers for a review on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 19:00):

saulecabrera commented on PR #14081:

Happy to take this one as well @alexcrichton.

Wasmtime GitHub notifications bot (Aug 05 2026 at 19:00):

saulecabrera requested saulecabrera for a review on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 19:00):

saulecabrera unassigned alexcrichton from PR #14081 winch: drc barriers.

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:12):

github-actions[bot] added the label wasmtime:api on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:12):

github-actions[bot] added the label wasmtime:config on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:12):

github-actions[bot] added the label wasmtime:docs on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:12):

github-actions[bot] added the label winch on PR #14081.

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:13):

github-actions[bot] commented on PR #14081:

Subscribe to Label Action

cc @saulecabrera

<details>
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:config", "wasmtime:docs", "winch"

Thus the following users have been cc'd because of the following labels:

• saulecabrera: winch

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Aug 05 2026 at 21:04):

github-actions[bot] commented on PR #14081:

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

• [ ] If you added a new Config method, you wrote extensive documentation for
  it.

  <details>

  Our documentation should be of the following form:

  ```text
  Short, simple summary sentence.

  More details. These details can be multiple paragraphs. There should be
  information about not just the method, but its parameters and results as
  well.

  Is this method fallible? If so, when can it return an error?

  Can this method panic? If so, when does it panic?

  Example

  Optional example here.
  ```

  </details>

• [ ] If you added a new Config method, or modified an existing one, you
  ensured that this configuration is exercised by the fuzz targets.

  <details>

  For example, if you expose a new strategy for allocating the next instance
  slot inside the pooling allocator, you should ensure that at least one of our
  fuzz targets exercises that new strategy.

  Often, all that is required of you is to ensure that there is a knob for this
  configuration option in [wasmtime_fuzzing::Config][fuzzing-config] (or one
  of its nested structs).

  Rarely, this may require authoring a new fuzz target to specifically test this
  configuration. See [our docs on fuzzing][fuzzing-docs] for more details.

  </details>

• [ ] If you are enabling a configuration option by default, make sure that it
  has been fuzzed for at least two weeks before turning it on by default.

[fuzzing-config]: https://github.com/bytecodealliance/wasmtime/blob/ca0e8d0a1d8cefc0496dba2f77a670571d8fdcab/crates/fuzzing/src/generators.rs#L182-L194
[fuzzing-docs]: https://docs.wasmtime.dev/contributing-fuzzing.html

---

<details>

To modify this label's message, edit the <code>.github/label-messager/wasmtime-config.md</code> file.

To add new label messages or remove existing label messages, edit the
<code>.github/label-messager.json</code> configuration file.

Learn more.

</details>

Wasmtime GitHub notifications bot (Aug 07 2026 at 12:24):

:memo: saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Aug 07 2026 at 12:24):

:speech_balloon: saulecabrera created PR review comment:

The original definition in the drc module declared this as an i64, any reason it changed here?

Wasmtime GitHub notifications bot (Aug 07 2026 at 12:24):

:speech_balloon: saulecabrera created PR review comment:

I cannot leave comments inline unfortunately, GitHub complains that this diff is too large to render.

Many barrier operations are common across both table.{get,set}, and I suspect we would need them down the line as well. I'd suggest dividing the common operations, such that they can be reused and also to be self-contained, which will also make them easier to understand and change, I had in mind something like:

In the codegen module:

fn emit_skip_if_null_or_i31(&mut self) -> Result<()>;
fn emit_load_gc_heap_base_and_len(&mut self) -> Result<(Reg, Reg)>;
fn emit_gc_ref_bounds_check(&mut self) -> Result<()>;
fn emit_gc_ref_addr(&mut self) -> Result<TypedReg>;

Additionally, we could introduce a drc module, to contain drc-specific operations (similar to the bounds.rs module):

pub fn emit_drc_read_barrier(&mut self, ..) -> Result<()>;
pub fn emit_drc_write_barrier(&mut self, ..) -> Result<()>;
fn emit_mutate_ref_count(&mut self, ..) -> Result<Reg>;
fn emit_push_over_approx_stack_root(&mut self, ..) -> Result<Reg>;
fn emit_maybe_force_gc(&mut self, ..) -> Result<()>;

Note that this is just a template/suggestion.

Also note that when emitting complex instruction sequences, we try to document each case as much as possible, I think we should try to do that here as well (see CodeGen::emit_compute_heap_address)

Wasmtime GitHub notifications bot (Aug 07 2026 at 18:39):

macovedj updated PR #14081.

Wasmtime GitHub notifications bot (Aug 07 2026 at 18:41):

:speech_balloon: macovedj created PR review comment:

Thanks for the suggestions! I added the methods you suggested, along with a little more. I also decided to put the first few in a gc module, but can change the name if you like. Let me know what you think!

Wasmtime GitHub notifications bot (Aug 07 2026 at 18:41):

:memo: macovedj submitted PR review.

Wasmtime GitHub notifications bot (Aug 10 2026 at 14:29):

alexcrichton commented on PR #14081:

cc @fitzgen on this as well as you're likely curious

In talking briefly about this in-person at the CG meeting last week, one possibility as well could be that the DRC collector may not be supported by Winch and instead only the copying/null collectors are supported which don't need barriers. Unless you've got a specific need for DRC @macovedj it seems ok to me if Winch only supports a subset of the GC collectors (notably the main one though, the copying collector). Although having hooks for barriers is always a pretty nice thing since it makes adding/refactoring in the future easier since the barriers are already outlined. Mostly just wanted to throw this out there that I don't personally think DRC is required in Winch to turn GC/exceptions on for Winch in the long-run.

Wasmtime GitHub notifications bot (Aug 10 2026 at 15:39):

macovedj commented on PR #14081:

cc @fitzgen on this as well as you're likely curious

In talking briefly about this in-person at the CG meeting last week, one possibility as well could be that the DRC collector may not be supported by Winch and instead only the copying/null collectors are supported which don't need barriers. Unless you've got a specific need for DRC @macovedj it seems ok to me if Winch only supports a subset of the GC collectors (notably the main one though, the copying collector). Although having hooks for barriers is always a pretty nice thing since it makes adding/refactoring in the future easier since the barriers are already outlined. Mostly just wanted to throw this out there that I don't personally think DRC is required in Winch to turn GC/exceptions on for Winch in the long-run.

Oh interesting. I don't have a strong need, and I think that not adding it would simplify the EH work ahead for winch. That said, I talked with @saulecabrera a bit offline and got the impression that this isn't that far from landing and that winch benefits from the cranelift parity. Happy to drop the PR if anybody has concerns, but also happy to keep moving it forward if the DRC support is attractive to everybody.

Wasmtime GitHub notifications bot (Aug 10 2026 at 15:52):

:memo: saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Aug 10 2026 at 15:52):

:speech_balloon: saulecabrera created PR review comment:

The refactoring looks good to me, thanks! I have on general comment around passing registers as arguments/scratch register usage. I wonder if for scratch registers we can default to using Winch's dedicated scratch register where possible. For other registers, I wonder if we can do the register allocation in each of the methods where possible as well.

The former will reduce register pressure and the latter will shorten the register lifetime, which we usually try to do as much as possible to prevent unwanted clobbering bugs.

See a example diff below covering the drc.rs and gc.rs modules:

<details>

index ea1b8dd19a..d3eb52d321 100644
--- a/winch/codegen/src/codegen/drc.rs
+++ b/winch/codegen/src/codegen/drc.rs
@@ -1,7 +1,7 @@
 use super::{Callee, CodeGen, CodeGenError, Emission, FnCall};
 use crate::{
     Result,
-    masm::{IntCmpKind, MacroAssembler, OperandSize, RegImm},
+    masm::{IntCmpKind, IntScratch, MacroAssembler, OperandSize, RegImm},
     reg::{Reg, writable},
     stack::{TypedReg, Val},
 };
@@ -63,9 +63,7 @@ where
         )?;

         let skip_barrier = self.masm.get_label()?;
-        let null_or_i31_scratch = self.context.any_gpr(self.masm)?;
-        self.emit_skip_if_gc_ref_is_null_or_i31(ref_reg, null_or_i31_scratch, skip_barrier)?;
-        self.context.free_reg(null_or_i31_scratch);
+        self.emit_skip_if_gc_ref_is_null_or_i31(ref_reg, skip_barrier)?;

         let (heap_reg, bound_reg) = self.emit_load_gc_heap_base_and_bound()?;

@@ -75,26 +73,25 @@ where
                 .vmoffsets
                 .vm_drc_header_next_over_approximated_stack_root(),
         ) + 4;
-        let extent_reg = self.context.any_gpr(self.masm)?;
-        self.emit_gc_ref_bounds_check(ref_reg, bound_reg, header_extent, extent_reg)?;
-        self.context.free_reg(extent_reg);
+        self.emit_gc_ref_bounds_check(ref_reg, bound_reg, header_extent)?;

-        // The bound is dead after the check, so reuse its register for the
-        // native object address and keep register pressure low.
-        let object_addr = bound_reg;
-        self.emit_gc_ref_addr(ref_reg, heap_reg, object_addr)?;
+        // The bound is dead after the check. Free it before requesting the
+        // object address so the allocator can hand the same register back and
+        // keep register pressure low.
+        self.context.free_reg(bound_reg);
+        let object_addr = self.emit_gc_ref_addr(ref_reg, heap_reg)?;

-        let bits_reg = self.context.any_gpr(self.masm)?;
-        self.emit_skip_if_in_over_approx_stack_roots(object_addr, bits_reg, skip_barrier)?;
+        self.emit_skip_if_in_over_approx_stack_roots(object_addr, skip_barrier)?;

-        let heap_data_reg =
-            self.emit_push_over_approx_stack_root(ref_reg, object_addr, bits_reg)?;
+        let (heap_data_reg, roots_len) =
+            self.emit_push_over_approx_stack_root(ref_reg, object_addr)?;

-        // `ref_reg` and the heap base are no longer needed. The remaining
-        // three registers are consumed and freed by `emit_maybe_force_gc`.
+        // Only the roots-list length and the heap data pointer are still live;
+        // both are consumed and freed by `emit_maybe_force_gc`.
         self.context.free_reg(ref_reg);
         self.context.free_reg(heap_reg);
-        self.emit_maybe_force_gc(bits_reg, heap_data_reg, object_addr)?;
+        self.context.free_reg(object_addr);
+        self.emit_maybe_force_gc(roots_len, heap_data_reg)?;

         self.masm.bind(skip_barrier)?;
         self.context.free_reg(storage_base);
@@ -126,17 +123,17 @@ where

         let ref_count_offset = self.env.vmoffsets.vm_drc_header_ref_count();
         let header_extent = i64::from(ref_count_offset) + 8;
-        let count_addr_reg = self.context.any_gpr(self.masm)?;
-        let count_reg = self.context.any_gpr(self.masm)?;

         // Retain the new heap reference before publishing it. This ordering
         // keeps self-assignment from temporarily dropping the final owner.
         let skip_inc = self.masm.get_label()?;
-        self.emit_skip_if_gc_ref_is_null_or_i31(new_ref.reg, count_addr_reg, skip_inc)?;
-        self.emit_gc_ref_bounds_check(new_ref.reg, bound_reg, header_extent, count_addr_reg)?;
-        self.emit_gc_ref_addr(new_ref.reg, heap_reg, count_addr_reg)?;
-        self.emit_mutate_ref_count(count_addr_reg, count_reg, RefCountMutation::Increment)?;
-        self.emit_store_ref_count(count_addr_reg, count_reg)?;
+        self.emit_skip_if_gc_ref_is_null_or_i31(new_ref.reg, skip_inc)?;
+        self.emit_gc_ref_bounds_check(new_ref.reg, bound_reg, header_extent)?;
+        let new_addr = self.emit_gc_ref_addr(new_ref.reg, heap_reg)?;
+        let count = self.emit_mutate_ref_count(new_addr, RefCountMutation::Increment)?;
+        self.emit_store_ref_count(new_addr, count)?;
+        self.context.free_reg(count);
+        self.context.free_reg(new_addr);

         self.masm.bind(skip_inc)?;
         // Publish the new value before releasing the old one because the
@@ -146,24 +143,24 @@ where
         // Release the old heap reference, storing a nonzero count inline and
         // delegating the zero-count case to `drop_gc_ref`.
         let skip_dec = self.masm.get_label()?;
-        self.emit_skip_if_gc_ref_is_null_or_i31(old_reg, count_addr_reg, skip_dec)?;
-        self.emit_gc_ref_bounds_check(old_reg, bound_reg, header_extent, count_addr_reg)?;
-        self.emit_gc_ref_addr(old_reg, heap_reg, count_addr_reg)?;
-        self.emit_mutate_ref_count(count_addr_reg, count_reg, RefCountMutation::Decrement)?;
+        self.emit_skip_if_gc_ref_is_null_or_i31(old_reg, skip_dec)?;
+        self.emit_gc_ref_bounds_check(old_reg, bound_reg, header_extent)?;
+        let old_addr = self.emit_gc_ref_addr(old_reg, heap_reg)?;
+        let count = self.emit_mutate_ref_count(old_addr, RefCountMutation::Decrement)?;

         let drop_old = self.masm.get_label()?;
         self.masm.branch(
             IntCmpKind::Eq,
-            count_reg,
+            count,
             RegImm::i64(0),
             drop_old,
             OperandSize::S64,
         )?;
-        self.emit_store_ref_count(count_addr_reg, count_reg)?;
+        self.emit_store_ref_count(old_addr, count)?;
         self.masm.jmp(skip_dec)?;

-        self.context.free_reg(count_reg);
-        self.context.free_reg(count_addr_reg);
+        self.context.free_reg(count);
+        self.context.free_reg(old_addr);
         self.context.free_reg(bound_reg);
         self.context.free_reg(heap_reg);
         self.context.free_reg(new_ref.reg);
@@ -184,43 +181,50 @@ where

     /// Branches to `skip` when `object_addr` is already linked into the
     /// over-approximated stack-roots list.
+    ///
+    /// Uses the dedicated scratch register, so it must not be called from
+    /// inside another `with_scratch` scope.
     fn emit_skip_if_in_over_approx_stack_roots(
         &mut self,
         object_addr: Reg,
-        scratch: Reg,
         skip: MachLabel,
     ) -> Result<()> {
         let reserved_offset = self.env.vmoffsets.vm_gc_header_reserved_bits();
-        self.masm.load(
-            self.masm.address_at_reg(object_addr, reserved_offset)?,
-            writable!(scratch),
-            OperandSize::S32,
-        )?;
-        self.masm.and(
-            writable!(scratch),
-            scratch,
-            RegImm::i32(DRC_HEADER_IN_OVER_APPROX_LIST_BIT as i32),
-            OperandSize::S32,
-        )?;
-        self.masm.branch(
-            IntCmpKind::Ne,
-            scratch,
-            scratch.into(),
-            skip,
-            OperandSize::S32,
-        )
+        self.masm.with_scratch::<IntScratch, _>(|masm, scratch| {
+            masm.load(
+                masm.address_at_reg(object_addr, reserved_offset)?,
+                scratch.writable(),
+                OperandSize::S32,
+            )?;
+            masm.and(
+                scratch.writable(),
+                scratch.inner(),
+                RegImm::i32(DRC_HEADER_IN_OVER_APPROX_LIST_BIT as i32),
+                OperandSize::S32,
+            )?;
+            masm.branch(
+                IntCmpKind::Ne,
+                scratch.inner(),
+                scratch.inner().into(),
+                skip,
+                OperandSize::S32,
+            )
+        })
     }

     /// Adds `gc_ref` to the DRC heap's over-approximated stack-roots list.
     ///
     /// The object is linked at the head, marked as linked, and retained for
     /// the list's ownership.
+    ///
+    /// Returns the DRC heap data pointer and the updated roots-list length,
+    /// both allocated here and owned by the caller, which must eventually free
+    /// them. The length feeds the collection-threshold check.
     fn emit_push_over_approx_stack_root(
         &mut self,
         gc_ref: Reg,
         object_addr: Reg,
-        scratch: Reg,
-    ) -> Result<Reg> {
+    ) -> Result<(Reg, Reg)> {
         let heap_data_offset = self.env.vmoffsets.ptr.vmctx_gc_heap_data();
         let roots_head_offset = u32::from(
             self.env
@@ -246,103 +250,109 @@ where
             writable!(heap_data_reg),
         )?;

-        // Link this object to the old head of the over-approximated list.
-        self.masm.load(
-            self.masm.address_at_reg(heap_data_reg, roots_head_offset)?,
-            writable!(scratch),
-            OperandSize::S32,
-        )?;
-        self.masm.store(
-            scratch.into(),
-            self.masm.address_at_reg(object_addr, next_offset)?,
-            OperandSize::S32,
-        )?;
-
-        // Mark the object as present so subsequent reads can take the fast
-        // path, and retain it f
[message truncated]

Wasmtime GitHub notifications bot (Aug 10 2026 at 16:01):

:speech_balloon: saulecabrera edited PR review comment.

Wasmtime GitHub notifications bot (Aug 10 2026 at 16:03):

saulecabrera commented on PR #14081:

@alexcrichton I agree that this is not required to land GC and EH. However, sometimes I find it a bit difficult to draw the line on when a compiler (Winch in my case) should support or not certain engine features. My assumption until has been that Winch should try to provide as much parity as possible with Cranelift, unless there's a real technical reason not to (e.g., supporting function inlining). In this case parity seemed the right default to me; but also we can decide not to land it if there are any specific concerns.

Wasmtime GitHub notifications bot (Aug 10 2026 at 16:17):

saulecabrera edited a comment on PR #14081:

@alexcrichton I agree that this is not required to land GC and EH. However, sometimes I find it a bit difficult to draw the line on when a compiler (Winch in my case) should support or not certain engine features. My assumption until now has been that Winch should try to provide as much parity as possible with Cranelift, unless there's a real technical reason not to (e.g., supporting function inlining). In this case parity seemed the right default to me; but also we can decide not to land it if there are any specific concerns.

Wasmtime GitHub notifications bot (Aug 10 2026 at 17:16):

macovedj updated PR #14081.

Wasmtime GitHub notifications bot (Aug 11 2026 at 14:37):

alexcrichton commented on PR #14081:

No yeah that all sounds reasonable to me. If it's significantly complicated for Winch to support something I personally think it's always in the cards to avoid implementing it, but that's not the case here so seems totally fine to land

Wasmtime GitHub notifications bot (Aug 12 2026 at 15:03):

:thumbs_up: saulecabrera submitted PR review:

LGTM, thanks!

Wasmtime GitHub notifications bot (Aug 12 2026 at 15:03):

saulecabrera added PR #14081 winch: drc barriers to the merge queue.

Wasmtime GitHub notifications bot (Aug 12 2026 at 15:29):

:check: saulecabrera merged PR #14081.

Wasmtime GitHub notifications bot (Aug 12 2026 at 15:29):

saulecabrera removed PR #14081 winch: drc barriers from the merge queue.

Last updated: Aug 30 2026 at 09:07 UTC