diff --git a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py index 71a1fb5da21..39763066605 100644 --- a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py +++ b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py @@ -1,5 +1,5 @@ # Coverage for the multi-frame blackhole build path: an INLINED callee that -# forces an outer frame while the walk is already inside a residual call. +# forces an outer frame through `sys._getframe(2)`. # # The walker executes a residual call concretely, so that level gets a real # frame from the interpreter's own call sequence; an inline push did not run diff --git a/pyre/bench/synth/trace_too_long_inline_multiframe.py b/pyre/bench/synth/trace_too_long_inline_multiframe.py new file mode 100644 index 00000000000..8f2f4432e4b --- /dev/null +++ b/pyre/bench/synth/trace_too_long_inline_multiframe.py @@ -0,0 +1,76 @@ +# ABORT_TOO_LONG while the authoritative walk is inside an inlined Python +# callee must continue from that callee's own MIFrame. The callee mutates +# three independently observable containers before the limit lands; replaying +# from the caller's CALL applies an iteration twice, while collapsing the +# callee onto the caller loses its locals/operand stack. +try: + import pypyjit +except ImportError: + pypyjit = None + + +if pypyjit is not None: + pypyjit.set_param("trace_limit=70,threshold=1,function_threshold=1") + + +def leaf(box, mapping, values, x): + box[0] += 1 + mapping["count"] = mapping["count"] + 1 + values.append(x) + a = x + 1 + b = a + 2 + c = b + 3 + d = c + 4 + e = d + 5 + f = e + 6 + g = f + 7 + h = g + 8 + i = h + 9 + j = i + 10 + return j + box[0] + mapping["count"] + + +def entry(box, mapping, values, x): + return leaf(box, mapping, values, x) + + +box = [0] +mapping = {"count": 0} +values = [] +total = 0 +for n in range(200): + total += entry(box, mapping, values, n) + +print(total, box[0], mapping["count"], len(values), sum(values)) + + +# The same handoff must preserve the innermost frame and pending exception +# when the blackhole finishes by unwinding instead of returning a value. +def raising_leaf(values, x): + values.append(x) + a = x + 1 + b = a + 2 + c = b + 3 + d = c + 4 + e = d + 5 + f = e + 6 + g = f + 7 + h = g + 8 + i = h + 9 + j = i + 10 + raise ValueError(j) + + +def raising_entry(values, x): + return raising_leaf(values, x) + + +raised_values = [] +raised_total = 0 +for n in range(100): + try: + raising_entry(raised_values, n) + except ValueError as exc: + raised_total += exc.args[0] + +print(raised_total, len(raised_values), sum(raised_values)) diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index cee028879cd..61a6d64659b 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -610,15 +610,6 @@ fn gc_prebuilt_remember_enabled() -> bool { }) } -/// Whether the per-return diagnostic dump is enabled -/// (`PYRE_INTERP_RETURN_LOG`). The probe sits on the RETURN_VALUE path, so an -/// uncached read would pay a `getenv` on every Python return. -#[cfg(not(feature = "sandbox"))] -fn interp_return_log_enabled() -> bool { - static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); - *ENABLED.get_or_init(|| std::env::var_os("PYRE_INTERP_RETURN_LOG").is_some()) -} - pub fn capture_pyframe_root_area() -> *const () { PYFRAME_ROOT_AREA.with(|area| area as *const _ as *const ()) } 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 aa71ace506c..a77bacb5405 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -83,6 +83,16 @@ thread_local! { const { std::cell::RefCell::new(None) }; } +/// The concrete red frame owned by the current inlined MIFrame. +/// +/// RPython carries this identity directly on every `MIFrame`; pyre's walker +/// brackets the corresponding per-thread execution state with +/// [`InlineConcreteFrameGuard`]. Vable writes use this accessor to keep that +/// frame's own heap image coherent for a later multi-frame blackhole handoff. +pub(crate) fn current_inline_concrete_frame() -> usize { + INLINE_CONCRETE_FRAME.with(|slot| slot.get() as usize) +} + pub(crate) struct EscapeFlushUndo { frame: usize, last_instr: isize, @@ -100,6 +110,11 @@ pub(crate) struct LatchedMultiFrameBlackhole { pub(crate) framestack: majit_metainterp::MIFrameStack, pub(crate) last_exc_value: i64, pub(crate) raising_exception: bool, + /// `ABORT_TOO_LONG` stops at an arbitrary post-step coordinate, so frame + /// 0's active operand stack must cross from the detached tracing snapshot + /// to the live red frame before the blackhole runs. The vable-force path + /// stops at a call resume marker and retains its existing handoff. + pub(crate) publish_root_stack: bool, } pub(crate) fn single_frame_blackhole_cell_ptr() @@ -229,15 +244,92 @@ pub(crate) fn latch_trace_too_long_blackhole( }); }); true + } else if ctx.fbw_mode.inline_subwalk { + let Some(framestack) = + build_multi_frame_miframe(ctx, resume_pc, InnermostMiframeBuild::TraceTooLong) + else { + return false; + }; + if !multi_frame_blackhole_preflight(ctx, &framestack) { + return false; + } + FBW_MULTI_FRAME_BLACKHOLE.with(|slot| { + *slot.borrow_mut() = Some(LatchedMultiFrameBlackhole { + framestack, + last_exc_value, + raising_exception: false, + publish_root_stack: true, + }); + }); + true } else { - // An inlined trace needs one independently materialized locals image - // per MIFrame. The opt-in multi-frame vable-force experiment still - // lacks that shape, so it cannot serve ABORT_TOO_LONG: this abort has - // already executed effects and has no safe entry-replay fallback. false } } +/// Read-only counterpart of every adopter gate that can reject a latched +/// multi-frame image. `ABORT_TOO_LONG` runs after the opcode's effects, so it +/// may publish the image only when the later handoff cannot fall back to entry +/// replay. RPython needs no split preflight: its per-frame red virtualizable +/// is already the live MIFrame state copied by +/// `convert_and_run_from_pyjitpl`. +fn multi_frame_blackhole_preflight( + ctx: &WalkContext<'_, '_, Sym>, + framestack: &majit_metainterp::MIFrameStack, +) -> bool { + if ctx.trace_ctx.virtualizable_info().is_none() || ctx.fbw_mode.snapshot_sym.is_null() { + return false; + } + let sym = unsafe { &*ctx.fbw_mode.snapshot_sym }; + let snapshot = sym.tracing_vable_frame_addr(); + let live_root = match ctx.trace_ctx.lookup_opref_concrete(sym.frame()) { + Some(majit_ir::Value::Ref(value)) if value.0 != 0 => value.0, + _ => sym.live_vable_frame_addr(), + }; + let root = if live_root != 0 { live_root } else { snapshot }; + if crate::state::concrete_nlocals(snapshot).is_none() + || crate::state::capture_frame_locals(root).is_none() + || !crate::state::can_write_back_outer_locals(ctx.trace_ctx, root) + || !crate::state::can_publish_frame_stack(snapshot, root) + { + return false; + } + + let mut seen = Vec::with_capacity(framestack.frames.len()); + for (index, frame) in framestack.frames.iter().enumerate() { + let Ok(jitcode_index) = i32::try_from(frame.jitcode.index()) else { + return false; + }; + let frame_reg = crate::state::portal_red_regs_at(jitcode_index).0; + if frame_reg == u16::MAX { + return false; + } + let Some(frame_ptr) = frame.ref_values.get(frame_reg as usize).copied().flatten() else { + return false; + }; + let frame_ptr = frame_ptr as usize; + let Some(stack_base) = crate::state::concrete_nlocals(frame_ptr) else { + return false; + }; + let Some(stack_depth) = crate::state::concrete_stack_depth(frame_ptr) else { + return false; + }; + let Some(array_len) = crate::state::concrete_frame_array_len(frame_ptr) else { + return false; + }; + if stack_depth < stack_base + || stack_depth > array_len + || (index == 0 && frame_ptr != root) + || (index > 0 && frame_ptr == root) + || seen.contains(&frame_ptr) + { + return false; + } + seen.push(frame_ptr); + } + true +} + fn build_single_frame_miframe( ctx: &WalkContext<'_, '_, Sym>, jitcode: std::sync::Arc, @@ -2516,6 +2608,7 @@ pub(crate) fn try_execute_residual_call_via_executor( framestack, last_exc_value, raising_exception, + publish_root_stack: false, }); }); } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/vable_ops.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/vable_ops.rs index 7187b321d3f..9b77e4c126d 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/vable_ops.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/vable_ops.rs @@ -241,6 +241,24 @@ pub(crate) fn getfield_vable_via_metainterp( /// /// `value_bank` selects the value register bank (`'i'`/`'r'`/`'f'`), /// mirroring `setfield_gc_via_heapcache`'s parameter shape. +fn current_inline_vable_target( + ctx: &WalkContext<'_, '_, Sym>, + vable: OpRef, +) -> Option { + let inline = current_inline_concrete_frame(); + if inline == 0 { + return None; + } + match ctx + .trace_ctx + .lookup_opref_concrete(vable) + .or_else(|| ctx.trace_ctx.recover_ref_value(vable, 8)) + { + Some(Value::Ref(value)) if value.as_usize() == inline => Some(inline), + _ => None, + } +} + pub(crate) fn setfield_vable_via_metainterp( code: &[u8], op: &DecodedOp, @@ -270,6 +288,10 @@ pub(crate) fn setfield_vable_via_metainterp( }; let descr = read_descr(code, op, 2, ctx)?; let concrete = vable_value_concrete(code, op, 1, ctx, value_bank, value); + let inline_field_index = ctx + .trace_ctx + .virtualizable_info() + .and_then(|info| info.static_field_by_descr(&descr)); // R7 parity: pyjitpl.py `_opimpl_setfield_vable(box, // valuebox, fielddescr, pc)` threads orgpc through // `_nonstandard_virtualizable(pc, ...)`; walker has `op.pc` for the @@ -277,6 +299,17 @@ pub(crate) fn setfield_vable_via_metainterp( let guards_before = ctx.trace_ctx.num_guards(); ctx.trace_ctx .vable_setfield(op.pc, obj, descr, value, concrete); + // `MIFrame` owns one red frame per inlined call. The trace shadow remains + // authoritative for optimization, while the matching concrete frame is + // its blackhole-resume image; mirror only own-frame standard-vable writes, + // never an outer/nonstandard virtualizable. + if let (Some(frame), Some(Value::Int(value)), Some(field_index)) = ( + current_inline_vable_target(ctx, obj), + concrete, + inline_field_index, + ) { + crate::state::store_live_frame_static_int(frame, field_index, value); + } walker_capture_inline_nonstandard_vable_guard(ctx, op.pc, guards_before)?; Ok((DispatchOutcome::Continue, op.next_pc)) } @@ -650,6 +683,11 @@ pub(crate) fn setarrayitem_vable_via_metainterp( value, concrete, ); + if index_value >= 0 + && let Some(frame) = current_inline_vable_target(ctx, vable) + { + crate::state::store_live_frame_array_slot(frame, index_value as usize, concrete); + } // Keep the inline concrete-locals shadow current so a later read of this // slot (after a may-force op clears the heapcache) recovers the concrete. // Seed BOTH maps: the read fallback prefers re-resolving the slot's OpRef diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 06db1eef5bd..7a56d5c09a1 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -4156,7 +4156,7 @@ pub(crate) fn frame_array_write_barrier( /// vable image after the guard-failure vsd correction cleared root slots /// in callee coordinates. A no-op for a null frame/array, an out-of-range /// slot, or a non-Ref value. -fn store_live_frame_array_slot(vable_ptr: usize, slot: usize, value: majit_ir::Value) { +pub(crate) fn store_live_frame_array_slot(vable_ptr: usize, slot: usize, value: majit_ir::Value) { let majit_ir::Value::Ref(r) = value else { return; }; @@ -4176,6 +4176,30 @@ fn store_live_frame_array_slot(vable_ptr: usize, slot: usize, value: majit_ir::V frame_array_write_barrier(vable_ptr as *mut u8, lp); } +/// Keep the scalar half of an inlined frame's red virtualizable coherent with +/// its MIFrame walk. Static-field indices are the `PyFrame` +/// `VirtualizableInfo` order used by the codewriter: 0 = `last_instr`, 2 = +/// `valuestackdepth`. Other fields are immutable frame identity/state and are +/// deliberately not mirrored here. +pub(crate) fn store_live_frame_static_int(vable_ptr: usize, field_index: usize, value: i64) { + if vable_ptr == 0 { + return; + } + match field_index { + 0 => unsafe { + *((vable_ptr + crate::frame_layout::PYFRAME_LAST_INSTR_OFFSET) as *mut isize) = + value as isize; + }, + 2 if value >= 0 => { + let depth = value as usize; + if concrete_frame_array_len(vable_ptr).is_some_and(|len| depth <= len) { + set_concrete_stack_depth(vable_ptr, depth); + } + } + _ => {} + } +} + /// pyframe.py:107-110: `locals_cells_stack_w` length = /// `co_nlocals + ncellvars + nfreevars + co_stacksize`. Returns the /// full heap-side array length (matching `virtualizable.py:86-99 diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index 04949d162a9..f745b941e5f 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -2068,17 +2068,19 @@ fn try_adopt_single_frame_blackhole( return false; } }; - let virtualizable_info = ctx - .virtualizable_info() - .map(std::sync::Arc::as_ptr) - .unwrap_or(std::ptr::null()); - if virtualizable_info.is_null() { + if ctx.virtualizable_info().is_none() { assert!( !trace_too_long, "preflighted trace-too-long virtualizable info disappeared" ); return false; } + // The resumed jitcodes operate on `PyFrame`, regardless of which + // translator-state vinfo happens to remain installed on `TraceCtx` at the + // walk epilogue. RPython gets this from each vable field descriptor's + // `get_vinfo()`; retain the canonical PyFrame Arc for the whole drive. + let pyframe_vinfo = crate::frame_layout::build_pyframe_virtualizable_info(); + let virtualizable_info = std::sync::Arc::as_ptr(&pyframe_vinfo); let Some(stack_base) = crate::state::concrete_nlocals(cf_addr) else { assert!( !trace_too_long, @@ -2406,14 +2408,12 @@ fn try_adopt_multi_frame_blackhole( return false; }; let depth = latched.framestack.len(); - let virtualizable_info = ctx - .virtualizable_info() - .map(std::sync::Arc::as_ptr) - .unwrap_or(std::ptr::null()); - if virtualizable_info.is_null() { + if ctx.virtualizable_info().is_none() { mfdbg!("no virtualizable_info"); return false; } + let pyframe_vinfo = crate::frame_layout::build_pyframe_virtualizable_info(); + let virtualizable_info = std::sync::Arc::as_ptr(&pyframe_vinfo); let Some(stack_base) = crate::state::concrete_nlocals(cf_addr) else { mfdbg!("cf_addr {cf_addr:#x} has no concrete nlocals"); return false; @@ -2500,10 +2500,14 @@ fn try_adopt_multi_frame_blackhole( // itself the escaping residual read the wrong frame at walk time, and // adopting committed that answer where the legacy escape/replay path // discarded it. `walker_ec_enter` / `walker_ec_leave` publish the callee - // frame on the execution context at the inlined-call push, which closes - // that hole; a `sys._getframe` executed later, inside the blackhole, was - // already correct because each level is published as it runs. + // frame on the execution context at the inlined-call push, and + // `ResidualFrameChainGuard` brackets the residual itself, which closes that + // hole; a `sys._getframe` executed later, inside the blackhole, was already + // correct because each level is published as it runs. // `synth/getframe_while_escaping_read_frame_identity` guards both readings. + // The root gate stays regardless: a residual-intermediate chain is not an + // MIFrame stack rooted at this portal and cannot reuse this walk's restart + // coordinate. mfdbg!( "chain root={root_addr:#x} cf_addr={cf_addr:#x} levels=[{}]", per_frame @@ -2582,26 +2586,64 @@ fn try_adopt_multi_frame_blackhole( // it the level reads what the live frame held before the walk began. Same // publish and same withdrawal as the single-frame arm. // - // The INNER levels get no counterpart, and that is a second thing standing - // between this path and its flip. The walk's virtualizable shadow covers - // the walked frame only, so an inlined callee's frame array keeps its - // pre-sub-walk contents while the values the sub-walk assigned sit in that - // level's register image; a local assigned inside the callee and read back - // after the escape therefore reads null. Measured on - // `scratchpad/exc_virt/s21_sigsegv.py` with the gate on: an inlined callee - // that stores `e.__traceback__` and then reads an attribute off it faults - // in `object_getattr_miss`, where the same shape through the single-frame - // arm is correct. Publishing them needs a per-level slot→value map — what - // the retired register-image rebuild built from `pcdep_trivia_at` — run - // before the drive rather than after it. + // Every INNER level already owns the concrete red frame created at the + // inline push. Its standard-vable writes are mirrored directly onto that + // frame while walking, matching RPython's one-red-frame-per-MIFrame shape; + // no slot side-table or root-frame anchor is involved. Frame 0 remains + // the sole detached-snapshot case and therefore needs the explicit publish + // below. let Some(mut locals_undo) = crate::state::capture_frame_locals(root_addr) else { mfdbg!("frame 0: {root_addr:#x} locals not capturable"); restore_links(&saved_links); return false; }; + let mut root_stack = if latched.publish_root_stack { + let Some(stack) = crate::state::capture_frame_stack_for_publish(cf_addr, root_addr) else { + mfdbg!("frame 0: active stack not capturable"); + restore_links(&saved_links); + return false; + }; + Some(stack) + } else { + None + }; + // Taking the latch removes it from the TLS extra-root walker. Root every + // MIFrame Ref bank and the pending exception across root-locals boxing, + // then copy forwarding updates back before the blackhole copies the banks. + // This is the multi-frame counterpart of the packed image roots in + // `try_adopt_single_frame_blackhole`. + let image_ref_locations: Vec<(usize, usize)> = latched + .framestack + .frames + .iter() + .enumerate() + .flat_map(|(frame_index, frame)| { + frame + .ref_values + .iter() + .enumerate() + .filter_map(move |(reg_index, value)| value.map(|_| (frame_index, reg_index))) + }) + .collect(); + let mut image_ref_roots: Vec = image_ref_locations + .iter() + .map(|&(frame_index, reg_index)| { + latched.framestack.frames[frame_index].ref_values[reg_index] + .expect("location came from Some") + }) + .collect(); + let image_exception_root = (latched.last_exc_value != 0).then(|| { + let index = image_ref_roots.len(); + image_ref_roots.push(latched.last_exc_value); + index + }); let undo_depth = majit_gc::shadow_stack::resume_ref_roots_depth(); unsafe { + majit_gc::shadow_stack::push_resume_ref_roots(image_ref_roots.as_mut_slice()); majit_gc::shadow_stack::push_resume_ref_roots(locals_undo.as_mut_slice()); + if let Some(stack) = root_stack.as_mut() { + majit_gc::shadow_stack::push_resume_ref_roots(stack.roots_mut()); + } } if !crate::state::write_back_outer_locals(ctx, root_addr) { crate::state::restore_frame_locals(root_addr, &locals_undo); @@ -2610,11 +2652,29 @@ fn try_adopt_multi_frame_blackhole( mfdbg!("frame 0: {root_addr:#x} locals publish declined"); return false; } + for (&(frame_index, reg_index), &forwarded) in image_ref_locations.iter().zip(&image_ref_roots) + { + latched.framestack.frames[frame_index].ref_values[reg_index] = Some(forwarded); + } + if let Some(index) = image_exception_root { + latched.last_exc_value = image_ref_roots[index]; + } + if let Some(stack) = root_stack.as_ref() + && !crate::state::publish_captured_frame_stack(root_addr, stack) + { + crate::state::restore_frame_locals(root_addr, &locals_undo); + majit_gc::shadow_stack::pop_resume_ref_roots_to(undo_depth); + restore_links(&saved_links); + mfdbg!("frame 0: active stack publish declined"); + return false; + } // As in the single-frame path, locals publication is the final recoverable // gate. Once the roots are released and the chain starts running, adoption // is irrevocable. majit_gc::shadow_stack::pop_resume_ref_roots_to(undo_depth); drop(locals_undo); + drop(image_ref_roots); + drop(root_stack); let ec = unsafe { (*(cf_addr as *mut pyre_interpreter::PyFrame)).execution_context as *mut pyre_interpreter::PyExecutionContext