From cfc16ab2f8dc742bc8cecd6a7b847535bc33c070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 10:11:09 +0200 Subject: [PATCH 1/2] perf(gc): align barrier gate load ordering --- .../src/expr/class_field_barrier_tests.rs | 10 +++++++--- crates/perry-codegen/src/expr/shadow_inline.rs | 14 ++++++++------ crates/perry-codegen/src/expr/shadow_slot.rs | 6 ++++-- crates/perry-codegen/src/expr/write_barrier.rs | 14 ++++++++------ crates/perry-codegen/tests/shadow_slot_hygiene.rs | 5 +++-- crates/perry-runtime/src/gc/barrier/mod.rs | 12 +++++++++++- crates/perry-runtime/src/gc/roots/shadow_stack.rs | 6 +++--- .../gc/tests/inline_generation_gate_contract.rs | 12 +++++++----- .../perry-runtime/src/gc/tests/shadow_stack_ops.rs | 4 +++- 9 files changed, 54 insertions(+), 29 deletions(-) diff --git a/crates/perry-codegen/src/expr/class_field_barrier_tests.rs b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs index 5f6caba4f2..cdbfc15932 100644 --- a/crates/perry-codegen/src/expr/class_field_barrier_tests.rs +++ b/crates/perry-codegen/src/expr/class_field_barrier_tests.rs @@ -381,13 +381,17 @@ fn the_class_field_barrier_sits_behind_a_live_parent_generation_test() { "the incremental clause is `{incremental_cmp}`:\n{body}" ); let count_reg = operand(incremental_cmp, 0).expect("icmp lhs"); + let count_load = def_of(&body, &count_reg).unwrap_or_default(); assert!( - def_of(&body, &count_reg) - .unwrap_or_default() - .contains(INCREMENTAL_GLOBAL), + count_load.contains(INCREMENTAL_GLOBAL), "the incremental clause does not read {INCREMENTAL_GLOBAL}; skipping \ the barrier also skips SATB shading:\n{body}" ); + assert!( + count_load.starts_with("load atomic i32") && count_load.contains(" monotonic, align 4"), + "the incremental gate uses `{count_load}` rather than the runtime's \ + Relaxed ordering (LLVM `monotonic`):\n{body}" + ); // The barrier must be on the TAKEN edge. A swapped `cond_br` compiles, // prints the right answer, and strands a child on the next minor GC. let successors: Vec<&str> = branch diff --git a/crates/perry-codegen/src/expr/shadow_inline.rs b/crates/perry-codegen/src/expr/shadow_inline.rs index 93e61971b4..f98f3b8e97 100644 --- a/crates/perry-codegen/src/expr/shadow_inline.rs +++ b/crates/perry-codegen/src/expr/shadow_inline.rs @@ -261,15 +261,17 @@ fn emit_inline_slot_write(ctx: &mut FnCtx<'_>, slot_idx: u32, what: InlineSlotWr /// Identical in kind to `emit_persistent_shadow_root_barrier` and to the /// runtime's own `root_shading_barrier`: a zero count *proves* this thread's /// `INCREMENTAL_MARK_BARRIER_VALID_PTRS` is null, because -/// `incremental_mark_barrier_enable` installs the thread-local before -/// incrementing the count. Skipping the call on a zero count is therefore -/// observationally identical, not a weaker barrier. +/// `incremental_mark_barrier_enable` increments the count before installing +/// the thread-local and disable clears the thread-local before decrementing +/// the count. Skipping the call on a zero count is therefore observationally +/// identical, not a weaker barrier. The LLVM `monotonic` load matches the +/// runtime's Rust `Relaxed` readers; this gate does not publish other memory. /// /// Terminates the current block with a branch to `done_label`. fn emit_inline_root_shading_barrier(ctx: &mut FnCtx<'_>, value_bits: &str, done_label: &str) { let active = ctx.block() - .load_atomic_seq_cst(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); + .load_atomic_monotonic(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); let needed = ctx.block().icmp_ne(I32, &active, "0"); let barrier_idx = ctx.new_block("ss.barrier"); let barrier_label = ctx.block_label(barrier_idx); @@ -535,9 +537,9 @@ mod tests { let body = roots_body(&rooted_local_ir()); assert!( body.contains( - "load atomic i32, ptr @PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT seq_cst" + "load atomic i32, ptr @PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT monotonic" ), - "inline bind must gate on the incremental-mark active count; \ + "inline bind must use the runtime's relaxed ordering for the incremental-mark gate; \ body:\n{body}" ); assert!( diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index 0aa859bb4f..7d90def217 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -272,11 +272,13 @@ pub(crate) fn emit_shadow_slot_bind_ptr(ctx: &mut FnCtx<'_>, slot_idx: u32, slot /// collector scanned roots still has to be shaded. Guarding on /// `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` inline keeps the common /// (no incremental cycle in flight) path down to a load, a compare, and a -/// not-taken branch instead of a TLS-touching call. +/// not-taken branch instead of a TLS-touching call. The load is LLVM +/// `monotonic`, matching the runtime's Rust `Relaxed` readers: the counter is +/// only a gate and does not publish accompanying memory. pub(crate) fn emit_persistent_shadow_root_barrier(ctx: &mut FnCtx<'_>, value_bits: &str) { let active = ctx.block() - .load_atomic_seq_cst(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); + .load_atomic_monotonic(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); let barrier_needed = ctx.block().icmp_ne(I32, &active, "0"); let barrier_idx = ctx.new_block("shadow.root.barrier"); let done_idx = ctx.new_block("shadow.root.barrier.done"); diff --git a/crates/perry-codegen/src/expr/write_barrier.rs b/crates/perry-codegen/src/expr/write_barrier.rs index 4724bb1172..918192a7e4 100644 --- a/crates/perry-codegen/src/expr/write_barrier.rs +++ b/crates/perry-codegen/src/expr/write_barrier.rs @@ -142,11 +142,13 @@ const GC_FLAG_TENURED_I8: &str = "32"; // 0x20 /// NOT a generational question and must never be dropped while a cycle is /// live. A zero count *proves* this thread's /// `INCREMENTAL_MARK_BARRIER_VALID_PTRS` is null, because -/// `incremental_mark_barrier_enable` installs the thread-local BEFORE -/// incrementing the count (`gc/barrier.rs:676–693`, where that ordering is -/// documented as load-bearing for exactly this reason). This is the same -/// gate, on the same global, that `expr/shadow_inline.rs` and -/// `expr/shadow_slot.rs` already emit for the root shading barrier. +/// `incremental_mark_barrier_enable` increments the count BEFORE installing +/// the thread-local, while disable clears the thread-local BEFORE +/// decrementing the count. This is the same gate, on the same global, that +/// `expr/shadow_inline.rs` and `expr/shadow_slot.rs` already emit for the +/// root shading barrier. It is an LLVM `monotonic` load (Rust `Relaxed`): +/// the counter is authoritative state, not a publication fence for other +/// memory. /// /// ## Why a live test and not a static claim /// @@ -169,7 +171,7 @@ pub(crate) fn emit_parent_may_need_remembering_check( let gc_flags = blk.load(I8, &gc_flags_ptr); let tenured_bits = blk.and(I8, &gc_flags, GC_FLAG_TENURED_I8); let is_tenured = blk.icmp_ne(I8, &tenured_bits, "0"); - let active = blk.load_atomic_seq_cst(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); + let active = blk.load_atomic_monotonic(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); let incremental_active = blk.icmp_ne(I32, &active, "0"); blk.or(I1, &is_tenured, &incremental_active) } diff --git a/crates/perry-codegen/tests/shadow_slot_hygiene.rs b/crates/perry-codegen/tests/shadow_slot_hygiene.rs index 47b0f430a9..210e7615c2 100644 --- a/crates/perry-codegen/tests/shadow_slot_hygiene.rs +++ b/crates/perry-codegen/tests/shadow_slot_hygiene.rs @@ -885,9 +885,10 @@ fn immutable_index_alias_binds_once_but_keeps_incremental_root_barrier() { ); assert!( main_ir.contains( - "load atomic i32, ptr @PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT seq_cst, align 4" + "load atomic i32, ptr @PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT monotonic, align 4" ) && main_ir.contains("shadow.root.barrier"), - "an inactive incremental collector should skip the TLS-backed root barrier call" + "the relaxed global gate should let an inactive incremental collector skip the \ + TLS-backed root barrier call" ); } diff --git a/crates/perry-runtime/src/gc/barrier/mod.rs b/crates/perry-runtime/src/gc/barrier/mod.rs index 06b54d7c2d..a31291f491 100644 --- a/crates/perry-runtime/src/gc/barrier/mod.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -800,13 +800,23 @@ pub(super) fn incremental_mark_barrier_disable() { /// pointer. Non-zero is conservative and falls through to the thread-local /// read, which then finds its own null and returns. /// +/// `Relaxed` is sufficient here. The counter publishes no accompanying data: +/// it only decides whether to pay for a thread-local read and barrier call. +/// Atomic coherence ensures that a thread which has incremented the counter +/// before installing its own pointer cannot later observe a value preceding +/// that increment; while its pointer remains installed, later counter values +/// also remain non-zero because that thread has not removed its contribution. +/// A zero observed by a thread with a null pointer merely skips a call that +/// would have returned immediately. No acquire/release relationship with +/// `ValidPointerSet` is required because that pointer is thread-local. +/// /// #7469: the point is to skip the *thread-local* read. On Darwin that read is /// an out-of-line `_tlv_get_addr` call on every heap-pointer store, and it was /// 91 of the 653 attributed `_tlv_get_addr` samples on `churn.ts` — all of them /// spent proving a null pointer was still null. This is a relaxed load of a /// static: `adrp` + `ldr` and a perfectly-predicted branch. #[inline(always)] -fn incremental_mark_barrier_globally_idle() -> bool { +pub(crate) fn incremental_mark_barrier_globally_idle() -> bool { PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::Relaxed) == 0 } diff --git a/crates/perry-runtime/src/gc/roots/shadow_stack.rs b/crates/perry-runtime/src/gc/roots/shadow_stack.rs index 3bd5c32995..3f17b47f12 100644 --- a/crates/perry-runtime/src/gc/roots/shadow_stack.rs +++ b/crates/perry-runtime/src/gc/roots/shadow_stack.rs @@ -385,8 +385,8 @@ pub(crate) fn bound_slot_meta(raw: usize) -> usize { /// /// `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` counts the threads whose /// thread-local pointer is currently non-null. `incremental_mark_barrier_enable` -/// installs the thread-local *and then* increments the count, both before -/// returning to the mutator, so on any thread: +/// increments the count *before* installing the thread-local; disable clears +/// the thread-local *before* decrementing the count. Thus, on any thread: /// /// > this thread's `VALID_PTRS` is non-null ⟹ the count is ≥ 1 /// @@ -402,7 +402,7 @@ pub(crate) fn bound_slot_meta(raw: usize) -> usize { /// this makes the runtime entry points agree with it. #[inline(always)] fn root_shading_barrier(value_bits: u64) { - if crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::SeqCst) != 0 { + if !crate::gc::incremental_mark_barrier_globally_idle() { shade_root_slot_value(value_bits); } } diff --git a/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs index 5dc65ea802..e4df96234c 100644 --- a/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs +++ b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs @@ -28,10 +28,12 @@ //! `barrier_child_prologue`'s `incremental_mark_barrier_value` — the //! insertion/SATB shading, which is not a generational question at all. A zero //! count *proves* this thread's `INCREMENTAL_MARK_BARRIER_VALID_PTRS` is null, -//! because `incremental_mark_barrier_enable` installs the thread-local BEFORE -//! incrementing the count; a non-zero count must force the call even for a -//! nursery parent. `the_incremental_clause_forces_the_call_for_a_young_parent` -//! pins that, and fails if the clause is dropped. +//! because `incremental_mark_barrier_enable` increments the count BEFORE +//! installing the thread-local and disable clears the pointer BEFORE +//! decrementing the count; a non-zero count must force the call even for a +//! nursery parent. The emitted LLVM `monotonic` load is this Rust model's +//! `Relaxed` load. `the_incremental_clause_forces_the_call_for_a_young_parent` +//! pins the clause, and fails if it is dropped. use super::super::*; use super::support::*; @@ -125,7 +127,7 @@ unsafe fn gated_slot_store( ) -> bool { *fields = child_bits; let flags = (*header_from_user_ptr(parent as *const u8)).gc_flags; - let active = crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::SeqCst); + let active = crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::Relaxed); if gate(flags, active) { js_write_barrier_slot(ptr_bits(parent as usize), fields as u64, child_bits); return true; diff --git a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs index 60a181dfd6..bd8e543684 100644 --- a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs +++ b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs @@ -301,6 +301,8 @@ fn bind_roots_the_value_present_at_the_call_not_a_later_store() { /// barrier is armed, `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` is /// non-zero. If that ever stopped holding, a zero count would no longer prove /// the barrier call is a no-op and the gate would start dropping shading. +/// This deliberately observes with the same `Relaxed` ordering used by the +/// runtime readers and by codegen's LLVM `monotonic` loads. #[test] fn active_count_is_nonzero_whenever_this_threads_barrier_is_armed() { let _guard = GcTestIsolationGuard::new(); @@ -311,7 +313,7 @@ fn active_count_is_nonzero_whenever_this_threads_barrier_is_armed() { let armed = IncrementalMarkBarrierTestGuard::new(&valid_ptrs); assert!(incremental_mark_barrier_active()); assert_ne!( - PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::SeqCst), + PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::Relaxed), 0, "armed barrier must be visible in the global gate" ); From 536ce35e88792fb65666e802a01efd61951cc07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 10:50:09 +0200 Subject: [PATCH 2/2] chore: add changelog for #7935 --- changelog.d/7935-barrier-gate-ordering.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 changelog.d/7935-barrier-gate-ordering.md diff --git a/changelog.d/7935-barrier-gate-ordering.md b/changelog.d/7935-barrier-gate-ordering.md new file mode 100644 index 0000000000..110ff50e4a --- /dev/null +++ b/changelog.d/7935-barrier-gate-ordering.md @@ -0,0 +1,19 @@ +### Align incremental-barrier gate ordering across generated and runtime code + +Generated write and root barriers read the incremental-mark active counter +with LLVM's sequentially-consistent ordering even though runtime barriers made +the same decision with Rust's relaxed ordering. The generated gates now use +LLVM `monotonic`, and the remaining shadow-stack runtime gate shares the +runtime's relaxed helper. + +The counter publishes no accompanying memory: arming increments it before +installing the current thread's barrier pointer, while disarming clears that +pointer before decrementing it. It is therefore authoritative only for whether +the current thread needs the TLS-backed barrier call, and needs no acquire +relationship with the thread-local pointer. Tests pin the relaxed ordering for +all three generated gate families and exercise the live armed-barrier premise. + +On `interp`, the change replaces 735 `ldar` instructions with matching `ldr` +instructions while preserving exact output and executable size. Two +order-reversed 31-pair sweeps on the quiet M1 mini found no measurable runtime +change (0.6288 s to 0.6284 s median; best cycles -0.14%).