diff --git a/majit/majit-gc/src/collector.rs b/majit/majit-gc/src/collector.rs index e7b40553131..0c26e7e5f0a 100644 --- a/majit/majit-gc/src/collector.rs +++ b/majit/majit-gc/src/collector.rs @@ -2053,40 +2053,65 @@ impl MiniMarkGC { } fn seed_major_root(&mut self, gcref: GcRef) { - if !gcref.is_null() && self.is_managed_heap_object(gcref.0) { - let hdr = unsafe { header_of(gcref.0) }; - // SAFETY: header_of returns a raw pointer; keep each access - // short-lived to avoid creating overlapping exclusive borrows. - let newly_marked = unsafe { - if !(*hdr).has_flag(flags::VISITED) { - (*hdr).set_flag(flags::VISITED); - true - } else { - false - } - }; - if newly_marked { - self.incr_state.gray_stack.push(gcref.0); - self.note_nonmoving_nursery_mark(gcref.0); - - // incminimark.py:1322-1340 requires marking worklists to - // contain no nursery objects after a minor. Pyre JITFRAME - // gcmap spills are collector metadata, not SETFIELD_GC - // writes: a frame first grayed in STATE_SCANNING can leave - // the active shadow stack before the next minor without a - // mutator barrier. Arm this newly seeded old root in the - // existing old_objects_pointing_to_young shape once, so that - // minor forwards any such spill before resetting nursery. - if !self.is_in_nursery(gcref.0) - && unsafe { (*hdr).has_flag(flags::TRACK_YOUNG_PTRS) } - { - unsafe { (*hdr).clear_flag(flags::TRACK_YOUNG_PTRS) }; - self.remembered_set.push(gcref.0); - } + if gcref.is_null() + || !self.is_managed_heap_object(gcref.0) + || !self.may_enter_marking_worklist(gcref.0) + { + return; + } + let hdr = unsafe { header_of(gcref.0) }; + // SAFETY: header_of returns a raw pointer; keep each access + // short-lived to avoid creating overlapping exclusive borrows. + let newly_marked = unsafe { + if !(*hdr).has_flag(flags::VISITED) { + (*hdr).set_flag(flags::VISITED); + true + } else { + false + } + }; + if newly_marked { + self.incr_state.gray_stack.push(gcref.0); + self.note_nonmoving_nursery_mark(gcref.0); + + // incminimark.py:1322-1340 requires marking worklists to + // contain no nursery objects after a minor. Pyre JITFRAME + // gcmap spills are collector metadata, not SETFIELD_GC + // writes: a frame first grayed in STATE_SCANNING can leave + // the active shadow stack before the next minor without a + // mutator barrier. Arm this newly seeded old root in the + // existing old_objects_pointing_to_young shape once, so that + // minor forwards any such spill before resetting nursery. + if !self.is_in_nursery(gcref.0) && unsafe { (*hdr).has_flag(flags::TRACK_YOUNG_PTRS) } { + unsafe { (*hdr).clear_flag(flags::TRACK_YOUNG_PTRS) }; + self.remembered_set.push(gcref.0); } } } + /// `incminimark.py:2739-2753 _collect_obj`: an object in the nursery is + /// never appended to `objects_to_trace` — "such an object is handled by + /// minor collections and shouldn't be specially handled by major + /// collections" — and `visit` (:2797-2799) asserts + /// `not self.is_in_nursery(obj)` on every popped entry. The worklist + /// outlives the mutator resuming, so a nursery address left on it is read + /// back after the next `reset_nursery` has recycled those bytes: the + /// header decodes to a garbage `type_id`. + /// + /// A young object stays reachable without the entry. When the minor + /// promotes it, `drag_out_root` greys the promoted copy + /// (`_trace_drag_out1_marking_phase`); when it is only reachable from an + /// old parent, that parent is in the remembered set and gets requeued + /// while MARKING (`_add_to_more_objects_to_trace`). + /// + /// The non-moving oldgen major is the one mode that marks the nursery in + /// place — it leaves those bytes untouched by contract, and + /// [`Self::note_nonmoving_nursery_mark`] clears the marks afterwards. + #[inline] + fn may_enter_marking_worklist(&self, addr: usize) -> bool { + self.oldgen_nonmoving_active || !self.is_in_nursery(addr) + } + /// Record a nursery object greyed during a non-moving major so its /// stale `flags::VISITED` is cleared as the strictly-last collection step. /// No-op (just a range check) outside a non-moving major; the normal @@ -2552,7 +2577,7 @@ impl MiniMarkGC { /// type system guarantees every `Ptr(GcStruct)` is GC-managed; it converges /// away once every `gc_ptr_offsets` target is a real GC allocation. fn grey_child(&mut self, addr: usize, holder_addr: usize, slot_addr: usize, site: &str) { - if self.is_managed_heap_object(addr) { + if self.is_managed_heap_object(addr) && self.may_enter_marking_worklist(addr) { let hdr = unsafe { header_of(addr) }; let type_id = unsafe { (*hdr).type_id() }; if type_id as usize >= self.types.len() { @@ -2586,6 +2611,12 @@ impl MiniMarkGC { // varsize GC-pointer array is never buffered (the gray stack already // retains the live children); only the bounded fixed-field offsets are // copied into the reused `mark_offsets` buffer. + // incminimark.py:2797-2799 `visit`: `ll_assert(not + // self.is_in_nursery(obj), "nursery object in 'objects_to_trace'")`. + debug_assert!( + self.may_enter_marking_worklist(obj_addr), + "nursery object {obj_addr:#x} in the marking worklist", + ); let type_id = unsafe { (*header_of(obj_addr)).type_id() }; // A non-moving major can trace a live nursery object without first // copying it into its reserved old-gen shadow. Keep that unoccupied diff --git a/pyre/bench/synth/_pending/exception_nested_exc_info_restore.py b/pyre/bench/synth/_pending/exception_nested_exc_info_restore.py deleted file mode 100644 index 345e66bad1c..00000000000 --- a/pyre/bench/synth/_pending/exception_nested_exc_info_restore.py +++ /dev/null @@ -1,54 +0,0 @@ -# PENDING — documents a PRE-EXISTING JIT bug, not a gap-10 regression. -# -# Nested try/except where each handler reads `sys.exc_info()`. After an inner -# handler unwinds, POP_EXCEPT must restore the slot to the prev its matching -# PUSH_EXC_INFO saved (the outer ValueError), and after the outer handler to -# None. Expected per-iteration signature 2*1 + 1*10 + 0*100 = 12 → 360000. -# -# The interpreter (PYRE_NO_JIT=1) is CORRECT (360000). The JIT is WRONG on -# BOTH tracers: trait gives 3320000, the FBW walker gives 3360000. Root: under -# JIT the nested handlers' POP_EXCEPT restores are not lowered to the EC -# `sys_exc_value` slot (the `sys.exc_info()` may-force between PUSH and POP ends -# the authoritative walk), so the slot keeps the inner exception after the -# handler exits. The B3 POP-restores-prev fix (FBW_EXC_PREV LIFO) corrects the -# single-handler shape (raise_catch / rc_small DCE) but does not reach these -# un-lowered nested POPs. Fixing requires keeping the walk authoritative across -# the in-handler `sys.exc_info()` read (or lowering it too). Lives here so the -# check.py synthetic suite stays green; promote back to ../ once fixed. -import sys - -N = 30000 - - -def classify(t): - if t is ValueError: - return 1 - if t is KeyError: - return 2 - if t is None: - return 0 - return 9 - - -def run(n): - acc = 0 - i = 0 - while i < n: - try: - raise ValueError("outer") - except ValueError: - try: - raise KeyError("inner") - except KeyError: - acc += classify(sys.exc_info()[0]) * 1 - acc += classify(sys.exc_info()[0]) * 10 - acc += classify(sys.exc_info()[0]) * 100 - i += 1 - return acc - - -def main(): - print(run(N)) - - -main() diff --git a/pyre/bench/synth/exception_nested_exc_info_restore.py b/pyre/bench/synth/exception_nested_exc_info_restore.py new file mode 100644 index 00000000000..45fd09f338c --- /dev/null +++ b/pyre/bench/synth/exception_nested_exc_info_restore.py @@ -0,0 +1,48 @@ +# Regression oracle: nested try/except where each handler reads +# `sys.exc_info()`. After an inner handler unwinds, POP_EXCEPT must restore the +# slot to the prev its matching PUSH_EXC_INFO saved (the outer ValueError), and +# after the outer handler to None. Expected per-iteration signature +# 2*1 + 1*10 + 0*100 = 12 → 360000. +# +# Was `_pending/`: the JIT answered 3320000 (trait) / 3360000 (FBW walker) +# because the nested handlers' POP_EXCEPT restores were not lowered to the EC +# `sys_exc_value` slot — the in-handler `sys.exc_info()` may-force ended the +# authoritative walk, so the slot kept the inner exception after the handler +# exited. Both tracers now answer 360000, matching the interpreter and CPython. +import sys + +N = 30000 + + +def classify(t): + if t is ValueError: + return 1 + if t is KeyError: + return 2 + if t is None: + return 0 + return 9 + + +def run(n): + acc = 0 + i = 0 + while i < n: + try: + raise ValueError("outer") + except ValueError: + try: + raise KeyError("inner") + except KeyError: + acc += classify(sys.exc_info()[0]) * 1 + acc += classify(sys.exc_info()[0]) * 10 + acc += classify(sys.exc_info()[0]) * 100 + i += 1 + return acc + + +def main(): + print(run(N)) + + +main() diff --git a/pyre/bench/synth/_pending/gc_bug_bridge_flavor_traceback_names.py b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.py similarity index 89% rename from pyre/bench/synth/_pending/gc_bug_bridge_flavor_traceback_names.py rename to pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.py index 5e1cdafa63d..36014ee43f9 100644 --- a/pyre/bench/synth/_pending/gc_bug_bridge_flavor_traceback_names.py +++ b/pyre/bench/synth/gc_bug_bridge_flavor_traceback_names.py @@ -1,11 +1,9 @@ -# KNOWN FAILING on cranelift: aborts with +# Regression oracle. Was `_pending/`: cranelift aborted 8/10 to 9/10 runs with # GC BUG: invalid type_id=... site=object_total_size # from `gc_alloc_nursery_shim` -> `alloc_with_type` -> `do_collect_nursery` -> # `incremental_mark_step`, i.e. a dangling nursery pointer on the MAJOR gray -# stack. dynasm and PYRE_JIT=0 are clean, which matches the known -# cranelift/wasm/Windows-only shape of that signature. -# -# Intermittent but high-rate - 8/10 to 9/10 runs - and needs no GC stress build. +# stack, while dynasm and the no-JIT run stayed clean. Now 20/20 clean on +# cranelift with the pinned output below. # # REFUTED lead, do not re-attempt: the unbarriered `(*frame).f_backref = # saved_topframeref` in ResidualFrameChainGuard::enter looks exactly like the diff --git a/pyre/bench/synth/_pending/loop_callee_shared_mutation.py b/pyre/bench/synth/loop_callee_shared_mutation.py similarity index 61% rename from pyre/bench/synth/_pending/loop_callee_shared_mutation.py rename to pyre/bench/synth/loop_callee_shared_mutation.py index 947b281e46c..c636cdfb0c9 100644 --- a/pyre/bench/synth/_pending/loop_callee_shared_mutation.py +++ b/pyre/bench/synth/loop_callee_shared_mutation.py @@ -4,13 +4,9 @@ # loop is JIT-compiled: the callee's traced iteration is committed concretely # during recording AND re-applied at the trace->compile boundary. # -# Expected: len(acc) == 2 * N. Under the bug the JIT prints 2*N + 3 (a constant +# Expected: len(acc) == 2 * N. Under the bug the JIT printed 2*N + 3 (a constant # over-count, independent of N, present only once N crosses the compile -# threshold). Both backends share the trace/resume layer, so both diverge. -# -# Kept under _pending/ (excluded by check.py's non-recursive `*.py` glob) so it -# does not fail the gate while #14 is open. Run explicitly with: -# python3 pyre/check.py --synthetic-only --synthetic-pattern '_pending/loop_callee_shared_mutation.py' +# threshold), on both backends, which share the trace/resume layer. N = 20000 diff --git a/pyre/bench/synth/_pending/sre_wasm_min.py b/pyre/bench/synth/sre_wasm_min.py similarity index 100% rename from pyre/bench/synth/_pending/sre_wasm_min.py rename to pyre/bench/synth/sre_wasm_min.py diff --git a/pyre/bench/synth/_pending/sre_wasm_min1.py b/pyre/bench/synth/sre_wasm_min1.py similarity index 100% rename from pyre/bench/synth/_pending/sre_wasm_min1.py rename to pyre/bench/synth/sre_wasm_min1.py diff --git a/pyre/pyre-interpreter/src/call.rs b/pyre/pyre-interpreter/src/call.rs index 85c16e68596..2e60d96203b 100644 --- a/pyre/pyre-interpreter/src/call.rs +++ b/pyre/pyre-interpreter/src/call.rs @@ -1334,6 +1334,30 @@ pub fn call_user_function( call_user_function_with_eval(frame, callable, args, eval_fn) } +/// Residual-call sibling of [`call_user_function_plain`] that keeps the +/// JIT-aware eval function. +/// +/// `blackhole.py:1225 bhimpl_residual_call_r_i` is `cpu.bh_call_i(func, ...)` +/// — it invokes the *translated function*, and when that function's graph +/// reaches a `jit_merge_point` (`execute_frame` does) the JIT is entered +/// normally. "Opaque to the trace" does not mean "the JIT is off inside": +/// upstream has no flag that disables it for the extent of a residual call. +/// `bhimpl_recursive_call_*` (`blackhole.py:1095-1132`) is not the only way +/// to reach the portal — it is the path the codewriter emits when the callee +/// is *statically* the portal graph. +/// +/// Re-entrant tracing is prevented where upstream prevents it, on the green +/// key: `warmstate.py:473-477` skips a hot back-edge while `JC_TRACING` is +/// set, which pyre mirrors with the `driver.is_tracing()` guard in +/// `maybe_compile_and_run`. +pub fn call_user_function_residual( + frame: &PyFrame, + callable: PyObjectRef, + args: &[PyObjectRef], +) -> PyResult { + call_user_function_with_eval(frame, callable, args, get_eval_fn()) +} + /// Plain interpreter-only user-function call. /// /// JIT residual helpers should use this instead of the injected eval override. diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index 9f65a15c2da..6d6f23570f8 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -48,6 +48,7 @@ thread_local! { last_exec_ctx: crate::call::capture_last_exec_ctx_cell(), import_roots: crate::importing::capture_import_root_area(), mapdict_method_cache: crate::pycode::capture_mapdict_method_cache_root_area(), + w_globals_stamped_codes: crate::pycode::capture_w_globals_stamped_code_root_area(), in_flight_exception: IN_FLIGHT_EXCEPTION.with(|cell| cell as *const _), bh_last_exception: majit_metainterp::blackhole::BH_LAST_EXC_VALUE .with(|cell| cell as *const _), @@ -64,6 +65,7 @@ struct PyFrameRootArea { last_exec_ctx: *const (), import_roots: *const (), mapdict_method_cache: *const (), + w_globals_stamped_codes: *const (), in_flight_exception: *const Cell, bh_last_exception: *const Cell, guard_exception: *const Cell, @@ -944,6 +946,19 @@ pub unsafe fn walk_pyframe_roots_area( area.mapdict_method_cache, &mut forward, ); + // `pycode.py:159-165 frame_stores_global` stamps `w_globals` + // permanently, and upstream traces it through the GC-managed + // `PyCode`. Box-immortal code objects are reached only when + // `walk_raw_code_roots` runs on a `frame.pycode` / `func.code` + // that happens to be walked, so a stamped code object sitting + // off the frame chain keeps a pre-move address once its + // globals dict is promoted. Forward every stamped slot. + crate::pycode::walk_w_globals_stamped_code_root_area( + area.w_globals_stamped_codes, + &mut |slot| { + visitor(&mut *(slot as *mut PyObjectRef as *mut majit_ir::GcRef)); + }, + ); } } } diff --git a/pyre/pyre-interpreter/src/pycode.rs b/pyre/pyre-interpreter/src/pycode.rs index 4a3a81f0659..20e7b2ed853 100644 --- a/pyre/pyre-interpreter/src/pycode.rs +++ b/pyre/pyre-interpreter/src/pycode.rs @@ -160,9 +160,12 @@ pub struct PyCode { /// `pycode.py:105 "w_globals?"`). Module globals are `malloc_typed`- /// immortal, but `exec`/custom-globals dicts are `try_gc_alloc` movable. /// The code object is Box-immortal, so the collector never reaches this - /// slot by tracing into it; `eval::walk_raw_code_roots` forwards it as a - /// root (via `walk_raw_function_roots` for `func.code` and the frame walk - /// for `frame.pycode`). Null until first stamped by `frame_stores_global`. + /// slot by tracing into it; every stamped code object is registered in + /// [`W_GLOBALS_STAMPED_CODES`] and forwarded from there on each + /// collection. `eval::walk_raw_code_roots` additionally forwards it + /// wherever a code object is already reached as a root (via + /// `walk_raw_function_roots` for `func.code` and the frame walk for + /// `frame.pycode`). Null until first stamped by `frame_stores_global`. pub w_globals: PyObjectRef, /// PyPy: `PyCode.hidden_applevel` (`pycode.py:111, 147`). Set by /// `pycompiler.compile(hidden_applevel=True)` for PyPy gateway/ @@ -1637,6 +1640,7 @@ pub unsafe fn w_code_set_w_globals(obj: PyObjectRef, w_globals: PyObjectRef) { if !w_globals.is_null() { let code_ptr = unsafe { (*(obj as *const PyCode)).code_ptr }; register_live_code_wrapper(code_ptr, obj); + register_w_globals_stamped_code(obj); } } @@ -1652,6 +1656,7 @@ pub unsafe fn w_code_frame_stores_global(obj: PyObjectRef, w_globals: PyObjectRe // Prebuilt-family store (see `w_code_set_w_globals`). pyre_object::gc_roots::mark_prebuilt_roots_dirty(); register_live_code_wrapper(code.code_ptr, obj); + register_w_globals_stamped_code(obj); return false; } !std::ptr::eq(code.w_globals, w_globals) @@ -1942,6 +1947,56 @@ thread_local! { /// registry retires when code objects become GC-managed. static MAPDICT_METHOD_CACHE_CODES: std::cell::RefCell> = std::cell::RefCell::new(std::collections::HashSet::new()); + /// Code objects whose `w_globals` has been stamped + /// (`pycode.py:159-165 frame_stores_global`). `w_globals` is a permanent + /// strong field: the first globals object a code object runs in is kept + /// for the code object's lifetime and never replaced. Upstream that + /// field is traced through the GC-managed `PyCode`; pyre code objects are + /// Box-immortal (`w_code_new` → `Box::into_raw`), so the only paths that + /// reach the slot are the opportunistic `walk_raw_code_roots` calls on + /// `frame.pycode` and `func.code`. Those miss any stamped code object + /// that is off the frame chain and not held in a walked frame slot at + /// collection time, which leaves a pre-move nursery address in + /// `w_globals` once its dict is promoted — the next call through that + /// code object then forwards a dangling pointer. This registry makes the + /// slot a root of its own, in the same family as + /// [`MAPDICT_METHOD_CACHE_CODES`]. Entries are immortal code pointers, + /// so they never dangle; the registry retires when code objects become + /// GC-managed. + static W_GLOBALS_STAMPED_CODES: std::cell::RefCell> = + std::cell::RefCell::new(std::collections::HashSet::new()); +} + +/// Record `obj` as a code object holding a stamped `w_globals` slot. +fn register_w_globals_stamped_code(obj: PyObjectRef) { + W_GLOBALS_STAMPED_CODES.with(|s| { + s.borrow_mut().insert(obj as usize); + }); +} + +pub(crate) fn capture_w_globals_stamped_code_root_area() -> *const () { + W_GLOBALS_STAMPED_CODES.with(|codes| codes as *const _ as *const ()) +} + +/// Forward the stamped `w_globals` slot of every registered code object. +/// +/// # Safety +/// `data` must come from [`capture_w_globals_stamped_code_root_area`], and the +/// owning thread must be quiesced. +pub(crate) unsafe fn walk_w_globals_stamped_code_root_area( + data: *const (), + forward: &mut dyn FnMut(&mut PyObjectRef), +) { + let codes = unsafe { + &*(*(data as *const std::cell::RefCell>)).as_ptr() + }; + for &code in codes.iter() { + let code = unsafe { &mut *(code as *mut PyCode) }; + if code.w_globals.is_null() { + continue; + } + forward(&mut code.w_globals); + } } /// Forward every filled `entry.w_method` slot during collection — the diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs index 3be52ce1aca..f5ace365db5 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs @@ -1043,6 +1043,53 @@ thread_local! { /// backstop abort costs one attempt per callee instead of storming. static FBW_FORITER_DEFERRED_DENY: std::cell::RefCell> = std::cell::RefCell::new(std::collections::HashSet::new()); + /// Code keys of the callees [`fbw_inline_callee_hazardous`] named when the + /// hazard arm of [`fbw_abort_nested_unjournaled_residual`] fired. The + /// inline callsite declines them from then on, so the call residualizes + /// and the enclosing trace never re-enters the identical abort. + /// + /// This is `disable_noninlinable_function` (`warmstate.py:331`, which sets + /// `JC_DONT_TRACE_HERE` = "do not inline calls to this function"): upstream + /// answers an abort attributable to one inlined callee by denying THAT + /// callee and retracing the enclosing loop, not by penalising the loop + /// (`pyjitpl.py:2818-2828`). Like the upstream flag the set has no removal + /// path — the hazard is a static property of the callee's `CodeObject`. + /// + /// Keying on the `CodeObject` alone is the full key for this decision, not + /// a truncation of one. The flag is consumed by `can_inline_callable` + /// (`warmstate.py:669-677`), whose only caller is `_opimpl_recursive_call` + /// (`pyjitpl.py:1376-1382`) — it passes the CALLEE's green args, and a + /// callee reached through a CALL is always entered at its own entry, so the + /// `next_instr` component is constant and `pycode` carries the whole + /// decision. The same holds here: the deny is recorded and queried for an + /// inline frame pushed at a CALL boundary (`inline_call.rs`), never for a + /// mid-body resume. + /// + /// Per-thread for the same reason as [`crate::trace::fbw_declined`]'s + /// `FBW_DECLINED_KEYS` and `RANGE_FORITER_DEMOTED`: pyre's walk state is + /// per-thread, and an inline hazard observed while tracing is a property of + /// the tracing thread's framestack. Sharing one memo while its siblings + /// stay per-thread would be the inconsistency. + /// + /// NOT yet ported: `warmstate.py:485-495` also treats the flag as "please + /// trace from here as soon as possible" — a denied cell that never had a + /// procedure token reaches `bound_reached` immediately, so the callee gets + /// its own trace instead of staying a plain residual forever. Since + /// residual calls re-enter the JIT the callee does reach its own threshold, + /// just on the ordinary counter rather than at once. + /// + /// A raw address is a sound permanent key only because `w_code_new` + /// (`pycode.rs`) allocates every `PyCode` with `Box::into_raw` and nothing + /// frees it: the address is unique for the process and never moves. + /// Upstream can key on the object because a `JitCell` holds its greens and + /// `should_remove_jitcell` (`warmstate.py:212`) prunes dead ones; this set + /// has neither, so it relies on that immortality. `eval.rs`'s `PyCode` + /// registration names the change that would end it — switching `w_code_new` + /// to `try_gc_alloc_stable`. At that point a reclaimed address can be + /// handed to a later code object and this set must gain a removal path or + /// a key that outlives the allocation. + static FBW_HAZARDOUS_INLINE_DENY: std::cell::RefCell> = + std::cell::RefCell::new(std::collections::HashSet::new()); } /// Marks the sub-walk of a callee admitted into a FOR_ITER body under @@ -1088,6 +1135,16 @@ fn fbw_foriter_deny_deferred_call(callee_code_key: usize) { }); } +pub(crate) fn fbw_hazardous_inline_denied(callee_code_key: usize) -> bool { + FBW_HAZARDOUS_INLINE_DENY.with(|c| c.borrow().contains(&callee_code_key)) +} + +fn fbw_deny_hazardous_inline(callee_code_key: usize) { + FBW_HAZARDOUS_INLINE_DENY.with(|c| { + c.borrow_mut().insert(callee_code_key); + }); +} + /// Whether the active inline sub-walk is one of the hazard classes the blanket /// nested-residual decline was masking, as opposed to a straight-line mutating /// callee (the #73 depth-≥2 payoff, which inlines). Two classes decline: @@ -1112,12 +1169,19 @@ fn fbw_foriter_deny_deferred_call(callee_code_key: usize) { /// The `w_code` field is the `jitcode_for` code key, resolved to the raw /// `CodeObject` via the jitcode index (the `current`-frame pattern, /// mod.rs:4664). -fn fbw_inline_callee_hazardous(ctx: &WalkContext<'_, '_, Sym>) -> bool { +/// +/// Returns the code key of the offending callee, which is the entity the +/// decline is a property of and therefore the one to deny — the same +/// attribution `find_biggest_function` (`pyjitpl.py:3538`) performs before +/// `disable_noninlinable_function`. Declining it at its own callsite makes +/// the next attempt residualize that call, so the surviving nest is +/// hazard-free and the enclosing loop can compile. +fn fbw_inline_callee_hazardous(ctx: &WalkContext<'_, '_, Sym>) -> Option { let session = ctx.session.borrow(); let mut seen: Vec = Vec::with_capacity(session.framestack.len()); for frame in session.framestack.iter() { if seen.contains(&frame.w_code) { - return true; + return Some(frame.w_code); } seen.push(frame.w_code); if let Some(idx) = crate::state::ensure_jitcode_index(frame.w_code as *const ()) { @@ -1127,13 +1191,13 @@ fn fbw_inline_callee_hazardous(ctx: &WalkContext<'_, '_, Sym>) -> if pyre_interpreter::code_has_for_iter(code) || pyre_interpreter::code_is_self_recursive(code) { - return true; + return Some(frame.w_code); } } } } } - false + None } pub(crate) fn fbw_abort_nested_unjournaled_residual( @@ -1168,14 +1232,28 @@ pub(crate) fn fbw_abort_nested_unjournaled_residual( // aborts before the hazardous body is committed. Every other nested // residual inlines. The hazard scan is last so the cheap checks // short-circuit it. - if !in_selfrec_fold + let nested = !in_selfrec_fold && !in_exception_string_inline - && !ctx.session.borrow().framestack.is_empty() - && (foriter_deferred_inline.is_some() || fbw_inline_callee_hazardous(ctx)) - { + && !ctx.session.borrow().framestack.is_empty(); + let hazardous_callee = if nested && foriter_deferred_inline.is_none() { + fbw_inline_callee_hazardous(ctx) + } else { + None + }; + if nested && (foriter_deferred_inline.is_some() || hazardous_callee.is_some()) { if let Some(callee_code_key) = foriter_deferred_inline { fbw_foriter_deny_deferred_call(callee_code_key); } + // Deny the named callee so the enclosing loop's next attempt + // residualizes that call instead of re-entering this abort. Without + // it the decline is a property of the framestack, which the next + // attempt rebuilds identically: the abort recurs byte-for-byte until + // the enclosing location is retired, so the loop never compiles at + // all. Upstream answers the same situation by denying the callee and + // letting the enclosing loop retrace (`pyjitpl.py:2818-2828`). + if let Some(callee_code_key) = hazardous_callee { + fbw_deny_hazardous_inline(callee_code_key); + } // The flush this latch feeds resumes the OUTERMOST caller at the CALL // that entered the inline region, re-executing that call from scratch, // while the walk's store journal is committed — a diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 3e2cc9dea9b..bee922fdb55 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -1414,6 +1414,43 @@ fn fbw_unpack_call_function_ex_args( Some((args, concretes)) } +thread_local! { + /// Memo for [`callee_body_has_abort_permanent`], keyed by the stable + /// `CodeObject` pointer. The answer is a static property of the callee's + /// assembled body, so it is computed once per code object instead of once + /// per inline attempt — the scan is O(body) and the callsite is reached on + /// every retrace of every caller. + /// + /// The raw address is a sound permanent key only because `w_code_new` + /// (`pycode.rs`) allocates every `PyCode` with `Box::into_raw` and nothing + /// frees it, so the address is unique for the process and never moves. + /// `eval.rs`'s `PyCode` registration names the change that would end that — + /// switching `w_code_new` to `try_gc_alloc_stable` — after which a + /// reclaimed address could return another code object's answer, and here + /// a stale `false` would admit an inline whose body does carry + /// `abort_permanent`. Same invariant as `FBW_HAZARDOUS_INLINE_DENY` + /// (`fbw_state.rs`). + static CALLEE_ABORT_PERMANENT_SEEN: std::cell::RefCell> = + const { std::cell::RefCell::new(std::collections::BTreeMap::new()) }; +} + +/// Whether the callee's assembled body contains an `abort_permanent` marker. +/// +/// Same scan `loop_inlines_abort_permanent_callee` runs over a `SubJitCodeBody` +/// (`trace.rs`), memoized here because this is the per-callsite path. +fn callee_body_has_abort_permanent(w_code: *const (), body: &SubJitCodeBody) -> bool { + let key = w_code as usize; + if let Some(hit) = CALLEE_ABORT_PERMANENT_SEEN.with(|m| m.borrow().get(&key).copied()) { + return hit; + } + let hit = + crate::jitcode_runtime::decoded_ops(body.code).any(|op| op.opname == "abort_permanent"); + CALLEE_ABORT_PERMANENT_SEEN.with(|m| { + m.borrow_mut().insert(key, hit); + }); + hit +} + pub(crate) fn try_walker_inline_user_call( ctx: &mut WalkContext<'_, '_, Sym>, op: &DecodedOp, @@ -1619,6 +1656,28 @@ pub(crate) fn try_walker_inline_resolved_user_call( if nparams > body.num_regs_r { return Ok(None); } + // Inlining a callee whose body carries an `abort_permanent` marker walks + // the sub-walk straight into it. That surfaces as + // `TraceAction::AbortPermanent`, which stamps `DONT_TRACE_HERE` on the + // CALLER loop's green key — so one unported opcode anywhere in a callee + // permanently un-JITs the loop that calls it. Upstream keeps this + // decision static and on the callee: `codewriter/policy.py:48-84` + // `look_inside_graph` reads whole-graph properties before tracing, and its + // own comment (:78-79) spells out the consequence of a "no" — "the call + // will be turned into a residual call". Answer the same way. + // + // `loop_inlines_abort_permanent_callee` (`trace.rs`) already screens this + // up front, but only for callees it can resolve statically out of globals + // and frame slots; a bound method, a container element or a call result + // reaches here unscreened. At this point the callee is concrete, so the + // screen is exact. + // + // Whole-body, like upstream's whole-graph test: a marker on a path this + // trace happens not to take still costs only the inline if we decline, + // where walking into it costs the whole loop, permanently. + if callee_body_has_abort_permanent(w_code, &body) { + return Ok(None); + } // The callee body resolves its `d`/`j` descr operands through its OWN // per-fn pool, not the caller's. Without this the sub-walk reads the // wrong descr at the first `getfield_vable_r` / `residual_call` @@ -1730,6 +1789,21 @@ pub(crate) fn try_walker_inline_resolved_user_call( !raw.is_null() && pyre_interpreter::code_is_self_recursive(&*raw) }; } + // A callee `fbw_abort_nested_unjournaled_residual` already named on its + // hazard arm residualizes from here on. The hazard is a static property of + // the callee's `CodeObject` (loop-bearing / self-recursive), so re-inlining + // it rebuilds the identical framestack and reaches the identical abort — + // the enclosing loop is retired for a decline that belongs to the callee. + // `warmstate.py:331` `disable_noninlinable_function` is the same answer: + // the flag it sets means "do not inline calls to this function", and the + // enclosing loop is left free to retrace. + // + // `bridge_rec_root_selfrec` is exempt: that admission carries its own + // `SELFREC_CA_FOLD_ACTIVE` exemption from the hazard arm (:2696), so its + // recursive residual is not what named the callee here. + if !bridge_rec_root_selfrec && fbw_hazardous_inline_denied(callee_code_key) { + return Ok(None); + } // An inline sub-walk inside a FOR_ITER body resumes a guard at the // caller's CALL boundary, so deopt re-executes the whole callee. Replaying // a live-heap mutation would double it; the nested-residual decline catches @@ -2113,6 +2187,35 @@ pub(crate) fn try_walker_inline_resolved_user_call( // the callee's residuals for real, and a residual that reads the chain // (`sys._getframe`, a traceback) must see the callee it is running in. let mut concrete_callee_frame = std::ptr::null_mut::(); + // Each precondition below answers "can the multiframe seed serve this + // callee". A "no" declines the INLINE — `Ok(None)`, this function's own + // did-not-inline answer, which every caller follows to the ordinary + // residual call. It must not be `Err`: that is + // `LoopBearingCalleeInlineUnsupported`, which `trace.rs` maps to + // `TraceAction::Abort`, discarding the whole enclosing loop trace. And + // because the arm does not call `fbw_decline`, the same static, callee-shaped + // precondition failed identically on every retrace, so the loop kept + // re-tracing and re-aborting instead of settling. + // + // The strict path already declines gracefully here (`break 'seed`); only + // the `try_multiframe` path aborted. Upstream never has this state: + // `pyjitpl.py` `do_residual_or_indirect_call` residualizes the callee it + // cannot follow, and the recursion-budget path calls `dont_trace_here` and + // then still falls through to `do_residual_call` — the enclosing trace + // survives either way. `rlib/jit.py`'s `ABORT_*` set (TOO_LONG, BRIDGE, + // BAD_LOOP, ESCAPE, FORCE_QUASIIMMUT, SEGMENTED_TRACE) has no + // cannot-inline-this-callee reason at all. + // + // The variant's own doc justifies abort-over-residual for a callee whose + // short inner loops would compile and deopt-storm — but + // `callee_fast_path_inlinable_allowing_forward_branch` already rejects every + // backward `goto_if_not` and every `switch`, so a `try_multiframe` callee + // provably has no inner loop and that rationale does not reach here. + // + // All of these sit before the first recorded op (the `GETFIELD_GC_R` + // below), so returning costs nothing but the inline. The + // POP_JUMP_IF_NONE scan is the one exception and still aborts — see the + // note at that site. if try_multiframe || strict_seed { 'seed: { // Branch-A frame shape only (mirror REC_CA): no cells. @@ -2122,14 +2225,14 @@ pub(crate) fn try_walker_inline_resolved_user_call( }; if raw.is_null() { if try_multiframe { - return Err(DispatchError::callee_inline_unsupported(op.pc)); + return Ok(None); } break 'seed; } let callee_code = unsafe { &*raw }; if pyre_interpreter::ncells(callee_code) != 0 { if try_multiframe { - return Err(DispatchError::callee_inline_unsupported(op.pc)); + return Ok(None); } break 'seed; } @@ -2146,6 +2249,19 @@ pub(crate) fn try_walker_inline_resolved_user_call( // POP_JUMP_IF_TRUE/FALSE stay inlinable: their `bool` truth folds in the // int bank, so no Ref rebox is needed. A strict straight-line callee // has no branch at all, so this scan never fires for it. + // + // This is the one precondition here that still aborts instead of + // returning `Ok(None)`, and deliberately so. Residualizing it does + // work — `bench/synth/_pending/gc_bug_bridge_flavor_traceback_names` + // goes from 98 aborts to 2 and + // `_pending/exception_nested_exc_info_restore` from 5 aborts to 0, + // both compiling loops they never compiled before — but the loops it + // newly compiles then print traceback tuples missing their outermost + // frame, diverging from the interpreter (that fixture pins its + // expected output in its header). The abort was masking a lost + // `PyTraceback` node on the compiled exception path, not preventing + // one. Restore `Ok(None)` here once that node is recorded; it is the + // largest single win left in this function. if (0..callee_code.instructions.len()).any(|pc| { matches!( pyre_interpreter::decode_instruction_at(callee_code, pc), @@ -2168,7 +2284,7 @@ pub(crate) fn try_walker_inline_resolved_user_call( crate::state::ensure_jitcode_index(callee_code_key as *const ()) else { if try_multiframe { - return Err(DispatchError::callee_inline_unsupported(op.pc)); + return Ok(None); } break 'seed; }; @@ -2179,7 +2295,7 @@ pub(crate) fn try_walker_inline_resolved_user_call( || ec_reg as usize >= callee_regs_r.len() { if try_multiframe { - return Err(DispatchError::callee_inline_unsupported(op.pc)); + return Ok(None); } break 'seed; } @@ -2194,7 +2310,7 @@ pub(crate) fn try_walker_inline_resolved_user_call( let sym_ptr = ctx.fbw_mode.snapshot_sym; if sym_ptr.is_null() { if try_multiframe { - return Err(DispatchError::callee_inline_unsupported(op.pc)); + return Ok(None); } break 'seed; } @@ -2362,6 +2478,15 @@ pub(crate) fn try_walker_inline_resolved_user_call( // the caller frame past its matching handler. Decline the inline so // the call stays residual, where the post-call catch resume // (`GuardCaptureScope::residual_call_catch_resume`) routes the raise. + // + // FIXME: this `Err` discards the whole enclosing loop trace, where the + // comment above describes only declining the inline. It cannot become + // `Ok(None)` in place like the seed preconditions below: by here the + // seed block has already recorded `GETFIELD_GC_R` + + // `emit_new_pyframe_inline_with_params` and stamped a concrete + // `FrameBox` onto that op, so returning would leave dead IR behind. + // Closing it means hoisting `decline_inline_caller_frame_for_catch_marker` + // ahead of the seed block. match compute_inline_caller_frame(ctx, op.pc) { Ok(pf) => Some(pf), Err(InlineCallerFrameDecline::TryBlockCatchMarker) => { diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index 537bfc02162..c5f0b6e17b5 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -4676,6 +4676,20 @@ fn full_body_walk_trace( fbw_decline(crate::driver::make_green_key(w_code, start_pc)); TraceAction::Abort } + // The exc-edge routing decision is `find_catch_for_exc_resume` + // + `exc_handler_rejoins_loop` over `(jitcode_code, position)` + // alone, so the same guard reaches it on every retrace — the + // premise the `AbortPermanent` decline below is written for. + // It cannot take that mapping: the abort is raised before any + // recording precisely so the guard resumes via the blackhole, + // which is the correct handling, not a location to retire. + // Record only the bridge-guard decline, so the guard stops + // re-walking the whole body (executing its residual calls + // concretely) to re-derive a static answer. + DE::ExcEdgeCrossFrameReturnUnsupported { .. } => { + fbw_bridge_decline(ctx); + TraceAction::Abort + } _ => TraceAction::Abort, } } diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index ffaac22c7b9..c1074d3743e 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -478,6 +478,83 @@ fn publish_residual_call_exception(exc_obj: i64) { store_jit_exception(exc_obj); } +/// The caller's residual-call exception pair, taken out of its cells for the +/// span of a nested JIT re-entry by [`park_residual_call_exception`]. +/// +/// A cell is the only root its exception has (`walk_bh_last_exc_value` / +/// `walk_jit_exc_value`), so a value taken out of one is pinned on the shadow +/// stack for the span and read back relocated: an RPython local holding a GC +/// object is a shadow-stack root, a Rust local is not. +struct ParkedResidualException { + /// `None` when both cells were empty — the ordinary case, since a + /// residual call is reached with no exception in flight. + scope: Option, + save: usize, + bh_pinned: bool, + backend_pinned: bool, +} + +/// Take the caller's `BH_LAST_EXC_VALUE` / backend `_store_exception` pair out +/// of their cells and leave both empty. +/// +/// Upstream never needs this: the raise a residual call reports lives in +/// `metainterp.last_exc_value`, a field of the MetaInterp instance that owns +/// the call, and `llmodel.py:194 _store_exception` is read back by the same +/// `bh_call_*` that armed it. Pyre keeps one cell per thread, so a nested +/// execution inside the call aliases the caller's. +fn park_residual_call_exception() -> ParkedResidualException { + let bh = majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.get()); + let backend = crate::eval::jit_exc_value_peek_backend(); + if bh == 0 && backend == 0 { + return ParkedResidualException { + scope: None, + save: 0, + bh_pinned: false, + backend_pinned: false, + }; + } + let scope = pyre_object::gc_roots::push_roots(); + let save = pyre_object::gc_roots::shadow_stack_len(); + let bh_pinned = bh != 0; + if bh_pinned { + pyre_object::gc_roots::pin_root(bh as pyre_object::PyObjectRef); + } + let backend_pinned = backend != 0; + if backend_pinned { + pyre_object::gc_roots::pin_root(backend as pyre_object::PyObjectRef); + } + majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.set(0)); + drain_backend_jit_exc(); + ParkedResidualException { + scope: Some(scope), + save, + bh_pinned, + backend_pinned, + } +} + +/// Restore what [`park_residual_call_exception`] took, discarding whatever the +/// nested execution left in the cells: a raise the nested callee already +/// handled is not this helper's to report, and the caller's +/// `GUARD_NO_EXCEPTION` would read it as a spurious pending exception — the +/// failure [`drain_backend_jit_exc`] names for the walker's snapshot side. +fn unpark_residual_call_exception(parked: ParkedResidualException) { + majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.set(0)); + drain_backend_jit_exc(); + if parked.scope.is_none() { + return; + } + let mut index = parked.save; + if parked.bh_pinned { + let bh = pyre_object::gc_roots::shadow_stack_get(index) as i64; + index += 1; + majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.set(bh)); + } + if parked.backend_pinned { + store_jit_exception(pyre_object::gc_roots::shadow_stack_get(index) as i64); + } +} + /// Drain the backend `_store_exception` cells (`jit_exc_clear`) without /// touching `BH_LAST_EXC_VALUE`. /// @@ -4449,14 +4526,36 @@ fn bh_call_fn_impl(callable: PyObjectRef, null_or_self: PyObjectRef, args: &[PyO pyre_interpreter::call::set_last_exec_ctx((*parent_frame_ptr).execution_context); } let parent_frame = unsafe { &*parent_frame_ptr }; - let result = { - // blackhole.py:1225 bhimpl_residual_call_* is an opaque CPU - // call. Only blackhole.py:1095 bhimpl_recursive_call_* reaches - // the portal runner, so nested Python CALLs from this residual - // path must stay on eval_frame_plain as well. - let _plain_guard = pyre_interpreter::call::force_plain_eval(); - pyre_interpreter::call::call_user_function_plain(parent_frame, callable, &call_args) - }; + // `blackhole.py:1225 bhimpl_residual_call_*` is opaque to the TRACE, + // not to the JIT: `cpu.bh_call_*(func, ...)` runs the translated + // callee, and a callee whose graph reaches a `jit_merge_point` + // (`execute_frame` does) enters the JIT normally. Pinning the callee + // and its whole subtree to `eval_frame_plain` here left a hot inner + // loop interpreted for as long as its caller's loop was compiled. + // `bhimpl_recursive_call_*` (`blackhole.py:1095-1132`) is not the + // only way back to the portal — it is the form the codewriter emits + // when the callee is *statically* the portal graph. + // + // Re-entrant TRACING stays blocked where upstream blocks it, on the + // green key (`warmstate.py:473-477` JC_TRACING, mirrored by + // `maybe_compile_and_run`'s `driver.is_tracing()`). + // + // Letting the callee re-enter the JIT also lets a nested compiled / + // blackhole execution run inside this helper, and that nesting writes + // the very cells this helper publishes to: `BH_LAST_EXC_VALUE` and the + // backend `_store_exception` pair. Upstream cannot alias them — the + // raise lives in `metainterp.last_exc_value`, a field of the + // MetaInterp instance that owns the call, and `llmodel.py:194 + // _store_exception` is read back by the same `bh_call_*` that armed + // it. Park the caller's pair across the nested run so this helper's + // own outcome is the only thing it publishes; a nested raise the + // callee handled internally would otherwise be read by the caller's + // `GUARD_NO_EXCEPTION` as a spurious pending exception (the failure + // `drain_backend_jit_exc` names for the walker's snapshot side). + let parked = park_residual_call_exception(); + let result = + pyre_interpreter::call::call_user_function_residual(parent_frame, callable, &call_args); + unpark_residual_call_exception(parked); pyre_interpreter::call::set_last_exec_ctx(saved_ctx); return match result { Ok(result) => result as i64, diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 91edfdb374a..c79fdccc566 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -3259,11 +3259,11 @@ fn install_gc_into_backend() { /// GC root walker. Mirrors `install_gc_into_backend`'s backend-selection cfg /// (cranelift wins over dynasm when both features are on; wasm on wasm32). #[cfg(target_arch = "wasm32")] -fn jit_exc_value_peek_backend() -> i64 { +pub(crate) fn jit_exc_value_peek_backend() -> i64 { majit_backend_wasm::jit_exc_value_peek() } #[cfg(all(feature = "cranelift", not(target_arch = "wasm32")))] -fn jit_exc_value_peek_backend() -> i64 { +pub(crate) fn jit_exc_value_peek_backend() -> i64 { majit_backend_cranelift::jit_exc_value_peek() } #[cfg(all( @@ -3271,11 +3271,11 @@ fn jit_exc_value_peek_backend() -> i64 { not(feature = "cranelift"), not(target_arch = "wasm32") ))] -fn jit_exc_value_peek_backend() -> i64 { +pub(crate) fn jit_exc_value_peek_backend() -> i64 { majit_backend_dynasm::jit_exc_value_peek() } #[cfg(not(any(target_arch = "wasm32", feature = "cranelift", feature = "dynasm")))] -fn jit_exc_value_peek_backend() -> i64 { +pub(crate) fn jit_exc_value_peek_backend() -> i64 { 0 } diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index c441e5fcf39..b2d299fa45f 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -6470,7 +6470,22 @@ impl CodeWriter { // `"abort_permanent"` to the builder, so the external push is // an exact mirror of the pre-existing internal behavior. macro_rules! emit_abort_permanent { + // Straight-line form: the arm has modelled this opcode's stack + // effect and the walk falls through to the next PC. The block is + // closed only when nothing follows to close it. ($py_pc:expr) => {{ + emit_abort_permanent!(@emit $py_pc, ($py_pc) + 1 >= code.instructions.len()) + }}; + // Terminal form: the marker ends this graph block. Used by the + // arms that `continue` out of the dispatch instead of completing + // their stack model — the `Call` nargs > 14 arm (which skips its + // `push_and_bump!`) and the `LoadFastCheck` unbound arm (which + // switches into a dedicated dead-end block that has no successor + // by construction). + ($py_pc:expr, closes_block) => {{ + emit_abort_permanent!(@emit $py_pc, true) + }}; + (@emit $py_pc:expr, $closes_block:expr) => {{ // Publish `last_instr` to the vable before the bail so the // blackhole hands the interpreter the right resume // coordinate. The blackhole replays codewriter jitcode that @@ -6518,23 +6533,45 @@ impl CodeWriter { None, ($py_pc) as i64, ); - // `abort_permanent` is a runtime terminator, but RPython's - // flow graph has no third terminal block beside returnblock - // and exceptblock (`model.py:18-19`). Keep the orthodox - // graph shape by linking the unreachable continuation to - // returnblock. Canonical flattening serializes - // `abort_permanent` first; its runtime dispatch never reaches - // the synthetic null return. Leaving this block with no exit - // would instead make `flatten.py:107-109` mistake its full - // FrameState input tuple for return arguments. - let abort_return = super::flow::Link::new( - vec![super::flow::Constant::none().into()], - Some(graph.returnblock.clone()), - None, - ) - .into_ref(); - append_exit(¤t_block.block(), abort_return); - needs_fallthrough = false; + // `abort_permanent` terminates the *run*, not the *graph*. + // Closing the block here would set `exits`, and the + // `block_closed_by_terminator` gate below then skips op + // dispatch for every later PC — so nothing behind this opcode + // is lowered, `merge_entry_by_green` loses every loop header + // that follows, and `compile_and_run_once` silently refuses + // (`n(target_pc).is_none()`) with no abort recorded. One + // `class` statement, `del`, or annotated assignment in a + // module prologue therefore made every loop in that module + // permanently un-JITtable. + // + // RPython has no counterpart to compare against because it + // has no unsupported opcodes: `pyopcode.py:865-870 + // LOAD_BUILD_CLASS` and `:777-778 LOAD_LOCALS` are plain + // value pushes and the tracer walks straight through them, + // and nothing in `flowcontext.py` blacklists the graph a + // hard-to-trace operation appears in. The parity-preserving + // shape is therefore to keep the graph connected: the arms + // above already leave a well-typed FrameState (the + // `push_fresh_ref` / depth adjustments), so the fall-through + // continues to be lowered and a real terminator closes the + // block. Runtime never reaches it — `abort_permanent` hands + // control back to the interpreter first. + // + // A block with no successor to close it must still be closed: + // an exit-less block makes `flatten.py:107-109` mistake its + // full FrameState input tuple for return arguments. Link it + // to returnblock, the orthodox second terminal block + // (`model.py:18-19`). + if $closes_block { + let abort_return = super::flow::Link::new( + vec![super::flow::Constant::none().into()], + Some(graph.returnblock.clone()), + None, + ) + .into_ref(); + append_exit(¤t_block.block(), abort_return); + needs_fallthrough = false; + } }}; } @@ -6701,6 +6738,54 @@ impl CodeWriter { }}; } + // `flowcontext.py:130-156 BlockRecorder.guessexception` closes the + // recording block AT the can-raise operation and resumes normal flow + // in a fresh `EggBlock`, so an opcode whose can-raise op is followed + // by its own exit wiring (`guessbool` setting the Bool exitswitch, + // FOR_ITER's exhaustion split) records the exception edge on the + // FIRST block and the branch on the SECOND. Pyre's arms build both + // in one pass, and the generic per-PC catch emission at the bottom of + // the dispatch runs too late — it finds the block already closed and + // skips the edge. This macro performs the `guessexception` cut in the + // middle of such an arm: attach the exception edge here, then hand the + // walker a fresh successor to wire the branch into. + // + // `$threaded` carries the Variables the successor consumes that the + // FrameState does not list — the can-raise op's own result feeding the + // branch. `unsimplify.py:59-76 split_block` threads exactly those + // through the link so `regalloc.py:26-77 make_dependencies`, which + // computes liveness per block from `inputargs`, sees them live. + macro_rules! emit_catch_exception_and_split { + ($catch_label:expr, $py_pc:expr, $threaded:expr) => {{ + let py_pc = $py_pc; + emit_catch_exception!($catch_label); + let mut next_state = current_state.clone(); + next_state.next_offset = py_pc; + next_state.blocklist = frame_blocks_for_offset(code, py_pc); + let next_block = + SpamBlockRef::new(graph.new_block(Vec::new()), Some(next_state.clone())); + all_walker_blocks.push(next_block.clone()); + // Identity split: every surviving Variable passes through + // unchanged, so `link_args` mirrors `inputargs`. + let mut inputargs: Vec = next_state.getvariables(); + for value in $threaded { + let value: super::flow::FlowValue = value.into(); + if let Some(variable) = value.as_variable() + && !inputargs.iter().any(|arg| arg == &value) + { + inputargs.push(variable.into()); + } + } + next_block.block().borrow_mut().inputargs = inputargs.clone(); + append_exit( + ¤t_block.block(), + super::flow::Link::new(inputargs, Some(next_block.block()), None).into_ref(), + ); + restore_canraise_exit_order(¤t_block.block()); + current_block = next_block; + }}; + } + // Dual emission for block `Label`. RPython parity: // `flatten.py:180` `self.emitline(Label(block))` marks block // entry; `assembler.py:157-158` records the label position in @@ -7791,13 +7876,12 @@ impl CodeWriter { // is expected to continue processing the canraise op's // normal-flow result and subsequent ops. // - // `emit_abort_permanent!` is NOT excluded: it appends a - // returnblock link, so `exits` is non-empty and every - // later PC lands here and skips dispatch. A body that - // aborts therefore stops emitting at that opcode — which - // is why `merge_entry_by_green` drops the loop headers - // behind it instead of taking every header a bytecode - // scan finds. + // `emit_abort_permanent!` does NOT reach this gate: it + // leaves the block open so the rest of the code object + // keeps lowering, and only appends a returnblock link when + // the abort is the last instruction (nothing follows to + // dispatch). `merge_entry_by_green` therefore keeps the + // loop headers behind an unsupported opcode. // // Reuses `block_closed_by_terminator` computed above the // loop-header `jit_merge_point` gate: the merge emission @@ -8213,6 +8297,14 @@ impl CodeWriter { cond_value, py_pc as i64, ); + // `bool` runs the operand's `__bool__`, so inside a + // `try` range it needs its own exception edge before + // `guessbool` closes the block with the two Bool + // exits. + if let Some(catch_label) = catch_for_pc[py_pc] { + emit_catch_exception_and_split!(catch_label, py_pc, [bool_value]); + exception_edge_handled = true; + } // flowcontext.py:756-763 `block.exitswitch = w_cond`. current_block.block().borrow_mut().exitswitch = Some(super::flow::ExitSwitch::Value(bool_value.into())); @@ -8293,6 +8385,13 @@ impl CodeWriter { cond_value, py_pc as i64, ); + // See PopJumpIfFalse — the `bool` can raise out of + // `__bool__`, so cut the block here when the PC is + // covered by a `try` range. + if let Some(catch_label) = catch_for_pc[py_pc] { + emit_catch_exception_and_split!(catch_label, py_pc, [bool_value]); + exception_edge_handled = true; + } // flowcontext.py:756-763 `block.exitswitch = w_cond`. current_block.block().borrow_mut().exitswitch = Some(super::flow::ExitSwitch::Value(bool_value.into())); @@ -8817,10 +8916,12 @@ impl CodeWriter { result.into() }; if nargs > 14 { - emit_abort_permanent!(py_pc); - // `abort_permanent` closes this graph block. - // Do not record the synthetic call result or - // any later operation on the closed block. + // `closes_block`: this arm skips the + // `push_and_bump!` below, so its stack model is + // incomplete and the fall-through must not be + // walked. Do not record the synthetic call + // result or any later operation on the block. + emit_abort_permanent!(py_pc, closes_block); continue; } push_and_bump!(call_result_value, py_pc); @@ -10030,22 +10131,11 @@ impl CodeWriter { // — the catch fires only on a non-null backend // exception. if let Some(catch_label) = catch_for_pc[py_pc] { - emit_catch_exception!(catch_label); - let mut b_state = current_state.clone(); - b_state.next_offset = py_pc; - b_state.blocklist = frame_blocks_for_offset(code, py_pc); - let block_b = SpamBlockRef::new( - graph.new_block(Vec::new()), - Some(b_state.clone()), - ); - all_walker_blocks.push(block_b.clone()); - block_b.block().borrow_mut().inputargs = b_state.getvariables(); - append_exit( - ¤t_block.block(), - output_link(¤t_state, &b_state, block_b.block()), + emit_catch_exception_and_split!( + catch_label, + py_pc, + [next_value.clone()] ); - restore_canraise_exit_order(¤t_block.block()); - current_block = block_b; } // Emit the exhaustion branch: ptr_nonzero(next) // selects between the continue arm (non-null → @@ -11624,7 +11714,11 @@ impl CodeWriter { current_block = unbound_block; current_state = unbound_state; - emit_abort_permanent!(py_pc); + // `closes_block`: the null arm's block is a + // dead end by construction — the bound arm + // already merged the fall-through PC above, so + // nothing follows to close this one. + emit_abort_permanent!(py_pc, closes_block); continue; } let code_const: super::flow::FlowValue = super::flow::Constant::new(