diff --git a/majit/majit-metainterp/src/jitdriver.rs b/majit/majit-metainterp/src/jitdriver.rs index aa6fda554c0..bf60b3f5195 100644 --- a/majit/majit-metainterp/src/jitdriver.rs +++ b/majit/majit-metainterp/src/jitdriver.rs @@ -2487,6 +2487,53 @@ impl JitDriver { Some((original_green_key, trace_meta)) } + /// pyjitpl.py:1641-1663, the loop arm of + /// `_create_segmented_trace_and_blackhole`: + /// + /// ```python + /// target_token = compile.compile_simple_loop( + /// metainterp, greenkey, metainterp.history.trace, + /// fake_runtime_boxes, enable_opts, cut_at, patch_jumpop_at_end=False) + /// jd_sd.warmstate.attach_procedure_to_interp( + /// greenkey, target_token.targeting_jitcell_token) + /// ``` + /// + /// Split out of the `TraceAction::SegmentedLoop` arm so the arm can host + /// the bridge else-arm (pyjitpl.py:1665-1668) beside it without the two + /// compiles pushing the shared handoff and blackhole tail apart. + fn compile_segmented_loop(&mut self, meta: S::Meta) { + let Some(green_key) = self.meta.compile_simple_loop(meta) else { + return; + }; + // pyjitpl.py:2760 reads the greenkey from current_merge_points; + // without staging the key here, the segmented-loop abort hook + // reports key 0. + self.meta.pending_abort_green_key = Some(green_key); + // pyjitpl.py:1662-1663 line-by-line: pass the compiled + // `target_token` returned by `compile_simple_loop` to + // `attach_procedure_to_interp`, NOT a fresh synthetic uncompiled + // JitCellToken. pyre's `compile_simple_loop` returns the + // green_key; the actual `Arc` is the `token` field + // of the freshly-installed `CompiledEntry` in `self.compiled_loops`. + let install_token = self + .meta + .compiled_loops + .get(&green_key) + .expect( + "compile_simple_loop returned Some(green_key) ⇒ \ + compiled_loops has the new CompiledEntry", + ) + .live_token(); + // `warmstate.py:339-348` redirect+record_jump_to chain routed + // through MetaInterp's caller-side helper. Skip the redirect when + // MemoryManager has already evicted the freshly-installed token + // (rare; eviction is independent of this insertion path). + if let Some(install_token) = install_token { + self.meta + .attach_procedure_with_redirect(green_key, install_token); + } + } + /// RPython rlib.jit.current_trace_length(). /// Returns the number of ops in the active trace, or -1 if not tracing. pub fn current_trace_length(&mut self) -> i64 { @@ -2798,13 +2845,21 @@ impl JitDriver { .trace_ctx() .and_then(|ctx| ctx.take_close_jump_into_key()) { - let mut compiled = false; + let mut result = crate::pyjitpl::BridgeCompileResult::Declined; + // Whether the close was actually evaluated. Both conditions below are + // transient — `partial_trace` clears with the retrace it belongs to, and a + // key acquires compiled targets the moment its loop compiles — and + // upstream re-tests them at every later visit of the same header + // (pyjitpl.py:3001-3007 runs once per visit, with no memory of the last). + // Only an attempt that ran and did not compile may be latched below. + let mut attempted = false; let mut continue_running_normally_values = None; // pyjitpl.py:3003 `if not self.partial_trace:` — a retrace must not bridge // into the very loop it exists to respecialize. if self.meta.partial_trace().is_none() && self.meta.has_compiled_targets(target_key) { + attempted = true; continue_running_normally_values = { let trace_meta = self.meta.trace_meta().cloned(); match (trace_meta, self.sym.as_ref()) { @@ -2828,15 +2883,12 @@ impl JitDriver { // (`compile_trace_from_interp`, compile.py:1002-1021). let bridge_origin = self.meta.bridge_info().map(|b| (b.trace_id, b.fail_index)); - compiled = match bridge_origin { - Some((trace_id, fail_index)) => matches!( - self.meta.close_bridge( - target_key, - trace_id, - fail_index, - &live_arg_boxes, - ), - crate::pyjitpl::BridgeCompileResult::Compiled + result = match bridge_origin { + Some((trace_id, fail_index)) => self.meta.close_bridge( + target_key, + trace_id, + fail_index, + &live_arg_boxes, ), // compile.py:269-270: a cross-loop CUT keeps its cut prefix // in `front_target_tokens[0]`, where a loop closed at its own @@ -2858,51 +2910,88 @@ impl JitDriver { // need not be reached — and declining the guard origin too // makes `pi/pi.jinseo` at `MAJIT_THRESHOLD=50` compute wrong // digits from byte 865, same length, no crash. - None if self.meta.is_cross_loop_cut_key(target_key) => false, + // + // What the widened arm produces, at the op level: the + // declined close returns here, the walk runs on and closes a + // second time one aheui instruction later, and the bridge it + // compiles carries that instruction's pop (`IntAdd -1`, two + // `GcLoadR`, two `GcStore`) ahead of a `Jump` into a + // four-input label, where the close that was declined jumped + // into a three-input one. So the cost is not the lost close + // but the resumed walk: declining and then giving the trace + // up outright reproduces the un-compiled output exactly. + None if self.meta.is_cross_loop_cut_key(target_key) => { + crate::pyjitpl::BridgeCompileResult::Declined + } None => match self.compile_trace_entry_data() { - Some((original_green_key, entry_meta)) => matches!( - self.meta.compile_trace_from_interp( + Some((original_green_key, entry_meta)) => { + let outcome = self.meta.compile_trace_from_interp( target_key, &live_arg_boxes, original_green_key, entry_meta, - ), - crate::CompileOutcome::Compiled { .. } - ), - None => false, + ); + self.meta.classify_compile_outcome(outcome) + } + None => crate::pyjitpl::BridgeCompileResult::Declined, }, }; } - if compiled { - // pyjitpl.py:3220 raise_if_successful → raise_continue_running_normally: - // the trace is over, the interpreter enters the loop it jumped into. - self.sym = None; - self.meta.clear_trace_session(); - self.note_continue_running_normally( - continue_running_normally_values, - None, - ); - self.meta.finish_trace_live(); - self.meta.clear_pending_abort(); - return; - } - // pyjitpl.py:3009-3010: `compile_trace` returned without raising, so the - // trace is NOT given up — tracing continues. Latch the decline so the walk - // does not re-run the optimizer over the same key, and re-enter the walk at - // the merge point's own pc (pyjitpl.py:1577 `self.pc = saved_pc`). - if let Some(ctx) = self.meta.trace_ctx() { - ctx.note_cross_loop_close_declined(target_key); - ctx.close_greens = None; - ctx.close_green_pc = None; - ctx.merge_point_resumed = true; + match result { + crate::pyjitpl::BridgeCompileResult::Compiled => { + // pyjitpl.py:3220 raise_if_successful → raise_continue_running_normally: + // the trace is over, the interpreter enters the loop it jumped into. + self.sym = None; + self.meta.clear_trace_session(); + self.note_continue_running_normally( + continue_running_normally_values, + None, + ); + self.meta.finish_trace_live(); + self.meta.clear_pending_abort(); + return; + } + crate::pyjitpl::BridgeCompileResult::RetraceNeeded => { + // pyjitpl.py:3003 and the sibling RetraceNeeded + // arm below: `compile_trace` armed + // `partial_trace`, so fall through to the + // reached_loop_header path that routes + // compile_loop -> compile_retrace in this call. + self.bridge_attempt_declined = true; + } + crate::pyjitpl::BridgeCompileResult::Declined => { + // pyjitpl.py:3009-3010: `compile_trace` returned without raising, so the + // trace is NOT given up — tracing continues. Latch the decline so the walk + // does not re-run the optimizer over the same key, and re-enter the walk at + // the merge point's own pc (pyjitpl.py:1577 `self.pc = saved_pc`). + // + // Only latch what an attempt actually rejected. A close the gate + // above never evaluated cost no optimizer pass, which is the whole + // reason the latch exists, and the gate's own conditions can be + // false now and true at the next visit of this header. + if let Some(ctx) = self.meta.trace_ctx() { + if attempted { + ctx.note_cross_loop_close_declined(target_key); + } + ctx.close_greens = None; + ctx.close_green_pc = None; + ctx.merge_point_resumed = true; + } + // The walk-final handoff staged at the top of this arm describes a trace + // that ENDED; discard it, same as the `take_keep_tracing_after_close` path + // at the bottom of this arm. + self.meta.single_pass_outcome = None; + self.meta.single_pass_scalar_values = None; + self.meta.single_pass_virt_array_values = None; + continue; + } + crate::pyjitpl::BridgeCompileResult::Failed => { + self.meta.abort_trace(false); + self.sym = None; + self.meta.clear_trace_session(); + return; + } } - // The walk-final handoff staged at the top of this arm describes a trace - // that ENDED; discard it, same as the `take_keep_tracing_after_close` path - // at the bottom of this arm. - self.meta.single_pass_outcome = None; - self.meta.single_pass_scalar_values = None; - self.meta.single_pass_virt_array_values = None; - continue; } // pyjitpl.py:2979-3036 reached_loop_header parity. // Path 1: bridge — only if has_compiled_targets (line 2982). @@ -3474,12 +3563,24 @@ impl JitDriver { } self.sym = None; } - TraceAction::SegmentedLoop => { - // pyjitpl.py:1658-1663 _create_segmented_trace_and_blackhole: - // target_token = compile.compile_simple_loop(...) - // warmstate.attach_procedure_to_interp(greenkey, token) - // + pyjitpl.py:1673 SwitchToBlackhole(ABORT_SEGMENTED_TRACE) - // + action @ (TraceAction::SegmentedLoop | TraceAction::SegmentedBridge { .. }) => { + // pyjitpl.py:1639-1668 _create_segmented_trace_and_blackhole, + // compile half. Upstream branches on whether the segmented + // trace owns a merge point and entered from the interpreter: + // loop: target_token = compile.compile_simple_loop(...) + // warmstate.attach_procedure_to_interp(greenkey, token) + // bridge: target_token = compile.compile_trace( + // metainterp, metainterp.resumekey, [exception_box]) + // if target_token is not token: compile.giveup() + // then leaves through one shared + // pyjitpl.py:1673 SwitchToBlackhole(ABORT_SEGMENTED_TRACE). + // `create_segmented_trace` made that choice while it still + // held the `TraceCtx`; the box it carries is the FINISH + // operand the bridge arm still has to record. + let bridge_exception_box = match action { + TraceAction::SegmentedBridge { exception_box } => Some(exception_box), + _ => None, + }; // pyjitpl.py:1671-1672 blackholes back to the interpreter // rather than jumping to compiled code. Publish the // single-pass handoff first — the walk executed everything @@ -3502,36 +3603,56 @@ impl JitDriver { self.meta.single_pass_virt_array_values = Some(elems); } } - let meta = self.meta.take_trace_meta().unwrap(); - if let Some(green_key) = self.meta.compile_simple_loop(meta) { - // pyjitpl.py:1662-1663 line-by-line: pass the compiled - // `target_token` returned by `compile_simple_loop` to - // `attach_procedure_to_interp`, NOT a fresh synthetic - // uncompiled JitCellToken. pyre's `compile_simple_loop` - // returns the green_key; the actual `Arc` - // is the `token` field of the freshly-installed - // `CompiledEntry` in `self.compiled_loops`. - let install_token = self - .meta - .compiled_loops - .get(&green_key) - .expect( - "compile_simple_loop returned Some(green_key) ⇒ \ - compiled_loops has the new CompiledEntry", - ) - .live_token(); - // `warmstate.py:339-348` redirect+record_jump_to chain - // routed through MetaInterp's caller-side helper. Skip - // the redirect when MemoryManager has already evicted - // the freshly-installed token (rare; eviction is - // independent of this insertion path). - if let Some(install_token) = install_token { - self.meta - .attach_procedure_with_redirect(green_key, install_token); + let abort_reason = match bridge_exception_box { + // pyjitpl.py:1665-1668, the else-arm: a guard-origin + // bridge has no merge point to make a loop out of, so + // the segment is closed as an ordinary bridge. + // `compile_finish_from_active_session` IS + // `compile.compile_trace(metainterp, self.resumekey, + // [exception_box])` for a bridge origin: it records the + // FINISH under `sd.exit_frame_with_exception_descr_ref` + // and returns `Err` exactly where upstream's + // `target_token is not token` reaches `compile.giveup()` + // (compile.py:27, `SwitchToBlackhole(ABORT_BRIDGE)`). + // + // The flag this arm answers to was already being armed + // with nothing to act on it: `prepare_trace_segmenting` + // (pyjitpl.py:2849-2857) sets FORCE_BRIDGE_SEGMENTING on + // the source loop token, `start_retrace_from_guard` + // reads it back into `force_finish_trace` + // (compile.py:725-731), and the bridge then ran on to + // the ordinary over-limit abort — the retracing-forever + // outcome the flag is set to prevent. Upstream's own + // note at pyjitpl.py:2854: "creating a segmented bridge + // is generally quite safe". + Some(exception_box) => { + // Read before the compile drains the tracer: the + // bridge's own key is what its abort hook reports. + let green_key = self.meta.trace_ctx().map(|ctx| ctx.green_key); + let result = self.meta.compile_finish_from_active_session( + &[exception_box], + vec![majit_ir::Type::Int], + /* exit_with_exception */ true, + ); + // On the giveup path `abort_trace_live` already + // staged this key; on the success path it was + // cleared again. Stage it either way so the hook + // below does not report key 0. + self.meta.pending_abort_green_key = green_key; + match result { + Ok(()) => crate::counters::ABORT_SEGMENTED_TRACE, + Err(stb) => stb.reason, + } } - } + None => { + let meta = self.meta.take_trace_meta().unwrap(); + self.compile_segmented_loop(meta); + crate::counters::ABORT_SEGMENTED_TRACE + } + }; // Blackhole transition: clear all driver tracing state. - // `take_trace_meta` above drained the M-ownership side; + // `take_trace_meta` / `compile_finish_from_active_session` + // above drained the M-ownership side; // `leave_profiler_tracing` fires the matching // `pyjitpl.py:2934 finally: profiler.end_tracing()`. // `clear_trace_session` is then a no-op for both effects @@ -3544,8 +3665,10 @@ impl JitDriver { // Counters.ABORT_SEGMENTED_TRACE)` — upstream's handler // (pyjitpl.py:2949) runs `aborted_tracing(stb.reason)`, so a // segmented trace counts as an abort with its own reason. - self.meta - .aborted_tracing(crate::counters::ABORT_SEGMENTED_TRACE); + // A bridge arm that gave up carries `ABORT_BRIDGE` instead, + // for the same reason: `giveup` raises its own + // SwitchToBlackhole and that is the reason the handler sees. + self.meta.aborted_tracing(abort_reason); } // Consumed inside the metainterp dispatch loop // (PyreMetaInterp::step_inline_frame pops the inline frame and diff --git a/majit/majit-metainterp/src/lib.rs b/majit/majit-metainterp/src/lib.rs index 2df329f0734..5c231f65d95 100644 --- a/majit/majit-metainterp/src/lib.rs +++ b/majit/majit-metainterp/src/lib.rs @@ -392,6 +392,23 @@ pub enum TraceAction { /// The trace has GUARD_ALWAYS_FAILS + unreachable FINISH appended. /// compile_simple_loop inserts a LABEL at entry for bridge attachment. SegmentedLoop, + /// The else-arm of the same split (pyjitpl.py:1665-1668): the segmented + /// trace is a guard-origin bridge, so there is no merge point to make a + /// loop out of and it is closed as an ordinary bridge instead. + /// + /// ```python + /// target_token = compile.compile_trace(metainterp, metainterp.resumekey, + /// [exception_box]) + /// if target_token is not token: + /// compile.giveup() + /// ``` + /// + /// The trace has GUARD_ALWAYS_FAILS appended but NOT the FINISH — the + /// driver's compile records it, so that it carries + /// `sd.exit_frame_with_exception_descr_ref` (the `token` upstream + /// compares the returned target against). `exception_box` is the + /// operand it finishes with, the same box the loop arm records. + SegmentedBridge { exception_box: OpRef }, /// Abort the current trace (recoverable — may retry later). Abort, /// Decline the current trace before compilation and return to residual diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 03bf5c81052..d5ec80826e9 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -11013,16 +11013,12 @@ impl MetaInterp { } } - /// pyjitpl.py:2982-2983: close_bridge — compile_trace wrapper that - /// maps CompileOutcome to BridgeCompileResult. - pub fn close_bridge( - &mut self, - green_key: u64, - trace_id: u64, - fail_index: u32, - finish_args: &[OpRef], - ) -> BridgeCompileResult { - let outcome = self.compile_trace(green_key, finish_args, Some((trace_id, fail_index))); + /// pyjitpl.py:2982-2983: the `compile_trace` outcome classification shared by + /// every close that goes through `compile_trace_inner` — the guard-origin + /// bridge (`close_bridge`) and the interp-origin entry bridge + /// (`compile_trace_from_interp`) alike, since `retrace_after_bridge` is armed + /// inside the shared compile path rather than per origin. + pub(crate) fn classify_compile_outcome(&self, outcome: CompileOutcome) -> BridgeCompileResult { match outcome { CompileOutcome::Compiled { .. } => BridgeCompileResult::Compiled, _ if self.retrace_after_bridge => { @@ -11040,6 +11036,19 @@ impl MetaInterp { } } + /// pyjitpl.py:2982-2983: close_bridge — compile_trace wrapper that + /// maps CompileOutcome to BridgeCompileResult. + pub fn close_bridge( + &mut self, + green_key: u64, + trace_id: u64, + fail_index: u32, + finish_args: &[OpRef], + ) -> BridgeCompileResult { + let outcome = self.compile_trace(green_key, finish_args, Some((trace_id, fail_index))); + self.classify_compile_outcome(outcome) + } + /// RPython-compatible helper name from compile.py. pub fn send_bridge_to_backend( &mut self, diff --git a/majit/majit-metainterp/src/pyjitpl/dispatch.rs b/majit/majit-metainterp/src/pyjitpl/dispatch.rs index a237e782b7c..1bf83949044 100644 --- a/majit/majit-metainterp/src/pyjitpl/dispatch.rs +++ b/majit/majit-metainterp/src/pyjitpl/dispatch.rs @@ -1556,14 +1556,18 @@ where /// waste the whole recording, so it is terminated here instead: an /// always-failing guard takes every execution back to the interpreter, /// and the FINISH behind it exists only to give the segment a - /// terminator. The compile half — `compile_simple_loop` plus - /// `attach_procedure_to_interp` (pyjitpl.py:1658-1663) — needs the - /// `MetaInterp` the walker does not hold, so it runs in the - /// [`TraceAction::SegmentedLoop`] arm of the driver. + /// terminator. The compile half needs the `MetaInterp` the walker + /// does not hold, so it runs in the driver, in one of the two arms + /// upstream branches to at pyjitpl.py:1639: /// - /// `compile_simple_loop` puts a LABEL at the segment's entry, which is - /// what lets a later trace close back into it; without it the segmented - /// loop could never be completed (pyjitpl.py:1641-1643). + /// * [`TraceAction::SegmentedLoop`] — `compile_simple_loop` plus + /// `attach_procedure_to_interp` (pyjitpl.py:1658-1663). + /// `compile_simple_loop` puts a LABEL at the segment's entry, which + /// is what lets a later trace close back into it; without it the + /// segmented loop could never be completed (pyjitpl.py:1641-1643). + /// * [`TraceAction::SegmentedBridge`] — `compile_trace(metainterp, + /// resumekey, [exception_box])` with the `target_token is not token` + /// give-up (pyjitpl.py:1665-1668). fn create_segmented_trace( &mut self, ctx: &mut TraceCtx, @@ -1584,13 +1588,44 @@ where mp_opcode_pc, /* after_residual_call */ false, ); - // pyjitpl.py:1633-1637: an unreachable FINISH carrying the - // AssertionError typeptr and `exit_frame_with_exception_descr_ref`. - // Pyre's FINISH takes neither — `record_finish` records the op with - // its result operand alone — and the op is unreachable behind a - // guard that always fails, so the operand is a placeholder. + // pyjitpl.py:1633-1636 `exception_box = ConstInt(ptr2int( + // llexception.typeptr))` — the AssertionError type pointer the + // unreachable FINISH escapes with. The op sits behind a guard that + // always fails, so pyre records the placeholder `ConstInt(0)` + // instead of resolving a type pointer for a value nothing reads. + // Both arms below take this same box, as upstream does. let exception_box = ctx.const_int(0); - ctx.record_finish(exception_box, majit_ir::Type::Int); + // pyjitpl.py:1639-1640 `if (metainterp.current_merge_points and + // isinstance(metainterp.resumekey, compile.ResumeFromInterpDescr)):` + // — a trace that owns a merge point and entered from the + // interpreter becomes a segmented loop; anything else (a + // guard-origin bridge) takes the else-arm. + let is_loop_trace = ctx.current_merge_points_first_greenkey().is_some() + && ctx.resumekey_original_loop_token().is_none(); + // pyjitpl.py:1637 `history.record1(rop.FINISH, exception_box, None, + // descr=token)`, recorded before the branch and seen by both arms. + // The loop arm keeps it here, where `record_finish` writes the op + // with its operand alone. The bridge arm leaves it to + // `compile_finish_from_active_session`, which is the port of + // pyjitpl.py:1666 `compile_trace(metainterp, resumekey, + // [exception_box])` and records the same FINISH through + // `recorder.finish(finish_args, finish_descr)` — carrying + // `sd.exit_frame_with_exception_descr_ref`, the descr upstream's + // `target_token is not token` test compares against. Recording it + // here as well would give that trace two terminators. + // + // The loop arm's FINISH therefore still carries no descr, which is + // the one place this stays short of pyjitpl.py:1637. The slot is + // there (`Op::setdescr`, and `Trace::record_op_with_descr` behind + // `recorder.finish`); what is missing is a `record_finish` that + // takes one. Left alone here because the loop arm reaches the + // backend through `compile_simple_loop`, where a FINISH that starts + // reporting `is_exception_exit` changes exit dispatch for a segment + // that compiles today — a change to make with its own measurement, + // not alongside the bridge arm's first one. + if is_loop_trace { + ctx.record_finish(exception_box, majit_ir::Type::Int); + } // pyjitpl.py:1671-1673: "we now need to blackhole back to the // interpreter instead of jumping to some existing code, because we // are at a really arbitrary place here." Under single-pass tracing @@ -1602,7 +1637,11 @@ where ctx.walk_final_pc = mp_green_pc.map(|p| p as usize); ctx.walk_final_reds = Vec::new(); // pyjitpl.py:1673 `raise SwitchToBlackhole(ABORT_SEGMENTED_TRACE)`. - TraceAction::SegmentedLoop + if is_loop_trace { + TraceAction::SegmentedLoop + } else { + TraceAction::SegmentedBridge { exception_box } + } } /// Resolve the box operand for a vable opcode. The canonical @@ -4932,16 +4971,12 @@ where // `jit_merge_point` op, which an arbitrary mid-walk position // has no counterpart for. if ctx.force_finish_trace() && ctx.num_ops() > ctx.trace_limit() * 4 / 5 { - // pyjitpl.py:1639-1640 `if metainterp.current_merge_points - // and isinstance(metainterp.resumekey, - // ResumeFromInterpDescr):` — the loop arm. A bridge takes - // upstream's `compile_trace(resumekey)` else-arm instead, - // which is not ported; it keeps aborting, as before. - let is_loop_trace = ctx.current_merge_points_first_greenkey().is_some() - && ctx.resumekey_original_loop_token().is_none(); - if is_loop_trace { - return self.create_segmented_trace(ctx, sym, mp_opcode_pc, mp_green_pc); - } + // The loop-vs-bridge split lives inside + // `create_segmented_trace`, where upstream keeps it + // (pyjitpl.py:1639) — the check reached here segments + // whatever trace it is in, exactly as + // `_create_segmented_trace_and_blackhole` does. + return self.create_segmented_trace(ctx, sym, mp_opcode_pc, mp_green_pc); } // pyjitpl.py:1547 `jitdriver_sd = // self.metainterp.staticdata.jitdrivers_sd[jdindex]` reads the @@ -5394,51 +5429,49 @@ where // `original_boxes[:num_green_args]` — the INNER // greenkey (pyjitpl.py:3183-3187). // - // Pyre's cut cannot do that attach: it stores under - // `cut_inner_green_key`, and the only key derivable - // here is `green_key_from_code_ptr(green_key_raw.0, - // pc)` with `JitState::code_ptr()` defaulting to 0 — - // not the driver's `GreenKey::hash_u64`. So cutting - // at a merge point that already holds a compiled loop - // would replace reachable code with code stored where - // nothing enters. Decline the cut and keep tracing; - // the trace then closes at its own header with this - // loop's body inlined, which is what it does at trip - // counts too low to reach the merge point twice. + // The JUMP is what the `already_compiled_here` arm + // below performs: it publishes the token key and + // returns `CloseLoop`, and the driver runs + // `close_bridge` (guard origin) or + // `compile_trace_from_interp` (interp origin). + // + // It is sound only because the key this arm derives + // is the one the interpreter ENTERS by. While the + // key was `green_key_from_code_ptr(green_key_raw.0, + // pc)` — `JitState::code_ptr()` defaulting to 0, not + // the driver's `GreenKey::hash_u64` — a compiled loop + // could sit under a key nothing enters, and jumping + // into it was measured as a logo miscompile (992635 + // against 996310) and a SIGSEGV. The four + // procedure-token consults now share + // `merge_point_green_key_hash`, so a loop is stored + // under the key it is reached by and the jump lands + // in code the interpreter can also enter. // - // The JUMP-into-ptoken half of :3001-3007 is not - // implemented here. It is not blocked on missing - // machinery — that was tried and measured: - // publishing the token key plus `close_greens` and - // returning `CloseLoop` reaches `close_bridge` for a - // guard origin and `compile_trace_from_interp` - // (through `compile_trace_entry_data`, which needs - // `header_pc == 0`) for an interp origin, and both - // land. cel's `nested_list_loop_varying_trip_count` - // then keeps its results and loses its 4 aborts, but - // its `spread 0..32` deopts go 959 → 1763 over 4000 - // rows and 1275 → 2601 over 16000: the residual - // per-row tail worsens ~2.6x. Jumping in ends the - // trace at the inner loop, where the ordinary close - // covered the whole outer iteration, and the exit - // guards that shape leaves behind do not converge. - // Routing the closing JUMP's target tokens off the - // token it enters instead of off the bridge origin - // (`unroll.py:196-197 cell_token = jump_op.getdescr()` - // — pyre's `compile_bridge` hands `optimize_bridge` - // the ORIGIN loop's `front_target_tokens`) recovers - // only 7% of that, so the gap is elsewhere. + // ⚠ The earlier measurement against this lever — + // cel's `nested_list_loop_varying_trip_count` keeping + // its results and losing its 4 aborts while `spread + // 0..32` deopts went 959 → 1763 over 4000 rows and + // 1275 → 2601 over 16000 — was taken BEFORE that key + // unification, i.e. against jumps into loops filed + // under keys nothing enters. It does not carry over + // and must be re-measured before being cited again. + // Same for the note that routing the closing JUMP's + // target tokens off the token it enters rather than + // off the bridge origin (`unroll.py:196-197 + // cell_token = jump_op.getdescr()` — pyre's + // `compile_bridge` hands `optimize_bridge` the ORIGIN + // loop's `front_target_tokens`) recovered only 7%. // - // Two consequences of declining, both narrower than - // upstream: - // * upstream reaches the `current_merge_points` - // scan whenever `compile_trace` does NOT raise; - // this arm returns to neither the scan nor the - // `append` (:3058-3060), so the merge point is - // never registered at all while a compiled loop - // sits at those greens. - // * upstream's is a JUMP into reachable code; ours - // re-traces that loop's body inline. + // One consequence of a DECLINED attempt is still + // narrower than upstream: upstream reaches the + // `current_merge_points` scan whenever `compile_trace` + // does not raise, while a declined attempt here + // returns to neither the scan nor the `append` + // (:3058-3060), so the merge point goes unregistered + // while a compiled loop sits at those greens. Kept + // deliberately — it is exactly the pre-JUMP behaviour, + // so the lever has a clean A/B. // // The token lookup below is unconditional, where // upstream guards it with `if not self.partial_trace:`