diff --git a/majit/majit-metainterp/src/jitdriver.rs b/majit/majit-metainterp/src/jitdriver.rs index c87db00f829..d1f3f32f131 100644 --- a/majit/majit-metainterp/src/jitdriver.rs +++ b/majit/majit-metainterp/src/jitdriver.rs @@ -4773,14 +4773,8 @@ impl JitDriver { // already recovered `state` to the resume point, so the // bridge sees the post-guard-failure values. if should_bridge && !portal_crn_handled && pc != usize::MAX { - let bridge_ok = self.start_bridge_tracing( - &descr_arc, - state, - env, - &raw_values, - pc, - guard_exc, - ); + let bridge_ok = + self.start_bridge_tracing(&descr_arc, state, env, &raw_values, pc); if crate::majit_log_enabled() { eprintln!( "[bridge] start_bridge_tracing (green resume) key={} trace={} fail={} resume_pc={} ok={}", @@ -6262,11 +6256,6 @@ impl JitDriver { env: &S::Env, raw_fail_values: &[i64], resume_pc: usize, - // llmodel.py:240 `cpu.grab_exc_value(deadframe)`: the pending exception - // grabbed at the guard failure, threaded so `setup_bridge_sym` seeds the - // bridge sym's standing exception (pyjitpl.py:3125). 0 when the guard - // carried no exception. - guard_exc: i64, ) -> bool { majit_metainterp::mc_diag_bump(12); // start_bridge_tracing entered // Same reason as the primary trace entry: the bridge compile decodes @@ -6489,11 +6478,6 @@ impl JitDriver { ctx.set_virtualizable_heap_ptr(ptr); } ctx.set_bridge_source_is_exception_guard(retrace.is_exception_guard); - // pyjitpl.py:3125 `_prepare_exception_resumption` grabs the exception - // BEFORE frame reconstruction; thread it onto the ctx so the pyre - // `setup_bridge_sym` override can seed the standing exception before it - // drains the inline-callee carrier. - ctx.set_bridge_guard_exc(guard_exc); ctx.bridge_target_header_pc = parent_header_pc; ctx.has_compiled_targets_fn = Some(Box::new(move |gk: u64| -> bool { let meta = unsafe { &*(meta_ptr as *const crate::pyjitpl::MetaInterp) }; @@ -6906,14 +6890,8 @@ impl JitDriver { let resume_pc = resume_pc.unwrap_or(guard_resume_pc); self.sync_after(state, &result_meta, descriptor.as_deref()); - let bridge_ok = self.start_bridge_tracing( - &descr_arc, - state, - env, - &raw_values, - resume_pc, - result_exc, - ); + let bridge_ok = + self.start_bridge_tracing(&descr_arc, state, env, &raw_values, resume_pc); if crate::majit_log_enabled() { eprintln!( "[bridge] start_bridge_tracing key={} trace={} fail={} resume_pc={} ok={}", @@ -8016,14 +7994,8 @@ mod tests { let descr_arc = std::sync::Arc::clone(&failure.descr_arc); drop(failure); - let started = driver.start_bridge_tracing( - &descr_arc, - &mut NonTraceableState, - &(), - &fail_values, - 0, - 0, - ); + let started = + driver.start_bridge_tracing(&descr_arc, &mut NonTraceableState, &(), &fail_values, 0); assert!(!started); assert!(!driver.meta.is_tracing()); } diff --git a/majit/majit-metainterp/src/optimizeopt/unroll.rs b/majit/majit-metainterp/src/optimizeopt/unroll.rs index b3f83b3cc26..347e8487d75 100644 --- a/majit/majit-metainterp/src/optimizeopt/unroll.rs +++ b/majit/majit-metainterp/src/optimizeopt/unroll.rs @@ -1849,11 +1849,18 @@ impl UnrollOptimizer { body_jump_arity, preamble_arity, exported_renamed_inputargs, ); } - // `compile.py:334 assert jump.numargs() == label.numargs()`. - // Upstream can assert because its `target_tokens[0]` is the start - // label whose args ARE `loop.inputargs` — the same loop-carried - // positions the body JUMP carries — so `unroll.py:238-242`'s - // arg-preserving retarget is sound by construction. + // Upstream has no arity check on this path at all, and the check + // below is pyre's own. `unroll.py:238-242 jump_to_preamble` only + // asserts `target_tokens[0].virtual_state is None` and retargets the + // JUMP with `copy_and_change`, keeping its args; `compile.py:334`'s + // `assert jump_op.numargs() == loop_info.label_op.numargs()` is + // guarded one line above by `if jump_op.getdescr() is + // loop_info.label_op.getdescr()`, which is exactly what + // `jump_to_preamble` has just stopped being true, and + // `compile_retrace` checks nothing. Upstream needs no check because + // its `target_tokens[0]` is the preamble start label carrying + // `start_state.renamed_inputargs` — the same loop-carried positions + // the body JUMP carries — so the retarget is sound by construction. // // A pyre RETRACE has no start label of its own (see // `emit_start_label`), so `target_tokens[0]` is the ORIGINAL loop's diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 5a729f7cd5c..84bf4c8b8f8 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -1561,8 +1561,8 @@ pub struct MetaInterp { /// Internal mutable counters for JIT compilation statistics. /// /// Holds only the pyre-specific lifetime counters (`loops_compiled`, -/// `loops_aborted`, `bridges_compiled`, `guard_failures`) that have no -/// `Counters.*` slot upstream. Every counter that maps to a +/// `retraces_compiled`, `loops_aborted`, `bridges_compiled`, `guard_failures`) +/// that have no `Counters.*` slot upstream. Every counter that maps to a /// `Counters.*` id (OPS / HEAPCACHED_OPS / RECORDED_OPS / GUARDS / /// OPT_OPS / OPT_GUARDS / OPT_GUARDS_SHARED / NV* / ABORT_* / /// FORCE_VIRTUALIZABLES / OPT_VECTORIZE_*) lives on @@ -1571,6 +1571,7 @@ pub struct MetaInterp { #[derive(Default, Clone, Debug)] pub(crate) struct JitStatsCounters { loops_compiled: usize, + retraces_compiled: usize, loops_aborted: usize, bridges_compiled: usize, guard_failures: usize, @@ -1580,6 +1581,7 @@ pub(crate) struct JitStatsCounters { #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct JitStats { pub loops_compiled: usize, + pub retraces_compiled: usize, pub loops_aborted: usize, pub bridges_compiled: usize, pub guard_failures: usize, @@ -4083,6 +4085,7 @@ impl MetaInterp { pub fn get_stats(&self) -> JitStats { JitStats { loops_compiled: self.stats.loops_compiled, + retraces_compiled: self.stats.retraces_compiled, loops_aborted: self.stats.loops_aborted, bridges_compiled: self.stats.bridges_compiled, guard_failures: self.stats.guard_failures, @@ -8194,6 +8197,7 @@ impl MetaInterp { // `ResumeDescr.rd_loop_token` inherits the source identity. let mut combined_ops = combined_ops; self.record_loop_or_bridge(&source_jct, &mut combined_ops, bridge_trace_id); + self.stats.retraces_compiled += 1; if crate::majit_log_enabled() { eprintln!( "[jit] attached retrace to guard at key={green_key}, guard={fail_index}, \ diff --git a/majit/majit-metainterp/src/trace_ctx.rs b/majit/majit-metainterp/src/trace_ctx.rs index 96637dfca6f..ec1f689ce78 100644 --- a/majit/majit-metainterp/src/trace_ctx.rs +++ b/majit/majit-metainterp/src/trace_ctx.rs @@ -635,18 +635,6 @@ pub struct TraceCtx { /// `descr_arc.is_guard_exc()` and read by static bridge setup/walkers /// that only receive `TraceCtx`. pub(crate) bridge_source_is_exception_guard: bool, - /// llmodel.py:240 `cpu.grab_exc_value(deadframe)`: the pending exception - /// value grabbed at the guard failure that triggered this bridge, threaded - /// from `start_bridge_tracing` so `setup_bridge_sym` can seed the bridge - /// sym's standing exception (pyjitpl.py:3125 `_prepare_exception_resumption` - /// grabs BEFORE frame reconstruction). Raw `PyObjectRef as i64`; 0 when the - /// guard carried no exception. Class is re-derived from the value's typeptr. - /// - /// Not itself a traced slot: the exception is kept alive for the whole - /// handoff by [`crate::blackhole::GuardExcRoot`], which `handle_fail` parks - /// before it starts the bridge, so the value read back here is still live - /// whether or not a collection ran during the resume decode. - pub(crate) bridge_guard_exc: i64, } /// A decoded-but-not-yet-built description of one inlined @@ -1554,7 +1542,6 @@ impl TraceCtx { bridge_inline_carrier: None, bridge_reg_indices: None, bridge_source_is_exception_guard: false, - bridge_guard_exc: 0, } } @@ -1640,7 +1627,6 @@ impl TraceCtx { bridge_inline_carrier: None, bridge_reg_indices: None, bridge_source_is_exception_guard: false, - bridge_guard_exc: 0, } } @@ -1679,16 +1665,6 @@ impl TraceCtx { } /// True only for bridge traces sourced from an exception guard descr. - pub fn set_bridge_guard_exc(&mut self, guard_exc: i64) { - self.bridge_guard_exc = guard_exc; - } - - /// The exception value grabbed at the guard failure that triggered this - /// bridge (0 when none). See `bridge_guard_exc`. - pub fn bridge_guard_exc(&self) -> i64 { - self.bridge_guard_exc - } - pub fn bridge_source_is_exception_guard(&self) -> bool { self.bridge_source_is_exception_guard } diff --git a/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats b/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats index 9df977d4495..2a8ae763685 100644 --- a/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats +++ b/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=647 diff --git a/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats b/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats index 9df977d4495..2a8ae763685 100644 --- a/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats +++ b/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=647 diff --git a/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats b/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats index 9df977d4495..2a8ae763685 100644 --- a/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats +++ b/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=647 diff --git a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats index 32fba6ef986..127a61f9c86 100644 --- a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats +++ b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats @@ -2,7 +2,10 @@ bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=809 diff --git a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats index 32fba6ef986..127a61f9c86 100644 --- a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats +++ b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=809 diff --git a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats index 32fba6ef986..127a61f9c86 100644 --- a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats +++ b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=809 diff --git a/pyre/bench/synth/getframe_caller_locals_after_resume.cranelift.jitstats b/pyre/bench/synth/getframe_caller_locals_after_resume.cranelift.jitstats new file mode 100644 index 00000000000..49eee67988f --- /dev/null +++ b/pyre/bench/synth/getframe_caller_locals_after_resume.cranelift.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +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 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=1 diff --git a/pyre/bench/synth/getframe_caller_locals_after_resume.dynasm.jitstats b/pyre/bench/synth/getframe_caller_locals_after_resume.dynasm.jitstats new file mode 100644 index 00000000000..49eee67988f --- /dev/null +++ b/pyre/bench/synth/getframe_caller_locals_after_resume.dynasm.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +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 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=1 diff --git a/pyre/bench/synth/getframe_caller_locals_after_resume.py b/pyre/bench/synth/getframe_caller_locals_after_resume.py new file mode 100644 index 00000000000..aa9368f9dd1 --- /dev/null +++ b/pyre/bench/synth/getframe_caller_locals_after_resume.py @@ -0,0 +1,40 @@ +# pyre-check: max-pypy-ratio=17 +# pypy's exec time is pinned to the startup-subtraction floor on most runs here, +# so the ratio is not a measurement. Nine local readings across the three +# backends span 1.8x-5.5x, and the ceiling is three times the slowest of them. +# +# A guard-failure resume inside an inlined callee must close that callee's +# execution-context scope before its blackhole advances to the caller +# (`executioncontext.py:91-107` leave). The blackhole run loop transfers the +# callee's return value and releases its interpreter, but releasing a +# BlackholeInterpreter does not restore `topframeref` on its own, so without +# the leave transition the completed callee stays the current frame. +# +# Two things then go wrong, and this fixture asserts both because either can +# hold while the other breaks: +# 1. a later `sys._getframe(1)` chains behind the stale callee and reads that +# callee's sparse locals image, so `f_locals['base']` raises KeyError; +# 2. the caller's own `locals()` selects the stale frame and answers with the +# callee's parameter set — no `sys._getframe` involved at the read. +# +# The first triggering call is correct either way: the damage is only +# observable once the resumed callee should have left. A single triggering +# call therefore passes with or without the fix. +import sys + + +def inner(k): + if k > 2997: # two triggering calls, not one + return sys._getframe(1).f_locals['base'] + return k + + +def outer(n): + base = 11 + acc = 0 + for i in range(n): + acc += inner(i) & 7 + return acc, sorted(locals().keys()) + + +print(outer(3000)) diff --git a/pyre/bench/synth/getframe_caller_locals_after_resume.wasm.jitstats b/pyre/bench/synth/getframe_caller_locals_after_resume.wasm.jitstats new file mode 100644 index 00000000000..49eee67988f --- /dev/null +++ b/pyre/bench/synth/getframe_caller_locals_after_resume.wasm.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +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 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=1 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats index b0b125c96eb..144b9884a47 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=1345 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats index b0b125c96eb..144b9884a47 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=1345 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats index b0b125c96eb..144b9884a47 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=1345 diff --git a/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats b/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats index 98787e008ab..c39598a6948 100644 --- a/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats +++ b/pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats @@ -7,5 +7,5 @@ field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=339 internal_compile_panics=0 -loops_aborted=13 -loops_compiled=67 +loops_aborted=14 +loops_compiled=66 diff --git a/pyre/bench/synth/retrace_accumulator_type_flip.cranelift.jitstats b/pyre/bench/synth/retrace_accumulator_type_flip.cranelift.jitstats index 8b03a4234c3..4f48a8d03c9 100644 --- a/pyre/bench/synth/retrace_accumulator_type_flip.cranelift.jitstats +++ b/pyre/bench/synth/retrace_accumulator_type_flip.cranelift.jitstats @@ -2,10 +2,14 @@ bridges_compiled=1 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=1202 +guard_failures=202 internal_compile_panics=0 -loops_aborted=5 +loops_aborted=0 loops_compiled=1 +retraces_compiled=1 diff --git a/pyre/bench/synth/retrace_accumulator_type_flip.dynasm.jitstats b/pyre/bench/synth/retrace_accumulator_type_flip.dynasm.jitstats index 8b03a4234c3..4f48a8d03c9 100644 --- a/pyre/bench/synth/retrace_accumulator_type_flip.dynasm.jitstats +++ b/pyre/bench/synth/retrace_accumulator_type_flip.dynasm.jitstats @@ -2,10 +2,14 @@ bridges_compiled=1 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=1202 +guard_failures=202 internal_compile_panics=0 -loops_aborted=5 +loops_aborted=0 loops_compiled=1 +retraces_compiled=1 diff --git a/pyre/bench/synth/retrace_accumulator_type_flip.py b/pyre/bench/synth/retrace_accumulator_type_flip.py index 2c0dead920c..6afd0e805f2 100644 --- a/pyre/bench/synth/retrace_accumulator_type_flip.py +++ b/pyre/bench/synth/retrace_accumulator_type_flip.py @@ -5,13 +5,18 @@ # one be BUILT: it defaults to 0 (`rpython/rlib/jit.py:595`), so no other fixture # in this corpus reaches `compile_retrace` at all. # -# What this currently covers is the retrace path up to and including its give-up: -# `retrace_needed` -> `cut_retrace_from` -> the unroll pass -> `jump_to_preamble` -# (`unroll.py:156/171`) -> the `compile.py:334` arity give-up. The retrace is not -# assembled, because pyre's start label is the portal entry contract while the -# optimized loop-carried set is wider — see MC_DIAG slot 57. When that contract -# is unified this fixture is what starts exercising a compiled retrace, and its -# jit-stats row is what will say so. +# It covers a retrace that is ASSEMBLED: `retrace_needed` -> `cut_retrace_from` +# -> the unroll pass -> the fresh target token, which the closing JUMP then +# matches. `loops_aborted` in the recorded jit-stats is what says so — it read 5 +# while every attempt gave up, and reads 0 now. +# +# The closure crosses bytecode offsets (the guard resumes deeper than the loop +# header it closes onto), so `close_loop_args_at` has to publish the merge +# point's own static stack depth rather than the depth the resumed frame still +# advertises. Publishing the stale deeper one pins a loop-VARIANT +# `valuestackdepth` as a constant, and the retrace then fails to match even its +# own label; the fallback is `jump_to_preamble` (`unroll.py:156/171`), which +# pyre refuses on an arity mismatch — MC_DIAG slot 57. # # CPython (the oracle) has no `pypyjit`; PyPy and pyre do. Guarding the import # keeps the output identical across all three while the param only binds where a diff --git a/pyre/bench/synth/retrace_accumulator_type_flip.wasm.jitstats b/pyre/bench/synth/retrace_accumulator_type_flip.wasm.jitstats index 8b03a4234c3..4f48a8d03c9 100644 --- a/pyre/bench/synth/retrace_accumulator_type_flip.wasm.jitstats +++ b/pyre/bench/synth/retrace_accumulator_type_flip.wasm.jitstats @@ -2,10 +2,14 @@ bridges_compiled=1 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=1202 +guard_failures=202 internal_compile_panics=0 -loops_aborted=5 +loops_aborted=0 loops_compiled=1 +retraces_compiled=1 diff --git a/pyre/bench/synth/str_search_index_bounds.cranelift.jitstats b/pyre/bench/synth/str_search_index_bounds.cranelift.jitstats index 5e4c371731b..b23794e0132 100644 --- a/pyre/bench/synth/str_search_index_bounds.cranelift.jitstats +++ b/pyre/bench/synth/str_search_index_bounds.cranelift.jitstats @@ -2,10 +2,7 @@ bridges_compiled=7 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_rolled_back_with_effects=0 -field_pos_attached_misplaced=0 -field_pos_spec_misplaced=0 -guard_failures=2096 +guard_failures=1897 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/str_search_index_bounds.dynasm.jitstats b/pyre/bench/synth/str_search_index_bounds.dynasm.jitstats index 5e4c371731b..b23794e0132 100644 --- a/pyre/bench/synth/str_search_index_bounds.dynasm.jitstats +++ b/pyre/bench/synth/str_search_index_bounds.dynasm.jitstats @@ -2,10 +2,7 @@ bridges_compiled=7 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_rolled_back_with_effects=0 -field_pos_attached_misplaced=0 -field_pos_spec_misplaced=0 -guard_failures=2096 +guard_failures=1897 internal_compile_panics=0 -loops_aborted=1 +loops_aborted=0 loops_compiled=5 diff --git a/pyre/bench/synth/type_dict_surrogate.py b/pyre/bench/synth/type_dict_surrogate.py index ab2a4951649..5cc17bc2e97 100644 --- a/pyre/bench/synth/type_dict_surrogate.py +++ b/pyre/bench/synth/type_dict_surrogate.py @@ -1,7 +1,11 @@ -# pyre-check: max-pypy-ratio=350 +# pyre-check: max-pypy-ratio=21 # pypy's exec time is pinned to the startup-subtraction floor here, so the -# ratio is not a measurement: the ceiling is the one recorded before the -# tightening, four times the slowest the CI runners observe (84.7x). +# ratio is not a measurement. The ceiling was 350 before the type-attribute +# fold: four times the slowest of an unfolded CI band of 21.5x-84.7x. Folded, +# twelve local readings across the three backends span 3.4x-7.0x, and the +# ceiling is three times the slowest of them. The unfolded band reached 2.13x +# the unfolded local reading, which puts a folded CI worst case near 15x, so +# the ceiling carries roughly 1.4x of headroom over it. N = 200000 S = '\udcff' # lone low surrogate diff --git a/pyre/check.py b/pyre/check.py index b32a5051815..59969667b15 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -831,7 +831,7 @@ def _parse_jit_stats(snapshot): "fbw_store_journal_rollback_failed", ) -# The three count-valued counters, and what a move in either direction means: +# The count-valued counters, and what a move in either direction means: # # * `guard_failures` counts every guard failure that re-enters the metainterp, # so it is what moves when a compiled loop's guards start failing more often — @@ -853,6 +853,10 @@ def _parse_jit_stats(snapshot): # under ordinary tuning", which made a bridge collapse (27 -> 0) invisible for # as long as `guard_failures` stayed inside its band — the whole dead-bridge # class this suite exists to catch. +# * `retraces_compiled` records that a requested retrace was assembled and +# attached. A fall means the retrace path stopped producing an artifact; +# without this positive signal, `loops_aborted` can hold at zero and the +# accompanying fall in `guard_failures` looks like an improvement. # * `fbw_blackhole_adopted_single_frame` and # `fbw_blackhole_adopted_multi_frame` count successful full-body-walk # blackhole adoptions: the walk handed the interpreter a resumable image @@ -887,6 +891,7 @@ def _parse_jit_stats(snapshot): JITSTATS_SNAPSHOT_FIELDS = JITSTATS_BADNESS_FIELDS + ( "loops_compiled", "bridges_compiled", + "retraces_compiled", "guard_failures", "fbw_blackhole_adopted_single_frame", "fbw_blackhole_adopted_multi_frame", @@ -903,11 +908,14 @@ def _parse_jit_stats(snapshot): # `loops_compiled` is inverted against the badness fields: it is the counter # that falls when the tracer stops admitting a frame at all, which aborts # nothing and *lowers* `guard_failures`, so a fall is the regression and a rise -# is the gain. The blackhole adoption counters are inverted the same way: a -# fall means the interpreter stopped receiving an image and went back to replay. +# is the gain. `retraces_compiled` has the same polarity: a fall means an +# assembled retrace stopped being attached. The blackhole adoption counters are +# inverted the same way: a fall means the interpreter stopped receiving an +# image and went back to replay. JITSTATS_REGRESSION_ON_RISE = JITSTATS_BADNESS_FIELDS + ("guard_failures",) JITSTATS_REGRESSION_ON_FALL = ( "loops_compiled", + "retraces_compiled", "fbw_blackhole_adopted_single_frame", "fbw_blackhole_adopted_multi_frame", ) diff --git a/pyre/extra_tests/parity_tests/builtin_new_argument_count.py b/pyre/extra_tests/parity_tests/builtin_new_argument_count.py new file mode 100644 index 00000000000..d450fafe3a3 --- /dev/null +++ b/pyre/extra_tests/parity_tests/builtin_new_argument_count.py @@ -0,0 +1,118 @@ +"""`type.__new__` and `bool.__new__` decide on the argument count. + +`typeobject.py:886-911 descr__new__` receives the metatype as its own gateway +parameter and the rest as `__args__`, so the one-versus-three form is settled by +`len(__args__.arguments_w)` before any argument is read. `_precheck_for_new` +(`typeobject.py:1001-1003`) then refuses a non-type metatype, and the metaclass +that survives `_calculate_metaclass` still has to pass +`W_TypeObject.check_user_subclass` (`typeobject.py:555-567`) on the way through +`allocate_instance`. + +The wordings differ between implementations, so only the exception type is +asserted here; each refusal below is one the reference raises too. +""" + +import sys + + +def raises_type_error(label, fn): + try: + result = fn() + except TypeError: + return + raise AssertionError(f"{label} returned {result!r} instead of raising TypeError") + + +# A non-type metatype is refused, not used to build a class. +raises_type_error("type.__new__(42, ...)", lambda: type.__new__(42, "A", (), {})) +raises_type_error("type.__new__(None, ...)", lambda: type.__new__(None, "A", (), {})) +raises_type_error("type.__new__('s', ...)", lambda: type.__new__("s", "A", (), {})) + +# A type that is not a subtype of `type` is refused after metaclass calculation. +raises_type_error("type.__new__(int, ...)", lambda: type.__new__(int, "A", (), {})) +raises_type_error("type.__new__(str, ...)", lambda: type.__new__(str, "A", (), {})) + +# Neither one nor three arguments behind the metatype. +raises_type_error("type.__new__()", lambda: type.__new__()) +raises_type_error("type.__new__(type)", lambda: type.__new__(type)) +raises_type_error("type.__new__(42, 'A', ())", lambda: type.__new__(42, "A", ())) +raises_type_error("type.__new__(type, 'A', ())", lambda: type.__new__(type, "A", ())) + +# Three arguments, so the count is settled and the name is what gets reported. +raises_type_error("type(1, (), {})", lambda: type(1, (), {})) +raises_type_error("type('A', [], {})", lambda: type("A", [], {})) +raises_type_error("type('A', (), [])", lambda: type("A", (), [])) + +class Meta(type): + pass + + +# The one-argument form belongs to `type` alone; every other metatype names only +# the three-argument one. The reference dropped the form altogether and refuses +# both, so the accepting half is asserted off it. +raises_type_error("type.__new__(Meta, 1)", lambda: type.__new__(Meta, 1)) +if sys.implementation.name != "cpython": + assert type.__new__(type, 1) is int + assert type.__new__(type, "s") is str + +# The three-argument form still builds a class, through `type` and a metaclass. +built = type("Built", (), {"x": 1}) +assert built.__name__ == "Built", built.__name__ +assert built.x == 1 +assert type(built) is type + +metabuilt = Meta("MetaBuilt", (), {}) +assert type(metabuilt) is Meta + + +class ViaSuper(type): + def __new__(mcls, name, bases, namespace): + return super().__new__(mcls, name, bases, namespace) + + +class UsesSuper(metaclass=ViaSuper): + pass + + +assert type(UsesSuper) is ViaSuper +assert UsesSuper.__name__ == "UsesSuper" + +# Heap types the interpreter builds for itself go through the same entry. +import ast # noqa: E402 + +assert isinstance(ast.parse("1"), ast.Module) + +# `bool.__new__` counts the class argument along with the value. +raises_type_error("bool(1, 2)", lambda: bool(1, 2)) +raises_type_error("bool(1, 2, 3)", lambda: bool(1, 2, 3)) +raises_type_error("bool(x=1)", lambda: bool(x=1)) +assert bool() is False +assert bool(1) is True + +if sys.implementation.name != "cpython": + # `descr__new__` names both accepted counts for `type` itself and only the + # three-argument form for any other metatype, and `_precheck_for_new` runs + # after that decision, so an unvalidated metatype reaches `%N`. + def message(fn): + try: + fn() + except TypeError as exc: + return str(exc) + raise AssertionError("expected TypeError") + + assert message(lambda: type.__new__(type)) == "type.__new__() takes 1 or 3 arguments" + assert ( + message(lambda: type.__new__(42, "A", ())) + == "?.__new__() takes exactly 3 arguments (1 given)" + ) + assert message(lambda: type.__new__(42, "A", (), {})) == "X is not a type object (int)" + assert ( + message(lambda: type.__new__(int, "A", (), {})) + == "type.__new__(int): int is not a subtype of type" + ) + assert ( + message(lambda: bool(1, 2)) + == "bool.__new__() takes from 1 to 2 positional arguments but 3 were given" + ) + +print("OK") diff --git a/pyre/extra_tests/parity_tests/str_padding_fill_operand.py b/pyre/extra_tests/parity_tests/str_padding_fill_operand.py new file mode 100644 index 00000000000..adb2a6a6f2c --- /dev/null +++ b/pyre/extra_tests/parity_tests/str_padding_fill_operand.py @@ -0,0 +1,69 @@ +"""`str.ljust` / `str.rjust` / `str.center` convert their fill operand differently. + +`descr_center` (`unicodeobject.py:1099-1101`) reads the operand with +`space.utf8_w`, which takes a `str` and nothing else. `descr_ljust` and +`descr_rjust` (`unicodeobject.py:1352,1371`) go through +`convert_arg_to_w_unicode` (`unicodeobject.py:175-184`) instead: it declines +`bytes` with its own wording and hands anything else to `decode_object` +(`unicodeobject.py:1727-1739`), which reads the operand as a buffer and decodes +it, so a `bytearray` or `memoryview` becomes a fill character rather than a +refusal. The length check that follows applies to the decoded result. + +The reference refuses every non-`str` operand, so the two buffer rows are +asserted only off it. +""" + +import sys + + +def raises_type_error(label, fn): + try: + result = fn() + except TypeError: + return + raise AssertionError(f"{label} returned {result!r} instead of raising TypeError") + + +assert "x".ljust(5, "-") == "x----" +assert "x".rjust(5, "-") == "----x" +assert "x".center(5, "-") == "--x--" +assert "x".ljust(5) == "x " +assert "abc".ljust(2, "-") == "abc" + +# A multi-character fill is refused by length, whatever its type. +raises_type_error("'x'.ljust(5, '--')", lambda: "x".ljust(5, "--")) +raises_type_error("'x'.rjust(5, '')", lambda: "x".rjust(5, "")) +raises_type_error("'x'.center(5, '--')", lambda: "x".center(5, "--")) + +# `bytes` is declined by name before any decode is attempted. +raises_type_error("'x'.ljust(5, b'-')", lambda: "x".ljust(5, b"-")) +raises_type_error("'x'.rjust(5, b'-')", lambda: "x".rjust(5, b"-")) + +# A non-buffer operand has nothing to decode. +raises_type_error("'x'.ljust(5, None)", lambda: "x".ljust(5, None)) +raises_type_error("'x'.rjust(5, 1)", lambda: "x".rjust(5, 1)) + +# `center` reads its operand strictly, so a buffer is refused there. +raises_type_error("'x'.center(5, bytearray(b'-'))", lambda: "x".center(5, bytearray(b"-"))) +raises_type_error("'x'.center(5, memoryview(b'-'))", lambda: "x".center(5, memoryview(b"-"))) + +if sys.implementation.name == "cpython": + raises_type_error("'x'.ljust(5, bytearray(b'-'))", lambda: "x".ljust(5, bytearray(b"-"))) + raises_type_error("'x'.rjust(5, memoryview(b'-'))", lambda: "x".rjust(5, memoryview(b"-"))) +else: + assert "x".ljust(5, bytearray(b"-")) == "x----" + assert "x".rjust(5, memoryview(b"-")) == "----x" + assert "x".ljust(5, bytearray("é", "utf-8")) == "xéééé" + # The decoded operand still has to be a single character. + raises_type_error( + "'x'.ljust(5, bytearray(b'--'))", lambda: "x".ljust(5, bytearray(b"--")) + ) + # The decode is strict, so a buffer that is not valid UTF-8 raises from it. + try: + "x".ljust(5, bytearray(b"\xff")) + except UnicodeDecodeError: + pass + else: + raise AssertionError("'x'.ljust(5, bytearray(b'\\xff')) did not raise") + +print("OK") diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index d04242c5cef..4ed1d771ba8 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -81,49 +81,90 @@ here as "S0 seam of the vable-owner rework toward `direct_assembler_call` scalar args" — **that judgement was wrong and is reversed in §1d**: the 2026-07-25 parity pass read `direct_assembler_call` and found its ON design is what upstream's `num_red_args` assert forbids. Retired. -Still kept: `PYRE_CARRIER_EXC_RESUME` (default-off; threads the guard-failure exception -into the bridge sym for the depth-2 carrier exception-resume slice #343/#126 — -inert until validated). Two parity gaps were listed here as pre-flip work. The -`bridge_guard_exc` GC-rooting is closed by §1e — which also measures the seed -site as **reachable** (170 bridge-route guard failures carry a live exception), -so this gate is a live adoption target rather than an inert one. - -The second is still open, and the row described it as "the unconditional -`execute_ll_raised` exception assign", which is not what the divergence is. - -pyre's standing-exception maintenance for an exception-guard bridge lives in -`seed_bridge_standing_exception_from_current` (`state.rs`), which is **not -gated** and already mirrors upstream's branch: it assigns `last_exc_value` / -`last_exc_box` when it finds an exception, and clears all four exception slots -when it does not (`_prepare_exception_resumption`'s -`else: clear_exception()`). The divergence is the **source**. Upstream takes it -from `cpu.grab_exc_value(deadframe)` — the exception the failing guard carried. -pyre takes it from `sym.current_exc_value`, falling back to -`get_current_exception()` — the *execution context's* current exception, which is -the `sys.exc_info()` mirror, a different slot with different lifetime rules. - -`PYRE_CARRIER_EXC_RESUME` is a **back-channel into that function**: its only -effect is to write `guard_exc` into `current_exc_value` beforehand so the ungated -code picks it up. Hence the `is_null` conjunct — it exists to avoid clobbering a -live `sys.exc_info` value, which also means the injection is suppressed exactly -when the EC already holds an exception. That is why forcing the gate on measures -as a no-op: **dynasm 336/336 with the gate forced on**, correctness results -matching the default run, and the seven live-exception producers of §1e among -them, despite the seed site being entered 170 times. - -So "inert until validated" should read **inert because the guard's exception -reaches `last_exc_value` only through a slot it does not belong in**. A green -corpus under the gate is not evidence about the gate. Two further deltas to -settle before any flip, both in that function: it early-returns when -`last_exc_box` is already set, and it sets `class_of_last_exc_is_const = true`, -whereas the `_prepare_exception_resumption` path reaches `execute_ll_raised` with -the default `constant=False`. +**`PYRE_CARRIER_EXC_RESUME` — RETIRED 2026-08-06, ON path deleted.** It was kept +here as "default-off; threads the guard-failure exception into the bridge sym for +the depth-2 carrier exception-resume slice #343/#126 — inert until validated", +with two parity gaps booked as pre-flip work, and was then upgraded to "a live +adoption target rather than an inert one" on the strength of §1e's reachability +census. **That upgrade was wrong and is reversed here.** §1e instruments +`handle_fail`, one layer above the seed site; the seed additionally required +`sym.current_exc_value.is_null()`, and — decisively — its write is discarded by +the walk-start seed. The site is reachable AND the gate was inert. Reachability +was never the question it failed. + +The account this replaces ran: pyre's standing-exception maintenance for an +exception-guard bridge lives in `seed_bridge_standing_exception_from_current` +(`state.rs`), not gated, sourcing the exception from `sym.current_exc_value` +falling back to `get_current_exception()` — the *execution context's* current +exception, the `sys.exc_info()` mirror, a different slot with different lifetime +rules than upstream's `cpu.grab_exc_value(deadframe)` — so the gate's only effect +was to write `guard_exc` into `current_exc_value` beforehand and let the ungated +code pick it up. **That was already only half true on the day it was written.** +`seed_standing_exception_for_walk` (`jitcode_dispatch/mod.rs`, called at walk +start from `bridge_subwalk.rs`) already had its present shape: it runs AFTER +`setup_bridge_sym` and sources from `BH_LAST_EXC_VALUE`, which +`trace_and_compile_from_bridge` (`call_jit.rs`) publishes from +`cpu.grab_exc_value`'s result on the exc-edge route, zeroes when the guard +carried no exception, and whose third combination (`pending_exc && +!route_exc_edge`) declines before any walk. For an exception-guard bridge that +function returns from one of its first two arms in every case — overwriting all +five exception slots on a non-null read, clearing them on a null one — so it +never reaches its own `last_exc_box` short-circuit on that flavour. On the +single-frame walk the pre-seed's only possible effect was to write a pointer the +walk seed then rewrote, and the source divergence named above is closed. + +Measured 2026-08-06 with a probe at BOTH seed sites across 369 synth benches: 23 +exception-guard bridges in 14 benches, 7 of which the gate would have seeded, and +in all 23 the value at the walk seed equalled the value at the setup seed, +pointer for pointer (the non-seeding cases read `0x0` at both). +`PYRE_CARRIER_EXC_RESUME=1` over the whole `bench/synth` corpus is dynasm +386/386, byte-identical to gate-off, with zero jit-stats movement — agreeing with +the earlier corpus run under the gate (**dynasm 336/336 with the gate forced +on**, correctness results matching the default run, and the seven live-exception +producers of §1e among them, despite the seed site being entered 170 times). + +"A green corpus under the gate is not evidence about the gate" still stands, and +it is why the 2026-08-06 evidence is the seed-site probe rather than the corpus: +the corpus could only ever show the no-op, never its cause. It also revises the +cause inferred here. The `is_null` conjunct was read as suppressing the +injection exactly when the EC already holds an exception; 7 of the 23 were not +suppressed at all, and the value they would have seeded is the value the walk +seed applies regardless. Inert by redundancy, not by suppression. + +**Scope of that redundancy, and what outlives the gate.** It is a property of +the `dispatch_via_miframe` leg. The multi-frame carrier leg does NOT run the +walk seed: `setup_bridge_sym` installs the inline carrier whenever +`resume_data.frames.len() > 1`, `trace_bytecode` returns through +`drive_bridge_carrier_walk` before the full-body-walk leg, and +`drive_bridge_frame_subwalk` seeds its sub-walk's `current_exception_seed` and +`class_of_last_exc_is_const` straight off `root_sym.last_exc_box()` — i.e. off +`seed_bridge_standing_exception_from_current`, with no `BH_LAST_EXC_VALUE` reader +anywhere on that leg. So for a multi-frame exception-guard bridge (only the +`unwind_to_live_frame` shape survives `call_jit.rs`'s pre-walk decline) both +original complaints are still live: the `sys.exc_info()` mirror as the source, +and the early return when `last_exc_box` is already set, neither of which +`_prepare_exception_resumption` has. That combination is unexercised by +`bench/synth` — the probe paired 23 for 23, so every exception-guard bridge in +the corpus took the single-frame leg — which is why the gate was never validated +and is why it is retired rather than flipped. If the slice is built, its seed +must come from `BH_LAST_EXC_VALUE`, the `grab_exc_value` source, not from +`current_exc_value`. + +The other pre-flip delta is **withdrawn as miscited**: it compared an +intermediate value to pyre's final one. `prepare_resume_from_failure` calls +`execute_ll_raised` with the default `constant=False`, then +`handle_possible_exception` three lines later (pyjitpl.py:3169), which ends +`self.class_of_last_exc_is_const = True` (pyjitpl.py:3416). Upstream's +post-resumption steady state is `True`, the same as pyre's. ## §1e — The grabbed guard exception is rooted for the whole handoff (2026-07-27) `bridge_guard_exc` was booked as a pre-flip gap for `PYRE_CARRIER_EXC_RESUME`. It is **not gate-specific**: the same grabbed pointer drives the default -blackhole resume, so the gate never bounded the exposure. +blackhole resume, so the gate never bounded the exposure. (That gate is retired +— §1b — and the `TraceCtx::bridge_guard_exc` carrier it was threaded through went +with it. This rooting did not: its three parking sites are on the blackhole and +guard-failure paths, none of them the deleted one.) `grab_exc_value` (`llmodel.py:240`) reads `jf_guard_exc` off the deadframe and drops the jitframe, which was the collector's only handle on the exception @@ -153,9 +194,14 @@ Instrumenting `handle_fail` counted **732,660** guard failures: | **NON-NULL** | **true** | **true** | **170** | So the window is entered with a live exception **34,790** times, and the 170 in -the last row are exactly the `bridge_guard_exc` read this section is about — the -`PYRE_CARRIER_EXC_RESUME` seed site is **reachable**, not inert. Seven benches -produce them: `inline_subwalk_property_mutates` and +the last row are the bridge-route guard failures this section is about — the +grabbed value `call_jit.rs` publishes into `BH_LAST_EXC_VALUE`. This read +"reachable, not inert" for the `PYRE_CARRIER_EXC_RESUME` seed site; **that +inference was wrong**. The instrumentation is in `handle_fail`, one layer above +that seed, which additionally required a null `current_exc_value` and whose write +the walk-start seed discards. The site was reachable AND the gate inert — see +§1b, retired 2026-08-06. Seven benches produce them: +`inline_subwalk_property_mutates` and `inline_subwalk_mutating_residual_abort` (11,482 each), `type_name_surrogate_reject` (9,462), `named_reraise_sibling_hot` (1,418), `exc_mixed_classes_bridge_flavor` (410), `handler_reraise_second_exc` (400), @@ -738,7 +784,7 @@ delete, do not count as gates.** - `PYRE_JIT_DISABLED` — a `OnceLock` cache name holding the `PYRE_JIT==0` result (`pyre-jit/src/eval.rs`); the env var is `PYRE_JIT` - `PYRE_STACKTOOBIG` — `pub static PyreStackTooBig` runtime symbol (`stack_check.rs`) -## §3 — Dead (12): no env read site +## §3 — Dead (13): no env read site No source reads these. Comment-only or absent. **Historical measurement notes are preserved in place per N7** (they record why code was deemed dead / what a @@ -758,6 +804,7 @@ census verified); they are not live gates and cost nothing. | PYRE_FULL_BODY_WALK | retired switch; the full-body walk is the sole tracer, so the OFF path (the deleted trait leg) is gone (#344) | | `_MULTIFRAME` | retired switch; reader and OFF path deleted once `walker_ec_enter` / `walker_ec_leave` closed the escaping-`sys._getframe` identity answer (§1d). Flipped default-ON and retired 2026-07-30; `_MULTIFRAME_DEPTH` is a separate live depth bound and is not this gate | | `_BLACKHOLE_RESUME` | retired switch; reader and OFF path deleted after #754 closed, with the multi-frame twin's retirement unblocking removal; it was flipped default-ON on 2026-07-25 | +| `PYRE_CARRIER_EXC_RESUME` | retired experiment; reader (`carrier_exc_resume_enabled`), the `setup_bridge_sym` pre-seed it guarded, `TraceCtx::bridge_guard_exc` and the `guard_exc` parameter of `start_bridge_tracing` all deleted 2026-08-06. The ON path measured inert — structurally redundant with the ungated walk-start `seed_standing_exception_for_walk` on the single-frame leg, and never exercised on the multi-frame carrier leg it was written for. §1b keeps the seed-site probe and both corpus runs | ## §4 — Live default-ON gates KEPT (retire when the epic closes) @@ -790,12 +837,13 @@ Kept as-is; listed for completeness. `_GIN`, `_INLINE_RECOG`, `PYRE_WASM_DUMP_ALL_TRACES`, `_DUMP_BAD_TRACE`, `_EXEC_TRACE`, `_JIT_STATS`, `PYRE_INTERP_RETURN_LOG`, `PYRE_NBODY_DEBUG`, `PYRE_DEBUG_CALL`, `PYRE_DEBUG_CLASS`. -- **Default-OFF experiments (2 remaining)** — triaged in §1b/§1c (4 retired - in the 2026-07-05 pass, 8 retired since then; `PYRE_P2_DRAIN` retired with +- **Default-OFF experiments (1 remaining)** — triaged in §1b/§1c (4 retired + in the 2026-07-05 pass, 9 retired since then; `PYRE_P2_DRAIN` retired with the framestack-walk deletion; `_VABLE_SCALAR_CA` retired 2026-07-25, see - §1d). Kept: `_CALLEE_VSTACK` (callee-local operand-stack mirror) and - `PYRE_CARRIER_EXC_RESUME`. For these the *ON* path is the unattested one, so - they are adoption targets rather than retirement targets. + §1d; `PYRE_CARRIER_EXC_RESUME` retired 2026-08-06 with its ON path deleted, + see §1b). Kept: `_CALLEE_VSTACK` (callee-local operand-stack mirror). Its + *ON* path is the unattested one, so it is an adoption target rather than a + retirement target. The single-frame resume-past-escape switch graduated out of this bucket on 2026-07-25 when it flipped default-ON. It is now retired alongside the multi-frame switch, with both readers and OFF paths deleted after diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 6fefa53698a..80ebe23a361 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -9636,6 +9636,58 @@ pub unsafe fn type_name_obj_fast_path(w_obj: PyObjectRef) -> Option<(PyObjectRef (!w_name.is_null()).then_some((metatype, w_name)) } +/// `typeobject.py:811-828 W_TypeObject.descr_getattribute` fast path for the +/// exact shape that returns a value from the class namespace unchanged. The +/// receiver must be a cacheable type whose metaclass is exactly `type`; a +/// metatype data descriptor, a missing class-MRO value, or any value with a +/// descriptor protocol declines. +/// +/// The value type must additionally be a non-heap builtin type. Its namespace +/// cannot be mutated from Python and it cannot be the target of a `__class__` +/// assignment, so an absent `__get__` remains absent for the life of the trace. +/// Consequently the fold needs only the receiver type's one version pin. +/// +/// # Safety +/// `w_obj` must be a valid object pointer (null tolerated). +pub unsafe fn type_attr_value_fast_path( + w_obj: PyObjectRef, + name: &Wtf8, +) -> Option<(PyObjectRef, u64, PyObjectRef)> { + if w_obj.is_null() || !pyre_object::typeobject::is_type(w_obj) { + return None; + } + let w_type = w_obj; + // `is_type` answers for the object's physical layout — every type object + // carries the same `ob_type` — so it says nothing about the metaclass. + // `getclass()` (baseobjspace.py) reads the metaclass off `w_class`; only + // `type` itself resolves the name through `type.__getattribute__`, so any + // other metaclass declines rather than have its `__getattribute__` + // override bypassed. + let metatype = crate::typedef::r#type(w_obj)?.as_ptr(); + if !std::ptr::eq(metatype, crate::typedef::w_type()) { + return None; + } + let version_tag = pyre_object::typeobject::w_type_get_version_tag(w_type); + if version_tag == 0 { + return None; + } + // typeobject.py:814-823: a metatype data descriptor preempts the class's + // own MRO, while a non-data metatype entry loses to the class value. + if lookup_in_type_wtf8(metatype, name).is_some_and(|descr| is_data_descr(descr)) { + return None; + } + let w_value = lookup_in_type_wtf8(w_type, name)?; + // typeobject.py:822 calls `space.get(w_value, w_None, self)`. Only a + // value with no descriptor protocol is returned unchanged. + let value_type = crate::typedef::r#type(w_value)?.as_ptr(); + if lookup_in_type(value_type, "__get__").is_some() + || pyre_object::w_type_is_heaptype(value_type) + { + return None; + } + Some((w_type, version_tag, w_value)) +} + /// `callmethod.py`'s `w_obj.getdictvalue(space, name)` shadowing check, /// restricted to a probe that neither allocates nor runs Python. /// diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index f285ad341cf..82162d3f7ac 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -4045,6 +4045,13 @@ pub fn is_builtin_len_function(callable: PyObjectRef) -> bool { } } +/// True iff `callable` is the builtin `getattr` function object — a +/// builtin-code function whose code wraps [`builtin_getattr`]. The JIT +/// walker uses this to recognize a plain `getattr(type, name)` residual. +pub fn is_builtin_getattr_function(callable: PyObjectRef) -> bool { + is_builtin_code_function(callable, builtin_getattr) +} + /// True iff `callable` is the builtin `locals` function object. /// /// The JIT walker uses this to recognize the `locals()` residual it can @@ -4128,6 +4135,25 @@ pub fn is_builtin_hash_function(callable: PyObjectRef) -> bool { } } +/// True iff `callable` is the canonical builtin `ord` function object. +/// Keep the wrapped-code identity test beside `len` / `repr`: mutable builtin +/// globals and user-visible function names are not specialization evidence. +pub fn is_builtin_ord_function(callable: PyObjectRef) -> bool { + unsafe { + if callable.is_null() || !crate::is_function(callable) { + return false; + } + let code = crate::function_get_code(callable) as PyObjectRef; + if code.is_null() || !crate::gateway::is_builtin_code(code) { + return false; + } + crate::gateway::builtin_code_fn_eq( + crate::gateway::builtin_code_get(code), + builtin_ord as crate::gateway::BuiltinCodeFn, + ) + } +} + /// `len(obj)` — return the length of an object. /// `len(obj)` — PyPy: operation.py len → space.len_w fn builtin_len(args: &[PyObjectRef]) -> Result { @@ -4897,65 +4923,49 @@ fn min_max_multiple_args( Ok(pyre_object::gc_roots::shadow_stack_get(best_item_slot)) } -/// `type(obj)` — return the type name as a string (simplified). -/// `type(obj)` — return the type of an object as a W_TypeObject. +/// typeobject.py:886 `descr__new__` — `type.__new__(metatype, name, bases, dict)` +/// and its one-argument form `type(obj)`. /// -/// PyPy: `space.type(w_obj)` → W_TypeObject +/// `descr__new__(space, w_typetype, __args__)` takes the metatype as its own +/// gateway parameter and everything behind it as `__args__`, so `pos[0]` is the +/// metatype and `pos[1..]` is `arguments_w`. A bound `__new__` does not prepend +/// its `__self__`, on the direct path or through `super()`, so the split is the +/// same however the call arrives. pub(crate) fn type_descr_new(args: &[PyObjectRef]) -> Result { - // type.__new__(metatype, name, bases, dict) - // May be called with extra self-binding from super(): - // [self, metatype, name, bases, dict] — 5 args - // [metatype, name, bases, dict] — 4 args - // [metatype, obj] — 2 args (type(obj)) - // Find the (name, bases, dict) triple by scanning for the first str arg. - // Also extract the metatype (first type arg before the name str). // The class-definition keywords arrive as a trailing `__pyre_kw__` - // dict (the builtin kwargs ABI); strip it before the arity scan and + // dict (the builtin kwargs ABI); strip it before the arity check and // hand it to __init_subclass__ via `type_descr_new_with_metaclass`. let (pos, kwargs) = split_builtin_kwargs(args); - let mut w_metaclass = pyre_object::PY_NULL; - for i in 0..pos.len() { - if unsafe { pyre_object::is_str(pos[i]) } && i + 2 < pos.len() { - // Extract metatype from preceding args - for j in 0..i { - if unsafe { pyre_object::is_type(pos[j]) } { - w_metaclass = pos[j]; - } - } - return type_descr_new_with_metaclass(&pos[i..], w_metaclass, kwargs); - } + // The gateway supplies `w_typetype` from a declared parameter, so upstream + // never sees this shape; pyre reads it out of the same slice and has to + // refuse it here, in `tp_new_wrapper`'s words. + if pos.is_empty() { + return Err(crate::PyError::type_error( + "type.__new__(): not enough arguments", + )); } - if pos.len() == 1 { - // `type.__new__(metatype)` — no name, bases or namespace follows, so - // `arguments_w` is empty and the count is neither one nor three. The - // arity is decided before `_precheck_for_new`, which is why a - // non-metatype is named here rather than refused as one. - return Err(crate::PyError::type_error(new_arity_message(pos[0]))); + + let w_typetype = pos[0]; + let arguments_w = &pos[1..]; + // The count decides the form before any argument is read; `w_typetype` is + // touched only to word the refusal, which is why `_precheck_for_new` runs + // after it and not before. + if arguments_w.len() != 1 && arguments_w.len() != 3 { + return Err(crate::PyError::type_error(new_arity_message(w_typetype))); } - if pos.len() == 2 { - precheck_for_new(pos[0])?; + + precheck_for_new(w_typetype)?; + if arguments_w.len() == 1 { // typeobject.py:901-908 — the one-argument form belongs to `type` // alone: `type(x)` reports the type of `x`, while `Metaclass(x)` is a // class statement missing its bases and its namespace. - if !unsafe { std::ptr::eq(pos[0], crate::typedef::w_type()) } { - return Err(crate::PyError::type_error(new_arity_message(pos[0]))); - } - return type_descr_new_without_metaclass(&pos[1..], kwargs); - } - // `descr__new__` (typeobject.py:885) keys the one-vs-three form on the - // argument *count*, and `_check_new_args` is what names an argument of - // the wrong type. The scan above keys on a str being present instead, - // so `type(1, (), {})` arrives here with its three arguments intact; - // hand that unambiguous `[metatype, name, bases, dict]` shape on so the - // name gets reported rather than the arity. - if pos.len() == 4 { - // Three arguments, so the count is settled and `_precheck_for_new` - // (typeobject.py:899) runs before either of them is read. - precheck_for_new(pos[0])?; - w_metaclass = pos[0]; - return type_descr_new_with_metaclass(&pos[1..], w_metaclass, kwargs); - } - Err(crate::PyError::type_error("type() takes 1 or 3 arguments")) + if !unsafe { std::ptr::eq(w_typetype, crate::typedef::w_type()) } { + return Err(crate::PyError::type_error(new_arity_message(w_typetype))); + } + return type_descr_new_without_metaclass(arguments_w, kwargs); + } + + type_descr_new_with_metaclass(arguments_w, w_typetype, kwargs) } /// typeobject.py:888-895 `descr__new__` — the wording for a `type.__new__` /// call whose argument count is neither one nor three. `type` itself names @@ -5383,7 +5393,7 @@ fn type_descr_new_with_metaclass( } else { bases }; - // CPython: calculate_metaclass — delegate to winner if different + // calculate_metaclass — delegate to winner if different let default_meta = if w_metaclass.is_null() { crate::typedef::w_type() } else { @@ -5392,7 +5402,14 @@ fn type_descr_new_with_metaclass( // A metaclass conflict among the bases (or an explicit metaclass that // is not a subclass of every base's metaclass) is a hard error, not a // silent fall-back to `default_meta`. - let w_winner = crate::call::calculate_metaclass(default_meta, w_effective_bases)?; + // + // `_calculate_metaclass` (typeobject.py:945) sees the bases as written. + // `(object,)` is substituted for an empty tuple only in + // `W_TypeObject.__init__` (`bases_w or [space.w_object]`), which runs + // after the winner is settled — supplying it here would weigh an + // explicit metatype against `type(object)` and report a conflict where + // the metatype itself is what upstream goes on to refuse. + let w_winner = crate::call::calculate_metaclass(default_meta, bases)?; if !std::ptr::eq(w_winner, default_meta) { // Winner is a different metaclass — delegate to its __new__ if let Some(w_metaclass_new) = @@ -5416,6 +5433,18 @@ fn type_descr_new_with_metaclass( } } let w_metaclass = w_winner; + // `_create_new_type` reaches the instance through + // `space.allocate_instance(W_TypeObject, w_typetype)`, and that runs + // `W_TypeObject.check_user_subclass` (typeobject.py:555-567) on the way + // in: the winning metatype has to be a subtype of `type` before a type + // is laid out for it. + if !unsafe { crate::baseobjspace::issubtype_w(w_metaclass, crate::typedef::w_type()) } { + let self_name = type_new_getname(crate::typedef::w_type()); + let subtype_name = type_new_getname(w_metaclass); + return Err(crate::PyError::type_error(format!( + "{self_name}.__new__({subtype_name}): {subtype_name} is not a subtype of {self_name}" + ))); + } // This is type.__new__'s own construction path. A different winning // metaclass above received the original bases without a C3 pre-check. @@ -16554,6 +16583,17 @@ fn builtin_dunder_import(args: &[PyObjectRef]) -> Result Result Result { if args.len() <= 2 { return Ok(CodePoint::from_char(' ')); } - if !unsafe { pyre_object::is_str(args[2]) } { + let decoded; + let raw = if unsafe { pyre_object::is_str(args[2]) } { + unsafe { w_str_get_wtf8(args[2]) } + } else { let type_name = arg_type_name(args[2]); - let message = if method == "center" { - format!("expected str, got {type_name} object") + if method == "center" { + return Err(crate::PyError::type_error(format!( + "expected str, got {type_name} object" + ))); } else if unsafe { pyre_object::is_bytes(args[2]) } { - format!("Can't convert '{type_name}' object to str implicitly") + return Err(crate::PyError::type_error(format!( + "Can't convert '{type_name}' object to str implicitly" + ))); } else { - let operand = if unsafe { pyre_object::is_none(args[2]) } { - "None".to_string() - } else { - format!("'{type_name}'") + let Some(buffer) = crate::baseobjspace::simple_buffer_bytes(args[2])? else { + let operand = if unsafe { pyre_object::is_none(args[2]) } { + "None".to_string() + } else { + format!("'{type_name}'") + }; + return Err(crate::PyError::type_error(format!( + "decoding to str: a bytes-like object is required, not {operand}" + ))); }; - format!("decoding to str: a bytes-like object is required, not {operand}") - }; - return Err(crate::PyError::type_error(message)); - } - let raw = unsafe { w_str_get_wtf8(args[2]) }; + let result = crate::typedef::decode_bytes_to_wtf8(buffer.as_bytes(), "utf-8", "strict"); + buffer.release(); + decoded = result?; + decoded.as_ref() + } + }; let mut iter = raw.code_points(); let first = iter.next(); if first.is_none() || iter.next().is_some() { diff --git a/pyre/pyre-interpreter/src/typedef.rs b/pyre/pyre-interpreter/src/typedef.rs index 3f0aa8e5393..1b2eebb5dea 100644 --- a/pyre/pyre-interpreter/src/typedef.rs +++ b/pyre/pyre-interpreter/src/typedef.rs @@ -3449,12 +3449,12 @@ fn bool_descr_new(args: &[PyObjectRef]) -> Result { if let Some(w_bool) = gettypefor(&pyre_object::BOOL_TYPE) { check_user_subclass(w_bool.as_ptr(), w_booltype)?; } - // boolobject.py: descr_new(space, w_booltype, w_obj) - // Takes exactly (cls) or (cls, obj). No extra args, no kwargs. + // boolobject.py:41-46 `descr_new` counts the class argument. if args.len() > 2 { - return Err(crate::PyError::type_error( - "bool expected at most 1 argument, got more", - )); + return Err(crate::PyError::type_error(format!( + "bool.__new__() takes from 1 to 2 positional arguments but {} were given", + args.len() + ))); } // args[1] = w_obj (default: False) let w_obj = args.get(1).copied().unwrap_or(pyre_object::PY_NULL); diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 97fab478dd3..14231e92ebe 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -3338,16 +3338,6 @@ pub(crate) fn try_catch_exception_at(code: &[u8], position: usize) -> Option bool { - static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); - *ENABLED.get_or_init(|| std::env::var_os("PYRE_CARRIER_EXC_RESUME").is_some()) -} - /// Mirror of `blackhole.rs BlackholeInterpreter::handle_exception_in_frame` /// for the walker: locate the `catch_exception/L` that owns an exception-guard /// resume position, forward case first, then the backward scan. @@ -4204,6 +4194,16 @@ fn seed_standing_exception_for_walk(sym: &mut Sym, trace_ctx: &mut // exception state a previous walk left on the sym. A preseeded sym is // kept only when no fresh signal exists — the multi-frame carrier walk // re-seeds per frame after the first frame drained the cell. + // For the walks that reach here this cell is the delivery path for + // `_prepare_exception_resumption`: `call_jit.rs` publishes + // `cpu.grab_exc_value`'s result before bridge tracing starts, zeroes it when + // the guard carried no exception, and declines the bridge outright in the + // third case. On an exception-guard bridge one of the two arms below always + // returns, so a pre-seed placed on the sym in `setup_bridge_sym` could only + // rewrite the pointer this read applies anyway. Scoped to this leg: the + // multi-frame carrier walk never runs this function — `trace.rs` routes it + // through `drive_bridge_carrier_walk`, whose sub-walk seeds itself off + // `root_sym.last_exc_box()` instead. let bh_exc = majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.get()); if bh_exc != 0 { let exc = bh_exc as pyre_object::PyObjectRef; diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs index 27beb1d35d5..83f65f0d582 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -2730,8 +2730,70 @@ pub(crate) fn try_execute_residual_call_via_executor( // for an elidable or loop-hoisted one. Feeds [`ESCAPE_OPCODE_WINDOW`]. let reentrant_residual = ei.check_is_elidable() || ei.extraeffect == majit_ir::ExtraEffect::LoopInvariant; - let provably_side_effect_free = - reentrant_residual || helper == majit_ir::PyreHelperKind::ForIterNext; + // PyPy traces through `str_descr_new` -> `space.str` and therefore knows + // that an exact builtin scalar cannot dispatch to user `__str__` code. + // Pyre reaches the same operation through the opaque `CallFn` helper; if + // it remains classified as arbitrary here, a loop-bearing inlined caller + // (for example `fold`'s `for ch in str(value)`) is denied at the nested + // residual and the whole bridge is thrown away once before that caller is + // learned as non-inlinable. + // + // This is deliberately an observed-value fact, not a blanket blessing of + // `str(x)`: both the callable and operand must be exact builtins in the + // concrete execution this walk may have to replay. Exact immutable scalar + // formatting allocates only its fresh result and cannot mutate live heap or + // enter a user frame, so replay has the same safety as an elidable call. + // A subclass/callable override misses pointer/type identity and keeps the + // ordinary nested-residual decline. + let observed_exact_scalar_str = + helper == majit_ir::PyreHelperKind::CallFn && args.len() == 3 && { + let callable = args[0] as pyre_object::PyObjectRef; + let operand = args[2] as pyre_object::PyObjectRef; + let str_type = + pyre_interpreter::typedef::gettypeobject(&pyre_object::pyobject::STR_TYPE); + !callable.is_null() + && !operand.is_null() + && std::ptr::eq(callable, str_type) + && unsafe { + pyre_object::is_int_or_long(operand) + || pyre_object::is_float(operand) + || pyre_object::is_complex(operand) + || pyre_object::is_str(operand) + || pyre_object::is_none(operand) + } + }; + // PyPy traces `space.iter(w_exact_unicode)` through + // `W_UnicodeObject.descr_iter` into a fresh sequence iterator. Pyre's + // tagged `GetIter` call has the same no-user-dispatch property for an + // exact string; subclasses retain the conservative decline. + // The wasm optimizer currently rejects the resulting longer trace as + // `InvalidLoop` (three compile aborts versus the conservative path's one + // trace-time decline). Keep its prior admission boundary until that + // backend can consume this shape; interpreter semantics are identical. + let native_exact_str_replay = !cfg!(target_arch = "wasm32"); + let observed_exact_str_iter = native_exact_str_replay + && helper == majit_ir::PyreHelperKind::GetIter + && args.len() == 1 + && { + let operand = args[0] as pyre_object::PyObjectRef; + !operand.is_null() && unsafe { pyre_object::is_str(operand) } + }; + // PyPy's `space.ord` reads the immutable unicode payload directly. Match + // both canonical builtin-code identity and the observed exact string so a + // rebound `ord` or a user object cannot enter this replay-safe class. + let observed_exact_str_ord = native_exact_str_replay + && helper == majit_ir::PyreHelperKind::CallFn + && args.len() == 3 + && pyre_interpreter::builtins::is_builtin_ord_function(args[0] as pyre_object::PyObjectRef) + && { + let operand = args[2] as pyre_object::PyObjectRef; + !operand.is_null() && unsafe { pyre_object::is_str(operand) } + }; + let provably_side_effect_free = reentrant_residual + || helper == majit_ir::PyreHelperKind::ForIterNext + || observed_exact_scalar_str + || observed_exact_str_iter + || observed_exact_str_ord; let writes_live_heap = call_descr.result_type() == majit_ir::Type::Void || matches!( helper, @@ -4957,28 +5019,29 @@ pub(crate) fn dispatch_residual_call_iRd_kind( return Ok((DispatchOutcome::Continue, op.next_pc)); } - // #171: virtualize a BUILD_TUPLE (`newtuple_from_array`) of any width as - // the canonical array-backed `W_TupleObject` shape, so a non-escaping tuple - // folds away rather than allocating through the opaque residual, and every - // consumer fold that reads `wrappeditems` applies to it. Falls through to - // the residual for any shape it cannot reproduce (SAFE — never declined). + // #195 / #73: virtualize an arity-2 plain-int BUILD_TUPLE + // (`newtuple_from_array`) as a `spec_ii` `new_with_vtable` + + // `value0` / `value1`, so the backing array build and the partner + // UNPACK_SEQUENCE reads DCE to a pure-int loop. Falls through to the + // opaque residual for any other shape (SAFE — never declined). if ctx.is_authoritative_executor && dst_bank == 'r' && ei.pyre_helper == majit_ir::PyreHelperKind::NewtupleFromArray - && try_walker_specialize_newtuple_object(ctx, op.pc, &r_args, dst, dst_bank)?.is_some() + && try_walker_specialize_newtuple(ctx, op.pc, &r_args, dst, dst_bank)?.is_some() { return Ok((DispatchOutcome::Continue, op.next_pc)); } - // #195 / #73: the arity-2 plain-int `spec_ii` shape (`new_with_vtable` + - // `value0` / `value1`) as the fallback for a pair the canonical fold above - // could not reproduce — it needs a const backing-array length, which the - // element probing here does not. Falls through to the opaque residual for - // any other shape (SAFE — never declined). + // The arities `makespecialisedtuple2` does not claim take the canonical + // array-backed `W_TupleObject` shape instead, so a non-escaping BUILD_TUPLE + // of any width folds away rather than allocating through the opaque + // residual. Reached only after the `spec_ii` fold above declines; falls + // through to the residual for any shape it cannot reproduce (SAFE — never + // declined). if ctx.is_authoritative_executor && dst_bank == 'r' && ei.pyre_helper == majit_ir::PyreHelperKind::NewtupleFromArray - && try_walker_specialize_newtuple(ctx, op.pc, &r_args, dst, dst_bank)?.is_some() + && try_walker_specialize_newtuple_object(ctx, op.pc, &r_args, dst, dst_bank)?.is_some() { return Ok((DispatchOutcome::Continue, op.next_pc)); } @@ -5112,6 +5175,17 @@ pub(crate) fn dispatch_residual_call_iRd_kind( return Ok((DispatchOutcome::Continue, op.next_pc)); } + // `getattr(type, name)` whose class-MRO value is returned unchanged: + // pin receiver, name, and the receiver version, then use the green value. + // Non-matching shapes fall through to the generic residual. + if ctx.is_authoritative_executor + && dst_bank == 'r' + && ei.pyre_helper == majit_ir::PyreHelperKind::CallFn + && try_walker_specialize_builtin_type_getattr(ctx, code, op, &r_args, dst)?.is_some() + { + return Ok((DispatchOutcome::Continue, op.next_pc)); + } + // An exact `range(...)` constructor call becomes a virtual W_Range whose // four wrapped-int fields can fold directly into GET_ITER virtualization. // Non-canonical callables and arguments fall through to the residual. @@ -6066,6 +6140,21 @@ pub(crate) fn dispatch_residual_call_iIRd_kind( )? { return Ok(inlined); } + // A type receiver whose class-MRO value needs no descriptor + // binding folds to that value under receiver + version pins. + if try_walker_specialize_load_type_attr( + ctx, + op.pc, + obj_opref, + w_code_ptr, + namei as usize, + dst, + dst_bank, + )? + .is_some() + { + return Ok((DispatchOutcome::Continue, op.next_pc)); + } } } } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs index 0d1ce37ebe1..ddec868c749 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs @@ -159,23 +159,18 @@ pub(crate) fn walker_capture_inline_nonstandard_vable_guard( if !ctx.trace_ctx.vable_snapshot_buildable() { return Err(DispatchError::GuardSnapshotVableUntyped { pc: op_pc }); } - // #73: give this promote guard the full multi-frame resume every RPython - // guard gets — `_nonstandard_virtualizable` (pyjitpl.py:1120) → - // `implement_guard_value(eqbox, pc)` (1916) → - // `generate_guard(GUARD_VALUE, resumepc=orgpc)` (2582) → - // `capture_resumedata(resumepc)` (2610) walking the WHOLE MIFrame chain - // (opencoder.py:819). When the paused-caller chain covers the full inline - // depth (the same gate as the standard multi-frame guard path, - // `walker_capture_snapshot_for_last_guard_impl`), publish the callee's OWN - // coordinate plus each paused caller instead of the single-frame sentinel - // collapse below — whose carried word is `NO_JITCODE_PC`, which - // `resolve_resume_pc_with_jitcode_pc` rejects, so the collapse - // unconditionally aborts (`GuardResumeCoordinateUnavailable`) every inline - // sub-walk emit of this guard. Stamp the last *guard* op, not the last op: - // `emit_force_virtualizable` records GETFIELD_GC / PTR_NE / COND_CALL after - // the promote. A chain that is not full, or a callee/caller frame the - // publisher cannot build, falls through to (or aborts the same as) the - // sentinel below — never a wrong resume. + // Give this promote guard the full inline resume represented by + // `_nonstandard_virtualizable` (pyjitpl.py:1120), + // `implement_guard_value(eqbox, pc)` (pyjitpl.py:1916), + // `generate_guard(GUARD_VALUE, resumepc=orgpc)` (pyjitpl.py:2582), + // and `capture_resumedata(resumepc)` (pyjitpl.py:2610) walking the full MIFrame chain + // (opencoder.py:819). Publish the callee's own coordinate when the inline + // chain is either the single callee frame or is fully covered by paused + // callers; both shapes are directly represented by the frame list. Stamp + // the last *guard* op, not the last op: `emit_force_virtualizable` records + // GETFIELD_GC / PTR_NE / COND_CALL after the promote. A chain that skips an + // intermediate inline frame still falls through to the sentinel below: + // never a wrong resume. let (n_parents, n_callees, parent_frames) = { let session = ctx.session.borrow(); ( @@ -192,7 +187,9 @@ pub(crate) fn walker_capture_inline_nonstandard_vable_guard( .collect::>(), ) }; - if n_parents > 0 && n_parents == n_callees { + let publish_inline_frames = + (n_parents == 0 && n_callees == 1) || (n_parents > 0 && n_parents == n_callees); + if publish_inline_frames { return walker_capture_multi_frame_inline_snapshot( ctx, op_pc, @@ -2275,8 +2272,7 @@ pub(crate) fn walker_capture_multi_frame_inline_snapshot( // use the same coordinate. let callee_jitcode_pc: i32 = match scope.branch_guard_jitcode_pc { Some(g) => g as i32, - None if after_residual_call => callee_pjc - .after_residual_marker_for_jitcode_pc(callee_op_pc) + None if after_residual_call => after_residual_guard_marker(&callee_pjc, callee_op_pc, None) .or_else(|| { // Fallback only, for the same reason as the single-frame path: // the sticky cursor names this frame's op only while no other @@ -2510,8 +2506,10 @@ pub(crate) fn walker_capture_multi_frame_inline_snapshot( // Publish the OUTERMOST caller's vable scalars for its resume coordinate so // the resume reader restores the caller's `PyFrame` at the CALL return // point rather than the stale loop-header seed the walker never crosses - // `set_orgpc` to update (mirror of the single-frame path above, 6366-6426). - publish_outermost_parent_vable_scalars(ctx, &parent_frames, callee_op_pc)?; + // `set_orgpc` to update. + if !parent_frames.is_empty() { + publish_outermost_parent_vable_scalars(ctx, &parent_frames, callee_op_pc)?; + } let (vable_boxes, vref_boxes) = ctx.trace_ctx.build_snapshot_vable_vref_boxes(); diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 5221aec5ee8..26d9b9fa342 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -16,6 +16,7 @@ //! call into these entry points. use super::*; +use rustpython_wtf8::Wtf8; /// `residual_call` shape `iRd>X` dispatcher. Reads `funcptr (i)`, /// R-list args, and `descr`, runs `_build_allboxes` to produce the @@ -3326,6 +3327,55 @@ pub(crate) fn try_walker_specialize_load_type_name_attr( Ok(Some(())) } +/// Fold `LOAD_ATTR` on a type receiver when +/// [`pyre_interpreter::type_attr_value_fast_path`] proves that +/// `typeobject.py:811-828` returns the class-MRO value unchanged. The exact +/// receiver and its version tag are pinned before the value is written as a +/// green constant. [`pyre_interpreter::mutated`] recursively invalidates +/// subclasses, so the one receiver pin covers reassignment or deletion on any +/// base class as well. +/// +/// The name needs no operand guard: the codewriter baked its `co_names` index +/// into the residual. This read-only, present-attribute fold cannot raise, so +/// unlike the classmethod method-load fold it is safe inside an inlined callee +/// sub-walk; resuming past it cannot repeat a side effect. +#[allow(clippy::too_many_arguments)] +pub(crate) fn try_walker_specialize_load_type_attr( + ctx: &mut WalkContext<'_, '_, Sym>, + op_pc: usize, + obj: OpRef, + w_code_ptr: usize, + name_idx: usize, + dst: usize, + dst_bank: char, +) -> Result, DispatchError> { + if !ctx.is_authoritative_executor || dst_bank != 'r' { + return Ok(None); + } + let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj) else { + return Ok(None); + }; + let Some(name) = walker_load_name_from_code(w_code_ptr, name_idx) else { + return Ok(None); + }; + let Some((w_type, _version_tag, w_value)) = (unsafe { + pyre_interpreter::type_attr_value_fast_path(concrete_obj, Wtf8::new(name.as_str())) + }) else { + return Ok(None); + }; + + let w_type_const = ctx.trace_ctx.const_ref(w_type as i64); + walker_emit_fold_guard_with_snapshot(ctx, op_pc, OpCode::GuardValue, &[obj, w_type_const])?; + ctx.trace_ctx + .heap_cache_mut() + .replace_box(obj, w_type_const); + walker_pin_type_version_tag(ctx, op_pc, w_type_const)?; + + let value_const = ctx.trace_ctx.const_ref(w_value as i64); + write_residual_call_result_to_dst(ctx, op_pc, dst, 'r', value_const)?; + Ok(Some(())) +} + /// Fold the `LOAD_ATTR`-method `getattr` residual for a receiver whose name /// resolves to a plain builtin-code function on its type — the `lst.append` /// shape [`try_walker_specialize_load_method_attr`] declines because upstream @@ -4148,9 +4198,11 @@ pub(crate) fn try_walker_specialize_newlist( Ok(Some(())) } -/// FBW virtualization of the array-backed BUILD_TUPLE, at arity 1 and 3 up. -/// Sibling of [`try_walker_specialize_newlist`] and -/// [`try_walker_specialize_newtuple`], which takes the arity-2 plain-int shape. +/// FBW virtualization of the array-backed BUILD_TUPLE — the arities +/// `makespecialisedtuple2` does not claim. Sibling of +/// [`try_walker_specialize_newtuple`] (arity-2 plain-int `spec_ii`) and +/// [`try_walker_specialize_newlist`], reached only after the `spec_ii` fold +/// declines, so that path stays byte-identical. /// /// `lower_tuple_build_hlop_to_insn` lowers BUILD_TUPLE to `new_array_clear` + /// per-index `setarrayitem_gc` + a `newtuple_from_array` residual. Re-emit the @@ -4161,16 +4213,19 @@ pub(crate) fn try_walker_specialize_newlist( /// and one that does escape materializes from the same fields the residual /// would have written. /// -/// Arity 2 is declined: `makespecialisedtuple2` -/// (`specialisedtupleobject.py:169-179`) is what the runtime calls there, so a -/// canonical virtual would be the one shape the interpreter never builds. The -/// trace alone stays self-consistent, but a side exit hands a real `Cls_ii` / -/// `Cls_ff` / `Cls_oo` — inline `value0` / `value1`, no `wrappeditems` block — -/// to a consumer the trace picked for the canonical layout, and +/// Arity 2 is `makespecialisedtuple2` territory (`Cls_ii` / `Cls_ff` / +/// `Cls_oo`, `specialisedtupleobject.py`): the runtime never builds an +/// array-backed tuple there, so emitting one would diverge from what the +/// blackhole rebuilds on deopt. Declined here — the `spec_ii` fold owns the +/// int-int case and the residual owns the rest. The empty tuple is declined +/// too (no element to recover a length from). +/// +/// Lifting that decline is not a trace-local question: the trace stays +/// self-consistent, but a side exit hands a real pair — inline `value0` / +/// `value1`, no `wrappeditems` block — to whatever consumer the trace picked +/// for the canonical layout, and /// [`try_walker_specialize_subscr_specialised_pair`] then reads a field that is -/// not there. [`try_walker_specialize_newtuple`] takes the pair shapes it can -/// build faithfully. The empty tuple is declined too (no element to recover a -/// length from). +/// not there. /// /// Returns `Ok(Some(()))` when folded; `Ok(None)` falls through to the opaque /// residual, which stays correct for any shape — a non-const array length or @@ -4226,9 +4281,8 @@ pub(crate) fn try_walker_specialize_newtuple_object( concretes.push(obj); } - // Concrete shadow: a fresh array-backed tuple from the element shadows, - // built by the same constructor the emit reproduces so the walk's own value - // and the traced object agree at every arity. A new allocation with no + // Concrete shadow: a fresh array-backed tuple from the element shadows + // (`w_tuple_new` parity for every arity but 2). A new allocation with no // heap mutation, safe during the walk like `wrapint`. Built before the // emit so a failure leaves no orphan ops in the trace. let result_concrete = pyre_object::w_tuple_new_array_backed(concretes); @@ -4257,12 +4311,6 @@ pub(crate) fn try_walker_specialize_newtuple_object( /// [`try_walker_specialize_unpack`] then folds the `value0` / `value1` /// reads off the virtual tuple, collapsing build→unpack to a pure-int loop. /// -/// Runs only after [`try_walker_specialize_newtuple_object`] declines, which it -/// does for a pair whose backing-array length never reached the heap-cache as a -/// constant; the element probing below recovers the arity without it. UNPACK -/// is the one consumer that folds off this shape, so the canonical arm is the -/// preferred one wherever it applies. -/// /// Returns `Ok(Some(()))` when folded (the caller returns `Continue`); /// `Ok(None)` to fall through to the opaque residual, which stays correct /// for any other shape (object tuple, arity ≠ 2, out-of-range long, tagged @@ -6750,6 +6798,122 @@ pub(crate) fn try_walker_specialize_builtin_len( Ok(Some(())) } +/// Fold plain `getattr(type, name)` when +/// [`pyre_interpreter::type_attr_value_fast_path`] proves that +/// `typeobject.py:811-828` returns the class-MRO value unchanged. The exact +/// callable, exact receiver, exact name object, and receiver version are pinned +/// before the value is written as a green constant. Pinning the callable makes +/// a rebound `getattr` side-exit instead of continuing to use the folded value. +/// The operand guards are tautologies when their inputs are already constants +/// and disappear during optimization. +/// [`pyre_interpreter::mutated`] recursively invalidates subclasses, so the +/// receiver's one quasi-immutable version watcher covers base-class mutation +/// and emits no per-iteration operations. +/// +/// Like the `len` fold this is safe in an inlined callee sub-walk: the oracle +/// proves a read-only present attribute, so it cannot raise or introduce a +/// side effect that resume would repeat. Every other shape declines before +/// emitting IR and falls through to the generic residual. +pub(crate) fn try_walker_specialize_builtin_type_getattr( + ctx: &mut WalkContext<'_, '_, Sym>, + code: &[u8], + op: &DecodedOp, + r_args: &[OpRef], + dst: usize, +) -> Result, DispatchError> { + // Plain `bh_call_fn(callable, PY_NULL, obj, name)` shape only. + if r_args.len() != 4 { + return Ok(None); + } + let arg_concretes = read_ref_var_list_concrete(code, op, 1, ctx); + let ( + ConcreteValue::Ref(concrete_callable), + ConcreteValue::Ref(null_or_self), + ConcreteValue::Ref(concrete_obj), + ConcreteValue::Ref(concrete_name), + ) = ( + arg_concretes[0], + arg_concretes[1], + arg_concretes[2], + arg_concretes[3], + ) + else { + return Ok(None); + }; + // A non-null `null_or_self` is a bound receiver `bh_call_fn_impl` + // prepends as arg0 — not a plain `getattr(type, name)` call. + if concrete_callable.is_null() + || !null_or_self.is_null() + || concrete_obj.is_null() + || concrete_name.is_null() + { + return Ok(None); + } + if !pyre_interpreter::builtins::is_builtin_getattr_function(concrete_callable) { + return Ok(None); + } + if !unsafe { pyre_object::is_exact_type(concrete_name, &pyre_object::pyobject::STR_TYPE) } { + return Ok(None); + } + let name = unsafe { pyre_object::w_str_get_wtf8(concrete_name) }; + let Some((w_type, _version_tag, w_value)) = + (unsafe { pyre_interpreter::type_attr_value_fast_path(concrete_obj, name) }) + else { + return Ok(None); + }; + + let callable_op = r_args[0]; + if !callable_op.is_constant() { + let expected = ctx.trace_ctx.const_ref(concrete_callable as i64); + walker_emit_fold_guard_with_snapshot( + ctx, + op.pc, + OpCode::GuardValue, + &[callable_op, expected], + )?; + ctx.trace_ctx + .heap_cache_mut() + .replace_box(callable_op, expected); + } + + let obj_ref = r_args[2]; + let w_type_const = ctx.trace_ctx.const_ref(w_type as i64); + if !obj_ref.is_constant() { + walker_emit_fold_guard_with_snapshot( + ctx, + op.pc, + OpCode::GuardValue, + &[obj_ref, w_type_const], + )?; + ctx.trace_ctx + .heap_cache_mut() + .replace_box(obj_ref, w_type_const); + } + + // The baked WTF-8 bytes remain constant only while this exact string is + // the name operand. Constant operands make this guard a removable + // tautology, so it costs nothing in the steady loop. + let name_ref = r_args[3]; + let name_const = ctx.trace_ctx.const_ref(concrete_name as i64); + if !name_ref.is_constant() { + walker_emit_fold_guard_with_snapshot( + ctx, + op.pc, + OpCode::GuardValue, + &[name_ref, name_const], + )?; + } + + // typeobject.py `promote(self.version_tag())`: this quasi-immutable watcher + // emits no per-iteration op. `mutated` (baseobjspace.rs) recurses through + // subclasses, so changing the attribute on any base invalidates this pin. + walker_pin_type_version_tag(ctx, op.pc, w_type_const)?; + + let value_const = ctx.trace_ctx.const_ref(w_value as i64); + write_residual_call_result_to_dst(ctx, op.pc, dst, 'r', value_const)?; + Ok(Some(())) +} + /// `range(stop)` / `range(start, stop)` / `range(start, stop, step)` with /// exact canonical machine-word ints: lower the opaque constructor residual /// to a virtual `W_Range` and four virtual wrapped-int fields. This lets the diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 340073f4989..70ae86a5171 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -4399,15 +4399,14 @@ pub(crate) fn concrete_nlocals(frame: usize) -> Option { Some(nlocals + ncells) } -/// Static operand-stack depth at `target_pc`, but only when it exceeds the -/// depth `frame` still advertises — the depth a closing JUMP must not fall -/// below when it retargets a merge point at a different bytecode offset. +/// Static operand-stack depth at `target_pc` when it differs from the depth +/// `frame` still advertises. A closing JUMP retargeted to a different bytecode +/// offset must publish the merge point's own live depth. /// /// `None` means "keep reading the frame": the frame or its code object cannot /// answer, the code has no liveness entry for `target_pc`, the depth would -/// overrun `locals_cells_stack_w`, or the frame already covers the merge -/// point. Never narrows — see `close_loop_args_at` for why only the widening -/// direction is a correction. +/// overrun `locals_cells_stack_w`, or the frame is already at the merge +/// point's depth. pub(crate) fn merge_point_stack_depth_to_recover(frame: usize, target_pc: usize) -> Option { let concrete = concrete_stack_depth(frame)?; let w_code = unsafe { @@ -4419,7 +4418,7 @@ pub(crate) fn merge_point_stack_depth_to_recover(frame: usize, target_pc: usize) if concrete_frame_array_len(frame).is_none_or(|len| depth > len) { return None; } - (depth > concrete).then_some(depth) + (depth != concrete).then_some(depth) } /// Return the absolute valuestackdepth. @@ -9555,24 +9554,6 @@ impl JitState for PyreJitState { return; } - // pyjitpl.py:3125-3165 `_prepare_exception_resumption` + `execute_ll_raised`: - // the exception grabbed at guard failure (`cpu.grab_exc_value`, threaded - // via `ctx.bridge_guard_exc`) becomes the bridge's standing exception. - // Seed `current_exc_value` here so `seed_bridge_standing_exception_from_current` - // below promotes it into `last_exc_value` / `last_exc_box` (the - // `dispatch_via_miframe` / carrier exc-edge precondition reads these). - // Without this seed the sym only sees `get_current_exception()`, which the - // blackhole has already cleared by carrier re-trace time. Gated while the - // #343/#126 depth-2 exception-resume slice is validated. - if crate::jitcode_dispatch::carrier_exc_resume_enabled() - && ctx.bridge_source_is_exception_guard() - { - let guard_exc = ctx.bridge_guard_exc(); - if guard_exc != 0 && sym.current_exc_value.is_null() { - sym.current_exc_value = guard_exc as pyre_object::PyObjectRef; - } - } - // virtualizable.py:139 load_list_of_boxes parity: decode each // RebuiltValue in the resume stream into a typed Value. The type // is the fixed Box kind the encoder recorded at numbering time @@ -13632,7 +13613,7 @@ mod tests { /// must recover that offset's own depth, or every operand-stack slot the /// header binds is force-nulled into the JUMP. #[test] - fn merge_point_stack_depth_recovers_a_deeper_header_and_never_narrows() { + fn merge_point_stack_depth_recovers_the_header_depth() { use pyre_interpreter::pyframe::PyFrame; ensure_test_callbacks(); @@ -13672,10 +13653,13 @@ mod tests { None, ); - // Frame already deeper than the header's static depth: never narrow — - // dropping a slot would lose a value the JUMP must carry. + // Stale frame (resumed at a deeper offset), closing on the shallower + // header: slots above the header's static depth are dead capacity. frame.valuestackdepth = deep_vsd + 1; - assert_eq!(merge_point_stack_depth_to_recover(frame_ptr, deep_pc), None); + assert_eq!( + merge_point_stack_depth_to_recover(frame_ptr, deep_pc), + Some(deep_vsd), + ); // An offset the code object has no liveness entry for keeps the frame. frame.valuestackdepth = base; diff --git a/pyre/pyre-jit-trace/src/trace_opcode.rs b/pyre/pyre-jit-trace/src/trace_opcode.rs index 141312ef44f..a2ffdcf3915 100644 --- a/pyre/pyre-jit-trace/src/trace_opcode.rs +++ b/pyre/pyre-jit-trace/src/trace_opcode.rs @@ -2058,11 +2058,10 @@ impl MIFrame { // // A bytecode offset has exactly one operand-stack depth, so the merge // point's depth is available statically from the same liveness table - // `maybe_compile_and_run` gates interpreter entry on. Take it when it - // exceeds what the frame advertises — never carry fewer slots than the - // target header requires. Narrowing is deliberately not done: a close - // whose frame is deeper than the header's static depth keeps its - // slots, since dropping one would lose a value the JUMP must carry. + // `maybe_compile_and_run` gates interpreter entry on. Use that exact + // depth in either direction. The full virtualizable array capacity is + // still carried below; slots above the header's live depth are dead + // capacity and are null-padded rather than counted as live stack. self.close_merge_point_vsd = target_pc.and_then(|pc| { crate::state::merge_point_stack_depth_to_recover(self.concrete_frame_addr, pc) }); diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 4b8cdb4869d..c239e5b31af 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -2231,6 +2231,57 @@ fn reject_non_exception_channel_value( ); } +/// `executioncontext.py:91-107 ExecutionContext.leave`'s frame-chain half, for a +/// frame a blackhole resumed into and has now finished. +/// +/// The profile-hook half (`if self.profilefunc: self._trace(frame, +/// 'leaveframe', w_exitvalue)`) stays with the interpreter's own +/// [`pyre_interpreter::PyExecutionContext::leave`], for the reason +/// `walker_ec_leave` states: `is_being_profiled` is a portal-driver green, so a +/// trace recorded with profiling off is only ever entered with profiling off, +/// and the blackhole resuming out of it inherits that key. +fn leave_resumed_blackhole_frame( + frame: &majit_metainterp::blackhole::BlackholeInterpreter, + got_exception: bool, +) { + let frame_ptr = frame.virtualizable_ptr as *mut PyFrame; + if frame_ptr.is_null() { + return; + } + let ec = unsafe { (*frame_ptr).execution_context as *mut pyre_interpreter::PyExecutionContext }; + if ec.is_null() { + return; + } + let frame_vref = unsafe { (*ec).topframeref }; + if !std::ptr::eq( + pyre_interpreter::executioncontext::vref_referent(frame_vref), + frame_ptr, + ) { + return; + } + // executioncontext.py:91-107 `leave`: a guard-failure blackhole resumes + // inside a frame whose `enter` already ran in compiled code. Advancing to + // `nextblackholeinterp` therefore has to close that still-open scope. The + // resume-data frame chain itself is not the application frame chain, and + // releasing a BlackholeInterpreter does not restore `topframeref`. + // + // Nothing here forces a vref. The guard above already established that + // `frame_vref` resolves to this frame, so `leave`'s own `frame_vref()` has + // nothing left to materialize, and the caller is reached through + // `vref_referent` rather than `get_f_back`: this runs once the compiled + // frame is gone, and forcing a vref that was finished with the NULL form + // is exactly what `force_pyframe_vref` refuses. + unsafe { + (*ec).topframeref = (*frame_ptr).f_backref; + if (*frame_ptr).escaped() || got_exception { + let f_back = pyre_interpreter::executioncontext::vref_referent((*frame_ptr).f_backref); + if !f_back.is_null() { + (*f_back).mark_as_escaped(); + } + } + } +} + /// resume.py:1312 blackhole_from_resumedata parity: /// Decode rd_numb via ResumeDataDirectReader, build blackhole chain, /// run _run_forever. @@ -2580,6 +2631,7 @@ pub fn blackhole_resume_via_rd_numb( let frame_ptr = bh.virtualizable_ptr as *mut PyFrame; let jitcode_index = bh.jitcode.try_index().map(|v| v as i32); let last_opcode_position = bh.last_opcode_position; + leave_resumed_blackhole_frame(&bh, true); release_bh_rd(bh); match next { Some(caller) => { @@ -2805,6 +2857,7 @@ pub fn blackhole_resume_via_rd_numb( } } } + leave_resumed_blackhole_frame(&bh, true); release_bh_rd(bh); // Re-read through the pin: the records above are allocation // points, so the pinned slot — not the stale local — is the live @@ -2912,6 +2965,7 @@ pub fn blackhole_resume_via_rd_numb( BhReturnType::Float => caller_bh.setup_return_value_f(bh.get_tmpreg_f()), BhReturnType::Void => {} } + leave_resumed_blackhole_frame(&bh, false); release_bh_rd(bh); bh = caller_bh; } @@ -3276,14 +3330,7 @@ pub fn trace_and_compile_from_bridge( // compile.py:714: start_retrace_from_guard + set bridge_info. let started = { let (driver, _) = crate::eval::driver_pair(); - driver.start_bridge_tracing( - descr_arc, - &mut jit_state, - &env, - raw_values, - resume_pc, - guard_exc, - ) + driver.start_bridge_tracing(descr_arc, &mut jit_state, &env, raw_values, resume_pc) }; if !started { if majit_metainterp::majit_log_enabled() { diff --git a/pyre/pyre-wasm-runner/src/main.rs b/pyre/pyre-wasm-runner/src/main.rs index 97bf651e48d..6ee275b9347 100644 --- a/pyre/pyre-wasm-runner/src/main.rs +++ b/pyre/pyre-wasm-runner/src/main.rs @@ -899,6 +899,7 @@ fn run(module_path: &PathBuf, source: &str, script: &Path) -> Result { }; let loops_compiled = counter("pyre_jit_loops_compiled", &mut missing); let bridges_compiled = counter("pyre_jit_bridges_compiled", &mut missing); + 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 internal_compile_panics = counter("pyre_jit_internal_compile_panics", &mut missing); @@ -955,6 +956,7 @@ fn run(module_path: &PathBuf, source: &str, script: &Path) -> Result { eprintln!( "[jit-stats] loops_compiled={loops_compiled} \ bridges_compiled={bridges_compiled} \ + retraces_compiled={retraces_compiled} \ loops_aborted={loops_aborted} \ guard_failures={guard_failures} \ internal_compile_panics={internal_compile_panics} \ diff --git a/pyre/pyre-wasm/src/lib.rs b/pyre/pyre-wasm/src/lib.rs index e625a28c058..5262066dd6d 100644 --- a/pyre/pyre-wasm/src/lib.rs +++ b/pyre/pyre-wasm/src/lib.rs @@ -433,6 +433,15 @@ pub extern "C" fn pyre_jit_loops_compiled() -> u64 { pyre_jit::eval::driver_pair().0.get_stats().loops_compiled as u64 } +#[cfg(all(target_arch = "wasm32", feature = "wasm-host"))] +#[unsafe(no_mangle)] +pub extern "C" fn pyre_jit_retraces_compiled() -> u64 { + pyre_jit::eval::driver_pair() + .0 + .get_stats() + .retraces_compiled as u64 +} + #[cfg(all(target_arch = "wasm32", feature = "wasm-host"))] #[unsafe(no_mangle)] pub extern "C" fn pyre_jit_bridges_compiled() -> u64 { diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index 2ec3e25188b..0fbba8aed6f 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -859,10 +859,11 @@ fn maybe_print_jit_stats() { ); let stats = pyre_jit::eval::driver_pair().0.get_stats(); eprintln!( - "[jit-stats] loops_compiled={} bridges_compiled={} loops_aborted={} \ + "[jit-stats] loops_compiled={} bridges_compiled={} retraces_compiled={} loops_aborted={} \ guard_failures={} internal_compile_panics={}", stats.loops_compiled, stats.bridges_compiled, + stats.retraces_compiled, stats.loops_aborted, stats.guard_failures, stats.internal_compile_panics,