diff --git a/majit/majit-backend/src/resume_guard_descr.rs b/majit/majit-backend/src/resume_guard_descr.rs index 5d26875b08a..e945e9cf522 100644 --- a/majit/majit-backend/src/resume_guard_descr.rs +++ b/majit/majit-backend/src/resume_guard_descr.rs @@ -39,7 +39,7 @@ use std::any::Any; use std::cell::UnsafeCell; use std::rc::Rc; -use std::sync::atomic::{AtomicPtr, AtomicU32, AtomicU64, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU32, AtomicU64, AtomicUsize, Ordering}; use std::sync::{Arc, OnceLock}; use majit_ir::{ @@ -167,6 +167,12 @@ pub struct ResumeGuardDescr { /// the meta Arc is the single source of truth. Sorted and deduped /// at write time so `is_force_token_slot` can use `binary_search`. pub force_token_slots: UnsafeCell>, + /// This guard is the eval-breaker word's back-edge poll, not a check on + /// traced values. Stamped once per emission by the optimizer, which is + /// where the guard's condition chain is still in hand; read by the + /// statistics counter to keep scheduled exits out of the guard-failure + /// total. See `descr.rs FailDescr::is_back_edge_poll`. + pub back_edge_poll: AtomicBool, /// `AbstractResumeGuardDescr.handle_fail` (`compile.py:701-717`) /// drives `must_compile` via `jitcounter.tick(status_hash)` in /// RPython. Pyre keeps a raw per-descr counter: @@ -302,6 +308,7 @@ impl Descr for ResumeGuardDescr { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -447,6 +454,12 @@ impl FailDescr for ResumeGuardDescr { // Safety: single-threaded JIT. unsafe { *self.source_op_index.get() = Some(source_op_index) }; } + fn is_back_edge_poll(&self) -> bool { + self.back_edge_poll.load(Ordering::Relaxed) + } + fn set_back_edge_poll(&self) { + self.back_edge_poll.store(true, Ordering::Relaxed); + } fn force_token_slots(&self) -> Vec { // Safety: single-threaded JIT. unsafe { (&*self.force_token_slots.get()).clone() } @@ -578,6 +591,7 @@ pub fn make_resume_guard_descr_typed(types: Vec) -> DescrRef { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), diff --git a/majit/majit-ir/src/descr.rs b/majit/majit-ir/src/descr.rs index d9b9095dd2c..51c463946c5 100644 --- a/majit/majit-ir/src/descr.rs +++ b/majit/majit-ir/src/descr.rs @@ -4055,6 +4055,30 @@ pub trait FailDescr: Descr { ); } + /// Pyre-only per-emission slot: this guard is the eval-breaker word's + /// back-edge poll rather than a check on traced values. + /// + /// Its failures are scheduled exits — the collector armed the word and the + /// loop has to leave machine code for the request to be serviced — so they + /// describe when a collection landed, not what the compiled code assumed. + /// The statistics counter reads this to keep the two apart + /// (`crate::eval_breaker_word::is_back_edge_poll_guard` decides it). + /// Default `false` for non-resume FailDescrs. + fn is_back_edge_poll(&self) -> bool { + false + } + + /// Pyre-only per-emission slot write. See `is_back_edge_poll`. Default + /// panics — only Resume-family guards own the slot. Callers must gate by + /// `is_resume_guard() || is_resume_guard_copied()`. + fn set_back_edge_poll(&self) { + panic!( + "set_back_edge_poll invoked on a FailDescr that does not \ + carry the per-emission back_edge_poll slot (only \ + ResumeGuardDescr / ResumeGuardCopiedDescr own it)" + ); + } + /// `compile.py:683` `AbstractResumeGuardDescr._attrs_ = ('status',)` /// — packs `ST_BUSY_FLAG` + type tag + hash on the resume-guard /// descr. `compile.py:741-745` `self.status` read for diff --git a/majit/majit-ir/src/eval_breaker_word.rs b/majit/majit-ir/src/eval_breaker_word.rs index bde809bf521..fe22d0d178c 100644 --- a/majit/majit-ir/src/eval_breaker_word.rs +++ b/majit/majit-ir/src/eval_breaker_word.rs @@ -136,6 +136,52 @@ pub fn take_gc() -> bool { EVAL_BREAKER_WORD.fetch_and(!EB_GC, Ordering::Relaxed) & EB_GC != 0 } +/// Depth of the operation chain between the poll's load and its guard. +/// +/// The recorder emits `RawLoadI -> IntAnd -> IntIsTrue -> GuardFalse`, so two +/// links separate the guard's condition from the load. The walk below allows +/// a few more so that an optimizer pass inserting or splitting one link does +/// not silently stop matching, and stops well before a long pure chain. +const POLL_CHAIN_DEPTH: usize = 6; + +/// Whether `guard`'s condition is a back-edge poll of this word. +/// +/// The poll is recorded as +/// `GuardFalse(IntIsTrue(IntAnd(RawLoadI(addr, 0), JIT_BREAKER_MASK)))`. The +/// match anchors on the `RawLoadI` of the published address rather than on the +/// whole shape: that load is the one link the recorder cannot drop (it must +/// stay non-pure and outside the always-pure range, or CSE forwards the +/// preamble's guarded-zero value into the loop body), so walking back to it +/// survives rewrites of the links in between. +/// +/// An unpublished address reads `0` and no poll is recorded, so the walk +/// declines rather than matching an unrelated load from a null constant. +pub fn is_back_edge_poll_guard(guard: &crate::resoperation::Op) -> bool { + let addr = eval_breaker_word_addr(); + if addr == 0 { + return false; + } + // Control-flow guards (`GUARD_NOT_FORCED`, `GUARD_NO_EXCEPTION`, ...) carry + // no condition operand at all. + if guard.num_args() == 0 { + return false; + } + let mut operand = guard.arg(0); + for _ in 0..POLL_CHAIN_DEPTH { + let Some(op) = operand.bound_op() else { + return false; + }; + if op.num_args() == 0 { + return false; + } + if op.opcode == crate::resoperation::OpCode::RawLoadI { + return op.arg(0).const_int() == Some(addr as i64); + } + operand = op.arg(0); + } + false +} + /// Every flag must fit in the word the poll actually loads. Checked per target, /// so a flag too wide for a 32-bit `usize` fails the wasm32 build rather than /// silently reading as unarmed there. @@ -151,6 +197,63 @@ const _: () = assert!( #[cfg(test)] mod tests { use super::*; + use crate::resoperation::{Op, OpCode}; + use crate::value::Const; + use std::rc::Rc; + + fn bind(op: Op) -> crate::operand::Operand { + crate::operand::Operand::from_bound_op(&Rc::new(op)) + } + + fn int(value: i64) -> crate::operand::Operand { + crate::operand::Operand::const_(Const::Int(value)) + } + + /// Build the recorded back-edge poll over `load_addr`, which the caller + /// varies to separate "this word" from "some other raw load". + fn poll_guard(load_addr: usize) -> Op { + let load = bind(Op::new(OpCode::RawLoadI, &[int(load_addr as i64), int(0)])); + let masked = bind(Op::new( + OpCode::IntAnd, + &[load, int(JIT_BREAKER_MASK as i64)], + )); + let armed = bind(Op::new(OpCode::IntIsTrue, &[masked])); + Op::new(OpCode::GuardFalse, &[armed]) + } + + /// The counter split turns on this predicate alone, so it has to separate + /// the poll from every other guard the same trace records. Only the + /// address published for this word matches: a raw load of a neighbouring + /// address reaches the same opcode chain, and a data guard reaches none of + /// it. + /// + /// `publish_addr` is idempotent and writes only the address holder, not the + /// word, so this leaves the process-global flag state alone. + #[test] + fn only_a_poll_of_this_words_address_is_recognised() { + publish_addr(); + let addr = eval_breaker_word_addr(); + assert_ne!(addr, 0, "publish_addr must make the address readable"); + + assert!(is_back_edge_poll_guard(&poll_guard(addr))); + // Same shape, a different word: the JIT records other raw loads. + assert!(!is_back_edge_poll_guard(&poll_guard( + addr + EVAL_BREAKER_WORD_SIZE + ))); + // A guard on traced values — the population whose failures the + // `guard_failures` total is meant to describe. + let value = bind(Op::new(OpCode::IntAdd, &[int(1), int(2)])); + let is_true = bind(Op::new(OpCode::IntIsTrue, &[value])); + assert!(!is_back_edge_poll_guard(&Op::new( + OpCode::GuardTrue, + &[is_true] + ))); + // A control-flow guard carries no condition operand at all. + assert!(!is_back_edge_poll_guard(&Op::new( + OpCode::GuardNotForced, + &[] + ))); + } /// One request arms the poll once: the taker reports it, clears it, and the /// next taker sees nothing. A taker that failed to clear would keep failing diff --git a/majit/majit-metainterp/src/compile.rs b/majit/majit-metainterp/src/compile.rs index 20defcc6ef0..bb8243caac3 100644 --- a/majit/majit-metainterp/src/compile.rs +++ b/majit/majit-metainterp/src/compile.rs @@ -3008,6 +3008,7 @@ pub fn make_fail_descr_with_index(fail_index: u32, num_live: usize) -> DescrRef fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -3100,6 +3101,7 @@ pub fn make_resume_guard_descr_typed(types: Vec) -> DescrRef { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -3380,6 +3382,7 @@ pub fn make_resume_at_position_descr_typed(types: Vec) -> DescrRef { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -3647,6 +3650,7 @@ pub fn make_resume_guard_forced_descr_typed(types: Vec) -> DescrRef { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -3898,6 +3902,7 @@ pub fn make_resume_guard_exc_descr_typed(types: Vec) -> DescrRef { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -3988,6 +3993,11 @@ pub struct ResumeGuardCopiedDescr { /// lives on the descr. Same per-emission scoping as /// `source_op_index` / `rd_locs` — owned per copied descr. force_token_slots: UnsafeCell>, + /// Pyre-only per-emission slot: this guard is the eval-breaker word's + /// back-edge poll. Same per-emission scoping as `source_op_index`; a + /// copy guards the same back edge as its donor, so the optimizer stamps + /// each copy as it emits it. + back_edge_poll: std::sync::atomic::AtomicBool, /// Pyre-only per-emission failure counter for bridge compilation /// thresholds. PyPy carries the equivalent jitcounter hash in /// `compile.py:683 AbstractResumeGuardDescr._attrs_ ('status',)` @@ -4134,6 +4144,7 @@ impl majit_ir::Descr for ResumeGuardCopiedDescr { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: std::sync::atomic::AtomicPtr::new(std::ptr::null_mut()), bridge_code_ptr_cache: Box::new(std::sync::atomic::AtomicUsize::new(0)), @@ -4312,6 +4323,26 @@ impl FailDescr for ResumeGuardCopiedDescr { fn set_source_op_index(&self, source_op_index: usize) { unsafe { *self.source_op_index.get() = Some(source_op_index) }; } + /// Per-emission like `force_token_slots` below, and deliberately NOT + /// chased through `prev`: the classification describes this guard's own + /// condition chain, which a sharer does not inherit from its donor. A + /// copied descr always answers `false`, and that is correct twice over. + /// A poll guard can never be the *sharer*: sharing requires + /// `!op.has_descr() && op.rd_resume_position < 0` (optimizer.rs + /// `emit_guard_operation`, mirrored in optimizeopt/mod.rs), whereas the + /// poll is emitted by `close_loop_args_at` through `generate_guard`, + /// which captures resume data and so always carries a resume position. + /// And when a poll guard is the *donor*, the sharer is some descrless + /// optimizer-created follow-up guard that is not itself a poll, so + /// reading through `prev` would misreport it as one. + fn is_back_edge_poll(&self) -> bool { + self.back_edge_poll + .load(std::sync::atomic::Ordering::Relaxed) + } + fn set_back_edge_poll(&self) { + self.back_edge_poll + .store(true, std::sync::atomic::Ordering::Relaxed); + } /// Per-emission `force_token_slots` (see field comment). Owned /// per copied descr so each emission's GC-root classification /// stays distinct — PyPy bakes the equivalent map inline per @@ -4450,6 +4481,7 @@ impl majit_ir::Descr for ResumeGuardCopiedExcDescr { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: std::sync::atomic::AtomicPtr::new(std::ptr::null_mut()), bridge_code_ptr_cache: Box::new(std::sync::atomic::AtomicUsize::new(0)), @@ -4571,6 +4603,12 @@ impl FailDescr for ResumeGuardCopiedExcDescr { fn set_source_op_index(&self, source_op_index: usize) { self.inner.set_source_op_index(source_op_index); } + fn is_back_edge_poll(&self) -> bool { + self.inner.is_back_edge_poll() + } + fn set_back_edge_poll(&self) { + self.inner.set_back_edge_poll(); + } fn force_token_slots(&self) -> Vec { self.inner.force_token_slots() } @@ -4654,6 +4692,7 @@ pub fn make_resume_guard_copied_descr(prev: DescrRef) -> DescrRef { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: std::sync::atomic::AtomicPtr::new(std::ptr::null_mut()), bridge_code_ptr_cache: Box::new(std::sync::atomic::AtomicUsize::new(0)), @@ -4694,6 +4733,7 @@ pub fn make_resume_guard_copied_exc_descr(prev: DescrRef) -> DescrRef { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: std::sync::atomic::AtomicPtr::new(std::ptr::null_mut()), bridge_code_ptr_cache: Box::new(std::sync::atomic::AtomicUsize::new(0)), @@ -4858,6 +4898,7 @@ impl majit_ir::Descr for CompileLoopVersionDescr { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -5078,6 +5119,7 @@ fn make_compile_loop_version_descr_with_payload(types: Vec, payload: RdPay fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), @@ -5580,6 +5622,7 @@ mod fail_descr_tests { fail_index_per_trace: AtomicU32::new(0), source_op_index: UnsafeCell::new(None), force_token_slots: UnsafeCell::new(Vec::new()), + back_edge_poll: std::sync::atomic::AtomicBool::new(false), fail_count: AtomicU32::new(0), trace_info: AtomicPtr::new(std::ptr::null_mut()), external_jump_target: OnceLock::new(), diff --git a/majit/majit-metainterp/src/optimizeopt/mod.rs b/majit/majit-metainterp/src/optimizeopt/mod.rs index b93067598d7..705ac4ee211 100644 --- a/majit/majit-metainterp/src/optimizeopt/mod.rs +++ b/majit/majit-metainterp/src/optimizeopt/mod.rs @@ -6529,6 +6529,16 @@ impl OptContext { fd.set_rd_consts(Some(rd_consts)); fd.set_rd_virtuals(descr_rd_virtuals); fd.set_rd_pendingfields(descr_pending); + // The back-edge poll of the eval-breaker word reaches this + // function like any other guard, and this is the last point where + // its condition chain and its descr are both in hand — the + // backends see only the emitted guard. Stamping here also covers + // every re-emission (unroll's preamble and body copies, and the + // poll a bridge records at its own back edge), because each mints + // a fresh descr through this same call. + if majit_ir::eval_breaker_word::is_back_edge_poll_guard(op) { + fd.set_back_edge_poll(); + } } // resume.py: RPython does NOT carry frame sizes out-of-band. // The decoder reads jitcode liveness (jitcode.position_info) at diff --git a/majit/majit-metainterp/src/optimizeopt/optimizer.rs b/majit/majit-metainterp/src/optimizeopt/optimizer.rs index b23e7fe005c..1f49b934ad1 100644 --- a/majit/majit-metainterp/src/optimizeopt/optimizer.rs +++ b/majit/majit-metainterp/src/optimizeopt/optimizer.rs @@ -2106,8 +2106,11 @@ impl Optimizer { /// failing a guard has no back edge and runs zero iterations while /// reporting a healthy-looking op total. /// - /// `label` names which of the three compile paths produced the trace, since - /// they differ in whether the body was unrolled. + /// `label` names which compile path produced the trace, since they differ + /// in whether the body was unrolled and whether it is a bridge. Bridges + /// must be logged too: they take trace ids from the same counter the loops + /// draw from, so omitting them leaves gaps that make a dumped trace + /// impossible to line up with the `trace=` field of a guard-failure log. pub fn log_optimized_trace(label: &str, ops: &[T], constants: &C) where V: std::fmt::Debug, diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 4df9193b964..50355d39673 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -1747,6 +1747,7 @@ pub(crate) struct JitStatsCounters { loops_aborted: usize, bridges_compiled: usize, guard_failures: usize, + back_edge_polls: usize, } /// Snapshot of cumulative JIT compilation statistics. @@ -1756,7 +1757,20 @@ pub struct JitStats { pub retraces_compiled: usize, pub loops_aborted: usize, pub bridges_compiled: usize, + /// Guards that failed because the compiled code's assumption did not hold. + /// Excludes the eval-breaker back-edge poll, which is counted separately — + /// see `back_edge_polls`. pub guard_failures: usize, + /// Failures of the eval-breaker word's back-edge poll: a compiled loop + /// leaving machine code so the interpreter can service a signal, a + /// stop-the-world request, or an owed collection. + /// + /// Split out because it measures *when* a collection landed rather than + /// anything about the compiled code, which makes it move with the heap + /// schedule and the host while `loops_compiled` / `bridges_compiled` sit + /// still. Folded into `guard_failures` it made that total unusable as an + /// exact-match baseline. + pub back_edge_polls: usize, /// issue compilation-panic: non-`InvalidLoop` panics swallowed during compilation /// (graceful degradation in release). Non-zero means the JIT was /// silently disabled for some traces by an internal bug. @@ -2257,8 +2271,19 @@ impl MetaInterp { !is_finish && !Self::is_jump_exit(is_finish, fail_index) } + /// `back_edge_poll` splits the total: a failing eval-breaker poll left + /// machine code because the collector asked for a safepoint, not because + /// the compiled code assumed something that turned out false. Only the + /// tally is split — the census, the per-guard warm-state counter, and the + /// hook see every failure, since bridge-compilation thresholds and guard + /// attribution still apply to the poll. #[inline] - fn record_guard_failure_event(&mut self, green_key: u64, fail_index: u32) { + fn record_guard_failure_event( + &mut self, + green_key: u64, + fail_index: u32, + back_edge_poll: bool, + ) { if guardlog_enabled() { eprintln!("@@@GUARD key={green_key} fail={fail_index}"); } @@ -2268,7 +2293,12 @@ impl MetaInterp { green_key, fail_index ); } - self.stats.guard_failures += 1; + let tally = if back_edge_poll { + &mut self.stats.back_edge_polls + } else { + &mut self.stats.guard_failures + }; + *tally += 1; crate::guard_census_record(green_key, fail_index); self.warm_state.log_guard_failure(fail_index); if let Some(ref hook) = self.hooks.on_guard_failure { @@ -4301,6 +4331,7 @@ impl MetaInterp { loops_aborted: self.stats.loops_aborted, bridges_compiled: self.stats.bridges_compiled, guard_failures: self.stats.guard_failures, + back_edge_polls: self.stats.back_edge_polls, internal_compile_panics: self.internal_compile_panics, } } @@ -10026,7 +10057,11 @@ impl MetaInterp { } if Self::should_record_guard_failure(effective_is_finish, fail_index) { - self.record_guard_failure_event(green_key, fail_index); + let back_edge_poll = result + .descr_arc + .as_fail_descr() + .is_some_and(|fd| fd.is_back_edge_poll()); + self.record_guard_failure_event(green_key, fail_index, back_edge_poll); } // pyjitpl.py:3119-3123: exc_class = ptr2int(exception_obj.typeptr) let exc_class = if result.exception_value.is_null() { @@ -10110,7 +10145,10 @@ impl MetaInterp { Self::finish_compiled_run_io(); if Self::should_record_guard_failure(is_finish, fail_index) { - self.record_guard_failure_event(green_key, fail_index); + let back_edge_poll = descr_arc + .as_fail_descr() + .is_some_and(|fd| fd.is_back_edge_poll()); + self.record_guard_failure_event(green_key, fail_index, back_edge_poll); } let exit_arity = exit_types.len(); @@ -10288,7 +10326,10 @@ impl MetaInterp { // in handle_fail → must_compile (compile.py:701-784). // must_compile handles tick. if Self::should_record_guard_failure(is_finish, fail_index) { - self.record_guard_failure_event(green_key, fail_index); + let back_edge_poll = descr_arc + .as_fail_descr() + .is_some_and(|fd| fd.is_back_edge_poll()); + self.record_guard_failure_event(green_key, fail_index, back_edge_poll); } let exit_arity = exit_types.len(); @@ -12749,6 +12790,11 @@ impl MetaInterp { eprintln!("--- bridge trace (after opt) ---"); eprint!("{}", majit_ir::format_trace(&optimized_ops, &constants)); } + crate::optimizeopt::optimizer::Optimizer::log_optimized_trace( + "compile_bridge", + &optimized_ops, + &constants, + ); // compile.py:27-29 giveup() parity: a bridge whose terminal JUMP // targets an already-compiled loop must supply exactly as many args diff --git a/majit/majit-translate/src/codewriter/call.rs b/majit/majit-translate/src/codewriter/call.rs index 3d2666eda83..5391fce6677 100644 --- a/majit/majit-translate/src/codewriter/call.rs +++ b/majit/majit-translate/src/codewriter/call.rs @@ -5946,10 +5946,22 @@ impl CallControl { // collectanalyze.py:27-33: analyze_simple_operation // RPython checks: malloc/malloc_varsize with flavor='gc' → True // LL_OPERATIONS[op.opname].canmallocgc → True - // majit codewriter graphs have no LL_OPERATIONS; the only - // operations that can trigger GC are transitive through calls. - // (All other OpKind variants are pure/field/array ops.) match &op.kind { + // collectanalyze.py:28-30 — `malloc` / `malloc_varsize` + // with `flavor='gc'`. These four variants are that + // operation on this side of jtransform: `New` and + // `NewWithVtable` are `malloc(GcStruct, flavor='gc')` + // (`rewrite_op_malloc`, jtransform.py:1012-1045), + // `NewArrayClear` is `new_array_clear` + // (jtransform.py:1858-1863), and `NewListClear` allocates + // a GcStruct plus a cleared items array + // (pyjitpl.py:792-798). This graph model carries no + // `flavor='raw'` allocation, so there is no flavour test + // to make — every allocation op here is a GC one. + OpKind::New { .. } + | OpKind::NewWithVtable { .. } + | OpKind::NewArrayClear { .. } + | OpKind::NewListClear { .. } => return true, OpKind::Call { target, .. } => { // graphanalyze.py:139-164: analyze_direct_call — recurse let callee_path = match self.target_to_path(target) { @@ -9320,6 +9332,69 @@ mod tests { cc.register_function_graph(path, graph.with_return_type("i64")); } + /// `collectanalyze.py:27-31 analyze_simple_operation` answers `True` for + /// `malloc` / `malloc_varsize` with `flavor='gc'`. This graph model spells + /// that operation four ways, and each has to answer on its own — a graph + /// reaching an allocation collects even when it calls nothing. + /// + /// The negative control is the point of the test as much as the positive + /// ones: before the allocation arms existed, `analyze_can_collect` could + /// only answer `true` through `close_stack` or `random_effects_on_gcobjs`, + /// and the latter is set by the `gc_effects` hint, which nothing in the + /// corpus carries. Every allocator therefore analysed as "cannot collect", + /// so an all-`false` verdict is exactly what the regression looks like and + /// a test that only checked the negative case would not have seen it. + #[test] + fn each_gc_allocation_op_collects_and_an_allocation_free_graph_does_not() { + let alloc_kinds = [ + OpKind::New { + owner: "W_IntObject".to_string(), + }, + OpKind::NewWithVtable { + owner: "W_FloatObject".to_string(), + vtable: 1, + }, + OpKind::NewArrayClear { + length: crate::flowspace::model::Variable::new(), + item_ty: ValueType::Ref(None), + array_type_id: None, + }, + OpKind::NewListClear { + length: crate::flowspace::model::Variable::new(), + item_ty: ValueType::Ref(None), + array_type_id: None, + }, + ]; + + for kind in alloc_kinds { + let label = format!("{kind:?}"); + let mut cc = CallControl::new(); + let mut graph = FunctionGraph::new("allocating"); + let start = graph.startblock; + graph.blocks[start.0] + .operations + .push(SpaceOperation { result: None, kind }); + graph.set_return(start, None); + let path = CallPath::from_segments(["allocating"]); + cc.register_function_graph(path.clone(), graph); + + let mut seen = HashSet::new(); + assert!( + cc.analyze_can_collect(&path, &mut seen), + "a graph whose only operation is {label} must analyse as collecting" + ); + } + + let mut cc = CallControl::new(); + let path = CallPath::from_segments(["allocation_free"]); + cc.register_function_graph(path.clone(), simple_graph("allocation_free")); + let mut seen = HashSet::new(); + assert!( + !cc.analyze_can_collect(&path, &mut seen), + "a graph with no allocation and no call must analyse as not collecting" + ); + } + /// Helper: create a FunctionGraph whose entry block routes to the /// canonical exceptblock, matching upstream's Link(..., exceptblock) /// shape for unconditional raise sites. diff --git a/pyre/bench/synth/arith_int_bool.cranelift.jitstats b/pyre/bench/synth/arith_int_bool.cranelift.jitstats index 4c105cf1961..f583f60500e 100644 --- a/pyre/bench/synth/arith_int_bool.cranelift.jitstats +++ b/pyre/bench/synth/arith_int_bool.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=2214 +guard_failures=2211 internal_compile_panics=0 loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/arith_int_bool.dynasm.jitstats b/pyre/bench/synth/arith_int_bool.dynasm.jitstats index 4c105cf1961..f583f60500e 100644 --- a/pyre/bench/synth/arith_int_bool.dynasm.jitstats +++ b/pyre/bench/synth/arith_int_bool.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=2214 +guard_failures=2211 internal_compile_panics=0 loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/bound_method_builtin_fold.cranelift.jitstats b/pyre/bench/synth/bound_method_builtin_fold.cranelift.jitstats index 156cfbff834..933245ec222 100644 --- a/pyre/bench/synth/bound_method_builtin_fold.cranelift.jitstats +++ b/pyre/bench/synth/bound_method_builtin_fold.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=459 +guard_failures=458 internal_compile_panics=0 loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/bound_method_builtin_fold.dynasm.jitstats b/pyre/bench/synth/bound_method_builtin_fold.dynasm.jitstats index 156cfbff834..933245ec222 100644 --- a/pyre/bench/synth/bound_method_builtin_fold.dynasm.jitstats +++ b/pyre/bench/synth/bound_method_builtin_fold.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=459 +guard_failures=458 internal_compile_panics=0 loops_aborted=0 loops_compiled=7 diff --git a/pyre/bench/synth/build_set_hashability.cranelift.jitstats b/pyre/bench/synth/build_set_hashability.cranelift.jitstats index a73414abd36..4e1e41a7bed 100644 --- a/pyre/bench/synth/build_set_hashability.cranelift.jitstats +++ b/pyre/bench/synth/build_set_hashability.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=4 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/build_set_hashability.dynasm.jitstats b/pyre/bench/synth/build_set_hashability.dynasm.jitstats index a73414abd36..4e1e41a7bed 100644 --- a/pyre/bench/synth/build_set_hashability.dynasm.jitstats +++ b/pyre/bench/synth/build_set_hashability.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=4 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/build_set_hashability.wasm.jitstats b/pyre/bench/synth/build_set_hashability.wasm.jitstats index d84b20777c9..4e1e41a7bed 100644 --- a/pyre/bench/synth/build_set_hashability.wasm.jitstats +++ b/pyre/bench/synth/build_set_hashability.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=3 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/bytes_split_whitespace_maxsplit.cranelift.jitstats b/pyre/bench/synth/bytes_split_whitespace_maxsplit.cranelift.jitstats index ac68d18c82e..651a3eaf3e9 100644 --- a/pyre/bench/synth/bytes_split_whitespace_maxsplit.cranelift.jitstats +++ b/pyre/bench/synth/bytes_split_whitespace_maxsplit.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=3 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/bytes_split_whitespace_maxsplit.dynasm.jitstats b/pyre/bench/synth/bytes_split_whitespace_maxsplit.dynasm.jitstats index ac68d18c82e..651a3eaf3e9 100644 --- a/pyre/bench/synth/bytes_split_whitespace_maxsplit.dynasm.jitstats +++ b/pyre/bench/synth/bytes_split_whitespace_maxsplit.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=3 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/bytes_split_whitespace_maxsplit.wasm.jitstats b/pyre/bench/synth/bytes_split_whitespace_maxsplit.wasm.jitstats index 0375a62df00..651a3eaf3e9 100644 --- a/pyre/bench/synth/bytes_split_whitespace_maxsplit.wasm.jitstats +++ b/pyre/bench/synth/bytes_split_whitespace_maxsplit.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=2 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/closure_per_call.wasm.jitstats b/pyre/bench/synth/closure_per_call.wasm.jitstats index 44510fd0054..5b523b8e3d7 100644 --- a/pyre/bench/synth/closure_per_call.wasm.jitstats +++ b/pyre/bench/synth/closure_per_call.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=415 +guard_failures=414 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/delete_negative_open_slice_hot.cranelift.jitstats b/pyre/bench/synth/delete_negative_open_slice_hot.cranelift.jitstats index f293f5a9b62..6fac6674aba 100644 --- a/pyre/bench/synth/delete_negative_open_slice_hot.cranelift.jitstats +++ b/pyre/bench/synth/delete_negative_open_slice_hot.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=1405 +guard_failures=1404 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/delete_negative_open_slice_hot.dynasm.jitstats b/pyre/bench/synth/delete_negative_open_slice_hot.dynasm.jitstats index f293f5a9b62..6fac6674aba 100644 --- a/pyre/bench/synth/delete_negative_open_slice_hot.dynasm.jitstats +++ b/pyre/bench/synth/delete_negative_open_slice_hot.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=1405 +guard_failures=1404 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/inline_gate_operand_provenance.wasm.jitstats b/pyre/bench/synth/inline_gate_operand_provenance.wasm.jitstats index 0a519406bcb..450564aefa7 100644 --- a/pyre/bench/synth/inline_gate_operand_provenance.wasm.jitstats +++ b/pyre/bench/synth/inline_gate_operand_provenance.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=5 +guard_failures=4 internal_compile_panics=0 loops_aborted=3 loops_compiled=7 diff --git a/pyre/bench/synth/instance_dict_reassign.cranelift.jitstats b/pyre/bench/synth/instance_dict_reassign.cranelift.jitstats index 0375a62df00..651a3eaf3e9 100644 --- a/pyre/bench/synth/instance_dict_reassign.cranelift.jitstats +++ b/pyre/bench/synth/instance_dict_reassign.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=2 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/instance_dict_reassign.dynasm.jitstats b/pyre/bench/synth/instance_dict_reassign.dynasm.jitstats index 0375a62df00..651a3eaf3e9 100644 --- a/pyre/bench/synth/instance_dict_reassign.dynasm.jitstats +++ b/pyre/bench/synth/instance_dict_reassign.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=2 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/math_isqrt_compare_bridge_resume.wasm.jitstats b/pyre/bench/synth/math_isqrt_compare_bridge_resume.wasm.jitstats index c86049cfd8a..a3797b634a3 100644 --- a/pyre/bench/synth/math_isqrt_compare_bridge_resume.wasm.jitstats +++ b/pyre/bench/synth/math_isqrt_compare_bridge_resume.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=403 +guard_failures=402 internal_compile_panics=0 loops_aborted=0 loops_compiled=4 diff --git a/pyre/bench/synth/newslice_step_hot.cranelift.jitstats b/pyre/bench/synth/newslice_step_hot.cranelift.jitstats index 27064635170..c3bab7dbb4a 100644 --- a/pyre/bench/synth/newslice_step_hot.cranelift.jitstats +++ b/pyre/bench/synth/newslice_step_hot.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=5 +guard_failures=2 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/newslice_step_hot.dynasm.jitstats b/pyre/bench/synth/newslice_step_hot.dynasm.jitstats index 27064635170..c3bab7dbb4a 100644 --- a/pyre/bench/synth/newslice_step_hot.dynasm.jitstats +++ b/pyre/bench/synth/newslice_step_hot.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=5 +guard_failures=2 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/newslice_step_hot.wasm.jitstats b/pyre/bench/synth/newslice_step_hot.wasm.jitstats index a73414abd36..c3bab7dbb4a 100644 --- a/pyre/bench/synth/newslice_step_hot.wasm.jitstats +++ b/pyre/bench/synth/newslice_step_hot.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=4 +guard_failures=2 internal_compile_panics=0 loops_aborted=0 loops_compiled=2 diff --git a/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats b/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats index badaa6495af..795616df41a 100644 --- a/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats +++ b/pyre/bench/synth/recursive_call_frame_relocation.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=638 +guard_failures=637 internal_compile_panics=0 loops_aborted=0 loops_compiled=3 diff --git a/pyre/bench/synth/str_fstring.cranelift.darwin.jitstats b/pyre/bench/synth/str_fstring.cranelift.darwin.jitstats index 5c093f4f38b..15e86455d8d 100644 --- a/pyre/bench/synth/str_fstring.cranelift.darwin.jitstats +++ b/pyre/bench/synth/str_fstring.cranelift.darwin.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=658 +guard_failures=657 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/str_fstring.cranelift.jitstats b/pyre/bench/synth/str_fstring.cranelift.jitstats index e45d14ff4ca..5c093f4f38b 100644 --- a/pyre/bench/synth/str_fstring.cranelift.jitstats +++ b/pyre/bench/synth/str_fstring.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=659 +guard_failures=658 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/str_fstring.cranelift.win32.github-actions.jitstats b/pyre/bench/synth/str_fstring.cranelift.win32.github-actions.jitstats index 5c093f4f38b..15e86455d8d 100644 --- a/pyre/bench/synth/str_fstring.cranelift.win32.github-actions.jitstats +++ b/pyre/bench/synth/str_fstring.cranelift.win32.github-actions.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=658 +guard_failures=657 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/str_fstring.dynasm.darwin.jitstats b/pyre/bench/synth/str_fstring.dynasm.darwin.jitstats index 5c093f4f38b..15e86455d8d 100644 --- a/pyre/bench/synth/str_fstring.dynasm.darwin.jitstats +++ b/pyre/bench/synth/str_fstring.dynasm.darwin.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=658 +guard_failures=657 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/str_fstring.dynasm.jitstats b/pyre/bench/synth/str_fstring.dynasm.jitstats index 5c093f4f38b..15e86455d8d 100644 --- a/pyre/bench/synth/str_fstring.dynasm.jitstats +++ b/pyre/bench/synth/str_fstring.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=658 +guard_failures=657 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/str_fstring.wasm.jitstats b/pyre/bench/synth/str_fstring.wasm.jitstats index 5c093f4f38b..15e86455d8d 100644 --- a/pyre/bench/synth/str_fstring.wasm.jitstats +++ b/pyre/bench/synth/str_fstring.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=658 +guard_failures=657 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/unpack_ex_hot.cranelift.jitstats b/pyre/bench/synth/unpack_ex_hot.cranelift.jitstats index ac68d18c82e..651a3eaf3e9 100644 --- a/pyre/bench/synth/unpack_ex_hot.cranelift.jitstats +++ b/pyre/bench/synth/unpack_ex_hot.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=3 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/unpack_ex_hot.dynasm.jitstats b/pyre/bench/synth/unpack_ex_hot.dynasm.jitstats index ac68d18c82e..651a3eaf3e9 100644 --- a/pyre/bench/synth/unpack_ex_hot.dynasm.jitstats +++ b/pyre/bench/synth/unpack_ex_hot.dynasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=3 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/bench/synth/unpack_ex_hot.wasm.jitstats b/pyre/bench/synth/unpack_ex_hot.wasm.jitstats index 0375a62df00..651a3eaf3e9 100644 --- a/pyre/bench/synth/unpack_ex_hot.wasm.jitstats +++ b/pyre/bench/synth/unpack_ex_hot.wasm.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=2 +guard_failures=1 internal_compile_panics=0 loops_aborted=0 loops_compiled=1 diff --git a/pyre/check.py b/pyre/check.py index ffdc77618a9..e3389a95d13 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -518,12 +518,17 @@ def pyre_env(): # 4MB nursery also sets a 32MB threshold, and where old-gen use crosses it # (:2437 `threshold_reached`) stayed free to move. # - # Crossing it is what reaches `guard_failures`. The major step's finalizer - # trigger arms the eval-breaker word, and every compiled loop's back edge - # polls that word through a real guard — + # Crossing it used to reach `guard_failures`, and no longer does. The major + # step's finalizer trigger arms the eval-breaker word, and every compiled + # loop's back edge polls that word through a real guard — # `RawLoadI(&EVAL_BREAKER_WORD) -> IntAnd(word, JIT_BREAKER_MASK) -> - # IntIsTrue -> GuardFalse` (trace_opcode.rs:2003-2015) — whose failure is - # counted like any other at pyjitpl.rs:2087. The trace is peeled, so that + # IntIsTrue -> GuardFalse` (trace_opcode.rs) — whose failure was counted + # like any other. It is now tallied separately as `back_edge_polls` + # (`is_back_edge_poll_guard` marks the guard, `record_guard_failure_event` + # splits the total), which is what removes the whole class rather than + # avoiding it; the rest of this note is kept because it describes the pins + # that are still in place and what they were measured to do. The trace is + # peeled, so that # poll appears twice, and which copy catches the armed bit sets the price: # the loop-body copy costs one bailout, the peeled-preamble copy costs two, # because resuming from it re-enters at the loop head and fails the @@ -533,11 +538,22 @@ def pyre_env(): # `bridges_compiled` identical across all three, and it is why the windows # runner disagreed with itself between jobs rather than against the tree. # - # Pushing the threshold past every fixture's working set removes the event + # Pushing the threshold past a fixture's working set removes the event # instead of relocating it. Measured: `recursive_call_frame_relocation` # reads 636 at every nursery from 3968KB to 8MB, and `closure_per_call` # reads 414 where it had alternated 415/416 — the pair a per-platform - # overlay could not hold. Host RAM cannot re-open it: `max_delta` + # overlay could not hold. + # + # "Past every fixture's working set" is what this pin was believed to do + # and is not what it does — the two fixtures above are not the suite. + # Measured across all 404 dynasm synthetic fixtures, comparing each one's + # count at this 256MB pin against the same fixture at 8GB (where no major + # collection happens at all): 11 of them still cross, for 21 poll failures + # in total. `bytes_split_whitespace_maxsplit` reads 4 here and 1 there, + # `build_set_hashability` 4 and 1, `str_fstring` 658 and 657. So the pin + # narrows the class and does not close it, which is why the counter split + # above exists — those 11 were the fixtures that flipped between hosts. + # Host RAM cannot re-open it: `max_delta` # (0.125 * total memory) enters only as an upper bound at collector.rs:3570 # and the `min_heap_size` floor is applied after it (:2452), so the floor # wins on every host. @@ -918,6 +934,14 @@ def _parse_jit_stats(snapshot): "fbw_blackhole_adopted_multi_frame", ) +# `back_edge_polls` is deliberately absent, and is the one counter that must +# stay absent. It reports how many times a compiled loop left machine code +# because the eval-breaker word was armed — a measure of when a collection +# landed, not of anything the compiler decided. Recording it would move the +# schedule-sensitivity this split was made to remove onto a new key instead of +# removing it. It is printed on the `[jit-stats]` line either way, so a reader +# diagnosing a `guard_failures` move can still see it. + # Which way each counter has to move to be a regression rather than a gain. # Both outcomes fail — a baseline that stopped describing the tree has to be diff --git a/pyre/pyre-wasm-runner/src/main.rs b/pyre/pyre-wasm-runner/src/main.rs index 1a4b1b2d37e..6a1c39b23a0 100644 --- a/pyre/pyre-wasm-runner/src/main.rs +++ b/pyre/pyre-wasm-runner/src/main.rs @@ -921,6 +921,7 @@ fn run(module_path: &Path, source: &str, script: &Path) -> Result { let retraces_compiled = counter("pyre_jit_retraces_compiled", &mut missing); let loops_aborted = counter("pyre_jit_loops_aborted", &mut missing); let guard_failures = counter("pyre_jit_guard_failures", &mut missing); + let back_edge_polls = counter("pyre_jit_back_edge_polls", &mut missing); let internal_compile_panics = counter("pyre_jit_internal_compile_panics", &mut missing); let descr_set_resolved = counter("pyre_jit_descr_set_resolved", &mut missing); let descr_set_absent = counter("pyre_jit_descr_set_absent", &mut missing); @@ -978,6 +979,7 @@ fn run(module_path: &Path, source: &str, script: &Path) -> Result { retraces_compiled={retraces_compiled} \ loops_aborted={loops_aborted} \ guard_failures={guard_failures} \ + back_edge_polls={back_edge_polls} \ internal_compile_panics={internal_compile_panics} \ descr_set_resolved={descr_set_resolved} \ descr_set_absent={descr_set_absent} \ diff --git a/pyre/pyre-wasm/src/lib.rs b/pyre/pyre-wasm/src/lib.rs index 9af1426202f..465b47a8275 100644 --- a/pyre/pyre-wasm/src/lib.rs +++ b/pyre/pyre-wasm/src/lib.rs @@ -498,6 +498,17 @@ pub extern "C" fn pyre_jit_guard_failures() -> u64 { pyre_jit::eval::driver_pair().0.get_stats().guard_failures as u64 } +/// Unlike the four above, this one is exported to be *read*, not gated: it +/// counts back-edge polls that found the eval-breaker word armed, which tracks +/// the collection schedule rather than anything the compiler decided. +/// `check.py` keeps it out of `JITSTATS_SNAPSHOT_FIELDS` for that reason, and +/// the runner prints it so a `guard_failures` move can be read against it. +#[cfg(all(target_arch = "wasm32", feature = "wasm-host"))] +#[unsafe(no_mangle)] +pub extern "C" fn pyre_jit_back_edge_polls() -> u64 { + pyre_jit::eval::driver_pair().0.get_stats().back_edge_polls as u64 +} + /// The descr-universe invariants, the remaining `JITSTATS_BADNESS_FIELDS`. The /// native backends print these from `descr_set_jit_stats`; the guest has no /// stderr, so it exports the counts and the runner prints the line. Without diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index da47134b48f..b1663120166 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -838,12 +838,13 @@ fn maybe_print_jit_stats() { let stats = pyre_jit::eval::driver_pair().0.get_stats(); eprintln!( "[jit-stats] loops_compiled={} bridges_compiled={} retraces_compiled={} loops_aborted={} \ - guard_failures={} internal_compile_panics={}", + guard_failures={} back_edge_polls={} internal_compile_panics={}", stats.loops_compiled, stats.bridges_compiled, stats.retraces_compiled, stats.loops_aborted, stats.guard_failures, + stats.back_edge_polls, stats.internal_compile_panics, ); // How those `guard_failures` are distributed: a handful of guards eating