diff --git a/.github/workflows/gc-native-roots.yml b/.github/workflows/gc-native-roots.yml index 89203832f1..bbc352c5de 100644 --- a/.github/workflows/gc-native-roots.yml +++ b/.github/workflows/gc-native-roots.yml @@ -294,13 +294,47 @@ jobs: # every function quietly lowered by the other backend, the arm # measuring the mode it was not testing. `--only-backend rs4gc` # rejects a single such fallback. - PERRY_RS4GC=1 ./target/perry-dev/perry \ - benchmarks/gc_ratchet/probes/09_try_catch_roots.ts \ - -o /tmp/rs4gc-report-probe --statepoint-report=json 2> /tmp/rs4gc-report.json # windows-latest exposes the toolcache python as `python`, not python3. py=python3; command -v python3 >/dev/null 2>&1 || py=python + + # The PORTABLE assertion, on every arm. `11_collect_at_depth` is + # deliberate: it contains no `try`, so it compiles under RS4GC + # everywhere. `09_try_catch_roots` does NOT — RS4GC cannot rewrite + # WinEH funclet pads, so `linker.rs`'s `rs4gc_funclet_refusal` rejects + # it on windows-msvc, and the probe loop above only tolerates that + # because it greps the compile log for "funclet". A report assertion + # pinned to a probe that cannot compile on one arm is a gate that + # fails for a reason unrelated to its subject. + # + # --only-backend proves the lowering ran on every function; the two + # --require-positive checks prove it PRODUCED something. Those counts + # come from the compact-map rewrite parsing the assembly LLVM + # emitted, which is the only honest source now that RS4GC decides + # what becomes a safepoint. Until #7368 the report counted at + # IR-emission time, #7348 deleted those writers with the bridge, and + # every compile printed `0 statepoints emitted` while its binary + # carried hundreds. A label check could not see that; these can. + PERRY_RS4GC=1 ./target/perry-dev/perry \ + benchmarks/gc_ratchet/probes/11_collect_at_depth.ts \ + -o /tmp/rs4gc-report-probe --statepoint-report=json 2> /tmp/rs4gc-report.json "$py" scripts/statepoint_report_assert.py /tmp/rs4gc-report.json \ - --only-backend rs4gc + --only-backend rs4gc \ + --require-positive records \ + --require-positive roots + + # The try-specific arm, everywhere RS4GC can compile a `try`. This is + # the coverage the probe above cannot give: 128 of 479 gap tests + # contain `try {}`, and RS4GC being the only backend that handles them + # is the reason the bridge could be deleted (#7339, #7348). + if [ "$RUNNER_OS" != "Windows" ]; then + PERRY_RS4GC=1 ./target/perry-dev/perry \ + benchmarks/gc_ratchet/probes/09_try_catch_roots.ts \ + -o /tmp/rs4gc-try-probe --statepoint-report=json 2> /tmp/rs4gc-try.json + "$py" scripts/statepoint_report_assert.py /tmp/rs4gc-try.json \ + --only-backend rs4gc \ + --require-positive records \ + --require-positive roots + fi # Walker liveness, on EVERY arm. A walker that visits zero frames # still lets most probes print the right answer, because other root diff --git a/changelog.d/7368-statepoint-report-count-from-map.md b/changelog.d/7368-statepoint-report-count-from-map.md new file mode 100644 index 0000000000..0a1c717ce8 --- /dev/null +++ b/changelog.d/7368-statepoint-report-count-from-map.md @@ -0,0 +1,65 @@ +### Fixed + +**`--statepoint-report` reported `0 statepoints emitted` for every compile since #7348.** + +#7348 deleted the explicit statepoint bridge, and with it the only callers of +`FunctionRecord::note_statepoint` and `note_skipped` — they lived in the bridge, +which counted safepoints as it emitted them. The methods survived with no +callers, so `statepoints`, `relocations`, `max_live_roots`, +`skipped_non_safepoints`, `live_roots_histogram` and both by-callee maps became +structurally zero in production. Measured on a real compile: + +``` +5 function(s), 5 bound native root slots (5 logical slots reserved) +5 textual calls: 5 with live roots, 0 without +0 statepoints emitted; 0 non-collecting calls skipped <-- binary had 120 +0 relocations; maximum 0 live roots at one safepoint +``` + +Counting at IR-emission time cannot work any more, and that is the real lesson: +**Perry no longer decides which calls become safepoints — `RewriteStatepointsForGC` +does, inside LLVM.** The only honest source is the compact-map rewrite, which +already parses the assembly LLVM actually emitted and computed exactly these +numbers before throwing them at `log::debug!`. The report now reads from there: + +``` +120 safepoints across 6 function(s) in 1 module(s) +36 live roots recorded, 0.30 per safepoint +``` + +An absent measurement no longer renders as a measured zero. `gc_map.modules == 0` +means "the rewrite never reported", the text report says +`Safepoint counts UNAVAILABLE` instead of printing zeros, and the JSON carries +`gc_map` separately from `totals` so a consumer can tell the two apart. +`schema_version` is now `2`. + +**The CI gate now asserts the counts, not just the label.** `gc-native-roots` +checked `--only-backend rs4gc`, which passed throughout the regression — the +backend label was correct, the numbers were fiction. It now also requires +`records > 0` and `roots > 0`. Verified against a synthetic report with the +#7348 shape: the label check reports 9 functions green while the count checks +exit 1. + +Note this is the *second* round of dead counters in this file (#7362 removed four +that never had a writer at all). The new test documents why the first invariant +missed this one: `every_rendered_counter_has_a_writer` called the mutators +itself, so "has a writer" passed while "is written" was false. The structural fix +is that the numbers now have exactly one producer and their absence is loud. + +Three review fixes on top of the above: + +- The report assertion ran on `09_try_catch_roots`, which contains four `try` + blocks — and RS4GC cannot rewrite WinEH funclet pads, so + `rs4gc_funclet_refusal` rejects that probe on `windows-msvc`. The probe loop + above tolerates it by grepping the compile log for "funclet"; this step did + not. The portable assertion now uses `11_collect_at_depth` (no `try`, compiles + on all four arms) and `09_try_catch_roots` keeps its own non-Windows step, so + the try-specific coverage is not lost. +- The `gc_map` doc claimed `records`/`roots` would be *absent* when unmeasured. + They are plain `u64` fields on a plain derive and always serialise; `modules` + is the sentinel. Corrected to describe what the code does. +- The "map never reported" guard in `statepoint_report_assert.py` fired for any + `--require-*`/`--print`, including fields that live in `totals` and are + counted at IR-emission time regardless of the rewrite. It is now scoped to + map-backed fields, so `--require-positive textual_calls` is answered from its + measured value instead of being failed by an unreported map it does not use. diff --git a/changelog.d/7369-gc-leaf-register-global-root.md b/changelog.d/7369-gc-leaf-register-global-root.md new file mode 100644 index 0000000000..fa3996a5ce --- /dev/null +++ b/changelog.d/7369-gc-leaf-register-global-root.md @@ -0,0 +1,48 @@ +### Changed + +**Two runtime helpers admitted to the GC-effect allowlist — and a measured null result on binary size.** + +`js_gc_register_global_root` was the single most frequent non-leaf callee in the +probe suite (148 call sites), and it is provably GC-leaf. Its entire body is: + +```rust +runtime_write_barrier_root_heap_word(*root); // shade one header +GLOBAL_ROOTS.with(|r| r.borrow_mut().push(root)); // TLS Vec push +``` + +The first call is exactly what `js_write_barrier_root_heap_word` — already +`CannotCollect` — wraps in one line. The second is a `Vec::push`, and the +"malloc count threshold" GC trigger does not apply to it: that counter is +`MALLOC_STATE.objects.len()`, a registry of Perry GC objects, and the +`#[global_allocator]` is plain mimalloc/System with no GC hook. +`js_typed_feedback_maybe_dump_trace` joins its already-admitted family siblings +(env read, JSON serialise, file write; empty body without `diagnostics`). + +**The result, measured A/B on the same tree, is that this buys nothing:** + +| probe | safepoints | roots | total bytes | `__text` | +|---|---:|---:|---:|---:| +| `06_string_retention` | 105 → 100 | 27 → 27 | 0 | −4 B | +| `09_try_catch_roots` | 343 → 339 | 259 → 259 | 0 | −4 B | +| `11_collect_at_depth` | 120 → 117 | 36 → 36 | 0 | −4 B | + +Root counts are **identical**. The 40 safepoints removed across the suite were +all rootless, and a safepoint with no live roots costs essentially nothing — +which is precisely what `docs/engine-plan.md` already says ("Statepoints have no +fixed cost… the axis is not 'statepoints are bigger', it is 'roots are bigger'"). + +This is worth recording as evidence rather than a win: **the safepoint-count +lever is not the binary-size lever.** Sequencing step 2's "reduce root density" +must attack live-root *sets*, not safepoint counts. Anyone reaching for the next +obvious helper should read the second test below first. + +Two tests come with it. `register_global_root_tracks_the_barrier_it_wraps` pins +the two classifications together so a future demotion of the barrier cannot +leave its wrapper claiming to be leaf. `allocating_helpers_are_not_cannot_collect` +pins `js_nanbox_string` **out** of the allowlist: at 120 call sites it is the +obvious next candidate and it reads as pure bit manipulation, but its +null-pointer guard calls `js_string_from_bytes` to allocate an empty string +rather than boxing null. + +Probe suite: 11/11 byte-identical to the Node oracle under `PERRY_RS4GC=1 +PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1`. diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index ceac76e0e1..a25ca75abb 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -13,8 +13,9 @@ use crate::native_value::{ NativeRep, SemanticKind, }; use crate::type_analysis::{is_array_expr, is_numeric_expr, is_string_expr, receiver_class_name}; -use crate::types::{DOUBLE, F32, I1, I16, I32, I64, I8}; +use crate::types::{DOUBLE, I32, I64}; +use super::index_set_typed_array::lower_inline_dyn_typed_array_set; use super::{ array_kind_fact, array_store_needs_layout_note, array_store_needs_write_barrier, buffer_access_materialization_reason, emit_array_numeric_write_note_on_block, @@ -1746,290 +1747,3 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { _ => unreachable!("expr/mod.rs dispatched a variant not handled by this submodule"), } } - -/// #5525 follow-up: guarded **inline** typed-array element STORE for an -/// `obj[i] = v` whose receiver static type is erased (`any`/unknown) but is, at -/// runtime, commonly an owning numeric typed array (bcryptjs's `P[i]=`/`S[i]=` -/// Int32Array boxes). Mirrors [`index_get::lower_inline_dyn_typed_array_get`]: -/// the same pointer / `PERRY_TA_VIEW_GUARD` / `PERRY_TA_KIND_CACHE` / index -/// guards, then a direct per-kind store into `header + 16 + idx*elem_size`, -/// falling back to `js_dyn_index_set` on any guard miss. The store result is the -/// assigned value (`val_double`), matching `js_dyn_index_set`'s return. -/// -/// Only the kinds with a simple ToInt32/ToUint32 truncating store (Int8/Uint8/ -/// Int16/Uint16/Int32/Uint32) or a direct float store (Float32/Float64) are -/// inlined — i.e. `kind <= KIND_FLOAT64` (7). Uint8ClampedArray (round-half-to- -/// even clamp), the BigInt kinds (ToBigInt / throw) and Float16 (f16 encode) are -/// excluded by the guard and defer to the runtime, which already owns them. The -/// integer truncation here (`toint32(value)` then narrow) is bit-identical to -/// the runtime `store_at`'s `to_uint32_bits(value) as `; the float store -/// is identical to `store_at`'s direct slot write — so behavior matches the -/// existing runtime fast path exactly. -fn lower_inline_dyn_typed_array_set( - ctx: &mut FnCtx<'_>, - obj_box: &str, - idx_d: &str, - val_double: &str, -) -> String { - let tag_mask = crate::nanbox::i64_literal(crate::nanbox::TAG_MASK); - let pointer_tag = crate::nanbox::POINTER_TAG_I64; - let pointer_mask = crate::nanbox::POINTER_MASK_I64; - - let fast_idx = ctx.new_block("tav.set.fast"); - let store_idx = ctx.new_block("tav.set.store"); - let slow_idx = ctx.new_block("tav.set.slow"); - let merge_idx = ctx.new_block("tav.set.merge"); - let fast_label = ctx.block_label(fast_idx); - let store_label = ctx.block_label(store_idx); - let slow_label = ctx.block_label(slow_idx); - let merge_label = ctx.block_label(merge_idx); - - // ---- entry: combined cache/kind/range guard -> fast | slow ---- - let entry_guard = { - let blk = ctx.block(); - let obj_bits = blk.bitcast_double_to_i64(obj_box); - let raw = blk.and(I64, &obj_bits, pointer_mask); - let tagged = blk.and(I64, &obj_bits, &tag_mask); - let is_ptr = blk.icmp_eq(I64, &tagged, pointer_tag); - let vg = blk.load(I64, "@PERRY_TA_VIEW_GUARD"); - let vg_zero = blk.icmp_eq(I64, &vg, "0"); - let slot = blk.lshr(I64, &raw, "3"); - let slot = blk.and(I64, &slot, "63"); - let entry_ptr = blk.gep( - "[64 x i64]", - "@PERRY_TA_KIND_CACHE", - &[(I64, "0"), (I64, &slot)], - ); - let entry_val = blk.load(I64, &entry_ptr); - let entry_addr = blk.lshr(I64, &entry_val, "8"); - let addr_match = blk.icmp_eq(I64, &entry_addr, &raw); - let kind = blk.and(I64, &entry_val, "255"); - // Stores inline only kinds with a trivial truncating/float store: - // kind <= KIND_FLOAT64 (7). Uint8Clamped (8), BigInt (9/10), Float16 - // (11), and the 0xFF sentinel all defer to the runtime. - let kind_ok = blk.icmp_ule(I64, &kind, "7"); - let idx_ge0 = blk.fcmp("oge", idx_d, "0.0"); - let idx_lt = blk.fcmp("olt", idx_d, "4294967296.0"); - let g = blk.and(I1, &is_ptr, &vg_zero); - let g = blk.and(I1, &g, &addr_match); - let g = blk.and(I1, &g, &kind_ok); - let g = blk.and(I1, &g, &idx_ge0); - blk.and(I1, &g, &idx_lt) - }; - ctx.block().cond_br(&entry_guard, &fast_label, &slow_label); - - // ---- fast: validate integer index + bounds -> store | slow ---- - ctx.current_block = fast_idx; - let (raw, idx_i64, kind) = { - let blk = ctx.block(); - let obj_bits = blk.bitcast_double_to_i64(obj_box); - let raw = blk.and(I64, &obj_bits, pointer_mask); - let slot = blk.lshr(I64, &raw, "3"); - let slot = blk.and(I64, &slot, "63"); - let entry_ptr = blk.gep( - "[64 x i64]", - "@PERRY_TA_KIND_CACHE", - &[(I64, "0"), (I64, &slot)], - ); - let entry_val = blk.load(I64, &entry_ptr); - let kind = blk.and(I64, &entry_val, "255"); - let idx_i64 = blk.fptosi(DOUBLE, idx_d, I64); - (raw, idx_i64, kind) - }; - let fast_ok = { - let blk = ctx.block(); - let idx_back = blk.sitofp(I64, &idx_i64, DOUBLE); - let is_int = blk.fcmp("oeq", &idx_back, idx_d); - let hdr_ptr = blk.inttoptr(I64, &raw); - let len = blk.load(I32, &hdr_ptr); - let len_i64 = blk.zext(I32, &len, I64); - let in_bounds = blk.icmp_ult(I64, &idx_i64, &len_i64); - blk.and(I1, &is_int, &in_bounds) - }; - ctx.block().cond_br(&fast_ok, &store_label, &slow_label); - - // ---- store: per-kind direct element store (data = header + 16) ---- - ctx.current_block = store_idx; - let data_base = { - let blk = ctx.block(); - blk.add(I64, &raw, "16") - }; - // ToInt32 of the value once (shared by all integer kinds). For float kinds - // we use the raw double directly. `toint32` matches the runtime - // `to_uint32_bits` (NaN/±Inf/±0 → 0, else trunc-toward-zero mod 2^32). - let val_i32 = ctx.block().toint32(val_double); - - let b_i8 = ctx.new_block("tav.s.i8"); - let b_u8 = ctx.new_block("tav.s.u8"); - let b_i16 = ctx.new_block("tav.s.i16"); - let b_u16 = ctx.new_block("tav.s.u16"); - let b_i32 = ctx.new_block("tav.s.i32"); - let b_u32 = ctx.new_block("tav.s.u32"); - let b_f32 = ctx.new_block("tav.s.f32"); - let b_f64 = ctx.new_block("tav.s.f64"); - let l_i8 = ctx.block_label(b_i8); - let l_u8 = ctx.block_label(b_u8); - let l_i16 = ctx.block_label(b_i16); - let l_u16 = ctx.block_label(b_u16); - let l_i32 = ctx.block_label(b_i32); - let l_u32 = ctx.block_label(b_u32); - let l_f32 = ctx.block_label(b_f32); - let l_f64 = ctx.block_label(b_f64); - - // Dispatch chain on `kind` (in the store block, after data_base/val_i32). - let chk = |ctx: &mut FnCtx<'_>, k: &str, hit: &str, next_idx: usize| { - let next_label = ctx.block_label(next_idx); - let cond = ctx.block().icmp_eq(I64, &kind, k); - ctx.block().cond_br(&cond, hit, &next_label); - }; - let c1 = ctx.new_block("tav.sd1"); - let c2 = ctx.new_block("tav.sd2"); - let c3 = ctx.new_block("tav.sd3"); - let c4 = ctx.new_block("tav.sd4"); - let c5 = ctx.new_block("tav.sd5"); - let c6 = ctx.new_block("tav.sd6"); - chk(ctx, "0", &l_i8, c1); - ctx.current_block = c1; - chk(ctx, "1", &l_u8, c2); - ctx.current_block = c2; - chk(ctx, "2", &l_i16, c3); - ctx.current_block = c3; - chk(ctx, "3", &l_u16, c4); - ctx.current_block = c4; - chk(ctx, "4", &l_i32, c5); - ctx.current_block = c5; - chk(ctx, "5", &l_u32, c6); - ctx.current_block = c6; - // remaining: kind 6 → f32, else (7) → f64. - let is_f32 = ctx.block().icmp_eq(I64, &kind, "6"); - ctx.block().cond_br(&is_f32, &l_f32, &l_f64); - - // Per-kind stores. Each: off = idx << shift; addr = data_base + off; - // store narrowed value; br merge. - emit_inline_ta_int_store( - ctx, - b_i8, - &idx_i64, - &data_base, - &merge_label, - "0", - &val_i32, - I8, - ); - emit_inline_ta_int_store( - ctx, - b_u8, - &idx_i64, - &data_base, - &merge_label, - "0", - &val_i32, - I8, - ); - emit_inline_ta_int_store( - ctx, - b_i16, - &idx_i64, - &data_base, - &merge_label, - "1", - &val_i32, - I16, - ); - emit_inline_ta_int_store( - ctx, - b_u16, - &idx_i64, - &data_base, - &merge_label, - "1", - &val_i32, - I16, - ); - emit_inline_ta_int_store( - ctx, - b_i32, - &idx_i64, - &data_base, - &merge_label, - "2", - &val_i32, - I32, - ); - emit_inline_ta_int_store( - ctx, - b_u32, - &idx_i64, - &data_base, - &merge_label, - "2", - &val_i32, - I32, - ); - // F32: fptrunc the double to float, store. - { - ctx.current_block = b_f32; - let blk = ctx.block(); - let off = blk.shl(I64, &idx_i64, "2"); - let addr = blk.add(I64, &data_base, &off); - let ptr = blk.inttoptr(I64, &addr); - let f = blk.fptrunc(DOUBLE, val_double, F32); - blk.store(F32, &f, &ptr); - blk.br(&merge_label); - } - // F64: store the double raw. - { - ctx.current_block = b_f64; - let blk = ctx.block(); - let off = blk.shl(I64, &idx_i64, "3"); - let addr = blk.add(I64, &data_base, &off); - let ptr = blk.inttoptr(I64, &addr); - blk.store(DOUBLE, val_double, &ptr); - blk.br(&merge_label); - } - - // ---- slow: the unchanged runtime setter ---- - ctx.current_block = slow_idx; - ctx.block().call( - DOUBLE, - "js_dyn_index_set", - &[(DOUBLE, obj_box), (DOUBLE, idx_d), (DOUBLE, val_double)], - ); - ctx.block().br(&merge_label); - - // ---- merge: assignment yields the stored value on every path ---- - ctx.current_block = merge_idx; - // All paths produce `val_double` as the expression result (matching - // `js_dyn_index_set`'s `return value`), so no phi is needed. - val_double.to_string() -} - -/// Emit one per-kind integer typed-array element store block for -/// [`lower_inline_dyn_typed_array_set`]: switches to `blk_idx`, computes the -/// element address (`data_base + (idx << shift)`), narrows the shared -/// ToInt32-coerced `val_i32` to `elem_ty`, stores it, and branches to -/// `merge_label`. -#[allow(clippy::too_many_arguments)] -fn emit_inline_ta_int_store( - ctx: &mut FnCtx<'_>, - blk_idx: usize, - idx_i64: &str, - data_base: &str, - merge_label: &str, - shift: &str, - val_i32: &str, - elem_ty: crate::types::LlvmType, -) { - ctx.current_block = blk_idx; - let blk = ctx.block(); - let off = blk.shl(I64, idx_i64, shift); - let addr = blk.add(I64, data_base, &off); - let ptr = blk.inttoptr(I64, &addr); - let narrowed = if elem_ty == I32 { - val_i32.to_string() - } else { - blk.trunc(I32, val_i32, elem_ty) - }; - blk.store(elem_ty, &narrowed, &ptr); - blk.br(merge_label); -} diff --git a/crates/perry-codegen/src/expr/index_set_typed_array.rs b/crates/perry-codegen/src/expr/index_set_typed_array.rs new file mode 100644 index 0000000000..fd622e9037 --- /dev/null +++ b/crates/perry-codegen/src/expr/index_set_typed_array.rs @@ -0,0 +1,297 @@ +//! Guarded **inline** typed-array element store for a type-erased receiver. +//! +//! Split out of `index_set.rs` when that file crossed the 2000-line cap +//! (#7342 pushed it to 2035). Pure mechanical move — the two functions below +//! are verbatim, and `index_set.rs` calls them through a `use super::` path +//! exactly as it called them locally before. + +use crate::types::{DOUBLE, F32, I1, I16, I32, I64, I8}; + +use super::FnCtx; + +/// #5525 follow-up: guarded **inline** typed-array element STORE for an +/// `obj[i] = v` whose receiver static type is erased (`any`/unknown) but is, at +/// runtime, commonly an owning numeric typed array (bcryptjs's `P[i]=`/`S[i]=` +/// Int32Array boxes). Mirrors [`index_get::lower_inline_dyn_typed_array_get`]: +/// the same pointer / `PERRY_TA_VIEW_GUARD` / `PERRY_TA_KIND_CACHE` / index +/// guards, then a direct per-kind store into `header + 16 + idx*elem_size`, +/// falling back to `js_dyn_index_set` on any guard miss. The store result is the +/// assigned value (`val_double`), matching `js_dyn_index_set`'s return. +/// +/// Only the kinds with a simple ToInt32/ToUint32 truncating store (Int8/Uint8/ +/// Int16/Uint16/Int32/Uint32) or a direct float store (Float32/Float64) are +/// inlined — i.e. `kind <= KIND_FLOAT64` (7). Uint8ClampedArray (round-half-to- +/// even clamp), the BigInt kinds (ToBigInt / throw) and Float16 (f16 encode) are +/// excluded by the guard and defer to the runtime, which already owns them. The +/// integer truncation here (`toint32(value)` then narrow) is bit-identical to +/// the runtime `store_at`'s `to_uint32_bits(value) as `; the float store +/// is identical to `store_at`'s direct slot write — so behavior matches the +/// existing runtime fast path exactly. +pub(super) fn lower_inline_dyn_typed_array_set( + ctx: &mut FnCtx<'_>, + obj_box: &str, + idx_d: &str, + val_double: &str, +) -> String { + let tag_mask = crate::nanbox::i64_literal(crate::nanbox::TAG_MASK); + let pointer_tag = crate::nanbox::POINTER_TAG_I64; + let pointer_mask = crate::nanbox::POINTER_MASK_I64; + + let fast_idx = ctx.new_block("tav.set.fast"); + let store_idx = ctx.new_block("tav.set.store"); + let slow_idx = ctx.new_block("tav.set.slow"); + let merge_idx = ctx.new_block("tav.set.merge"); + let fast_label = ctx.block_label(fast_idx); + let store_label = ctx.block_label(store_idx); + let slow_label = ctx.block_label(slow_idx); + let merge_label = ctx.block_label(merge_idx); + + // ---- entry: combined cache/kind/range guard -> fast | slow ---- + let entry_guard = { + let blk = ctx.block(); + let obj_bits = blk.bitcast_double_to_i64(obj_box); + let raw = blk.and(I64, &obj_bits, pointer_mask); + let tagged = blk.and(I64, &obj_bits, &tag_mask); + let is_ptr = blk.icmp_eq(I64, &tagged, pointer_tag); + let vg = blk.load(I64, "@PERRY_TA_VIEW_GUARD"); + let vg_zero = blk.icmp_eq(I64, &vg, "0"); + let slot = blk.lshr(I64, &raw, "3"); + let slot = blk.and(I64, &slot, "63"); + let entry_ptr = blk.gep( + "[64 x i64]", + "@PERRY_TA_KIND_CACHE", + &[(I64, "0"), (I64, &slot)], + ); + let entry_val = blk.load(I64, &entry_ptr); + let entry_addr = blk.lshr(I64, &entry_val, "8"); + let addr_match = blk.icmp_eq(I64, &entry_addr, &raw); + let kind = blk.and(I64, &entry_val, "255"); + // Stores inline only kinds with a trivial truncating/float store: + // kind <= KIND_FLOAT64 (7). Uint8Clamped (8), BigInt (9/10), Float16 + // (11), and the 0xFF sentinel all defer to the runtime. + let kind_ok = blk.icmp_ule(I64, &kind, "7"); + let idx_ge0 = blk.fcmp("oge", idx_d, "0.0"); + let idx_lt = blk.fcmp("olt", idx_d, "4294967296.0"); + let g = blk.and(I1, &is_ptr, &vg_zero); + let g = blk.and(I1, &g, &addr_match); + let g = blk.and(I1, &g, &kind_ok); + let g = blk.and(I1, &g, &idx_ge0); + blk.and(I1, &g, &idx_lt) + }; + ctx.block().cond_br(&entry_guard, &fast_label, &slow_label); + + // ---- fast: validate integer index + bounds -> store | slow ---- + ctx.current_block = fast_idx; + let (raw, idx_i64, kind) = { + let blk = ctx.block(); + let obj_bits = blk.bitcast_double_to_i64(obj_box); + let raw = blk.and(I64, &obj_bits, pointer_mask); + let slot = blk.lshr(I64, &raw, "3"); + let slot = blk.and(I64, &slot, "63"); + let entry_ptr = blk.gep( + "[64 x i64]", + "@PERRY_TA_KIND_CACHE", + &[(I64, "0"), (I64, &slot)], + ); + let entry_val = blk.load(I64, &entry_ptr); + let kind = blk.and(I64, &entry_val, "255"); + let idx_i64 = blk.fptosi(DOUBLE, idx_d, I64); + (raw, idx_i64, kind) + }; + let fast_ok = { + let blk = ctx.block(); + let idx_back = blk.sitofp(I64, &idx_i64, DOUBLE); + let is_int = blk.fcmp("oeq", &idx_back, idx_d); + let hdr_ptr = blk.inttoptr(I64, &raw); + let len = blk.load(I32, &hdr_ptr); + let len_i64 = blk.zext(I32, &len, I64); + let in_bounds = blk.icmp_ult(I64, &idx_i64, &len_i64); + blk.and(I1, &is_int, &in_bounds) + }; + ctx.block().cond_br(&fast_ok, &store_label, &slow_label); + + // ---- store: per-kind direct element store (data = header + 16) ---- + ctx.current_block = store_idx; + let data_base = { + let blk = ctx.block(); + blk.add(I64, &raw, "16") + }; + // ToInt32 of the value once (shared by all integer kinds). For float kinds + // we use the raw double directly. `toint32` matches the runtime + // `to_uint32_bits` (NaN/±Inf/±0 → 0, else trunc-toward-zero mod 2^32). + let val_i32 = ctx.block().toint32(val_double); + + let b_i8 = ctx.new_block("tav.s.i8"); + let b_u8 = ctx.new_block("tav.s.u8"); + let b_i16 = ctx.new_block("tav.s.i16"); + let b_u16 = ctx.new_block("tav.s.u16"); + let b_i32 = ctx.new_block("tav.s.i32"); + let b_u32 = ctx.new_block("tav.s.u32"); + let b_f32 = ctx.new_block("tav.s.f32"); + let b_f64 = ctx.new_block("tav.s.f64"); + let l_i8 = ctx.block_label(b_i8); + let l_u8 = ctx.block_label(b_u8); + let l_i16 = ctx.block_label(b_i16); + let l_u16 = ctx.block_label(b_u16); + let l_i32 = ctx.block_label(b_i32); + let l_u32 = ctx.block_label(b_u32); + let l_f32 = ctx.block_label(b_f32); + let l_f64 = ctx.block_label(b_f64); + + // Dispatch chain on `kind` (in the store block, after data_base/val_i32). + let chk = |ctx: &mut FnCtx<'_>, k: &str, hit: &str, next_idx: usize| { + let next_label = ctx.block_label(next_idx); + let cond = ctx.block().icmp_eq(I64, &kind, k); + ctx.block().cond_br(&cond, hit, &next_label); + }; + let c1 = ctx.new_block("tav.sd1"); + let c2 = ctx.new_block("tav.sd2"); + let c3 = ctx.new_block("tav.sd3"); + let c4 = ctx.new_block("tav.sd4"); + let c5 = ctx.new_block("tav.sd5"); + let c6 = ctx.new_block("tav.sd6"); + chk(ctx, "0", &l_i8, c1); + ctx.current_block = c1; + chk(ctx, "1", &l_u8, c2); + ctx.current_block = c2; + chk(ctx, "2", &l_i16, c3); + ctx.current_block = c3; + chk(ctx, "3", &l_u16, c4); + ctx.current_block = c4; + chk(ctx, "4", &l_i32, c5); + ctx.current_block = c5; + chk(ctx, "5", &l_u32, c6); + ctx.current_block = c6; + // remaining: kind 6 → f32, else (7) → f64. + let is_f32 = ctx.block().icmp_eq(I64, &kind, "6"); + ctx.block().cond_br(&is_f32, &l_f32, &l_f64); + + // Per-kind stores. Each: off = idx << shift; addr = data_base + off; + // store narrowed value; br merge. + emit_inline_ta_int_store( + ctx, + b_i8, + &idx_i64, + &data_base, + &merge_label, + "0", + &val_i32, + I8, + ); + emit_inline_ta_int_store( + ctx, + b_u8, + &idx_i64, + &data_base, + &merge_label, + "0", + &val_i32, + I8, + ); + emit_inline_ta_int_store( + ctx, + b_i16, + &idx_i64, + &data_base, + &merge_label, + "1", + &val_i32, + I16, + ); + emit_inline_ta_int_store( + ctx, + b_u16, + &idx_i64, + &data_base, + &merge_label, + "1", + &val_i32, + I16, + ); + emit_inline_ta_int_store( + ctx, + b_i32, + &idx_i64, + &data_base, + &merge_label, + "2", + &val_i32, + I32, + ); + emit_inline_ta_int_store( + ctx, + b_u32, + &idx_i64, + &data_base, + &merge_label, + "2", + &val_i32, + I32, + ); + // F32: fptrunc the double to float, store. + { + ctx.current_block = b_f32; + let blk = ctx.block(); + let off = blk.shl(I64, &idx_i64, "2"); + let addr = blk.add(I64, &data_base, &off); + let ptr = blk.inttoptr(I64, &addr); + let f = blk.fptrunc(DOUBLE, val_double, F32); + blk.store(F32, &f, &ptr); + blk.br(&merge_label); + } + // F64: store the double raw. + { + ctx.current_block = b_f64; + let blk = ctx.block(); + let off = blk.shl(I64, &idx_i64, "3"); + let addr = blk.add(I64, &data_base, &off); + let ptr = blk.inttoptr(I64, &addr); + blk.store(DOUBLE, val_double, &ptr); + blk.br(&merge_label); + } + + // ---- slow: the unchanged runtime setter ---- + ctx.current_block = slow_idx; + ctx.block().call( + DOUBLE, + "js_dyn_index_set", + &[(DOUBLE, obj_box), (DOUBLE, idx_d), (DOUBLE, val_double)], + ); + ctx.block().br(&merge_label); + + // ---- merge: assignment yields the stored value on every path ---- + ctx.current_block = merge_idx; + // All paths produce `val_double` as the expression result (matching + // `js_dyn_index_set`'s `return value`), so no phi is needed. + val_double.to_string() +} + +/// Emit one per-kind integer typed-array element store block for +/// [`lower_inline_dyn_typed_array_set`]: switches to `blk_idx`, computes the +/// element address (`data_base + (idx << shift)`), narrows the shared +/// ToInt32-coerced `val_i32` to `elem_ty`, stores it, and branches to +/// `merge_label`. +#[allow(clippy::too_many_arguments)] +fn emit_inline_ta_int_store( + ctx: &mut FnCtx<'_>, + blk_idx: usize, + idx_i64: &str, + data_base: &str, + merge_label: &str, + shift: &str, + val_i32: &str, + elem_ty: crate::types::LlvmType, +) { + ctx.current_block = blk_idx; + let blk = ctx.block(); + let off = blk.shl(I64, idx_i64, shift); + let addr = blk.add(I64, data_base, &off); + let ptr = blk.inttoptr(I64, &addr); + let narrowed = if elem_ty == I32 { + val_i32.to_string() + } else { + blk.trunc(I32, val_i32, elem_ty) + }; + blk.store(elem_ty, &narrowed, &ptr); + blk.br(merge_label); +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 5be137438d..dc7190c134 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -1739,6 +1739,7 @@ mod ta_param_f64_read; pub(crate) use index_get::packed_f64_loop_index_parts; pub(crate) use masked_window::masked_window_fact_for_index; mod index_set; +mod index_set_typed_array; mod instance_misc1; pub(crate) use instance_misc1::builtin_parent_reserved_class_id; pub(crate) mod class_field_inline_guard; diff --git a/crates/perry-codegen/src/gc_call_effects.rs b/crates/perry-codegen/src/gc_call_effects.rs index adb85f49de..c4474eb66a 100644 --- a/crates/perry-codegen/src/gc_call_effects.rs +++ b/crates/perry-codegen/src/gc_call_effects.rs @@ -56,6 +56,28 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect { | "js_write_barrier_slot" | "js_write_barrier_root_heap_word" | "js_write_barrier_root_nanbox" + // `gc/roots.rs`: registers one module-level global as a root. Audited + // 2026-08-04 and admitted because its whole body is two calls that are + // already covered: + // + // runtime_write_barrier_root_heap_word(*root) <- `js_write_barrier_ + // root_heap_word` immediately above is a ONE-LINE wrapper around + // this exact function, and is already CannotCollect. It shades + // one header and calls `push_mark_seed`, which is a TLS + // `Vec::push` (`gc/trace.rs`) — no trace, no sweep, no trigger. + // GLOBAL_ROOTS.with(|r| r.borrow_mut().push(root)) <- a TLS Vec. + // + // The `Vec::push` is the only thing worth pausing on, because CLAUDE.md + // lists a "malloc count threshold" as a GC trigger. It does not apply: + // that counter is `MALLOC_STATE.objects.len()`, a registry of Perry GC + // objects, and the `#[global_allocator]` is plain mimalloc/System with + // no GC hook. A raw Rust allocation cannot arm a trigger — which is the + // case the module doc above already carves out. + // + // Worth the audit: at 148 call sites across the probe suite this is the + // single most frequent non-leaf callee, all of it module-init code + // registering `@perry_global_*` roots. + | "js_gc_register_global_root" // `gc/layout.rs`: side-table metadata updates only. | "js_gc_note_slot_layout" | "js_gc_note_slot_layout_aware" @@ -71,6 +93,11 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect { | "js_typed_feedback_class_field_set_guard" | "js_typed_feedback_observe_property_get" | "js_typed_feedback_observe_property_set" + // Same family, audited 2026-08-04: under `diagnostics` it reads an env + // var, serialises the counters with serde_json and writes a file; + // without the feature the body is empty. No Perry allocation, no + // re-entry into generated code, no route into collection. + | "js_typed_feedback_maybe_dump_trace" // Refcount writes and array-layout observations; none enters GC. | "js_string_addref" | "js_string_addref_if_heap_string" @@ -130,6 +157,38 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect { mod tests { use super::*; + /// `js_gc_register_global_root` is `js_write_barrier_root_heap_word` plus + /// a TLS `Vec::push`, so the two must never be classified differently — + /// if a future audit demotes the barrier, this catches the sibling that + /// would otherwise keep claiming to be leaf. + #[test] + fn register_global_root_tracks_the_barrier_it_wraps() { + assert_eq!( + classify_direct_callee("js_gc_register_global_root"), + classify_direct_callee("js_write_barrier_root_heap_word"), + "js_gc_register_global_root's entire body is that barrier plus a \ + TLS Vec::push; they cannot have different GC effects" + ); + } + + /// The helpers that *do* allocate must stay out of `CannotCollect`, and + /// this pins the two that read as pure but are not. + /// + /// `js_nanbox_string` looks like bit manipulation and mostly is — but its + /// null-pointer guard calls `js_string_from_bytes` to allocate an empty + /// string rather than boxing null. At 120 call sites it is the obvious + /// thing to reach for next; it is not admissible. + #[test] + fn allocating_helpers_are_not_cannot_collect() { + for name in ["js_nanbox_string", "js_string_from_bytes", "js_array_alloc"] { + assert_ne!( + classify_direct_callee(name), + GcCallEffect::CannotCollect, + "{name} can allocate and must not be marked gc-leaf" + ); + } + } + #[test] fn audited_runtime_bookkeeping_cannot_collect() { for name in [ diff --git a/crates/perry-codegen/src/gc_map.rs b/crates/perry-codegen/src/gc_map.rs index 946c1e9689..72b590f910 100644 --- a/crates/perry-codegen/src/gc_map.rs +++ b/crates/perry-codegen/src/gc_map.rs @@ -960,6 +960,11 @@ pub fn compact_and_assemble( stats.records, stats.roots, ); + // The statepoint report's safepoint counts come from here and nowhere + // else. Perry does not choose which calls become safepoints — RS4GC + // does, inside LLVM — so this parse of the emitted assembly is the + // only place the real numbers exist. + crate::statepoint_report::note_gc_map(stats.functions, stats.records, stats.roots); } assemble(clang, target, asm_path, obj_path) diff --git a/crates/perry-codegen/src/statepoint_report.rs b/crates/perry-codegen/src/statepoint_report.rs index f5fd1c83d1..01c9b4ea77 100644 --- a/crates/perry-codegen/src/statepoint_report.rs +++ b/crates/perry-codegen/src/statepoint_report.rs @@ -24,13 +24,6 @@ pub struct FunctionRecord { textual_calls: u64, calls_without_live_roots: u64, calls_with_live_roots: u64, - skipped_non_safepoints: u64, - statepoints: u64, - relocations: u64, - max_live_roots: usize, - live_roots_histogram: BTreeMap, - statepoints_by_callee: BTreeMap, - skipped_by_callee: BTreeMap, } impl FunctionRecord { @@ -57,28 +50,52 @@ impl FunctionRecord { self.calls_with_live_roots += 1; } } +} - pub(crate) fn note_skipped(&mut self, callee: &str) { - self.skipped_non_safepoints += 1; - *self - .skipped_by_callee - .entry(callee.to_string()) - .or_default() += 1; - } +/// What the compact-map rewrite actually found in the emitted assembly. +/// +/// This is the ONLY honest post-RS4GC source for these numbers. Perry no +/// longer chooses which calls become safepoints — `RewriteStatepointsForGC` +/// does, inside LLVM — so counting at IR-emission time cannot work, and the +/// counters that tried were silently zero (see `note_gc_map`'s callers and +/// the regression note on `render_text`). +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Serialize)] +pub struct GcMapTotals { + /// Functions carrying at least one safepoint with a live root. + pub functions: u64, + /// Safepoints, i.e. stack-map records. + pub records: u64, + /// (safepoint, root) pairs — the relocations LLVM emitted. + pub roots: u64, + /// Modules whose stack map was compacted. Zero here with non-zero + /// function records is the "instrument not wired" case. + pub modules: u64, +} + +static GC_MAP: Mutex = Mutex::new(GcMapTotals { + functions: 0, + records: 0, + roots: 0, + modules: 0, +}); - fn note_emitted_roots(&mut self, live_roots: usize) { - self.max_live_roots = self.max_live_roots.max(live_roots); - *self.live_roots_histogram.entry(live_roots).or_default() += 1; +/// Record one module's compact-map result. Called from `gc_map::compact_and_assemble`. +pub(crate) fn note_gc_map(functions: usize, records: usize, roots: usize) { + if !enabled() { + return; + } + if let Ok(mut totals) = GC_MAP.lock() { + totals.functions += functions as u64; + totals.records += records as u64; + totals.roots += roots as u64; + totals.modules += 1; } +} - pub(crate) fn note_statepoint(&mut self, callee: &str, live_roots: usize) { - self.statepoints += 1; - self.relocations += live_roots as u64; - self.note_emitted_roots(live_roots); - *self - .statepoints_by_callee - .entry(callee.to_string()) - .or_default() += 1; +fn take_gc_map() -> GcMapTotals { + match GC_MAP.lock() { + Ok(mut totals) => std::mem::take(&mut *totals), + Err(_) => GcMapTotals::default(), } } @@ -130,13 +147,6 @@ struct Totals { textual_calls: u64, calls_without_live_roots: u64, calls_with_live_roots: u64, - skipped_non_safepoints: u64, - statepoints: u64, - relocations: u64, - max_live_roots: usize, - live_roots_histogram: BTreeMap, - statepoints_by_callee: BTreeMap, - skipped_by_callee: BTreeMap, } fn totals(records: &[FunctionRecord]) -> Totals { @@ -150,19 +160,6 @@ fn totals(records: &[FunctionRecord]) -> Totals { out.textual_calls += record.textual_calls; out.calls_without_live_roots += record.calls_without_live_roots; out.calls_with_live_roots += record.calls_with_live_roots; - out.skipped_non_safepoints += record.skipped_non_safepoints; - out.statepoints += record.statepoints; - out.relocations += record.relocations; - out.max_live_roots = out.max_live_roots.max(record.max_live_roots); - for (width, count) in &record.live_roots_histogram { - *out.live_roots_histogram.entry(*width).or_default() += count; - } - for (callee, count) in &record.statepoints_by_callee { - *out.statepoints_by_callee.entry(callee.clone()).or_default() += count; - } - for (callee, count) in &record.skipped_by_callee { - *out.skipped_by_callee.entry(callee.clone()).or_default() += count; - } } out } @@ -183,6 +180,10 @@ fn render_ranked_map(out: &mut String, heading: &str, values: &BTreeMap String { + render_text_with(records, take_gc_map()) +} + +fn render_text_with(records: &[FunctionRecord], gc_map: GcMapTotals) -> String { let totals = totals(records); let mut out = String::from( "Perry native-stack GC report (--statepoint-report)\n\ @@ -207,33 +208,37 @@ pub fn render_text(records: &[FunctionRecord]) -> String { "{} textual calls: {} with live roots, {} without", totals.textual_calls, totals.calls_with_live_roots, totals.calls_without_live_roots ); + // Everything above is counted at IR-emission time. Everything below comes + // from the compact-map rewrite, which parses the assembly LLVM actually + // emitted — the only honest source now that RS4GC, not Perry, decides + // which calls become safepoints. + if gc_map.modules == 0 { + out.push_str( + "\nSafepoint counts UNAVAILABLE: the compact-map rewrite never reported.\n\ + The report was rendered anyway rather than printing zeros, because a\n\ + confident `0 statepoints emitted` is indistinguishable from a real\n\ + zero and that is exactly how these counts silently died once before\n\ + (#7348 removed their only writers with the bridge, and nothing\n\ + noticed). Compile with PERRY_RS4GC=1 on a target whose map is\n\ + rewritten, and without a cached .o.\n", + ); + return out; + } + let _ = writeln!( out, - "{} statepoints emitted; {} non-collecting calls skipped", - totals.statepoints, totals.skipped_non_safepoints + "{} safepoints across {} function(s) in {} module(s)", + gc_map.records, gc_map.functions, gc_map.modules ); + let mean = if gc_map.records == 0 { + 0.0 + } else { + gc_map.roots as f64 / gc_map.records as f64 + }; let _ = writeln!( out, - "{} relocations; maximum {} live roots at one safepoint\n", - totals.relocations, totals.max_live_roots - ); - - if !totals.live_roots_histogram.is_empty() { - out.push_str("Live roots per emitted safepoint\n"); - for (width, count) in &totals.live_roots_histogram { - let _ = writeln!(out, " {width:>4} root(s): {count:>6} safepoint(s)"); - } - out.push('\n'); - } - render_ranked_map( - &mut out, - "Most frequent explicit statepoint callees", - &totals.statepoints_by_callee, - ); - render_ranked_map( - &mut out, - "Calls omitted by the GC-effect audit", - &totals.skipped_by_callee, + "{} live roots recorded, {mean:.2} per safepoint\n", + gc_map.roots ); out } @@ -242,13 +247,27 @@ pub fn render_text(records: &[FunctionRecord]) -> String { struct JsonReport<'a> { schema_version: u32, totals: Totals, + /// Safepoint counts from the compact-map rewrite. + /// + /// **`modules` is the sentinel, not field absence.** Every count here is a + /// plain `u64` and serialises unconditionally, so `records: 0` alone cannot + /// tell a consumer whether the program had no safepoints or the rewrite + /// never reported. `modules == 0` means the latter — treat the other counts + /// as unmeasured. That distinction is what went missing in #7348, when the + /// old emission-time counters lost their writers and kept printing zero. + gc_map: GcMapTotals, functions: &'a [FunctionRecord], } pub fn render_json(records: &[FunctionRecord]) -> String { + render_json_with(records, take_gc_map()) +} + +fn render_json_with(records: &[FunctionRecord], gc_map: GcMapTotals) -> String { serde_json::to_string_pretty(&JsonReport { - schema_version: 1, + schema_version: 2, totals: totals(records), + gc_map, functions: records, }) .unwrap_or_else(|error| format!("{{\"error\":\"{error}\"}}")) @@ -258,70 +277,117 @@ pub fn render_json(records: &[FunctionRecord]) -> String { mod tests { use super::*; - #[test] - fn text_and_json_expose_root_pressure_and_fallbacks() { - let mut record = FunctionRecord::new("probe", "statepoint", 3, 2); + fn probe_record() -> FunctionRecord { + let mut record = FunctionRecord::new("probe", "rs4gc", 3, 2); + // Both call shapes: `calls_without_live_roots` only moves for a call + // with an empty live set, so a fixture of all-live calls would accuse + // a perfectly live field of having no writer. record.note_call(2); - record.note_statepoint("@may_collect", 2); - record.note_call(1); - record.note_skipped("@js_gc_temp_root_get"); - record.note_call(1); + record.note_call(0); + record + } - let text = render_text(std::slice::from_ref(&record)); + #[test] + fn text_and_json_expose_root_pressure() { + let record = probe_record(); + let map = GcMapTotals { + functions: 1, + records: 4, + roots: 9, + modules: 1, + }; + + let text = render_text_with(std::slice::from_ref(&record), map); assert!(text.contains("2 bound native root slots")); - assert!(text.contains("1 non-collecting calls skipped")); - assert!(text.contains("@js_gc_temp_root_get")); + assert!(text.contains("4 safepoints across 1 function(s) in 1 module(s)")); + // 9 / 4 — the mean must come out of the real counts, not a placeholder. + assert!( + text.contains("9 live roots recorded, 2.25 per safepoint"), + "{text}" + ); - let json = render_json(&[record]); + let json = render_json_with(&[record], map); let parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); - assert_eq!(parsed["schema_version"], 1); - assert_eq!(parsed["totals"]["relocations"], 2); + assert_eq!(parsed["schema_version"], 2); + assert_eq!(parsed["gc_map"]["records"], 4); + assert_eq!(parsed["gc_map"]["roots"], 9); } /// Every counter this report prints must have a writer. /// - /// It did not. `plain_stack_maps`, `stack_map_operands`, + /// Two rounds of this have now been needed, and the second is the reason + /// the first was not enough. + /// + /// **Round one.** `plain_stack_maps`, `stack_map_operands`, /// `statepoint_fallbacks` and `fallbacks_by_callee` were declared, summed - /// and rendered, and **no mutator ever wrote them** — `git log -S - /// note_fallback` finds nothing, so they were never populated, not even - /// before the plain-map bridge was deleted. The report printed - /// "0 statepoint parser fallback(s)" as reassurance, a test asserted that - /// zero, and the comment above that assert said the structural zero "is - /// the point". A counter that cannot be non-zero is not evidence; it is - /// CLAUDE.md's fourth failure mode with the subject removed entirely. + /// and rendered with no mutator writing them, ever. The report printed + /// "0 statepoint parser fallback(s)" as reassurance and a test asserted + /// that zero, above a comment saying the structural zero "is the point". /// - /// The real fail-closed guarantee is in `gc_map.rs`, which returns `Err` - /// on an unparseable or uncompactable map, so a fallback fails the BUILD - /// rather than incrementing a number nobody reads. + /// **Round two, which this test originally missed.** `statepoints`, + /// `relocations`, `max_live_roots` and the skip counters *did* have + /// mutators — and #7348 deleted their only production callers along with + /// the explicit bridge. The methods survived, so the invariant below still + /// passed: the test called them itself. Meanwhile every real compile + /// printed `0 statepoints emitted` while its binary carried thousands. /// - /// This test pins the invariant that let the dead fields hide: a totals - /// field that is always zero for a record with real activity is either - /// unwritten or misrendered. + /// The lesson is that "has a writer" is weaker than "is written". The + /// structural fix is not a bigger assertion here — it is that the numbers + /// now come from exactly one place (`note_gc_map`, fed by the compact-map + /// rewrite) and that their absence renders as an explicit UNAVAILABLE + /// notice rather than a zero. See `absent_map_counts_say_so_instead_of_zero`. #[test] fn every_rendered_counter_has_a_writer() { - let mut record = FunctionRecord::new("f", "rs4gc", 2, 2); - // Both call shapes: `calls_without_live_roots` only moves for a call - // with an empty live set, so a fixture of all-live calls would accuse - // a perfectly live field of having no writer. - record.note_call(1); - record.note_call(0); - record.note_statepoint("@js_alloc", 1); - record.note_skipped("@js_gc_temp_root_get"); - - let json = render_json(&[record]); + let json = render_json_with( + &[probe_record()], + GcMapTotals { + functions: 1, + records: 1, + roots: 1, + modules: 1, + }, + ); let parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); - let totals = parsed["totals"].as_object().expect("totals is an object"); - - for (name, value) in totals { - // Maps and histograms carry their own emptiness; scalars are the - // ones that silently read as "checked, and fine". - let Some(n) = value.as_u64() else { continue }; - assert_ne!( - n, 0, - "totals.{name} is zero for a record with a call, a statepoint \ - and a skip — it has no writer, or nothing reaches it. Give it \ - one or delete the field; do not print a number that cannot move." - ); + + for section in ["totals", "gc_map"] { + let obj = parsed[section] + .as_object() + .unwrap_or_else(|| panic!("{section} is an object")); + for (name, value) in obj { + let Some(n) = value.as_u64() else { continue }; + assert_ne!( + n, 0, + "{section}.{name} is zero for a record with real activity — \ + it has no writer, or nothing reaches it. Give it one or \ + delete the field; do not print a number that cannot move." + ); + } } } + + /// A missing measurement must not render as a measured zero. + /// + /// This is the guard that would have caught #7348 the day it landed. + /// `--statepoint-report` kept printing `0 statepoints emitted; 0 + /// non-collecting calls skipped` after its writers were deleted, and + /// nothing distinguished that from a program with genuinely no safepoints. + #[test] + fn absent_map_counts_say_so_instead_of_zero() { + let text = render_text_with(&[probe_record()], GcMapTotals::default()); + assert!( + text.contains("Safepoint counts UNAVAILABLE"), + "an unreported map must say so:\n{text}" + ); + assert!( + !text.contains("0 safepoints"), + "a confident zero is the bug this test exists for:\n{text}" + ); + + let json = render_json_with(&[probe_record()], GcMapTotals::default()); + let parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!( + parsed["gc_map"]["modules"], 0, + "a JSON consumer must be able to tell not-measured from zero" + ); + } } diff --git a/docs/engine-plan.md b/docs/engine-plan.md index df08e69092..d4a407f9e3 100644 --- a/docs/engine-plan.md +++ b/docs/engine-plan.md @@ -215,6 +215,65 @@ flipping it globally, and neither is correctness: So the honest state is: *aarch64-viable, globally blocked on two pieces of scope that are both already identified.* +### ★ Update 2026-08-04 — both adoption gates are closed; statepoints run everywhere + +The section above closes with *"aarch64-viable, globally blocked on two pieces +of scope."* Both pieces are now done, so that sentence should not be carried +forward either. + +**1. x86-64 is no longer blocked (#7333 → #7349).** The `_Unwind_GetGR(ctx, 7)` +segfault is real and unfixable as stated — libgcc tracks only the columns CFI +restores, and RSP is derived from the CFA rather than tracked. The fix was to +stop asking for it: #7349 derives the SP-relative base from `_Unwind_GetCFA`, +which does work, with a per-architecture return-address adjustment (x86-64's +`call` pushes a return address, aarch64's `bl` does not — 8 bytes vs 0). +x86-64 Linux is a first-class arm of `gc-native-roots`, not a pinned refusal; +`statepoints-refuse-x86` is deleted along with the job that hosted it. + +**2. Windows works (#7354 → #7355).** `RtlVirtualUnwind` steps a `CONTEXT` +outward and yields `Rip`/`Rsp`/`Rbp` directly, so the CFA derivation above is +not needed there. It is the one walker with no Itanium unwinder beneath it. + +**Platform status, measured rather than assumed:** + +| shape | map | walker | state | +|---|---|---|---| +| aarch64 + Mach-O (macOS/iOS/iPadOS/tvOS) | `__PERRY_GCMAP` | x29 chain, unwinder fallback | ✅ CI arm | +| x86-64 + ELF | `.perry_gcmap` | unwinder + CFA-derived SP | ✅ CI arm | +| x86-64 + PE | `.pgcmap` | `RtlVirtualUnwind` | ✅ CI arm | +| aarch64 + ELF | `.perry_gcmap` | x29 chain | ✅ CI arm (#7360) | +| watchOS / visionOS | ready | ready | compiler-side ✅; see below | +| ARM64 Windows | refused | none | open | + +watchOS and visionOS are **not** blocked by Perry. `cargo check -p perry-runtime` +succeeds on stable for both with any feature set excluding `dyn-eval`; with it, +they fail three crates away in `psm`, whose Mach-O guard enumerates +`darwin/macos/ios/tvos` and omits `watchos`/`visionos`, so both fall to the ELF +branch and emit `.type`/`.size`. Verified by patching that one line: both then +build with full default features. They regressed on 2026-07-18 when `dyn-eval` +joined `default` (#6584) — nothing about the platforms changed. #7364 pins the +whole Apple target set compiler-side. + +**One mechanism, not two.** `PERRY_STATEPOINTS` is deleted and the plain-map +bridge with it; `PERRY_RS4GC` is the only spelling, and the last stale references +went in #7362. The kill-policy line above — *"a mode that still exists is a +decision that hasn't been made"* — no longer applies to this pair, because the +losing mode stopped compiling. + +**What the gate now proves.** Until 2026-08-04 the Unix arms reported +`frames_visited: 7, locations_visited: 0` — they would have passed with a walker +that visited nothing, since other root sources covered the probes. Windows +walked deep only by accident of heap sizing. `11_collect_at_depth` collects at +maximum recursion depth with one live root per frame (macOS 228/221, x86-64 +Linux 231/221, both byte-matching the oracle), so `--require-locations` now +gates every arm (#7359). + +**⇒ The remaining gate on adoption is `llvm-inprocess` becoming a default cargo +feature**, since RS4GC is the only invoke-capable backend. That is #7301's +scope and is in flight. Correctness and platform scope are no longer the +blockers; sequencing step 2 below (root density) is, because adopting today +would regress binary size on root-dense code. + ### ★ Binary size, measured 2026-08-04 — it is a ROOT-DENSITY problem, not a metadata one The note above says *"closing that axis needs **fewer roots**, not a tighter @@ -361,7 +420,8 @@ landed (#7314) and became *reachable* (#7339) and *selectable* (#7340). The spin signature is a stale `GC_TYPE_STRING` at minor #0. 4. **Then the adoption fork.** Flipping statepoints on by default additionally needs `llvm-inprocess` to become a default cargo feature (#7301's scope, since - RS4GC is the only invoke-capable backend) and x86-64 to work (#7333). + RS4GC is the only invoke-capable backend). ~~and x86-64 to work (#7333)~~ — + x86-64 landed in #7349 and Windows in #7355; see the 2026-08-04 update above. 5. **After the collector is trustworthy:** re-derive the RSS numbers (#7056). 6. **Do not** re-measure GC pacing, or update the README's performance table, mid-cycle. diff --git a/scripts/statepoint_report_assert.py b/scripts/statepoint_report_assert.py index ae6e356f46..f1f883e3f9 100755 --- a/scripts/statepoint_report_assert.py +++ b/scripts/statepoint_report_assert.py @@ -10,16 +10,22 @@ unable to fail", #4). Each mode has a signature in this report that the other modes cannot produce: - * explicit statepoint bridge -> every record `"backend": "statepoint"`, - `statepoints > 0`, `plain_stack_maps == 0`, `statepoint_fallbacks == 0` - * RS4GC (`PERRY_RS4GC=1`) -> every record `"backend": "rs4gc"`. RS4GC - bails per function to the explicit bridge on any unrecognised root-alloca - shape, so "did it run" and "did it run everywhere" are different - questions and only `--only-backend` answers the second. - * safepoint-only contract -> `skipped_non_safepoints` strictly up and - `statepoints` strictly down against the same build without it, which is + * RS4GC (`PERRY_RS4GC=1`) -> every record `"backend": "rs4gc"`, and + `gc_map.records > 0`. The explicit bridge is deleted, so there is no + second backend to fall back to; `--only-backend` still answers "did it + run *everywhere*", which is a different question from "did it run". + * safepoint density -> `gc_map.records` and `gc_map.roots`, which is what `--print` is for. +**Fields are looked up in `totals` first, then `gc_map`.** That split is not +cosmetic. `totals` is counted at IR-emission time; `gc_map` comes from the +compact-map rewrite parsing the assembly LLVM actually emitted. Since RS4GC — +not Perry — decides which calls become safepoints, `gc_map` is the only honest +source for safepoint counts. #7348 deleted the emission-time writers along with +the bridge and the report went on printing `0 statepoints emitted` for every +compile; `gc_map.modules == 0` now means "not measured" and is distinguishable +from a real zero. + Usage: statepoint_report_assert.py REPORT [--only-backend NAME] [--require-positive FIELD]... @@ -85,28 +91,57 @@ def main() -> int: else: print(f"backend {args.only_backend}: {counts[args.only_backend]} function(s)") + gc_map = report.get("gc_map", {}) + + def lookup(field: str): + """`totals` first, then `gc_map` — see the module docstring.""" + if field in totals: + return totals[field] + return gc_map.get(field) + + def where(field: str) -> str: + """Which section answered, so the printed line is not misleading.""" + return "totals" if field in totals else "gc_map" + + # A report whose map never reported cannot answer a SAFEPOINT question, and + # must not be allowed to satisfy a --require-zero by absence. + # + # Scoped to fields that actually come from the map. `totals` fields are + # counted at IR-emission time and are measured whether or not the rewrite + # ran, so `--require-positive textual_calls` must not be failed by an + # unreported map it does not depend on. + requested = [*args.require_positive, *args.require_zero] + if args.print_field: + requested.append(args.print_field) + map_backed = [f for f in requested if f not in totals and f in gc_map] + if map_backed and not gc_map.get("modules"): + failures.append( + f"gc_map.modules == 0: the compact-map rewrite never reported, so " + f"{', '.join(sorted(map_backed))} are NOT MEASURED rather than zero" + ) + for field in args.require_positive: - value = totals.get(field) + value = lookup(field) if not isinstance(value, int): - failures.append(f"totals.{field} missing from the report") + failures.append(f"{field} missing from the report (checked totals and gc_map)") elif value <= 0: - failures.append(f"totals.{field} == {value}, expected > 0 (the mode did nothing)") + failures.append(f"{where(field)}.{field} == {value}, expected > 0 (the mode did nothing)") else: - print(f"totals.{field} = {value}") + print(f"{where(field)}.{field} = {value}") for field in args.require_zero: - value = totals.get(field) + value = lookup(field) if not isinstance(value, int): - failures.append(f"totals.{field} missing from the report") + failures.append(f"{field} missing from the report (checked totals and gc_map)") elif value != 0: - failures.append(f"totals.{field} == {value}, expected 0") + failures.append(f"{where(field)}.{field} == {value}, expected 0") else: - print(f"totals.{field} = 0") + print(f"{where(field)}.{field} = 0") if args.print_field: - value = totals.get(args.print_field) + value = lookup(args.print_field) if not isinstance(value, int): - sys.exit(f"::error::totals.{args.print_field} missing from the report") + sys.exit(f"::error::{args.print_field} missing from the report (checked totals and gc_map)") print(value) for message in failures: