diff --git a/majit/majit-metainterp/src/optimizeopt/heap.rs b/majit/majit-metainterp/src/optimizeopt/heap.rs index f7619d78861..71b10e69690 100644 --- a/majit/majit-metainterp/src/optimizeopt/heap.rs +++ b/majit/majit-metainterp/src/optimizeopt/heap.rs @@ -994,6 +994,11 @@ impl OptHeap { /// array, so a sparse high slot costs one entry and nothing else. const HEADER_FIELD_SLOT_BASE: u32 = 0x8000_0000; + /// Slot base for a field descriptor the parent's slot list does not place + /// (see [`Self::field_slot_index`]). Disjoint from both the dense + /// `index_in_parent` space and [`Self::HEADER_FIELD_SLOT_BASE`]. + const UNSLOTTED_FIELD_SLOT_BASE: u32 = 0x4000_0000; + /// Compute the `PtrInfo._fields` slot for a field descriptor. /// /// RPython uses `descr.get_index()` only for `info._fields[index]` @@ -1013,17 +1018,19 @@ impl OptHeap { /// which is what `make_equal_to`'s `Box.type` invariant fires on. /// /// So use the position only where the parent's list actually holds this - /// field at it, and otherwise fall back to the descr's own key, which for - /// an unnumbered field is minted out of its payload and cannot collide - /// with a position. + /// field at it. A field that list does not place carries no position, and + /// the descr's own `index()` is not one either: every `VirtualizableInfo` + /// field descriptor is minted with index `0`, so answering with it puts + /// the vable's array-pointer read on the same slot as the vable token + /// store, and the read comes back with the token. Key those by offset in + /// a band of their own instead. /// - /// That fallback does NOT cover the header words, which is why they are - /// answered before it: `PyObject.ob_type` carries `index()` `0`, a real - /// position, so routing it through the fallback returns it to the very slot - /// the check just refused. They resolve through no field list at all -- - /// `OptVirtualize` folds `is_typeptr` from `known_class` and `is_w_class` - /// from the virtual's class identity -- so give them a band above the - /// positional space instead, keyed by offset to keep the two apart. + /// The header words are answered before all of that, in a band of their + /// own. They resolve through no field list at all -- `OptVirtualize` + /// folds `is_typeptr` from `known_class` and `is_w_class` from the + /// virtual's class identity -- and an offset alone would not separate a + /// header word from an unplaced field naming the same word, so the two + /// bands stay apart. pub(crate) fn field_slot_index(descr: &DescrRef) -> u32 { let descr_idx = descr.index(); let Some(field_descr) = descr.as_field_descr() else { @@ -1045,7 +1052,16 @@ impl OptHeap { if holds_this_field { index as u32 } else { - descr_idx + // The parent places this field nowhere, so there is no slot number + // for it — and `descr.index()` is not one either. Slot numbers are + // `index_in_parent` values, small and dense; a descriptor minted + // without a parent slot would land on top of whatever the parent + // really holds at that slot, and a read through one descriptor + // would then answer with the value stored through the other. + // Offset is the identity such a field does carry, so key it by + // that: descriptors naming the same word alias, descriptors naming + // different words never do. + Self::UNSLOTTED_FIELD_SLOT_BASE + field_descr.offset() as u32 } } @@ -6223,8 +6239,12 @@ mod tests { let pos100 = ctx.materialize_operand_at(OpRef::ref_op(100)); ctx.set_ptr_info(&pos100, PtrInfo::instance(None, None)); let val101 = ctx.materialize_operand_at(OpRef::ref_op(101)); + // Seed the slot the reader will consult: `field_slot_index`, not the + // descriptor's own key. A descriptor with no parent carries no slot + // number, so the two differ. + let slot = OptHeap::field_slot_index(&descr); ctx.with_ptr_info_mut(&pos100, |info| { - info.setfield(descr.index(), val101.clone()); + info.setfield(slot, val101.clone()); }) .unwrap(); pass.produce_potential_short_preamble_ops(&mut sb, &mut ctx); diff --git a/majit/majit-metainterp/src/optimizeopt/optimizer.rs b/majit/majit-metainterp/src/optimizeopt/optimizer.rs index f067924a3ef..3ae3d7974e1 100644 --- a/majit/majit-metainterp/src/optimizeopt/optimizer.rs +++ b/majit/majit-metainterp/src/optimizeopt/optimizer.rs @@ -4618,7 +4618,22 @@ impl Optimizer { // innermost level may absorb what the current propagate queues. ctx.extra_pending.push(pending); let result = self.drain_innermost_pending(ctx); - ctx.extra_pending.pop(); + let unfinished = ctx + .extra_pending + .pop() + .expect("the active extra-operation drain level must remain installed"); + if result.is_err() { + // `InvalidLoop` unwinds the recursive Rust drain in place of + // RPython's ordinary exception unwinding. Keep both operations + // emitted by the failing propagation and the untouched tail from + // this level available to the caller's unwind. Each outer level + // appends its own untouched tail in turn, so no parked producer is + // silently discarded and a reused context never observes a + // half-drained scheduling state. + ctx.extra_operations_after.extend(unfinished); + } else { + debug_assert!(unfinished.is_empty()); + } result } @@ -6176,6 +6191,30 @@ mod tests { } } + struct QueueExtraThenInvalidate; + + impl Optimization for QueueExtraThenInvalidate { + fn propagate_forward( + &mut self, + op: &Op, + _op_rc: &majit_ir::OpRc, + ctx: &mut OptContext, + ) -> OptimizationResult { + if op.opcode == OpCode::IntAdd { + ctx.emit_extra( + ctx.current_pass_idx, + Op::new(OpCode::IntMul, &[op.arg(0), op.arg(1)]), + ); + ctx.signal_invalid_loop("test invalid loop while draining extras"); + } + OptimizationResult::PassOn + } + + fn name(&self) -> &'static str { + "queue_extra_then_invalidate" + } + } + #[test] fn test_optimizer_passthrough() { let mut opt = Optimizer::new(); @@ -6192,6 +6231,28 @@ mod tests { assert_eq!(result[0].opcode, OpCode::IntAdd); } + #[test] + fn invalid_loop_preserves_queued_and_unprocessed_extra_operations() { + let mut opt = Optimizer::new(); + opt.add_pass(Box::new(QueueExtraThenInvalidate)); + let mut ctx = OptContext::new(2); + let lhs = rooted_resop_operand(Type::Int, 0); + let rhs = rooted_resop_operand(Type::Int, 1); + ctx.emit_extra_at(0, Op::new(OpCode::IntAdd, &[lhs.clone(), rhs.clone()])); + ctx.emit_extra_at(0, Op::new(OpCode::IntSub, &[lhs, rhs])); + + let result = opt.drain_extra_operations_from(0, &mut ctx); + + assert!(result.is_err()); + assert!(ctx.extra_pending.is_empty()); + let queued: Vec<_> = ctx + .extra_operations_after + .iter() + .map(|(_, op)| op.opcode) + .collect(); + assert_eq!(queued, [OpCode::IntMul, OpCode::IntSub]); + } + #[test] fn test_restart_from_extra_operation_rediscovers_first_pass() { let hits = Rc::new(Cell::new(0)); diff --git a/majit/majit-metainterp/src/optimizeopt/unroll.rs b/majit/majit-metainterp/src/optimizeopt/unroll.rs index 4c1342c657a..44810dde5c1 100644 --- a/majit/majit-metainterp/src/optimizeopt/unroll.rs +++ b/majit/majit-metainterp/src/optimizeopt/unroll.rs @@ -7666,7 +7666,11 @@ mod tests { // source of truth, matching RPython's HeapOp.produce_op → opinfo.setfield. let obj_box = ctx2.get_box_replacement_operand_opt(targetargs[0]).unwrap(); let pop = ctx2 - .with_ptr_info_mut(&obj_box, |info| info.take_preamble_field(0)) + .with_ptr_info_mut(&obj_box, |info| { + info.take_preamble_field(crate::optimizeopt::heap::OptHeap::field_slot_index( + &field_descr, + )) + }) .flatten(); assert!(pop.is_some(), "PreambleOp must be in PtrInfo._fields"); let pop = pop.unwrap(); @@ -7777,7 +7781,11 @@ mod tests { // the Const as the body-visible Box. let obj_box = ctx2.get_box_replacement_operand_opt(targetargs[0]).unwrap(); let pop = ctx2 - .with_ptr_info_mut(&obj_box, |info| info.take_preamble_field(0)) + .with_ptr_info_mut(&obj_box, |info| { + info.take_preamble_field(crate::optimizeopt::heap::OptHeap::field_slot_index( + &field_descr, + )) + }) .flatten(); assert!( pop.is_some(), diff --git a/majit/majit-metainterp/src/trace_ctx.rs b/majit/majit-metainterp/src/trace_ctx.rs index d7e938adcae..5e9c8ab52f9 100644 --- a/majit/majit-metainterp/src/trace_ctx.rs +++ b/majit/majit-metainterp/src/trace_ctx.rs @@ -1362,6 +1362,57 @@ impl TraceCtx { self.virtualref_boxes.last().copied() } + /// The current concrete address of one `virtualref_boxes` entry. + /// + /// The `usize` beside each box is the address the object had when the pair + /// was pushed, and the object it names is movable: a minor collection + /// relocates it and forwards the stamp, leaving the pushed copy naming the + /// old address. `opimpl_virtual_ref_finish` documents the same hazard on + /// the same list. Read the address back through `concrete_of_opref` — + /// pyre's `getref_base()` — so a pair that has moved still matches, and + /// keep the pushed copy only for an entry carrying no stamp at all. + fn virtualref_entry_ptr(&self, entry: (OpRef, usize)) -> usize { + match self.concrete_of_opref(entry.0) { + Some(Value::Ref(r)) => r.as_usize(), + _ => entry.1, + } + } + + /// Resolve a live tracing-time vref back to its `[virtualbox, vrefbox]` + /// pair. This is the paired walk `vrefs_after_residual_call` makes over + /// `MetaInterp.virtualref_boxes` (`pyjitpl.py`): callers that execute + /// `jit_force_virtual(vref)` need the paired virtual box that + /// `stop_tracking_virtualref` publishes through `VIRTUAL_REF_FINISH`. + /// + /// Search from the innermost pair because frame-chain vrefs are nested in + /// the same order as `virtualref_boxes`. A stopped pair has had its vref + /// entry replaced by `CONST_NULL`, exactly as upstream, so it cannot match. + pub fn live_virtualref_pair_for_ptr(&self, vref_ptr: usize) -> Option<(OpRef, OpRef)> { + if vref_ptr == 0 { + return None; + } + self.virtualref_boxes + .chunks_exact(2) + .rev() + .find(|pair| self.virtualref_entry_ptr(pair[1]) == vref_ptr) + .map(|pair| (pair[0].0, pair[1].0)) + } + + /// Find the virtual box for a concrete object named by either a live or an + /// already-stopped vref pair. `stop_tracking_virtualref` replaces only + /// `virtualref_boxes[i + 1]` with `CONST_NULL`; the adjacent virtual box + /// remains in the upstream list until `virtual_ref_finish` pops the scope. + pub fn virtualref_virtual_for_object_ptr(&self, object_ptr: usize) -> Option { + if object_ptr == 0 { + return None; + } + self.virtualref_boxes + .chunks_exact(2) + .rev() + .find(|pair| self.virtualref_entry_ptr(pair[0]) == object_ptr) + .map(|pair| pair[0].0) + } + /// `pyjitpl.py rebuild_state_after_failure`'s /// `self.virtualref_boxes = virtualref_boxes`. A bridge resumes into its /// parent's still-open `virtual_ref` scopes, so the pairs the parent guard @@ -5186,6 +5237,59 @@ impl TraceCtx { ) } + /// The array half of `virtualizable.py write_boxes`, emitted into the trace + /// for one array field of the STANDARD virtualizable. + /// + /// `pyjitpl.py synchronize_virtualizable` runs that write-back after every + /// vable store, but only against the recording-time virtualizable: upstream + /// readers of a virtualizable array are traced through and read the boxes, + /// so the compiled trace never needs the array itself to be current. A + /// consumer that reads the array at run time instead needs the same writes + /// emitted, which is what this records. + /// + /// `items` is `(element index, box)` pairs; only the listed slots are + /// written, so a caller covering a sub-range of the array leaves the rest + /// alone. The emission shape is `gen_store_back_in_vable`'s array loop — + /// one `getfield_gc_r` of `array_pointer_field_descr` followed by a + /// `setarrayitem_gc` per item under `array_item_descr`. Neither the token + /// store nor `forced_virtualizable` is touched: this writes the image out, + /// it does not force the virtualizable. + /// + /// The shadow is left alone — it already holds these values and stays + /// authoritative for the rest of the trace. + pub fn vable_array_region_write_back( + &mut self, + vable_opref: OpRef, + array_index: usize, + items: &[(i64, OpRef)], + ) -> bool { + let Some(info) = self.virtualizable_info.clone() else { + return false; + }; + if array_index >= info.array_fields.len() { + return false; + } + let field_descr = info.array_pointer_field_descr(array_index); + let array_descr = info.array_item_descr(array_index); + let array_opref = self.vable_getfield_ref_descr(vable_opref, field_descr.clone()); + // `executor.execute` for the read: the array base has to carry its + // concrete half, or the consumer below reaches the backend with an + // operand no producer answers for. Same step every other recorded + // vable array-base read takes. + let vable_concrete = self.concrete_of_opref(vable_opref); + self.stamp_vable_array_base(array_opref, vable_concrete, &field_descr); + self.heapcache_getfield_now_known(vable_opref, field_descr.index(), array_opref); + for &(item_index, value) in items { + let index = self.const_int(item_index); + self.profiler() + .count_ops(OpCode::SetarrayitemGc, crate::counters::OPS); + self.profiler() + .count_ops(OpCode::SetarrayitemGc, crate::counters::RECORDED_OPS); + self.vable_setarrayitem_descr(array_opref, index, value, array_descr.clone()); + } + true + } + /// `_opimpl_setarrayitem_vable` body with the `_nonstandard_virtualizable` /// decision already taken by the caller (see /// [`Self::nonstandard_virtualizable`]). diff --git a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.cranelift.jitstats b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.cranelift.jitstats index 9400653eeeb..4e1e41a7bed 100644 --- a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.cranelift.jitstats +++ b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.cranelift.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=1 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.dynasm.jitstats b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.dynasm.jitstats index 9400653eeeb..4e1e41a7bed 100644 --- a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.dynasm.jitstats +++ b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.dynasm.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=1 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.wasm.jitstats b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.wasm.jitstats index 9400653eeeb..4e1e41a7bed 100644 --- a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.wasm.jitstats +++ b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.wasm.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=1 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_bridge_force_after_store_declined.cranelift.jitstats b/pyre/bench/synth/getframe_bridge_force_after_store_declined.cranelift.jitstats index 959c5f5446c..6ec1f9b3871 100644 --- a/pyre/bench/synth/getframe_bridge_force_after_store_declined.cranelift.jitstats +++ b/pyre/bench/synth/getframe_bridge_force_after_store_declined.cranelift.jitstats @@ -1,14 +1,15 @@ -bridges_compiled=0 +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=20 +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=4114 +guard_failures=201 internal_compile_panics=0 -loops_aborted=20 +loops_aborted=0 loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_bridge_force_after_store_declined.dynasm.jitstats b/pyre/bench/synth/getframe_bridge_force_after_store_declined.dynasm.jitstats index 959c5f5446c..6ec1f9b3871 100644 --- a/pyre/bench/synth/getframe_bridge_force_after_store_declined.dynasm.jitstats +++ b/pyre/bench/synth/getframe_bridge_force_after_store_declined.dynasm.jitstats @@ -1,14 +1,15 @@ -bridges_compiled=0 +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=20 +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=4114 +guard_failures=201 internal_compile_panics=0 -loops_aborted=20 +loops_aborted=0 loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_bridge_force_after_store_declined.py b/pyre/bench/synth/getframe_bridge_force_after_store_declined.py index 2fb8bf670ff..6358628d27d 100644 --- a/pyre/bench/synth/getframe_bridge_force_after_store_declined.py +++ b/pyre/bench/synth/getframe_bridge_force_after_store_declined.py @@ -1,36 +1,13 @@ -# Companion to getframe_bridge_force_after_store, -# carrying that file's shape at a force the constant-depth -# `sys._getframe` arm DECLINES, so the machinery it documents stays -# covered now that its own call site folds. +# Historical `_declined` companion to `getframe_bridge_force_after_store`. +# The name predates the standard-frame `f_locals` specialization: an exact +# `_getframe(0)` result now creates its CPython 3.14 `FrameLocalsProxy` without +# residualizing the getter or forcing the portal virtualizable. # -# `try_walker_specialize_sys_getframe` (jitcode_dispatch/specialize.rs) takes -# depth 0 at the top walk level, where `getframe`'s answer IS the portal -# virtualizable and no force is needed, so the CALL folds here as well. The -# force this file needs comes from the `.f_locals` getset on the folded -# result: it reads the virtualizable's own fields, so it routes through -# `force_frame_before_locals_read` on the portal and escapes it. -# -# A nonzero depth does NOT serve. `getframe` forces only the frame it RETURNS, -# and one below the portal is not the traced virtualizable, so nothing escapes. -# -# The counters recorded for this file are the ones its original -# carried before the fold; a diff against them is a real change in the escape -# machinery, not in the arm. -# -# The bridge forced-vable escape of `getframe_bridge_force_plain`, with one -# un-journaled store ahead of the forcing call. -# -# `box.n = i` lowers to a Void `store_attr_fn` residual: it writes live heap, so -# it bumps the executed-effect odometer, and no journal covers it. Rolling the -# walk back therefore cannot undo the store, and the legacy entry replay would -# apply it a second time. The escape has to capture its operand-stack mirror and -# resume forward instead, which is what the recorded -# `fbw_blackhole_adopted_single_frame` pins; `fbw_rolled_back_with_effects` back -# above zero means the capture broke and the store is running twice again. -# -# The forcing residual itself never contributes an effect: the force branch -# returns before the odometer bump, so a bridge escape needs a second, earlier -# effectful op to register at all -- which is exactly what this file adds. +# That is also what the PyPy oracle does through its `@jit.unroll_safe` +# `fast2locals`: one loop, one bridge, no forcings, no virtualizable forcings, +# and no aborts. The rare bridge and the preceding effectful `box.n = i` +# remain useful coverage that the optimized frame read neither duplicates nor +# loses the store. import sys _gf = sys._getframe diff --git a/pyre/bench/synth/getframe_bridge_force_after_store_declined.wasm.jitstats b/pyre/bench/synth/getframe_bridge_force_after_store_declined.wasm.jitstats index 959c5f5446c..6ec1f9b3871 100644 --- a/pyre/bench/synth/getframe_bridge_force_after_store_declined.wasm.jitstats +++ b/pyre/bench/synth/getframe_bridge_force_after_store_declined.wasm.jitstats @@ -1,14 +1,15 @@ -bridges_compiled=0 +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=20 +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=4114 +guard_failures=201 internal_compile_panics=0 -loops_aborted=20 +loops_aborted=0 loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_bridge_force_plain_declined.cranelift.jitstats b/pyre/bench/synth/getframe_bridge_force_plain_declined.cranelift.jitstats index 959c5f5446c..6ec1f9b3871 100644 --- a/pyre/bench/synth/getframe_bridge_force_plain_declined.cranelift.jitstats +++ b/pyre/bench/synth/getframe_bridge_force_plain_declined.cranelift.jitstats @@ -1,14 +1,15 @@ -bridges_compiled=0 +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=20 +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=4114 +guard_failures=201 internal_compile_panics=0 -loops_aborted=20 +loops_aborted=0 loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_bridge_force_plain_declined.dynasm.jitstats b/pyre/bench/synth/getframe_bridge_force_plain_declined.dynasm.jitstats index 959c5f5446c..6ec1f9b3871 100644 --- a/pyre/bench/synth/getframe_bridge_force_plain_declined.dynasm.jitstats +++ b/pyre/bench/synth/getframe_bridge_force_plain_declined.dynasm.jitstats @@ -1,14 +1,15 @@ -bridges_compiled=0 +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=20 +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=4114 +guard_failures=201 internal_compile_panics=0 -loops_aborted=20 +loops_aborted=0 loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_bridge_force_plain_declined.py b/pyre/bench/synth/getframe_bridge_force_plain_declined.py index 3987f6b6b51..1f7ba36c3b7 100644 --- a/pyre/bench/synth/getframe_bridge_force_plain_declined.py +++ b/pyre/bench/synth/getframe_bridge_force_plain_declined.py @@ -1,42 +1,13 @@ -# Companion to getframe_bridge_force_plain, -# carrying that file's shape at a force the constant-depth -# `sys._getframe` arm DECLINES, so the machinery it documents stays -# covered now that its own call site folds. +# Historical `_declined` companion to `getframe_bridge_force_plain`. An exact +# portal `_getframe(0).f_locals` now stays in the trace: the getter constructs +# the CPython 3.14 write-through proxy around the same red frame and does not +# force it. # -# `try_walker_specialize_sys_getframe` (jitcode_dispatch/specialize.rs) takes -# depth 0 at the top walk level, where `getframe`'s answer IS the portal -# virtualizable and no force is needed, so the CALL folds here as well. The -# force this file needs comes from the `.f_locals` getset on the folded -# result: it reads the virtualizable's own fields, so it routes through -# `force_frame_before_locals_read` on the portal and escapes it. -# -# A nonzero depth does NOT serve. `getframe` forces only the frame it RETURNS, -# and one below the portal is not the traced virtualizable, so nothing escapes. -# -# The counters recorded for this file are the ones its original -# carried before the fold; a diff against them is a real change in the escape -# machinery, not in the arm. -# -# Coverage for the forced-vable escape on a BRIDGE walk, which the rest of the -# corpus never produces: of 138 forced escapes across the synth fixtures, zero -# are `bridge=true`. -# -# Shape, each clause load-bearing: -# * the `for` loop compiles on the common arm; -# * `i % 97 == 0` is the rare arm, so its guard fails ~4124 times -- past -# `DEFAULT_TRACE_EAGERNESS` -- and `start_bridge_tracing` sets -# `ctx.is_bridge_trace`, making the walk over the rare arm a bridge walk; -# * `_gf(0).f_locals` forces the virtualizable through a residual that stays -# in the portal frame: the `_gf(0)` call folds, and the getset behind -# `.f_locals` is a builtin, so `frame_entry_count()` does not move and no -# user Python frame is entered; -# * the call sits directly in the portal frame's loop body, so the framestack -# is empty and this is not an inline sub-walk. -# -# With nothing else in the rare arm the escape's mirror image resolves and the -# walk adopts a single-frame blackhole terminal. Its sibling -# `getframe_bridge_force_after_store` puts an un-journaled store ahead of the -# forcing call and takes the replay path instead. +# PyPy reaches the same optimized shape through `@jit.unroll_safe fast2locals`: +# one loop, one bridge, no forcings, no virtualizable forcings, and no aborts. +# The rare `i % 97 == 0` arm still pins bridge compilation and the live locals +# result, but no longer endorses a single-frame blackhole escape that upstream +# never performs. import sys _gf = sys._getframe diff --git a/pyre/bench/synth/getframe_bridge_force_plain_declined.wasm.jitstats b/pyre/bench/synth/getframe_bridge_force_plain_declined.wasm.jitstats index 959c5f5446c..6ec1f9b3871 100644 --- a/pyre/bench/synth/getframe_bridge_force_plain_declined.wasm.jitstats +++ b/pyre/bench/synth/getframe_bridge_force_plain_declined.wasm.jitstats @@ -1,14 +1,15 @@ -bridges_compiled=0 +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=20 +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=4114 +guard_failures=201 internal_compile_panics=0 -loops_aborted=20 +loops_aborted=0 loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_inline_subwalk_multiframe.cranelift.jitstats b/pyre/bench/synth/getframe_inline_subwalk_multiframe.cranelift.jitstats index f03920a1a18..651a3eaf3e9 100644 --- a/pyre/bench/synth/getframe_inline_subwalk_multiframe.cranelift.jitstats +++ b/pyre/bench/synth/getframe_inline_subwalk_multiframe.cranelift.jitstats @@ -2,14 +2,14 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=5 +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=0 +guard_failures=1 internal_compile_panics=0 -loops_aborted=5 +loops_aborted=0 loops_compiled=1 retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_inline_subwalk_multiframe.dynasm.jitstats b/pyre/bench/synth/getframe_inline_subwalk_multiframe.dynasm.jitstats index f03920a1a18..651a3eaf3e9 100644 --- a/pyre/bench/synth/getframe_inline_subwalk_multiframe.dynasm.jitstats +++ b/pyre/bench/synth/getframe_inline_subwalk_multiframe.dynasm.jitstats @@ -2,14 +2,14 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=5 +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=0 +guard_failures=1 internal_compile_panics=0 -loops_aborted=5 +loops_aborted=0 loops_compiled=1 retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py index 367687ed68c..6a32c24acbf 100644 --- a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py +++ b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py @@ -9,43 +9,21 @@ # `base`. Residualizing the depth-zero lookup instead forces the published # callee during tracing and prevents this loop from compiling. The specialized # path therefore records the callee frame and creates its `FrameLocalsProxy` -# without forcing the outer standard virtualizable; positive-depth lookup stays -# on the established virtual-reference walk. +# without forcing the outer standard virtualizable. The positive-depth walk +# must carry each frame's own red box, close the corresponding live +# `virtual_ref` pair around `jit_force_virtual`, and let the optimizer forward +# the force to that pair's virtual frame. # # A real PyPy run compiles one loop with no bridges, forcings, virtualizable # forcings, or aborts. Pyre must print the same value, compile one loop, and # report no escape abort or frame-blackhole adoption for this fixture. # -# The positive-depth clause has a measured cost, and the recorded counters -# encode it rather than endorse it. `leaf`'s `_getframe(2)` reaches `main`'s -# frame, which is the virtualizable of `main`'s compiled loop, and materializes -# it. `mid(i)` stays a residual `CallMayForce` in that loop, so the -# `GuardNotForced` behind it fails on every machine-code entry: `MAJIT_STATS=1` -# reports `mc_entered` equal to `guard_failures`, with `back_edge_polls=0` and -# `bridges_compiled=0`, so the compiled loop leaves for the blackhole before it -# ever crosses its back edge. The attribution is a one-run check -- a `leaf` -# holding only the depth-zero read records `guard_failures=1`, one holding only -# the `_getframe(2)` read reproduces the full count. -# -# The count is a known shortfall, not a number to preserve -- but it will not -# come down from the `sys._getframe` arm. Letting the inline level take that -# arm's own per-level lowering, by deleting the decline in front of it, was -# measured: the fixture still answers correctly, `guard_failures` stays at -# exactly the recorded value, and the `Finish` trace gains three -# `GuardNotForced`s, because the lowering forces once per hop rather than not -# at all. Reaching `main`'s frame object from `leaf` means materializing it -# however the walk is spelled. -# -# What removes the cost is inlining `mid` -> `leaf` into `main`'s loop, where -# the depth-2 lookup lands on the trace's own virtualizable and needs no force: -# `PYPYLOG=jit-summary: pypy3 -P` on this file reports one loop, no -# bridges, `forcings: 0` and no aborts, against the two traces pyre records. -# -# `loops_compiled` counts those two traces: `main`'s loop, and a linear -# `Finish` trace for the inlined `mid` -> `leaf` chain, whose own four -# `GuardNotForced`s never fail. The wasm baseline records `guard_failures=1` -# instead because that backend always materializes the virtualizable, which -# makes `GuardNotForced` a no-op there. +# The remaining single guard failure is also present in the depth-zero-only +# control; the positive-depth frame walk itself compiles without an abort, +# bridge, forcing, or blackhole adoption. In the optimized loop, the temporary +# `JIT_FORCE_VIRTUAL`/`GUARD_NOT_FORCED` pair emitted for the orthodox residual +# bracket is gone: `VIRTUAL_REF_FINISH` lets the optimizer forward it to the +# paired virtual frame, just as in PyPy. import sys diff --git a/pyre/bench/synth/getframe_inline_subwalk_multiframe.wasm.jitstats b/pyre/bench/synth/getframe_inline_subwalk_multiframe.wasm.jitstats index f03920a1a18..651a3eaf3e9 100644 --- a/pyre/bench/synth/getframe_inline_subwalk_multiframe.wasm.jitstats +++ b/pyre/bench/synth/getframe_inline_subwalk_multiframe.wasm.jitstats @@ -2,14 +2,14 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=5 +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=0 +guard_failures=1 internal_compile_panics=0 -loops_aborted=5 +loops_aborted=0 loops_compiled=1 retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.cranelift.jitstats b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.cranelift.jitstats index 9400653eeeb..4e1e41a7bed 100644 --- a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.cranelift.jitstats +++ b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.cranelift.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=1 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.dynasm.jitstats b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.dynasm.jitstats index 9400653eeeb..4e1e41a7bed 100644 --- a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.dynasm.jitstats +++ b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.dynasm.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=1 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.py b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.py index d15b022c4d3..9b9ed5c470b 100644 --- a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.py +++ b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.py @@ -1,25 +1,13 @@ # pyre-check: max-pypy-ratio=32 -# Companion to getframe_residual_callee_own_frame, -# carrying that file's shape at a force the constant-depth -# `sys._getframe` arm DECLINES, so the machinery it documents stays -# covered now that its own call site folds. +# Historical `_declined` companion to `getframe_residual_callee_own_frame`. +# Exact `_getframe(0).f_locals` now remains traced and owns the callee's exact +# red frame, so it neither forces that frame nor clears the caller's +# virtualizable token. # -# `try_walker_specialize_sys_getframe` (jitcode_dispatch/specialize.rs) takes -# only depth 0 at the top walk level, where `getframe`'s answer IS the portal -# virtualizable and no force is needed. -# The `_getframe` call itself folds here; the added `.f_locals` read is the -# forcing residual, and it forces the SAME frame, so the shape below is -# unchanged apart from where the force comes from. -# -# The counters recorded for this file are the ones its original -# carried before the fold; a diff against them is a real change in the escape -# machinery, not in the arm. -# -# Regression guard: a residual (may-force) callee that inspects its OWN frame -# via sys._getframe() must not clear the traced CALLER's virtualizable tracing -# token. Clearing it raised a spurious frame-escape with no committed resume pc, -# replaying the loop body from entry and double-applying the callee's -# non-journaled STORE_ATTR side effect -- a JIT-only wrong answer (c.n > loops). +# The recorded shape is two loops, no bridge, no forcings, no virtualizable +# forcings, and no aborts. The observable regression guard remains the +# non-journaled `STORE_ATTR`: frame inspection must execute `bump` exactly once +# per iteration, so `c.n` must stay equal to the loop count. import sys diff --git a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.wasm.jitstats b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.wasm.jitstats index 9400653eeeb..4e1e41a7bed 100644 --- a/pyre/bench/synth/getframe_residual_callee_own_frame_declined.wasm.jitstats +++ b/pyre/bench/synth/getframe_residual_callee_own_frame_declined.wasm.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=1 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.cranelift.jitstats b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.cranelift.jitstats index 77817899ecb..651a3eaf3e9 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.cranelift.jitstats +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.cranelift.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=0 +guard_failures=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=0 +loops_aborted=0 +loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.dynasm.jitstats b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.dynasm.jitstats index a893551fa0c..651a3eaf3e9 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.dynasm.jitstats +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.dynasm.jitstats @@ -3,13 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=0 +guard_failures=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=0 +loops_aborted=0 +loops_compiled=1 retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.py b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.py index c3ba428296c..1d0a05ff3a5 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.py +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.py @@ -1,54 +1,11 @@ -# Companion to getframe_root_loop_force_blackhole_crn, -# carrying that file's shape at a force the constant-depth -# `sys._getframe` arm DECLINES, so the machinery it documents stays -# covered now that its own call site folds. -# -# `try_walker_specialize_sys_getframe` (jitcode_dispatch/specialize.rs) takes -# depth 0 at the top walk level, where `getframe`'s answer IS the portal -# virtualizable and no force is needed, so the CALL folds here as well. The -# force this file needs comes from the `.f_locals` getset on the folded -# result: it reads the virtualizable's own fields, so it routes through -# `force_frame_before_locals_read` on the portal and escapes it. -# -# A nonzero depth does NOT serve. `getframe` forces only the frame it RETURNS, -# and one below the portal is not the traced virtualizable, so nothing escapes. -# -# The counters recorded for this file are the ones its original -# carried before the fold; a diff against them is a real change in the escape -# machinery, not in the arm. -# -# Regression guard: SIGSEGV from a NULL operand-stack slot written by the -# single-frame blackhole's ContinueRunningNormally handoff. -# -# The walk roots at `main`, which HAS a Python loop, so its portal jitcode -# carries a jit_merge_point at the loop header. A sys._getframe force inside the -# loop latched a blackhole image built at the resume pc just past the residual; -# driving it ran to the loop back edge and raised ContinueRunningNormally at the -# merge point. The MIFrame is seeded from the live colors at the BUILD pc, but -# the merge point has its own live set, so a Ref color live at the merge but not -# at the build read back NULL -- and apply_blackhole_crn had no NULL guard, so it -# wrote a null into a live operand-stack slot. Resuming there faulted the -# interpreter (EXC_BAD_ACCESS at 0x0 in baseobjspace::next), exit 139, JIT-only. -# -# The walk's own flush declines exactly this case ("NULL operand-stack shadow -# slot (mid-expression)"); the blackhole path did not. -# -# Guarding it post-drive is NOT sufficient and this fixture also pins that: a -# post-drive decline discards a region the drive already executed and hands it -# back to the replay, which turned 199990000 into 200005595. -# -# This shape is also the one that exposed the two defects behind that NULL. The -# image seeded only the colors live at the build pc rather than the whole -# concrete bank; and the walk synchronized the virtualizable into the -# snapshot_for_tracing copy while the image's vable identity pointed at the live -# frame, so the drive read locals one Python iteration stale and accumulated -# total += 1039 where i was already 1040. -# -# None of that is reachable here: the CRN handoff does not rebuild the frame -# from the terminal register banks, so there is no NULL to write and nothing -# to decline. The drive runs and this file adopts it five times. Its effects are idempotent, though, which is exactly -# what hid the residual heap half of a post-drive decline — see the -# `_nonidempotent` sibling. +# Historical `_declined` companion to `getframe_root_loop_force_blackhole_crn`. +# Exact portal `_getframe(0).f_locals` now creates its write-through proxy in +# the trace, so this shape never enters the blackhole CRN handoff. +# +# PyPy reports one loop, no bridge, no forcings, no virtualizable forcings, and +# no aborts. The fixture still pins the result and locals view at a loop back +# edge; its old NULL-slot/blackhole history belongs to the sibling that +# deliberately exercises a real force, not to this upstream-force-free path. import sys diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.wasm.jitstats b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.wasm.jitstats index 77817899ecb..651a3eaf3e9 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.wasm.jitstats +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_declined.wasm.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=0 +guard_failures=1 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=0 +loops_aborted=0 +loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.cranelift.jitstats b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.cranelift.jitstats index 77817899ecb..d7b142df687 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.cranelift.jitstats +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.cranelift.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=0 +guard_failures=5 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=0 +loops_aborted=0 +loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.dynasm.jitstats b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.dynasm.jitstats index a893551fa0c..d7b142df687 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.dynasm.jitstats +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.dynasm.jitstats @@ -3,13 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=0 +guard_failures=5 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=0 +loops_aborted=0 +loops_compiled=1 retraces_compiled=0 diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.py b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.py index bf777a2dbab..8127deca969 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.py +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.py @@ -1,43 +1,10 @@ -# Companion to getframe_root_loop_force_blackhole_crn_nonidempotent, -# carrying that file's shape at a force the constant-depth -# `sys._getframe` arm DECLINES, so the machinery it documents stays -# covered now that its own call site folds. +# Historical `_declined` companion to the non-idempotent blackhole CRN +# regression. Exact portal `_getframe(0).f_locals` is now force-free, so this +# upstream shape stays in the compiled loop and never enters that handoff. # -# `try_walker_specialize_sys_getframe` (jitcode_dispatch/specialize.rs) takes -# depth 0 at the top walk level, where `getframe`'s answer IS the portal -# virtualizable and no force is needed, so the CALL folds here as well. The -# force this file needs comes from the `.f_locals` getset on the folded -# result: it reads the virtualizable's own fields, so it routes through -# `force_frame_before_locals_read` on the portal and escapes it. -# -# A nonzero depth does NOT serve. `getframe` forces only the frame it RETURNS, -# and one below the portal is not the traced virtualizable, so nothing escapes. -# -# The counters recorded for this file are the ones its original -# carried before the fold; a diff against them is a real change in the escape -# machinery, not in the arm. -# -# Regression guard: a blackhole drive that is declined after the fact must not -# hand its already-executed region back to a replay. -# -# Same shape as `getframe_root_loop_force_blackhole_crn`: the walk roots at -# `main`, whose portal jitcode carries a jit_merge_point at the loop header, and -# a sys._getframe force inside the loop latches a blackhole image that drives to -# the back edge and raises ContinueRunningNormally there. -# -# The difference is the effect in the driven region. The sibling accumulates -# into a local and adds an already-present element to a set, so BOTH halves of a -# post-drive decline are invisible once the frame is restored: the local comes -# back with the undo, and re-running `set.add` of the same string changes -# nothing. A list append does not have that property. With the CRN arm forced -# to decline, that sibling prints its correct 199990000 while this file prints -# 20005 appends for 20000 iterations — one extra per declined drive. -# -# That is the measurement behind `adopt_blackhole_crn`: a decline taken after -# the drive cannot be repaired by any frame-level undo, because the residual -# calls the drive ran are heap effects and not frame state. So the CRN handoff -# resumes on the frame the blackhole already wrote instead of validating a -# rebuilt register image that could reject it. +# PyPy reports one loop, no bridge, no forcings, no virtualizable forcings, and +# no aborts. The list append remains a strong observable guard: eliminating +# the unnecessary force must still append exactly once per Python iteration. import sys diff --git a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.wasm.jitstats b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.wasm.jitstats index 77817899ecb..d7b142df687 100644 --- a/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.wasm.jitstats +++ b/pyre/bench/synth/getframe_root_loop_force_blackhole_crn_nonidempotent_declined.wasm.jitstats @@ -3,12 +3,13 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=5 +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=0 +guard_failures=5 internal_compile_panics=0 -loops_aborted=5 -loops_compiled=0 +loops_aborted=0 +loops_compiled=1 +retraces_compiled=0 diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index 48f020bd517..b745077b07b 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -847,20 +847,21 @@ fn simple_namespace_replace(args: &[PyObjectRef]) -> crate::PyResult { /// /// `try_walker_specialize_sys_getframe` /// (`pyre-jit-trace/src/jitcode_dispatch/specialize.rs`) reproduces upstream's -/// traced-through form for the one level it can resolve — depth 0 at the top -/// walk level, where the answer IS the portal virtualizable — so those call -/// sites reach neither this function nor its force. Over the `getframe_*` -/// corpus that took `loops_aborted` 155 → 71 and `loops_compiled` 6 → 22. +/// traced-through depth-0 form for either the portal or an inline MIFrame. It +/// also resolves a guarded positive-depth inline chain when the exact portal +/// result is immediately consumed by `f_locals`; `fast2locals` is +/// `@jit.unroll_safe`, so upstream reaches that mapping by reading the +/// virtualizable boxes instead of forcing, and the specialized `f_locals` +/// writes the locals region back out of the image in the dropped force's +/// place. /// /// Every other shape still arrives here, and the force stays load-bearing for -/// it. A call site inside an INLINED callee is the one the arm must keep -/// declining: that frame carries `last_instr = -1` -/// (`pyre-jit-trace/src/helpers.rs`) with nothing updating it through the body, -/// so folding it would compile a `_getframe().f_lineno` reporting the `def` -/// line where the residual's force answers correctly today. The `*_declined` -/// fixtures under `pyre/bench/synth` hold each folded shape's escape at a read -/// the arm does not cover — `_getframe(0).f_locals`, whose getset forces the -/// portal on its own — so the machinery behind this force keeps its coverage. +/// it. In particular, generic positive-depth consumers and inline +/// `_getframe().f_lineno` / `f_lasti` remain residual until each app-level frame +/// getter carries the same per-frame red identity and resume coordinate. The +/// guarded positive-depth specialization admits only its completed immediate +/// `f_locals` slice, so it cannot bake a stale inline `last_instr` into those +/// other getters. pub fn getframe(depth: i64) -> crate::PyResult { let ec = current_execution_context(); let mut current = if ec.is_null() { 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 65f9715b7ff..ba54f6f6aef 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -4451,7 +4451,7 @@ pub(crate) fn maybe_walker_vable_and_vrefs_before_residual_call( ctx: &mut WalkContext<'_, '_, Sym>, jit_pc: usize, ) { - maybe_record_inline_callee_last_instr(ctx, jit_pc); + record_and_publish_inline_callee_last_instr(ctx, jit_pc); walker_vable_and_vrefs_before_residual_call(ctx.trace_ctx); } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 5b0203dab84..4d719b60000 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -2557,6 +2557,65 @@ fn walker_specialize_traceback_walk_field( Ok(Some(())) } +/// Write the standard virtualizable's locals region into the array a folded +/// `f_locals` hands out. +/// +/// `pyframe.py fast2locals` — the body behind `getdictscope`, and so behind +/// `f_locals` — is `@jit.unroll_safe`, and the `locals_cells_stack_w[i]` reads +/// it unrolls are `getarrayitem_vable_r` against the virtualizable BOXES. The +/// getter therefore neither forces the virtualizable nor reads its array, which +/// is what makes folding it legitimate at all. +/// +/// pyre answers `f_locals` with the 3.14 `FrameLocalsProxy`, which reads the +/// frame's array lazily instead of copying out of it at the call, so the values +/// have to be IN that array by the time the proxy is handed out. +/// `pyjitpl.py synchronize_virtualizable` (`virtualizable.py write_boxes`) is +/// the write-back that puts them there. Upstream runs it against the +/// recording-time virtualizable after every vable store; both halves are +/// needed here, because the values have to be in the array for the walk's own +/// read of the proxy AND for the compiled run's. So this mirrors the region +/// onto the concrete frame and emits the same store into the trace. Without +/// it the residual getter's read barrier was the only thing writing the region +/// out, and folding the getter silently dropped every local the traced body +/// had assigned. +/// +/// Only the locals/cells region is written back. `write_boxes` covers the +/// whole array, but the operand-stack region above `nlocals` is not reachable +/// through the proxy and its shadow slots read NULL outside a merge point +/// (see [`crate::state::flush_locals_region_to_frame`]), so writing those back +/// would destroy the values the walk is holding. +/// +/// A slot the shadow cannot answer declines the whole write-back, and with it +/// the fold, leaving the residual force in place. The validation pass runs +/// before the first emission, so a decline emits nothing. +fn walker_write_back_standard_frame_locals( + ctx: &mut WalkContext<'_, '_, Sym>, + frame_op: OpRef, + concrete_frame: usize, +) -> bool { + let Some(info) = ctx.trace_ctx.virtualizable_info().cloned() else { + return false; + }; + let base = info.num_static_extra_boxes; + let Some(nlocals) = crate::state::concrete_nlocals(concrete_frame) else { + return false; + }; + // `Value::Void` is the shadow's "no concrete half" sentinel rather than an + // unbound local, so a slot carrying it cannot be written back. + let mut slots = Vec::with_capacity(nlocals); + for slot in 0..nlocals { + match ctx.trace_ctx.virtualizable_entry_at(base + slot) { + Some((_, majit_ir::Value::Void)) | None => return false, + Some((value, _)) => slots.push((slot as i64, value)), + } + } + if !crate::state::flush_locals_region_to_frame(ctx.trace_ctx, concrete_frame) { + return false; + } + ctx.trace_ctx + .vable_array_region_write_back(frame_op, 0, &slots) +} + /// `mapdict.py LOAD_ATTR_caching` full-body-walker fast path for a /// plain (non-method) instance attribute. When the concrete receiver is a /// monomorphic instance whose attribute resolves to a boxed plain storage slot @@ -2614,13 +2673,28 @@ pub(crate) fn try_walker_specialize_load_attr( // CPython 3.14 exposes an optimized frame's locals as a fresh // `FrameLocalsProxy`. Constructing that proxy does not read fast locals; // its operations synchronize through the frame when they are actually - // used. Keep the inline callee's own red frame as the proxy owner instead - // of residualizing the getter, whose explicit read barrier would force the - // outer standard virtualizable while this MIFrame is still active. + // used. Keep the exact per-MIFrame red receiver as the proxy owner instead + // of residualizing the getter, whose explicit read barrier would force a + // live virtualizable while an inline MIFrame is still active. + // + // There are two identities the walker can prove here: the current inline + // callee's shadow frame, whose locals region the walk flushes itself at the + // escape, or the standard portal frame, gated by BOTH its red box and its + // concrete pointer. For the portal the dropped force was also what wrote + // the locals region out of the virtualizable image, so the fold has to + // write that region itself. let inline_frame = current_inline_concrete_frame(); - if name == "f_locals" - && inline_frame != 0 + let is_inline_frame = inline_frame != 0 && concrete_obj as usize == inline_frame + && ctx + .callee_shadow + .as_ref() + .is_some_and(|shadow| shadow.concrete_frame == inline_frame && shadow.frame_box == obj); + let is_standard_frame = ctx.trace_ctx.standard_virtualizable_box() == Some(obj) + && ctx.trace_ctx.standard_virtualizable_ptr() == Some(concrete_obj as usize); + + if name == "f_locals" + && (is_inline_frame || is_standard_frame) && unsafe { (*concrete_obj).ob_type } == &pyre_interpreter::pyframe::FRAME_TYPE && unsafe { (*(concrete_obj as *const pyre_interpreter::PyFrame)) @@ -2628,10 +2702,6 @@ pub(crate) fn try_walker_specialize_load_attr( .flags .contains(pyre_interpreter::CodeFlags::OPTIMIZED) } - && ctx - .callee_shadow - .as_ref() - .is_some_and(|shadow| shadow.concrete_frame == inline_frame && shadow.frame_box == obj) { let w_type = pyre_interpreter::typedef::gettypeobject(&pyre_interpreter::pyframe::FRAME_TYPE); @@ -2639,6 +2709,11 @@ pub(crate) fn try_walker_specialize_load_attr( if version_tag == 0 || unsafe { (*concrete_obj).w_class } != w_type { return Ok(None); } + if is_standard_frame + && !walker_write_back_standard_frame_locals(ctx, obj, concrete_obj as usize) + { + return Ok(None); + } let concrete_proxy = pyre_interpreter::pyframe::frame_locals_proxy::new(concrete_obj); walker_guard_exception_attr_slot(ctx, op_pc, obj, concrete_obj, w_type, version_tag)?; let proxy = ctx.trace_ctx.call_ref_typed_with_effect( @@ -8987,11 +9062,13 @@ pub(crate) fn try_walker_specialize_builtin_locals( /// frame is always the portal, so the residual always escapes. Removing it /// removes the force with it, and nothing has to replace it: `last_instr` is /// published onto the portal frame at every may-force boundary -/// (`LiveLastInstrGuard`), and every getset that reads a virtualizable field -/// off the handed-out frame (`f_locals`, and `f_lasti` / `f_lineno` through -/// their own `jit_getattr` residual) is itself such a boundary. Of those only -/// `f_locals` also FORCES: measured against this fold, swapping a fixture's -/// forcing read for `f_lasti` or `f_lineno` leaves `loops_aborted` at 0. +/// (`LiveLastInstrGuard`). Generic `f_lasti` / `f_lineno` readers retain that +/// residual boundary. An exact optimized-frame `f_locals` read is specialized +/// below instead: `pyframe.py fast2locals` is `@jit.unroll_safe`, so upstream +/// traces through it and reads the virtualizable boxes rather than forcing. +/// The force it drops was also the only writer of the frame's locals region, +/// which pyre's `FrameLocalsProxy` reads; the fold therefore performs that +/// write-back itself (`walker_write_back_standard_frame_locals`). /// /// Emitted shape, following `getframe`'s body line by line: /// `guard_value(callable)`; `guard_class` + exact-class + `getfield_gc_i` on @@ -9034,6 +9111,78 @@ pub(crate) fn try_walker_specialize_builtin_locals( /// forced `f_backref` is null, or a hop whose result is hidden. /// Declines after emission rewind to the pre-specialization trace position and /// reset the heap cache before falling through. +fn next_op_is_f_locals_for_getframe_result( + code: &[u8], + op: &DecodedOp, + ctx: &WalkContext<'_, '_, Sym>, + getframe_dst: usize, +) -> bool { + let Some(mut next) = crate::jitcode_runtime::decode_op_at(code, op.next_pc) else { + return false; + }; + while next.opname == "live" + || next.opname.starts_with("setarrayitem_vable") + || next.opname.starts_with("setfield_vable") + { + let Some(after_bookkeeping) = crate::jitcode_runtime::decode_op_at(code, next.next_pc) + else { + return false; + }; + next = after_bookkeeping; + } + let helper_kind = residual_call::residual_call_descr_index_in_body(code, &next) + .and_then(|index| ctx.descr_refs.at(index)) + .and_then(|descr| { + descr + .as_call_descr() + .map(|call| call.get_extra_info().pyre_helper) + }); + if next.key != "residual_call_ir_r/iIRd>r" + || helper_kind != Some(majit_ir::PyreHelperKind::LoadAttr) + { + return false; + } + + // `iIRd>r`: funcbox, Int var-list, Ref var-list, descr, result. The + // LoadAttr helper's lists are `[name_idx]` and `[obj, code]`. + let Some(&i_len_byte) = code.get(next.pc + 2) else { + return false; + }; + let i_len = i_len_byte as usize; + if i_len != 1 { + return false; + } + let Some(&name_reg) = code.get(next.pc + 3) else { + return false; + }; + let r_len_pc = next.pc + 3 + i_len; + if code.get(r_len_pc) != Some(&2) { + return false; + } + let (Some(&obj_reg), Some(&code_reg)) = (code.get(r_len_pc + 1), code.get(r_len_pc + 2)) else { + return false; + }; + if obj_reg as usize != getframe_dst { + return false; + } + let (Some(&name_op), Some(&code_op)) = ( + ctx.registers_i.get(name_reg as usize), + ctx.registers_r.get(code_reg as usize), + ) else { + return false; + }; + let (Some(majit_ir::Value::Int(name_idx)), Some(majit_ir::Value::Ref(w_code))) = ( + ctx.trace_ctx.box_value(name_op), + ctx.trace_ctx.box_value(code_op), + ) else { + return false; + }; + if name_idx < 0 || w_code.as_usize() == 0 { + return false; + } + walker_load_name_from_code(w_code.as_usize(), name_idx as usize).as_deref() == Some("f_locals") +} + pub(crate) fn try_walker_specialize_sys_getframe( ctx: &mut WalkContext<'_, '_, Sym>, code: &[u8], @@ -9101,14 +9250,12 @@ pub(crate) fn try_walker_specialize_sys_getframe( // virtual frame upstream. let inline_ptr = current_inline_concrete_frame(); let inline_level = ctx.fbw_mode.inline_subwalk || inline_ptr != 0; - // A depth-zero lookup returns this MIFrame's own red frame directly. A - // positive depth has to force each intervening virtual reference through - // `_do_jit_force_virtual`; until the inline walker carries that operation - // with the same per-level resume state, keep the established residual path - // instead of treating the concrete recording-time chain as its substitute. - if inline_level && depth_value != 0 { + let (Some(standard_vable_op), Some(standard_vable_ptr)) = ( + ctx.trace_ctx.standard_virtualizable_box(), + ctx.trace_ctx.standard_virtualizable_ptr(), + ) else { return Ok(None); - } + }; let (vable_op, vable_ptr) = if inline_level { let Some(shadow) = ctx.callee_shadow.as_ref() else { return Ok(None); @@ -9119,13 +9266,7 @@ pub(crate) fn try_walker_specialize_sys_getframe( } (shadow.frame_box, inline_ptr) } else { - let (Some(op), Some(ptr)) = ( - ctx.trace_ctx.standard_virtualizable_box(), - ctx.trace_ctx.standard_virtualizable_ptr(), - ) else { - return Ok(None); - }; - (op, ptr) + (standard_vable_op, standard_vable_ptr) }; let ec = pyre_interpreter::call::getexecutioncontext() as *mut pyre_interpreter::PyExecutionContext; @@ -9149,27 +9290,89 @@ pub(crate) fn try_walker_specialize_sys_getframe( } frame }; - // A hop whose `f_backref` is a live `JitVirtualRef` names another INLINED - // frame, and this walk publishes a forced pair only for the level it owns - // (`pyjitpl.py vrefs_after_residual_call`). The emitted - // `jit_force_virtual` then reaches a vref the optimizer materialises with a - // null `forced` field, so the hop lands on whatever that read produces - // rather than on the caller's frame. Scan the record-time chain for one - // before anything is emitted: the seed below forces the walk's own vref for - // real and finishes its pair, so a later decline would leave the residual - // `getframe` a shorter chain than the interpreter's. Non-virtual slots - // hold the frame pointer itself (`_jit_vref.py:40`), so following them here - // forces nothing. - { + // Validate the entire concrete chain before emitting or forcing anything. + // A tracing-time `JitVirtualRef` is admissible only when it is still one of + // `MetaInterp.virtualref_boxes`: that is the pair + // `vrefs_after_residual_call` will publish if this walk forces it. Reading + // `forced` here does not force or change the token; vrefs created during + // tracing already carry the real recording-time frame there + // (`virtualref.py virtual_ref_during_tracing`). This all-or-nothing gate + // keeps a later decline from shortening the concrete frame chain before + // the generic residual gets a chance to run. + // + // A hidden hop declines outright. `executioncontext.py + // getnextframe_nohidden` skips a hidden frame WITHOUT consuming a depth + // level, so one raw `f_backref` per level only reproduces `getframe`'s walk + // on a chain that has none; the emitted traversal pins that with its + // per-hop `guard_false(hidden_applevel)`. + let final_concrete_frame = { let mut scan = frame; for _ in 0..depth_value { let raw = unsafe { (*scan).f_backref }; - if raw.is_null() - || unsafe { majit_metainterp::virtualref::ptr_is_virtual_ref(raw as *const u8) } - { + if raw.is_null() { return Ok(None); } - scan = raw; + if unsafe { majit_metainterp::virtualref::ptr_is_virtual_ref(raw as *const u8) } { + let referent = unsafe { + majit_metainterp::virtualref::vref_forced(raw as *const u8) + as *mut pyre_interpreter::PyFrame + }; + if referent.is_null() + || (ctx + .trace_ctx + .live_virtualref_pair_for_ptr(raw as usize) + .is_none() + && ctx + .trace_ctx + .virtualref_virtual_for_object_ptr(referent as usize) + .is_none()) + { + if fbw_debug_abort_enabled() { + let pairs = ctx.trace_ctx.snapshot_virtualref_boxes(); + eprintln!( + "[getframe-decline] depth={depth_value} vref {:#x} referent={:#x} has no pair; tracked={pairs:?}", + raw as usize, referent as usize, + ); + } + return Ok(None); + } + scan = referent; + } else { + scan = raw; + } + if unsafe { (*scan).hide() } { + return Ok(None); + } + } + scan + }; + + // Until every app-level frame getter is lowered through its own red frame, + // admitting an arbitrary positive-depth result would expose it to a + // generic residual whose single live-coordinate slot cannot describe a + // nested caller chain. The completed slice is the outer standard frame + // immediately consumed by `f_locals`: that getter is specialized below and + // its locals write-back names the same frame, so it crosses no such + // residual boundary. Preflight its whole static shape before emitting any + // part of `_getframe`. + if inline_level && depth_value > 0 { + let standard_frame = final_concrete_frame as usize == standard_vable_ptr + && unsafe { (*final_concrete_frame).ob_header.ob_type } + == &pyre_interpreter::pyframe::FRAME_TYPE + && unsafe { + (*final_concrete_frame) + .code() + .flags + .contains(pyre_interpreter::CodeFlags::OPTIMIZED) + }; + let w_type = + pyre_interpreter::typedef::gettypeobject(&pyre_interpreter::pyframe::FRAME_TYPE); + if !standard_frame + || unsafe { (*final_concrete_frame).ob_header.w_class } != w_type + || unsafe { pyre_object::typeobject::w_type_get_version_tag(w_type) } == 0 + || !next_op_is_f_locals_for_getframe_result(code, op, ctx, dst) + { + return Ok(None); } } @@ -9252,36 +9455,111 @@ pub(crate) fn try_walker_specialize_sys_getframe( majit_ir::Value::Ref(majit_ir::GcRef(raw_ptr as usize)), ); - let next_ptr = pyre_interpreter::executioncontext::force_vref(raw_ptr); - if next_ptr.is_null() || unsafe { (*next_ptr).hide() } { + let raw_is_vref = + unsafe { majit_metainterp::virtualref::ptr_is_virtual_ref(raw_ptr as *const u8) }; + let (next_op, next_ptr) = if raw_is_vref { + // `_do_jit_force_virtual` sees the vref box as a known + // non-standard virtualizable and returns None, so + // `do_residual_call` executes the may-force call. Run the exact + // vref bracket around that concrete force: the post half records + // `VIRTUAL_REF_FINISH(vref, virtual)` before the CALL and replaces + // the tracked vref with CONST_NULL (`pyjitpl.py`). The optimizer + // can then forward JIT_FORCE_VIRTUAL to the paired frame instead + // of materialising a vref whose `forced` field is null. + let live_pair = ctx.trace_ctx.live_virtualref_pair_for_ptr(raw_ptr as usize); + let referent = unsafe { + majit_metainterp::virtualref::vref_forced(raw_ptr as *const u8) + as *mut pyre_interpreter::PyFrame + }; + let virtual_op = live_pair.map(|pair| pair.0).unwrap_or_else(|| { + ctx.trace_ctx + .virtualref_virtual_for_object_ptr(referent as usize) + .expect("the pre-emission frame-chain census accepted this stopped vref") + }); + // A field read can produce an alias box even though its concrete + // value is the tracked vref. Upstream's heapcache normally hands + // `_do_jit_force_virtual` the tracked box directly. Preserve + // that identity for the optimizer after proving the alias at + // runtime; `VIRTUAL_REF_FINISH` and JIT_FORCE_VIRTUAL must name + // the same vref box for `optimize_jit_force_virtual` to forward + // the result to `virtual_op`. + let force_arg = if let Some((_, vref_op)) = live_pair { + if raw_op != vref_op { + let is_tracked_vref = + ctx.trace_ctx.record_op(OpCode::PtrEq, &[raw_op, vref_op]); + ctx.trace_ctx + .set_opref_concrete(is_tracked_vref, majit_ir::Value::Int(1)); + walker_emit_fold_guard_with_snapshot( + ctx, + op.pc, + OpCode::GuardTrue, + &[is_tracked_vref], + )?; + } + vref_op + } else { + raw_op + }; + maybe_walker_vable_and_vrefs_before_residual_call(ctx, op.pc); + ctx.trace_ctx.vrefs_before_residual_call(); + let next_ptr = pyre_interpreter::executioncontext::force_vref(raw_ptr); + ctx.trace_ctx.vrefs_after_residual_call(); + let force_fn = crate::helpers::jit_force_vref as *const (); + let forced_op = ctx.trace_ctx.call_typed_with_effect( + OpCode::CallMayForceR, + force_fn, + &[force_arg], + &[majit_ir::Type::Ref], + majit_ir::Type::Ref, + majit_ir::EffectInfo::new( + majit_ir::ExtraEffect::ForcesVirtualOrVirtualizable, + majit_ir::OopSpecIndex::JitForceVirtual, + ), + ); + ctx.trace_ctx.set_opref_concrete( + forced_op, + majit_ir::Value::Ref(majit_ir::GcRef(next_ptr as usize)), + ); + ctx.trace_ctx.record_guard(OpCode::GuardNotForced, &[], 0); + walker_capture_snapshot_for_last_guard(ctx, op.pc)?; + // `VirtualRefFinish(vref, virtual)` immediately before the call is + // the optimizer proof that `forced_op == virtual_op`. Preserve + // the orthodox force in IR while letting the source walker follow + // the same forwarded box immediately. + (virtual_op, next_ptr) + } else if raw_ptr as usize == standard_vable_ptr { + // `_do_jit_force_virtual`: the standard virtualizable identity + // short-circuits before residual-call preparation. Heapcache + // normally gives us the same OpRef; keep the runtime proof for an + // alias box, matching its PTR_EQ + implement_guard_value arm. + if raw_op != standard_vable_op { + let is_standard = ctx + .trace_ctx + .record_op(OpCode::PtrEq, &[raw_op, standard_vable_op]); + ctx.trace_ctx + .set_opref_concrete(is_standard, majit_ir::Value::Int(1)); + walker_emit_fold_guard_with_snapshot( + ctx, + op.pc, + OpCode::GuardTrue, + &[is_standard], + )?; + } + (standard_vable_op, raw_ptr) + } else { + if fbw_debug_abort_enabled() { + eprintln!( + "[getframe-decline] depth={depth_value} non-vref hop {:#x} != standard {standard_vable_ptr:#x}", + raw_ptr as usize + ); + } ctx.trace_ctx.cut_trace_with_snapshots(pre_emit_pos); ctx.trace_ctx.heap_cache_mut().reset(); return Ok(None); + }; + if next_ptr.is_null() || unsafe { (*next_ptr).hide() } { + unreachable!("the pre-emission frame-chain census accepted this hop") } - - maybe_walker_vable_and_vrefs_before_residual_call(ctx, op.pc); - let force_fn = crate::helpers::jit_force_vref as *const (); - let next_op = ctx.trace_ctx.call_typed_with_effect( - OpCode::CallMayForceR, - force_fn, - &[raw_op], - &[majit_ir::Type::Ref], - majit_ir::Type::Ref, - majit_ir::EffectInfo::new( - majit_ir::ExtraEffect::ForcesVirtualOrVirtualizable, - majit_ir::OopSpecIndex::JitForceVirtual, - ), - ); - ctx.trace_ctx.set_opref_concrete( - next_op, - majit_ir::Value::Ref(majit_ir::GcRef(next_ptr as usize)), - ); - // `pyjitpl.py:2163-2165` short-circuits a known non-standard - // virtualizable to `None`; the caller turns that into this residual - // `jit_force_virtual` call, so the ptr_eq/guard_value half is not - // emitted for hop results. - ctx.trace_ctx.record_guard(OpCode::GuardNotForced, &[], 0); - walker_capture_snapshot_for_last_guard(ctx, op.pc)?; walker_emit_fold_guard_with_snapshot(ctx, op.pc, OpCode::GuardNonnull, &[next_op])?; let code_op = crate::state::opimpl_getfield_gc_r( @@ -9335,10 +9613,27 @@ pub(crate) fn try_walker_specialize_sys_getframe( // the ordinary residual force path flushes, without forcing the outer // portal virtualizable or aborting its trace. if inline_level && depth_value == 0 { - maybe_record_inline_callee_last_instr(ctx, op.pc); + residual_call::record_and_publish_inline_callee_last_instr(ctx, op.pc); disarm_folded_inline_callee_after_escape(ctx, op.pc)?; } + // A positive-depth inline walk can land on the standard portal frame. + // Its symbolic `last_instr` is already current in `virtualizable_boxes` + // (the caller CALL boundary was mirrored there before descending), and + // residual-call preparation will emit that shadow's store before any + // runtime frame reader. Keep the recording-time concrete frame in step + // with the same value so a getter executed while recording observes the + // coordinate the compiled trace will publish, rather than baking the + // frame's stale pre-inline heap value into the trace. + if cur_op == standard_vable_op + && cur_ptr as usize == standard_vable_ptr + && let Some((_, majit_ir::Value::Int(last_instr))) = ctx + .trace_ctx + .virtualizable_entry_at(crate::virtualizable_spec::LAST_INSTR_VABLE_FIELD_INDEX) + { + unsafe { (*cur_ptr).last_instr = last_instr as isize }; + } + // `f.mark_as_escaped()` — vm.py. `escaped` is not one of the six fields // `interp_jit.py:25-30` declares, so the store cannot force; it is // load-bearing at `executioncontext.py leave`, which forces the diff --git a/pyre/pyre-jit-trace/src/virtualizable_spec.rs b/pyre/pyre-jit-trace/src/virtualizable_spec.rs index 5cf6e33400a..798c827b81a 100644 --- a/pyre/pyre-jit-trace/src/virtualizable_spec.rs +++ b/pyre/pyre-jit-trace/src/virtualizable_spec.rs @@ -48,6 +48,12 @@ pub const LOCALS_CELLS_STACK_W_VABLE_ARRAY_INDEX: usize = 0; /// `virtualizable_entry_at` index a reader of `getorcreatedebug()` uses. pub const DEBUGDATA_VABLE_FIELD_INDEX: usize = 3; +/// Canonical vable-field index for `last_instr`. +/// +/// The standard-frame shadow carries the portal frame's symbolic Python +/// coordinate while an inlined callee is running. +pub const LAST_INSTR_VABLE_FIELD_INDEX: usize = 0; + const _: () = { assert!( !PYFRAME_VABLE_ARRAYS.is_empty(), @@ -80,6 +86,26 @@ const _: () = { PYFRAME_VABLE_FIELDS[DEBUGDATA_VABLE_FIELD_INDEX].1 == DEBUGDATA_VABLE_FIELD_INDEX, "debugdata must be registered at the expected vable field index" ); + assert!( + PYFRAME_VABLE_FIELDS[LAST_INSTR_VABLE_FIELD_INDEX].1 == LAST_INSTR_VABLE_FIELD_INDEX, + "last_instr must be registered at the expected vable field index" + ); + let name = PYFRAME_VABLE_FIELDS[LAST_INSTR_VABLE_FIELD_INDEX] + .0 + .as_bytes(); + let expected = b"last_instr"; + assert!( + name.len() == expected.len(), + "PYFRAME_VABLE_FIELDS[0] name mismatch" + ); + let mut i = 0; + while i < expected.len() { + assert!( + name[i] == expected[i], + "PYFRAME_VABLE_FIELDS[0] name mismatch" + ); + i += 1; + } let name = PYFRAME_VABLE_FIELDS[DEBUGDATA_VABLE_FIELD_INDEX] .0 .as_bytes();