diff --git a/majit/majit-backend-wasm/src/codegen.rs b/majit/majit-backend-wasm/src/codegen.rs index f5e6ec29421..bf07984f2e9 100644 --- a/majit/majit-backend-wasm/src/codegen.rs +++ b/majit/majit-backend-wasm/src/codegen.rs @@ -1373,7 +1373,7 @@ pub fn build_wasm_module( g.is_finish && !crate::failguard::meta_descr_is_exit_frame_with_exception(&g.meta_descr) }) .map(|g| g.fail_index) - .unwrap_or(0); + .unwrap_or(crate::failguard::WASM_CA_FINISH_FI_UNKNOWN); // CA frames execute the source loop and this bridge on the same frozen // geometry. `compile_bridge` rejects a bridge that needs more slots, so // no global floor or speculative slack is needed here. @@ -2001,18 +2001,16 @@ fn build_function( guard_idx += 1; } OpCode::GuardValue => { - let arg0 = op.arg(0).to_opref(); - let is_float = - !arg0.is_constant() && value_types[arg0.raw() as usize] == ValType::F64; - if is_float { - emit_resolve_f64(&mut sink, constants, value_types, arg0); - emit_resolve_f64(&mut sink, constants, value_types, op.arg(1).to_opref()); - sink.f64_ne(); - } else { - emit_resolve(&mut sink, constants, value_types, arg0); - emit_resolve(&mut sink, constants, value_types, op.arg(1).to_opref()); - sink.i64_ne(); - } + // GUARD_VALUE checks bit-equality against the promoted constant: + // Value::eq (value.rs) compares floats by to_bits() (0.0 != -0.0, + // NaN == same-bit NaN, per history.py same_constant), which the + // dynasm/cranelift siblings implement as an integer bit-compare. + // emit_resolve pushes an F64 operand's i64 bits, so i64_ne is the + // correct compare for both int and float — an IEEE f64.ne would + // wrongly pass -0.0 == +0.0 (and fail NaN == same-bit NaN). + emit_resolve(&mut sink, constants, value_types, op.arg(0).to_opref()); + emit_resolve(&mut sink, constants, value_types, op.arg(1).to_opref()); + sink.i64_ne(); emit_guard_if_exit( &mut sink, constants, @@ -2674,34 +2672,20 @@ fn build_function( } // ── String/Unicode ops (direct memory access) ── - OpCode::Strlen | OpCode::Unicodelen => { - let vi = op.pos.get().raw(); - if !OpRef::raw_is_constant(vi) { - emit_resolve(&mut sink, constants, value_types, op.arg(0).to_opref()); - sink.i32_wrap_i64(); - // Length at offset 8 (after ob_type pointer on wasm32) - sink.i64_load(mem64(8)); - sink.local_set(1 + vi); - } - } - OpCode::Strgetitem | OpCode::Unicodegetitem => { - let vi = op.pos.get().raw(); - if !OpRef::raw_is_constant(vi) { - // str[index]: base + header_size + index - emit_resolve(&mut sink, constants, value_types, op.arg(0).to_opref()); - sink.i32_wrap_i64(); - emit_resolve(&mut sink, constants, value_types, op.arg(1).to_opref()); // index - sink.i32_wrap_i64(); - sink.i32_add(); - // String data starts after header (assume 16 bytes: ob_type + length) - sink.i32_load8_u(MemArg { - offset: 16, - align: 0, - memory_index: 0, - }); - sink.i64_extend_i32_u(); - sink.local_set(1 + vi); - } + // strlen/strgetitem/unicodelen/unicodegetitem were lowered with a + // hardcoded layout (length as an 8-byte load of a 4-byte word field; + // item as a 1-byte, stride-1 read at a fixed offset) that is wrong for + // UNICODE (4-byte code units, stride 4) and folds garbage into a str + // length's high bits — a silent wrong value on wasm, where offset is + // valid linear memory and does not trap. pyre models strings/unicode + // as Array(Char) and routes these through the descr-driven + // GETARRAYITEM/ARRAYLEN paths, so no producer emits these ops; decline + // them (interpreter fallback) rather than ship a wrong hardcoded read. + OpCode::Strlen | OpCode::Unicodelen | OpCode::Strgetitem | OpCode::Unicodegetitem => { + return Err(BackendError::Unsupported(format!( + "wasm codegen: string/unicode direct-memory op {:?} (no descr-driven layout)", + op.opcode + ))); } // ── GC memory ops ── @@ -2950,7 +2934,12 @@ fn build_function( sink.i32_wrap_i64(); sink.i64_load(mem64(vtable_off as u64)); sink.i32_wrap_i64(); - sink.i64_load(mem64(offset2 as u64)); + emit_sized_int_load( + &mut sink, + offset2 as u64, + std::mem::size_of::(), + true, + ); } else { // assembler.py:1957-1969 gcremovetypeptr path. // MOV32 loc_tmp, mem(loc_object, 0) @@ -2974,7 +2963,7 @@ fn build_function( sink.i64_const((guard_gc_type_info.sizeof_ti + offset2) as i64); sink.i64_add(); sink.i32_wrap_i64(); - sink.i64_load(mem64(0)); + emit_sized_int_load(&mut sink, 0, std::mem::size_of::(), true); } // Stack: [..., loc_tmp (i64)] diff --git a/majit/majit-backend-wasm/src/lib.rs b/majit/majit-backend-wasm/src/lib.rs index ff3f706d32f..135343b5377 100644 --- a/majit/majit-backend-wasm/src/lib.rs +++ b/majit/majit-backend-wasm/src/lib.rs @@ -1818,7 +1818,7 @@ impl majit_backend::Backend for WasmBackend { && !failguard::meta_descr_is_exit_frame_with_exception(&descr.meta_descr) }) .map(|descr| descr.fail_index) - .unwrap_or(0); + .unwrap_or(failguard::WASM_CA_FINISH_FI_UNKNOWN); // For a pending self target this is the exact map already embedded in // the module's CA arm. Reuse it for the published metadata so the // loop and its self-callee have demonstrably identical geometry. A diff --git a/pyre/bench/synth/comprehension_param_range_call_flush.py b/pyre/bench/synth/comprehension_param_range_call_flush.py new file mode 100644 index 00000000000..b4e48d9c361 --- /dev/null +++ b/pyre/bench/synth/comprehension_param_range_call_flush.py @@ -0,0 +1,28 @@ +# A module-scope hot loop calling a helper that returns an inlined list +# comprehension over `range(n)` with `n` a parameter. Once the loop reaches the +# trace threshold it records the `len(f())` body; recording the CALL to +# `f` aborts and the walk forward-flushes the caller (module) frame at the CALL +# boundary so the interpreter re-runs the call from there. That flush rebuilds +# the caller's operand stack from the walk's live/shadow sources — but the +# CALL's `LOAD_CONST`'d argument has no concrete Ref shadow, so its slot +# resolves to NULL. The flush must decline (fall back to the legacy replay) +# rather than commit the NULL; committing it left the next call's argument slot +# unbound, so `f` raised `UnboundLocalError` on its parameter (`n`). +# +# The trigger is specific: the caller loop must be at MODULE scope (its CALL +# operands come from LOAD_NAME / LOAD_CONST, not LOAD_FAST), the inner +# `range(n)` must be large enough to compile + bridge the comprehension loop, +# and the outer loop must run enough to reach the trace threshold so the two +# transitions coincide. + + +def f(n): + return [i for i in range(n)] + + +t = 0 +k = 0 +while k < 220: + t += len(f(300)) + k += 1 +print(t) diff --git a/pyre/bench/synth/int_mul_ovf_bignum_promote.py b/pyre/bench/synth/int_mul_ovf_bignum_promote.py new file mode 100644 index 00000000000..75a09005b81 --- /dev/null +++ b/pyre/bench/synth/int_mul_ovf_bignum_promote.py @@ -0,0 +1,27 @@ +# pyre-check: max-pypy-ratio=30 + +# Overflow-crossing int multiply on a JIT-hot path. The inner loop is traced +# while `scale` is small (a*a stays in machine-int range, so the recorded +# GUARD_NO_OVERFLOW passes), then a large `scale` makes a*a overflow a 64-bit +# int and it must promote to a big int. A backend that drops the overflow check +# silently wraps the product instead of promoting, giving a wrong answer. +def hot(scale, n): + acc = 0 + i = 0 + while i < n: + a = scale + (i & 1) # loop-variant: cannot fold to a constant + acc = acc + a * a + i = i + 1 + return acc + + +def main(): + warm = 0 + for _ in range(120): + warm = warm + hot(3, 20000) # a in {3,4}; a*a tiny, never overflows + # Big scale: a ~ 5e9, a*a = 2.5e19 overflows int64 (and uint64) -> big int. + print(hot(5000000000, 20000)) + print(warm) + + +main() 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 f7252c1b681..ef8fece93a2 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs @@ -741,6 +741,39 @@ pub(crate) fn fbw_store_journal_commit() { FBW_FORITER_INFLIGHT.with(|c| c.borrow_mut().clear()); } +/// Record a bridge/retrace recording walk's range-iterator cursor before its +/// eager advance, so the abort path can restore it ([`FBW_BRIDGE_ITER_JOURNAL`]). +/// Called from the range FOR_ITER specialization ONLY while `is_bridge_trace`. +pub(crate) fn fbw_bridge_iter_journal_push( + iter: pyre_object::PyObjectRef, + pre_current: i64, + pre_remaining: i64, +) { + FBW_BRIDGE_ITER_JOURNAL.with(|j| j.borrow_mut().push((iter, pre_current, pre_remaining))); +} + +/// Non-commit epilogue for a bridge/retrace recording walk: restore each +/// range iterator to the cursor it held before the walk advanced it, in +/// reverse push order. The interpreter resume then re-consumes the item the +/// aborted recording had taken, so the iteration is executed exactly once. +pub(crate) fn fbw_bridge_iter_journal_rollback() { + FBW_BRIDGE_ITER_JOURNAL.with(|j| { + let mut entries = j.borrow_mut(); + while let Some((iter, pre_current, pre_remaining)) = entries.pop() { + unsafe { + pyre_object::functional::w_range_iter_set_cursor(iter, pre_current, pre_remaining); + } + } + }); +} + +/// Commit epilogue: a committed bridge recording keeps its advanced cursor +/// (the compiled bridge adopts it as the authoritative continuation), so drop +/// the undo log without restoring. +pub(crate) fn fbw_bridge_iter_journal_clear() { + FBW_BRIDGE_ITER_JOURNAL.with(|j| j.borrow_mut().clear()); +} + /// Record the in-flight FOR_ITER continuation (#57 Option C): the consumed /// item the `for_iter_next` residual produced and its FOR_ITER body coordinate. /// Called from the residual diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 6bf3796cb7b..1606138393f 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -4249,6 +4249,17 @@ thread_local! { static FBW_FORITER_INFLIGHT: std::cell::RefCell> = const { std::cell::RefCell::new(Vec::new()) }; + /// Undo log for a bridge/retrace recording walk's eager range-iterator + /// cursor advance. The main walk leaves the advance unjournaled and relies + /// on in-flight FOR_ITER forward-delivery to recover the consumed item on + /// abort; the bridge/retrace abort path has no such delivery, so a bridge + /// walk records `(iter, pre_current, pre_remaining)` here and restores the + /// cursor when it does NOT commit — leaving the recording side-effect + /// neutral so the interpreter resume re-consumes the item exactly once. + /// Only populated while `is_bridge_trace`; empty (no-op) on the main walk. + static FBW_BRIDGE_ITER_JOURNAL: std::cell::RefCell> = + const { std::cell::RefCell::new(Vec::new()) }; + static FBW_UNJOURNALED_VALUE_UNAVAILABLE: std::cell::Cell = const { std::cell::Cell::new(false) }; static FBW_UNJOURNALED_SYMBOLIC: std::cell::Cell = diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs index d6d9cae664d..051899dbc2d 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs @@ -1132,10 +1132,11 @@ pub(crate) fn collect_call_stack_overrides( } } // Use the virtualizable shadow only after the live register/vstack - // sources. A mid-opcode shadow stack slot can be NULL because it was not - // mirrored, while the corresponding live color still has the value. The - // remaining NULL is the CALL's real null-or-self sentinel and must be - // preserved as an explicit slot override. + // sources, and only for slots those sources left unresolved. A mid-opcode + // shadow stack slot can be NULL because it was not mirrored, while the + // corresponding live color still holds the value — so a NULL reaching this + // fallback is an UNRESOLVED slot, not a proven null (see the per-slot note + // below), and is dropped rather than committed. let base = ctx .trace_ctx .virtualizable_info() @@ -1147,7 +1148,20 @@ pub(crate) fn collect_call_stack_overrides( } if let Some((_opref, Value::Ref(value))) = ctx.trace_ctx.virtualizable_entry_at(base + slot) { - overrides.push((slot, value.as_usize() as pyre_object::PyObjectRef)); + // Only a positively-resolved (non-null) shadow value is a faithful + // stack slot. The live vstack/color sources above already emit + // every genuine null-or-self sentinel they can resolve + // (`concrete_ref_for_opref` yields an explicit null Ref for a + // PUSH_NULL box). A slot that reaches this shadow fallback with a + // NULL Ref is one the walk could not resolve — e.g. an + // unmaterialized `LOAD_CONST` operand whose concrete value was + // never mirrored — not a real null. Leaving it ABSENT makes the + // outer-call flush validation decline, so the legacy replay + // rebuilds the frame from its start state instead of resuming the + // interpreter over a NULL where a live object belongs. + if value.as_usize() != 0 { + overrides.push((slot, value.as_usize() as pyre_object::PyObjectRef)); + } } } overrides diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index f99de0056d1..3105d3152ef 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -5022,6 +5022,12 @@ pub(crate) fn try_walker_specialize_for_iter_next( ctx.trace_ctx .set_opref_concrete(current, Value::Int(concrete_current)); + if ctx.trace_ctx.is_bridge_trace { + // A bridge/retrace recording walk has no in-flight forward-delivery on + // abort, so journal the pre-advance cursor for restore if the walk does + // not commit (keeps the aborted recording side-effect neutral). + fbw_bridge_iter_journal_push(iter_obj, concrete_current, concrete_remaining); + } fbw_foriter_inflight_capture(concrete_item_ptr, body); // Range iteration stays at the C level, so the operand-stack mirror // remains valid and must receive the item produced by FOR_ITER. Its diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index 1db4c1372eb..7a7a4d9b2ef 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -2688,8 +2688,15 @@ fn run_perfn_walk( let journal = crate::jitcode_dispatch::fbw_store_journal_len(); if committed { crate::jitcode_dispatch::fbw_store_journal_commit(); + // A committed bridge recording keeps its advanced iterator cursor (the + // compiled bridge / adopted end state owns the iteration count). + crate::jitcode_dispatch::fbw_bridge_iter_journal_clear(); } else { crate::jitcode_dispatch::fbw_store_journal_rollback(); + // A bridge/retrace recording that does not commit restores the + // iterator cursor it eagerly advanced, so the interpreter resume + // re-consumes the in-flight item exactly once (no drop). + crate::jitcode_dispatch::fbw_bridge_iter_journal_rollback(); } if authoritative && std::env::var_os("PYRE_FBW_CENSUS").is_some() { let mut end = match &walk_result { diff --git a/pyre/pyre-wasm-runner/src/main.rs b/pyre/pyre-wasm-runner/src/main.rs index e9fcb14bbdd..8681219ade3 100644 --- a/pyre/pyre-wasm-runner/src/main.rs +++ b/pyre/pyre-wasm-runner/src/main.rs @@ -87,6 +87,11 @@ struct Host { /// executed this run. Per-store (not a global static): the increment sites /// already hold the `Caller`, and the runner is single-threaded. jit_compile_count: u64, + /// `PYRE_WASM_JIT_STATS` diagnostic: total wall time spent inside + /// `Module::new` (wasmtime Cranelift-compiling guest-emitted trace + /// modules), in nanoseconds. Isolates host-compile latency — the warmup + /// tax — from steady-state execution. + jit_compile_time_ns: u128, jit_execute_count: u64, /// Diagnostic: per-op residual-call host crossings (`env.jit_call` /// trampoline invocations). Compare against `jit_execute_count` to test @@ -102,6 +107,8 @@ struct Host { } fn main() { + let t0_main = std::time::Instant::now(); + let startup_trace = std::env::var_os("PYRE_WASM_STARTUP_TRACE").is_some(); let mut module_path: Option = None; let mut script: Option = None; let mut inspect = false; @@ -174,7 +181,15 @@ fn main() { .expect("spawn worker thread"); match worker.join() { - Ok(Ok(code)) => std::process::exit(code), + Ok(Ok(code)) => { + if startup_trace { + eprintln!( + "[startup] TOTAL(main-entry..worker-joined) {:.1}ms", + t0_main.elapsed().as_secs_f64() * 1e3 + ); + } + std::process::exit(code) + } Ok(Err(e)) => fatal(&e), Err(_) => fatal("worker thread panicked"), } @@ -224,9 +239,26 @@ fn run(module_path: &PathBuf, source: &str) -> Result { if guest_profile_out.is_some() { config.epoch_interruption(true); } + // Diagnostic: PYRE_WASM_STARTUP_TRACE=1 prints a fixed-startup breakdown + // (module load/deserialize, main-module instantiate, guest bootstrap+run) + // so the warmup tax can be attributed to host module-load vs guest + // interpreter bootstrap. + let startup_trace = std::env::var_os("PYRE_WASM_STARTUP_TRACE").is_some(); + let mut startup_mark = std::time::Instant::now(); + let mut startup_lap = |label: &str| { + if startup_trace { + eprintln!( + "[startup] {label} {:.1}ms", + startup_mark.elapsed().as_secs_f64() * 1e3 + ); + startup_mark = std::time::Instant::now(); + } + }; let engine = Engine::new(&config)?; + startup_lap("engine_new"); let module = load_main_module(&engine, module_path)?; + startup_lap("load_module"); let mut store = Store::new(&engine, Host::default()); if guest_profile_out.is_some() { @@ -263,6 +295,7 @@ fn run(module_path: &PathBuf, source: &str) -> Result { let instance = linker .instantiate(&mut store, &module) .context("instantiate main module")?; + startup_lap("instantiate"); let memory = instance .get_memory(&mut store, "memory") @@ -309,6 +342,7 @@ fn run(module_path: &PathBuf, source: &str) -> Result { // livelock): the JIT counters are populated at compile time, before the run // finishes, and the diag exports just read statics that survive a trap. let run_result = run_python.call(&mut store, (in_ptr, len)); + startup_lap("run_python(bootstrap+script)"); // After a fuel-exhaustion trap the store has no fuel, so the diagnostic // export calls below would themselves immediately trap and read as 0. // Refill so the readout reflects the real (compile-time) counter values. @@ -442,9 +476,10 @@ fn run(module_path: &PathBuf, source: &str) -> Result { .ok(); let host = store.data(); eprintln!( - "[jit-stats] compiles={} executes={} jit_calls={} linear_mem={} gc_oldgen={} gc_nursery={} \ + "[jit-stats] compiles={} compile_ms={:.1} executes={} jit_calls={} linear_mem={} gc_oldgen={} gc_nursery={} \ gc_minors={} gc_majors={} heap_live_bytes={} heap_live_count={}", host.jit_compile_count, + host.jit_compile_time_ns as f64 / 1.0e6, guest_jit_execute_count.unwrap_or(host.jit_execute_count), host.jit_call_count, lin_mem, @@ -515,6 +550,18 @@ fn run(module_path: &PathBuf, source: &str) -> Result { use std::io::Write; std::io::stdout().write_all(&out)?; std::io::stdout().flush()?; + startup_lap("dealloc+stdout"); + // Exit without running the wasmtime `Store`/`Module`/`Engine` destructors. + // Dropping them frees ~40MB of mapped code + linear memory that the OS + // reclaims on process exit anyway; on a short run that teardown is ~0.2s — + // the dominant fixed startup tax, larger than the module load and far + // larger than trace compilation. stdout is already flushed above and the + // diagnostic stats print to (unbuffered) stderr, so nothing is lost. + // `PYRE_WASM_FULL_TEARDOWN=1` restores the drops for leak diagnostics. + if std::env::var_os("PYRE_WASM_FULL_TEARDOWN").is_none() { + std::io::stderr().flush().ok(); + std::process::exit(0); + } Ok(0) } @@ -790,7 +837,10 @@ fn jit_compile(caller: &mut Caller<'_, Host>, bytes_ptr: u32, bytes_len: u32) -> Err(pe) => eprintln!("[jit_compile_wasm] wat print failed: {pe}"), } } - let module = match Module::new(&engine, &bytes) { + let compile_start = std::time::Instant::now(); + let module_result = Module::new(&engine, &bytes); + caller.data_mut().jit_compile_time_ns += compile_start.elapsed().as_nanos(); + let module = match module_result { Ok(m) => m, Err(e) => { if std::env::var_os("PYRE_WASM_DUMP_BAD_TRACE").is_some() {