Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 6 additions & 34 deletions majit/majit-metainterp/src/jitdriver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4773,14 +4773,8 @@ impl<S: JitState> JitDriver<S> {
// already recovered `state` to the resume point, so the
// bridge sees the post-guard-failure values.
if should_bridge && !portal_crn_handled && pc != usize::MAX {
let bridge_ok = self.start_bridge_tracing(
&descr_arc,
state,
env,
&raw_values,
pc,
guard_exc,
);
let bridge_ok =
self.start_bridge_tracing(&descr_arc, state, env, &raw_values, pc);
if crate::majit_log_enabled() {
eprintln!(
"[bridge] start_bridge_tracing (green resume) key={} trace={} fail={} resume_pc={} ok={}",
Expand Down Expand Up @@ -6262,11 +6256,6 @@ impl<S: JitState> JitDriver<S> {
env: &S::Env,
raw_fail_values: &[i64],
resume_pc: usize,
// llmodel.py:240 `cpu.grab_exc_value(deadframe)`: the pending exception
// grabbed at the guard failure, threaded so `setup_bridge_sym` seeds the
// bridge sym's standing exception (pyjitpl.py:3125). 0 when the guard
// carried no exception.
guard_exc: i64,
) -> bool {
majit_metainterp::mc_diag_bump(12); // start_bridge_tracing entered
// Same reason as the primary trace entry: the bridge compile decodes
Expand Down Expand Up @@ -6489,11 +6478,6 @@ impl<S: JitState> JitDriver<S> {
ctx.set_virtualizable_heap_ptr(ptr);
}
ctx.set_bridge_source_is_exception_guard(retrace.is_exception_guard);
// pyjitpl.py:3125 `_prepare_exception_resumption` grabs the exception
// BEFORE frame reconstruction; thread it onto the ctx so the pyre
// `setup_bridge_sym` override can seed the standing exception before it
// drains the inline-callee carrier.
ctx.set_bridge_guard_exc(guard_exc);
ctx.bridge_target_header_pc = parent_header_pc;
ctx.has_compiled_targets_fn = Some(Box::new(move |gk: u64| -> bool {
let meta = unsafe { &*(meta_ptr as *const crate::pyjitpl::MetaInterp<S::Meta>) };
Expand Down Expand Up @@ -6906,14 +6890,8 @@ impl<S: JitState> JitDriver<S> {
let resume_pc = resume_pc.unwrap_or(guard_resume_pc);
self.sync_after(state, &result_meta, descriptor.as_deref());

let bridge_ok = self.start_bridge_tracing(
&descr_arc,
state,
env,
&raw_values,
resume_pc,
result_exc,
);
let bridge_ok =
self.start_bridge_tracing(&descr_arc, state, env, &raw_values, resume_pc);
if crate::majit_log_enabled() {
eprintln!(
"[bridge] start_bridge_tracing key={} trace={} fail={} resume_pc={} ok={}",
Expand Down Expand Up @@ -8016,14 +7994,8 @@ mod tests {
let descr_arc = std::sync::Arc::clone(&failure.descr_arc);
drop(failure);

let started = driver.start_bridge_tracing(
&descr_arc,
&mut NonTraceableState,
&(),
&fail_values,
0,
0,
);
let started =
driver.start_bridge_tracing(&descr_arc, &mut NonTraceableState, &(), &fail_values, 0);
assert!(!started);
assert!(!driver.meta.is_tracing());
}
Expand Down
17 changes: 12 additions & 5 deletions majit/majit-metainterp/src/optimizeopt/unroll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1849,11 +1849,18 @@ impl UnrollOptimizer {
body_jump_arity, preamble_arity, exported_renamed_inputargs,
);
}
// `compile.py:334 assert jump.numargs() == label.numargs()`.
// Upstream can assert because its `target_tokens[0]` is the start
// label whose args ARE `loop.inputargs` — the same loop-carried
// positions the body JUMP carries — so `unroll.py:238-242`'s
// arg-preserving retarget is sound by construction.
// Upstream has no arity check on this path at all, and the check
// below is pyre's own. `unroll.py:238-242 jump_to_preamble` only
// asserts `target_tokens[0].virtual_state is None` and retargets the
// JUMP with `copy_and_change`, keeping its args; `compile.py:334`'s
// `assert jump_op.numargs() == loop_info.label_op.numargs()` is
// guarded one line above by `if jump_op.getdescr() is
// loop_info.label_op.getdescr()`, which is exactly what
// `jump_to_preamble` has just stopped being true, and
// `compile_retrace` checks nothing. Upstream needs no check because
// its `target_tokens[0]` is the preamble start label carrying
// `start_state.renamed_inputargs` — the same loop-carried positions
// the body JUMP carries — so the retarget is sound by construction.
//
// A pyre RETRACE has no start label of its own (see
// `emit_start_label`), so `target_tokens[0]` is the ORIGINAL loop's
Expand Down
8 changes: 6 additions & 2 deletions majit/majit-metainterp/src/pyjitpl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1561,8 +1561,8 @@ pub struct MetaInterp<M: Clone> {
/// Internal mutable counters for JIT compilation statistics.
///
/// Holds only the pyre-specific lifetime counters (`loops_compiled`,
/// `loops_aborted`, `bridges_compiled`, `guard_failures`) that have no
/// `Counters.*` slot upstream. Every counter that maps to a
/// `retraces_compiled`, `loops_aborted`, `bridges_compiled`, `guard_failures`)
/// that have no `Counters.*` slot upstream. Every counter that maps to a
/// `Counters.*` id (OPS / HEAPCACHED_OPS / RECORDED_OPS / GUARDS /
/// OPT_OPS / OPT_GUARDS / OPT_GUARDS_SHARED / NV* / ABORT_* /
/// FORCE_VIRTUALIZABLES / OPT_VECTORIZE_*) lives on
Expand All @@ -1571,6 +1571,7 @@ pub struct MetaInterp<M: Clone> {
#[derive(Default, Clone, Debug)]
pub(crate) struct JitStatsCounters {
loops_compiled: usize,
retraces_compiled: usize,
loops_aborted: usize,
bridges_compiled: usize,
guard_failures: usize,
Expand All @@ -1580,6 +1581,7 @@ pub(crate) struct JitStatsCounters {
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct JitStats {
pub loops_compiled: usize,
pub retraces_compiled: usize,
pub loops_aborted: usize,
pub bridges_compiled: usize,
pub guard_failures: usize,
Expand Down Expand Up @@ -4083,6 +4085,7 @@ impl<M: Clone> MetaInterp<M> {
pub fn get_stats(&self) -> JitStats {
JitStats {
loops_compiled: self.stats.loops_compiled,
retraces_compiled: self.stats.retraces_compiled,
loops_aborted: self.stats.loops_aborted,
bridges_compiled: self.stats.bridges_compiled,
guard_failures: self.stats.guard_failures,
Expand Down Expand Up @@ -8194,6 +8197,7 @@ impl<M: Clone> MetaInterp<M> {
// `ResumeDescr.rd_loop_token` inherits the source identity.
let mut combined_ops = combined_ops;
self.record_loop_or_bridge(&source_jct, &mut combined_ops, bridge_trace_id);
self.stats.retraces_compiled += 1;
if crate::majit_log_enabled() {
eprintln!(
"[jit] attached retrace to guard at key={green_key}, guard={fail_index}, \
Expand Down
24 changes: 0 additions & 24 deletions majit/majit-metainterp/src/trace_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,18 +635,6 @@ pub struct TraceCtx {
/// `descr_arc.is_guard_exc()` and read by static bridge setup/walkers
/// that only receive `TraceCtx`.
pub(crate) bridge_source_is_exception_guard: bool,
/// llmodel.py:240 `cpu.grab_exc_value(deadframe)`: the pending exception
/// value grabbed at the guard failure that triggered this bridge, threaded
/// from `start_bridge_tracing` so `setup_bridge_sym` can seed the bridge
/// sym's standing exception (pyjitpl.py:3125 `_prepare_exception_resumption`
/// grabs BEFORE frame reconstruction). Raw `PyObjectRef as i64`; 0 when the
/// guard carried no exception. Class is re-derived from the value's typeptr.
///
/// Not itself a traced slot: the exception is kept alive for the whole
/// handoff by [`crate::blackhole::GuardExcRoot`], which `handle_fail` parks
/// before it starts the bridge, so the value read back here is still live
/// whether or not a collection ran during the resume decode.
pub(crate) bridge_guard_exc: i64,
}

/// A decoded-but-not-yet-built description of one inlined
Expand Down Expand Up @@ -1554,7 +1542,6 @@ impl TraceCtx {
bridge_inline_carrier: None,
bridge_reg_indices: None,
bridge_source_is_exception_guard: false,
bridge_guard_exc: 0,
}
}

Expand Down Expand Up @@ -1640,7 +1627,6 @@ impl TraceCtx {
bridge_inline_carrier: None,
bridge_reg_indices: None,
bridge_source_is_exception_guard: false,
bridge_guard_exc: 0,
}
}

Expand Down Expand Up @@ -1679,16 +1665,6 @@ impl TraceCtx {
}

/// True only for bridge traces sourced from an exception guard descr.
pub fn set_bridge_guard_exc(&mut self, guard_exc: i64) {
self.bridge_guard_exc = guard_exc;
}

/// The exception value grabbed at the guard failure that triggered this
/// bridge (0 when none). See `bridge_guard_exc`.
pub fn bridge_guard_exc(&self) -> i64 {
self.bridge_guard_exc
}

pub fn bridge_source_is_exception_guard(&self) -> bool {
self.bridge_source_is_exception_guard
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=647
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=647
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=647
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=809
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=809
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=809
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=2
internal_compile_panics=0
loops_aborted=0
loops_compiled=1
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=2
internal_compile_panics=0
loops_aborted=0
loops_compiled=1
40 changes: 40 additions & 0 deletions pyre/bench/synth/getframe_caller_locals_after_resume.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# pyre-check: max-pypy-ratio=17
# pypy's exec time is pinned to the startup-subtraction floor on most runs here,
# so the ratio is not a measurement. Nine local readings across the three
# backends span 1.8x-5.5x, and the ceiling is three times the slowest of them.
#
# A guard-failure resume inside an inlined callee must close that callee's
# execution-context scope before its blackhole advances to the caller
# (`executioncontext.py:91-107` leave). The blackhole run loop transfers the
# callee's return value and releases its interpreter, but releasing a
# BlackholeInterpreter does not restore `topframeref` on its own, so without
# the leave transition the completed callee stays the current frame.
#
# Two things then go wrong, and this fixture asserts both because either can
# hold while the other breaks:
# 1. a later `sys._getframe(1)` chains behind the stale callee and reads that
# callee's sparse locals image, so `f_locals['base']` raises KeyError;
# 2. the caller's own `locals()` selects the stale frame and answers with the
# callee's parameter set — no `sys._getframe` involved at the read.
#
# The first triggering call is correct either way: the damage is only
# observable once the resumed callee should have left. A single triggering
# call therefore passes with or without the fix.
import sys


def inner(k):
if k > 2997: # two triggering calls, not one
return sys._getframe(1).f_locals['base']
return k


def outer(n):
base = 11
acc = 0
for i in range(n):
acc += inner(i) & 7
return acc, sorted(locals().keys())


print(outer(3000))
14 changes: 14 additions & 0 deletions pyre/bench/synth/getframe_caller_locals_after_resume.wasm.jitstats
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=2
internal_compile_panics=0
loops_aborted=0
loops_compiled=1
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1345
Expand Down
3 changes: 3 additions & 0 deletions pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1345
Expand Down
3 changes: 3 additions & 0 deletions pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ bridges_compiled=5
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1345
Expand Down
4 changes: 2 additions & 2 deletions pyre/bench/synth/pickle_terminal_raise_resume.wasm.jitstats
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=339
internal_compile_panics=0
loops_aborted=13
loops_compiled=67
loops_aborted=14
loops_compiled=66
Loading
Loading