diff --git a/majit/majit-backend-dynasm/src/x86/assembler.rs b/majit/majit-backend-dynasm/src/x86/assembler.rs index 5894f75eadd..f3ad067ed94 100644 --- a/majit/majit-backend-dynasm/src/x86/assembler.rs +++ b/majit/majit-backend-dynasm/src/x86/assembler.rs @@ -1470,7 +1470,18 @@ impl<'a> Assembler386<'a> { } } Loc::Immed(i) => { - let v = i.value as i32; + // regloc.py:456-464 — an immediate that does not fit in 32 + // bits cannot use the imm32 form (the encoder would truncate + // it and the CPU sign-extend the low half, e.g. an + // 0xFFFF_FFFF_FFFF mask becoming an all-ones no-op); + // materialize it into the scratch register and retry as the + // reg-reg form. + let Ok(v) = i32::try_from(i.value) else { + let scratch = crate::regloc::X86_64_SCRATCH_REG; + dynasm!(self.mc ; .arch x64 ; mov Rq(scratch.value), QWORD i.value); + self.emit_binop_reg_loc(opcode, dst_reg, &Loc::Reg(scratch)); + return; + }; match opcode { OpCode::IntAdd | OpCode::IntAddOvf | OpCode::NurseryPtrIncrement => { dynasm!(self.mc ; .arch x64 ; add Rq(dst_reg), v); @@ -1487,15 +1498,12 @@ impl<'a> Assembler386<'a> { OpCode::IntXor => { dynasm!(self.mc ; .arch x64 ; xor Rq(dst_reg), v); } - OpCode::IntMul | OpCode::IntMulOvf if i32::try_from(i.value).is_ok() => { + OpCode::IntMul | OpCode::IntMulOvf => { // imul r64, r64, imm32 (sign-extended) — one instruction // instead of materializing the constant into a scratch reg. dynasm!(self.mc ; .arch x64 ; imul Rq(dst_reg), Rq(dst_reg), v); } - _ => { - let scratch = crate::regloc::X86_64_SCRATCH_REG.value; - dynasm!(self.mc ; .arch x64 ; mov Rq(scratch), QWORD i.value ; imul Rq(dst_reg), Rq(scratch)); - } + _ => {} } } _ => {} @@ -2989,15 +2997,22 @@ impl<'a> Assembler386<'a> { match (a0, src) { (Loc::Reg(a), Loc::Reg(s)) => dynasm!(self.mc ; .arch x64 ; lea Rq(dst.value), [Rq(a.value) + Rq(s.value)]), - (Loc::Reg(a), Loc::Immed(i)) => { - let v = i.value as i32; - dynasm!(self.mc ; .arch x64 - ; lea Rq(dst.value), [Rq(a.value) + v]) - } - (Loc::Immed(i), Loc::Reg(s)) => { - let v = i.value as i32; - dynasm!(self.mc ; .arch x64 - ; lea Rq(dst.value), [Rq(s.value) + v]) + (Loc::Reg(a), Loc::Immed(i)) | (Loc::Immed(i), Loc::Reg(a)) => { + // The `_consider_lea` route guarantees a fitting + // disp32, but the `consider_binop_symm` fallback + // reaches this arm with an arbitrary 64-bit + // constant (regloc.py:456-464); materialize a wide + // one into the scratch register and use the + // base+index form. + if let Ok(v) = i32::try_from(i.value) { + dynasm!(self.mc ; .arch x64 + ; lea Rq(dst.value), [Rq(a.value) + v]) + } else { + let scratch = crate::regloc::X86_64_SCRATCH_REG.value; + dynasm!(self.mc ; .arch x64 + ; mov Rq(scratch), QWORD i.value + ; lea Rq(dst.value), [Rq(a.value) + Rq(scratch)]) + } } (Loc::Immed(i0), Loc::Immed(i1)) => { let sum = i0.value.wrapping_add(i1.value); diff --git a/majit/majit-backend-dynasm/tests/basic_loop.rs b/majit/majit-backend-dynasm/tests/basic_loop.rs index 7263d6deea1..3f653bfd1d4 100644 --- a/majit/majit-backend-dynasm/tests/basic_loop.rs +++ b/majit/majit-backend-dynasm/tests/basic_loop.rs @@ -672,3 +672,71 @@ fn test_guard_no_exception_and_always_fails_emit_side_exits() { "GUARD_ALWAYS_FAILS should side-exit unconditionally" ); } + +#[test] +fn test_int_binop_wide_immediate_is_not_truncated() { + // An immediate that does not fit in 32 bits must not use the imm32 + // form: the encoder would truncate it and the CPU sign-extend the low + // half, so `x & 0xFFFF_FFFF_FFFF` degenerated to `x & -1` (a no-op). + // regloc.py:456-464 routes such immediates through the scratch register. + let mask = 0xFFFF_FFFF_FFFFi64; + let mut backend = DynasmBackend::new(); + backend.attach_default_test_descrs(); + let mut token = JitCellToken::new(45); + + let inputargs = vec![InputArg::from_type(Type::Int, 0)]; + let i0 = inputargs[0].opref(); + + let and_op = Op::new(OpCode::IntAnd, &[rb(i0), rb(OpRef::const_int(mask))]); + and_op.pos.set(OpRef::int_op(1)); + + let finish_op = Op::new(OpCode::Finish, &[rb(OpRef::int_op(1))]); + finish_op.pos.set(OpRef::void_op(2)); + finish_op.set_fail_arg_types(vec![Type::Int]); + finish_op.setfailargs(vec![rb(OpRef::int_op(1))].into()); + + let ops_rc: Vec> = vec![Rc::new(and_op), Rc::new(finish_op)]; + let result = backend.compile_loop(&inputargs, &ops_rc, &mut token); + assert!(result.is_ok(), "compile_loop failed: {:?}", result.err()); + + let frame = backend.execute_token(&token, &[Value::Int(-1)]); + assert!(backend.get_latest_descr(&frame).is_finish()); + assert_eq!( + backend.get_int_value(&frame, 0), + mask, + "-1 & 0xFFFF_FFFF_FFFF must keep only the low 48 bits" + ); +} + +#[test] +fn test_int_add_wide_immediate_is_not_truncated() { + // Twin of the AND test for the LEA-form `int_add` emitter: a 2^32 + // addend truncated to imm32 would add 0. + let addend = 1i64 << 32; + let mut backend = DynasmBackend::new(); + backend.attach_default_test_descrs(); + let mut token = JitCellToken::new(46); + + let inputargs = vec![InputArg::from_type(Type::Int, 0)]; + let i0 = inputargs[0].opref(); + + let add_op = Op::new(OpCode::IntAdd, &[rb(i0), rb(OpRef::const_int(addend))]); + add_op.pos.set(OpRef::int_op(1)); + + let finish_op = Op::new(OpCode::Finish, &[rb(OpRef::int_op(1))]); + finish_op.pos.set(OpRef::void_op(2)); + finish_op.set_fail_arg_types(vec![Type::Int]); + finish_op.setfailargs(vec![rb(OpRef::int_op(1))].into()); + + let ops_rc: Vec> = vec![Rc::new(add_op), Rc::new(finish_op)]; + let result = backend.compile_loop(&inputargs, &ops_rc, &mut token); + assert!(result.is_ok(), "compile_loop failed: {:?}", result.err()); + + let frame = backend.execute_token(&token, &[Value::Int(7)]); + assert!(backend.get_latest_descr(&frame).is_finish()); + assert_eq!( + backend.get_int_value(&frame, 0), + addend + 7, + "7 + 2^32 must not truncate the immediate" + ); +} diff --git a/majit/majit-backend-wasm/src/codegen.rs b/majit/majit-backend-wasm/src/codegen.rs index 70037a94d9f..4248ef4b456 100644 --- a/majit/majit-backend-wasm/src/codegen.rs +++ b/majit/majit-backend-wasm/src/codegen.rs @@ -2359,20 +2359,20 @@ fn build_function( // int_signext(val, num_bytes): sign-extend from num_bytes width let vi = op.pos.get().raw(); if !OpRef::raw_is_constant(vi) { - emit_resolve(&mut sink, constants, value_types, op.arg(0).to_opref()); - // num_bytes (arg(1)) is always a compile-time constant; - // resolve it like every other emit-time const so a genuine - // pool miss panics via missing_emit_const instead of silently - // defaulting to 8 (which zeroes the shift and skips the - // narrowing, passing an un-truncated integer through). + // The static shift below needs num_bytes (arg(1)) as an + // emit-time constant. A non-constant width is still a valid + // IR shape — int_signext/ii>i is a two-operand blackhole op + // and the cranelift backend resolves arg(1) as a runtime + // operand — just one this backend does not lower, so decline + // for interpreter fallback rather than aborting the compile. let arg1 = op.arg(1).to_opref(); - let num_bytes = const_operand_value(constants, arg1).unwrap_or_else(|| { - panic!( - "wasm int_signext: num_bytes operand (raw={}) is not a \ - resolvable compile-time constant", + let Some(num_bytes) = const_operand_value(constants, arg1) else { + return Err(BackendError::Unsupported(format!( + "wasm int_signext: non-constant num_bytes operand (raw={})", arg1.raw() - ) - }); + ))); + }; + emit_resolve(&mut sink, constants, value_types, op.arg(0).to_opref()); let shift = 64 - num_bytes * 8; if shift > 0 && shift < 64 { sink.i64_const(shift); @@ -2679,8 +2679,11 @@ fn build_function( // 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. + // GETARRAYITEM/ARRAYLEN paths, so no producer emits these ops (verified + // with PYRE_DUMP_PERFN_JITCODE: a str-subscript / len / compare / find + // hot loop traces to GETARRAYITEM, never STRGETITEM). Decline them + // (interpreter fallback) rather than ship a descr-driven lowering that + // no trace exercises — a valid but untestable path here. OpCode::Strlen | OpCode::Unicodelen | OpCode::Strgetitem | OpCode::Unicodegetitem => { return Err(BackendError::Unsupported(format!( "wasm codegen: string/unicode direct-memory op {:?} (no descr-driven layout)", @@ -2689,22 +2692,29 @@ fn build_function( } // ── GC memory ops ── - // GC_LOAD/GC_STORE and their indexed forms are produced only by the - // GC rewrite (majit-gc/src/rewrite.rs): the true semantics are - // offset=arg1, size=arg2 (load) / value=arg2, size=arg3 (store), with - // no FieldDescr attached. The wasm backend does not run the GC rewrite, - // so these never reach here. The prior lowering read a nonexistent - // field_offset_from_descr (→ 0) and, for GcStore, stored arg(1) (the - // offset operand) as the value — a silent miscompile. Panic loudly like - // LoadFromGcTable rather than emit a wrong memory access. - OpCode::GcLoadI - | OpCode::GcLoadR - | OpCode::GcLoadF - | OpCode::GcLoadIndexedI + // The indexed forms are also wired as real frontend blackhole ops + // (blackhole.rs `gc_load_indexed_{i,f}` / `gc_store_indexed_{i,f}`), + // so an llop/buffer trace can carry them into the backend. This + // backend has no descr-driven lowering for them, so decline + // (interpreter fallback) rather than aborting the whole compile. + OpCode::GcLoadIndexedI | OpCode::GcLoadIndexedR | OpCode::GcLoadIndexedF - | OpCode::GcStore | OpCode::GcStoreIndexed => { + return Err(BackendError::Unsupported(format!( + "wasm codegen: indexed GC op {:?} (no descr-driven layout)", + op.opcode + ))); + } + // The bare GC_LOAD/GC_STORE forms are produced only by the GC rewrite + // (majit-gc/src/rewrite.rs): the true semantics are offset=arg1, + // size=arg2 (load) / value=arg2, size=arg3 (store), with no FieldDescr + // attached. The wasm backend does not run the GC rewrite, so these + // never reach here. The prior lowering read a nonexistent + // field_offset_from_descr (→ 0) and, for GcStore, stored arg(1) (the + // offset operand) as the value — a silent miscompile. Panic loudly like + // LoadFromGcTable rather than emit a wrong memory access. + OpCode::GcLoadI | OpCode::GcLoadR | OpCode::GcLoadF | OpCode::GcStore => { panic!( "wasm backend: {:?} is unsupported (GC_LOAD/GC_STORE); \ the GC rewrite must not run for wasm", @@ -2934,10 +2944,14 @@ fn build_function( sink.i32_wrap_i64(); sink.i64_load(mem64(vtable_off as u64)); sink.i32_wrap_i64(); + // subclassrange_min is an 8-byte i64 on every target + // (pyobject.rs `PyType::subclassrange_min: AtomicI64`); read + // the full field width, not the wasm32 4-byte `usize`, or the + // guard truncates/sign-extends the object's min. emit_sized_int_load( &mut sink, offset2 as u64, - std::mem::size_of::(), + std::mem::size_of::(), true, ); } else { @@ -2963,7 +2977,8 @@ fn build_function( sink.i64_const((guard_gc_type_info.sizeof_ti + offset2) as i64); sink.i64_add(); sink.i32_wrap_i64(); - emit_sized_int_load(&mut sink, 0, std::mem::size_of::(), true); + // 8-byte i64 subclassrange_min (see the vtable path above). + emit_sized_int_load(&mut sink, 0, std::mem::size_of::(), true); } // Stack: [..., loc_tmp (i64)] diff --git a/pyre/bench/synth/exception_metadata_jitstress.py b/pyre/bench/synth/exception_metadata_jitstress.py new file mode 100644 index 00000000000..074add9a63f --- /dev/null +++ b/pyre/bench/synth/exception_metadata_jitstress.py @@ -0,0 +1,159 @@ +# JIT-stress twin of exception_metadata_hot: `pypyjit.set_param` lowers the +# trace/function thresholds to 1 so recording fires on the earliest iterations +# of every section rather than only after the ~1600-iteration warmup. That +# forces the recording pass onto each traceback/context/exc_info shape on every +# run and every backend — the param hook reaches the wasm guest, which sees no +# environment — turning the historically platform-dependent recording-path +# coverage of these shapes into a deterministic check. The output is identical +# to exception_metadata_hot; only the thresholds and iteration count differ. +# +# CPython (the oracle) has no `pypyjit`; PyPy and pyre do. Guarding the import +# keeps the output identical across all three while the thresholds only bind +# where a JIT exists. +import sys + +try: + import pypyjit + + pypyjit.set_param("threshold=1,function_threshold=1") +except ImportError: + pass + +N = 400 + + +def out(key, value): + print(f"{key} = {value}") + + +def thrower(i): + raise KeyError(i) + + +def returns_through_finally(i): + try: + if i % 3 == 0: + raise ValueError(i) + return "ok" + except ValueError: + return "caught" + finally: + pass + + +# Same-frame raise/except: the handler sees a traceback, anchored at this +# frame and at one line. +same_frame_tb = 0 +tb_linenos = set() +tb_frame_is_self = 0 +for i in range(N): + try: + raise ValueError(i) + except ValueError as e: + traceback = e.__traceback__ + same_frame_tb += traceback is not None + tb_linenos.add(traceback.tb_lineno) + tb_frame_is_self += traceback.tb_frame is sys._getframe() +out("same_frame_tb", same_frame_tb) +out("same_frame_tb_lineno_count", len(tb_linenos)) +out("same_frame_tb_frame_is_self", tb_frame_is_self) + +# Cross-frame raise: the traceback spans the helper frame and this one. +cross_frame_depths = set() +for i in range(N): + try: + thrower(i) + except KeyError as e: + depth = 0 + traceback = e.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + cross_frame_depths.add(depth) +out("cross_frame_tb_depths", sorted(cross_frame_depths)) + +# Raising inside a handler chains exactly one __context__ link — an implicit +# chain that keeps growing would show up here as a larger set. +context_lengths = set() +for i in range(N): + try: + try: + raise ValueError(i) + except ValueError: + raise TypeError(i) + except TypeError as e: + length = 0 + context = e.__context__ + seen = set() + while context is not None and id(context) not in seen: + seen.add(id(context)) + length += 1 + context = context.__context__ + context_lengths.add(length) +out("context_chain_lengths", sorted(context_lengths)) + +# `raise ... from ...` sets __cause__ and suppresses the context. +cause_ok = 0 +for i in range(N): + try: + try: + raise ValueError(i) + except ValueError as cause: + raise TypeError(i) from cause + except TypeError as e: + cause_ok += isinstance(e.__cause__, ValueError) and e.__suppress_context__ +out("cause_ok", cause_ok) + +# sys.exc_info reports the handled exception inside the handler and nothing +# once it has been left. +exc_info_inside = 0 +exc_info_after_none = 0 +for i in range(N): + try: + raise ValueError(i) + except ValueError: + info = sys.exc_info() + exc_info_inside += info[0] is ValueError and info[1].args == (i,) and info[2] is not None + exc_info_after_none += sys.exc_info() == (None, None, None) +out("exc_info_inside", exc_info_inside) +out("exc_info_after_none", exc_info_after_none) + +# The handler binds the raised object itself and unbinds the name on exit. +raised_identity = 0 +name_cleared = 0 +for i in range(N): + raised = ValueError(i) + try: + raise raised + except ValueError as e: + raised_identity += e is raised + try: + e + except NameError: + name_cleared += 1 +out("raised_identity", raised_identity) +out("name_cleared", name_cleared) + +# A bare re-raise keeps the original traceback rather than restarting it. +reraise_depths = set() +for i in range(N): + try: + try: + thrower(i) + except KeyError: + raise + except KeyError as e: + depth = 0 + traceback = e.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + reraise_depths.add(depth) +out("reraise_tb_depths", sorted(reraise_depths)) + +# return-from-except under a finally. +finally_counts = {} +for i in range(N): + result = returns_through_finally(i) + finally_counts[result] = finally_counts.get(result, 0) + 1 +out("finally_counts", sorted(finally_counts.items())) diff --git a/pyre/bench/synth/exception_reraise_tb_depth_hot.py b/pyre/bench/synth/exception_reraise_tb_depth_hot.py new file mode 100644 index 00000000000..6cb514dcafe --- /dev/null +++ b/pyre/bench/synth/exception_reraise_tb_depth_hot.py @@ -0,0 +1,64 @@ +# A bare re-raise caught in the same frame keeps the original traceback: no +# node is attached at a re-raise coordinate (RaiseWithExplicitTraceback, +# attach_tb=False). The module-level loop makes the recording iteration itself +# execute the re-raise chain, which historically prepended spurious nodes for +# the bare-raise and handler-cleanup coordinates on exactly that iteration +# (depth 4 instead of 2). Named re-raise (`raise e`) must still attach its +# node (depth 3), and a `finally` passthrough attaches nothing (depth 2). +N = 4000 + + +def thrower(i): + raise KeyError(i) + + +bare_depths = set() +bare_bad = 0 +for i in range(N): + try: + try: + thrower(i) + except KeyError: + raise + except KeyError as e: + depth = 0 + traceback = e.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + bare_depths.add(depth) + bare_bad += depth != 2 +print("bare_depths =", sorted(bare_depths)) +print("bare_bad =", bare_bad) + +named_depths = set() +for i in range(N): + try: + try: + thrower(i) + except KeyError as e: + raise e + except KeyError as e2: + depth = 0 + traceback = e2.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + named_depths.add(depth) +print("named_depths =", sorted(named_depths)) + +finally_depths = set() +for i in range(N): + try: + try: + thrower(i) + finally: + pass + except KeyError as e3: + depth = 0 + traceback = e3.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + finally_depths.add(depth) +print("finally_depths =", sorted(finally_depths)) diff --git a/pyre/bench/synth/exception_reraise_tb_depth_jitstress.py b/pyre/bench/synth/exception_reraise_tb_depth_jitstress.py new file mode 100644 index 00000000000..5e1e492c2d8 --- /dev/null +++ b/pyre/bench/synth/exception_reraise_tb_depth_jitstress.py @@ -0,0 +1,79 @@ +# JIT-stress twin of exception_reraise_tb_depth_hot: `pypyjit.set_param` +# lowers the trace/function thresholds to 1 so trace recording fires on the +# earliest iterations rather than only after the ~1600-iteration warmup. That +# makes the recording pass land on the re-raise sections on every run and every +# backend (the param hook reaches the wasm guest, which sees no environment), +# turning the historically platform-dependent recording-path coverage into a +# deterministic check. +# +# Traceback shape invariants (identical to the non-stress twin): a same-frame +# bare re-raise keeps the original traceback (depth 2, no node at the re-raise +# coordinate), a named re-raise `raise e` attaches its node (depth 3), and a +# `finally` passthrough attaches nothing (depth 2). +# CPython (the oracle) has no `pypyjit`; PyPy and pyre do. Guarding the import +# keeps the output identical across all three while the thresholds only bind +# where a JIT exists. +try: + import pypyjit + + pypyjit.set_param("threshold=1,function_threshold=1") +except ImportError: + pass + +N = 600 + + +def thrower(i): + raise KeyError(i) + + +bare_depths = set() +bare_bad = 0 +for i in range(N): + try: + try: + thrower(i) + except KeyError: + raise + except KeyError as e: + depth = 0 + traceback = e.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + bare_depths.add(depth) + bare_bad += depth != 2 +print("bare_depths =", sorted(bare_depths)) +print("bare_bad =", bare_bad) + +named_depths = set() +for i in range(N): + try: + try: + thrower(i) + except KeyError as e: + raise e + except KeyError as e2: + depth = 0 + traceback = e2.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + named_depths.add(depth) +print("named_depths =", sorted(named_depths)) + +finally_depths = set() +for i in range(N): + try: + try: + thrower(i) + finally: + pass + except KeyError as e3: + depth = 0 + traceback = e3.__traceback__ + while traceback is not None: + depth += 1 + traceback = traceback.tb_next + finally_depths.add(depth) +print("finally_depths =", sorted(finally_depths)) diff --git a/pyre/bench/synth/str_getitem_len_hot.py b/pyre/bench/synth/str_getitem_len_hot.py new file mode 100644 index 00000000000..d0d7ee0416c --- /dev/null +++ b/pyre/bench/synth/str_getitem_len_hot.py @@ -0,0 +1,39 @@ +# Hot-loop str/unicode subscript and length over every string kind: ASCII / +# latin1 (1-byte code units), BMP (2-byte), and non-BMP astral (4-byte). The +# subscripts emit residual STRGETITEM/UNICODEGETITEM and the lengths STRLEN/ +# UNICODELEN, exercising the backend's descr-driven item/length reads at each +# item size (item_size 1/2/4, with the STR null-terminator base adjustment). +# A rolling ordinal checksum makes a wrong offset or width a visibly wrong +# number rather than a silent pass. Deterministic; output asserted cpython==pypy. + + +def hot_checksum(n, s): + length = len(s) + acc = 0 + for i in range(n): + acc = (acc * 131 + ord(s[i % length]) + length) & 0xFFFFFFFFFFFF + return acc + + +def hot_len(n, s): + acc = 0 + for i in range(n): + acc += len(s) + return acc + + +def main(): + ascii_s = "The quick brown fox jumps over the lazy dog 0123456789!?" + latin1_s = "café déjà vu naïve résumé — ¡Hola! ½¾ ©®µ" + bmp_s = "αβγδεζ ελληνικά Ω ДЖЕМ кириллица 日本語 テスト" + astral_s = "𝕒𝕓𝕔𝕕 𝟙𝟚𝟛𝟜 😀🎉🚀 𐀀𐀁 mixed astral" + + n = 60000 + print("ascii", hot_checksum(n, ascii_s)) + print("latin1", hot_checksum(n, latin1_s)) + print("bmp", hot_checksum(n, bmp_s)) + print("astral", hot_checksum(n, astral_s)) + print("len", hot_len(n, ascii_s), hot_len(n, bmp_s), hot_len(n, astral_s)) + + +main() diff --git a/pyre/check.py b/pyre/check.py index dc6480a8306..6ed80177bdf 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -1134,6 +1134,28 @@ def _performance_gate_passed( return False, elapsed, baseline_time, "" + def _gate_fail_detail(self, backend, baseline, measured, baseline_time, limit): + """One-line FAIL detail using the exact numbers the gate compared. + + The gate decides on startup-subtracted exec times + (``_exec_time(backend, measured) <= _exec_time(baseline, baseline_time) + * limit``), so those exec times — not the raw run times — are printed, + alongside their true measured ratio and the gate threshold. On a FAIL + the ratio necessarily exceeds the threshold, so every number on the + line is arithmetically self-consistent: exec_measured / exec_baseline + equals the shown ratio, which is above the shown gate. + """ + exec_m = self._exec_time(backend, measured) + exec_b = self._exec_time(baseline, baseline_time) + if exec_b in (None, "-") or float(exec_b) <= 0: + ratio = "-" + else: + ratio = f"{float(exec_m) / float(exec_b):.1f}x" + return ( + f"exec {exec_m:.2f}s > {baseline} {exec_b:.2f}s " + f"ratio {ratio} > gate {float(limit):g}x" + ) + def _run_backend_bench( self, backend, name, script, timeout, vs_cpython, vs_pypy, t_cpython, t_pypy, pypy_output, @@ -1199,12 +1221,12 @@ def _ratio(elapsed_val, pypy_val): [PYTHON3, script], pypy_output, "cpython", ) if not passed: - self._record( - backend, False, name, - f"{checked_elapsed:.2f}s > cpython {checked_baseline:.2f}s x{vs_cpython}", + detail = self._gate_fail_detail( + backend, "cpython", checked_elapsed, checked_baseline, vs_cpython, ) + self._record(backend, False, name, detail) suffix = f" ({retry_note})" if retry_note else "" - print(f"{red('SLOWER')} pyre {checked_elapsed:.2f}s > cpython {checked_baseline:.2f}s x{vs_cpython}{suffix}") + print(f"{red('SLOWER')} pyre {detail}{suffix}") self._append_comparison( backend, name, t_cpython, t_pypy, fmt_time(f"{elapsed:.2f}"), f"({ratio} vs pypy)", @@ -1220,12 +1242,12 @@ def _ratio(elapsed_val, pypy_val): [PYPY3, script], pypy_output, "pypy", ) if not passed: - self._record( - backend, False, name, - f"{checked_elapsed:.2f}s > pypy {checked_baseline:.2f}s x{vs_pypy}", + detail = self._gate_fail_detail( + backend, "pypy", checked_elapsed, checked_baseline, vs_pypy, ) + self._record(backend, False, name, detail) suffix = f" ({retry_note})" if retry_note else "" - print(f"{red('SLOWER')} pyre {checked_elapsed:.2f}s > pypy {checked_baseline:.2f}s x{vs_pypy}{suffix}") + print(f"{red('SLOWER')} pyre {detail}{suffix}") self._append_comparison( backend, name, t_cpython, t_pypy, fmt_time(f"{elapsed:.2f}"), f"({ratio} vs pypy)", diff --git a/pyre/pyre-interpreter/src/call.rs b/pyre/pyre-interpreter/src/call.rs index 296b05c665b..230084344ba 100644 --- a/pyre/pyre-interpreter/src/call.rs +++ b/pyre/pyre-interpreter/src/call.rs @@ -255,6 +255,29 @@ pub fn set_jit_param(name: &str, value: i64) { } } +// `rlib/jit.py:842 set_user_param` — the positional-string form +// (`"name=value,…"`, `"off"`, `"default"`) that `pypyjit.set_param(str)` +// routes through. The JIT owns the authoritative parser, so this forwards the +// whole string and returns `Err(())` on a malformed string (rlib/jit.py:853). +type SetJitParamStringFn = fn(text: &str) -> Result<(), ()>; +static SET_JIT_PARAM_STRING_HOOK: OnceLock = OnceLock::new(); + +/// Register the hook that applies a JIT-parameter string via the JIT +/// runtime's `set_user_param`. Called by pyre-jit at startup. +pub fn register_set_jit_param_string_hook(f: SetJitParamStringFn) { + let _ = SET_JIT_PARAM_STRING_HOOK.set(f); +} + +/// Apply a JIT-parameter string. `Ok(())` when the hook is absent (JIT-disabled +/// build) so a `pypyjit.set_param("…")` call is inert rather than an error +/// there; `Err(())` only on a malformed string once the JIT is present. +pub fn set_jit_param_string(text: &str) -> Result<(), ()> { + match SET_JIT_PARAM_STRING_HOOK.get() { + Some(hook) => hook(text), + None => Ok(()), + } +} + /// jd1 (`unpackiterable_driver`) merge-point hook. pyre-interpreter cannot /// import pyre-jit (its upper crate), so the JIT registers this at boot and the /// `unpackiterable_driver.jit_merge_point` marker calls through it. Mirrors the diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 77780a8fa31..68d1a8c61cd 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -541,6 +541,9 @@ pub fn install_builtin_modules() { pyre_install_module!("__pypy__" => crate::module::__pypy__::init); pyre_install_module!("__pypy__.builders" => crate::module::__pypy__::builders::init); + // pypyjit — runtime JIT-parameter control (`set_param`). + pyre_install_module!("pypyjit" => crate::module::pypyjit::init); + pyre_install_module!(atexit); // faulthandler installs host signal handlers and writes tracebacks to a raw // fd, neither of which is mediated; like the other host-access modules below diff --git a/pyre/pyre-interpreter/src/module/mod.rs b/pyre/pyre-interpreter/src/module/mod.rs index 1abf3b9f4a6..c655696da5a 100644 --- a/pyre/pyre-interpreter/src/module/mod.rs +++ b/pyre/pyre-interpreter/src/module/mod.rs @@ -85,6 +85,7 @@ pub mod posix; #[cfg(all(unix, not(feature = "sandbox")))] pub mod pwd; pub mod pyexpat; +pub mod pypyjit; #[cfg(not(feature = "sandbox"))] pub mod resource; #[cfg(not(feature = "sandbox"))] diff --git a/pyre/pyre-interpreter/src/module/pypyjit/mod.rs b/pyre/pyre-interpreter/src/module/pypyjit/mod.rs new file mode 100644 index 00000000000..ac3591ee3d7 --- /dev/null +++ b/pyre/pyre-interpreter/src/module/pypyjit/mod.rs @@ -0,0 +1,82 @@ +//! `pypyjit` module — PyPy: pypy/module/pypyjit/ +//! +//! Exposes `set_param`, the runtime JIT-parameter control from +//! `interp_jit.py:138-167`. The JIT itself lives in the higher `pyre-jit` +//! crate, so this routes through the `SET_JIT_PARAM_STRING_HOOK` / +//! `SET_JIT_PARAM_HOOK` that pyre-jit registers at boot (`call.rs`). Because +//! the hooks are in-process function pointers rather than an env lever, a +//! `pypyjit.set_param(...)` call configures the warmstate on every backend +//! including the wasm guest (which sees no environment). + +/// interp_jit.py:138-167 — `set_param(space, __args__)`. +/// +/// Accepts the PyPy calling conventions: +/// * `set_param("name=value,name=value")` / `set_param("off")` / +/// `set_param("default")` — the positional string form. +/// * `set_param(name=value, ...)` — keyword arguments. +/// +/// Both forms funnel through the JIT's authoritative `set_user_param` parser +/// (`call::set_jit_param_string`) so no parameter table is duplicated here. +fn set_param( + args: &[pyre_object::PyObjectRef], +) -> Result { + let (pos, kwds) = crate::builtins::split_builtin_kwargs(args); + + // interp_jit.py:147-148 — at most one non-keyword argument. + if pos.len() > 1 { + return Err(crate::PyError::type_error(format!( + "set_param() takes at most 1 non-keyword argument, {} given", + pos.len() + ))); + } + + // interp_jit.py:151-156 — positional string → set_user_param(None, text). + if let Some(&text_obj) = pos.first() { + let text = crate::baseobjspace::text_w(text_obj)?; + if crate::call::set_jit_param_string(&text).is_err() { + return Err(crate::PyError::new( + crate::PyErrorKind::ValueError, + "error in JIT parameters string".to_string(), + )); + } + } + + // interp_jit.py:157-167 — keyword arguments. Re-serialize each `name=value` + // pair into one parameter string so the JIT-side parser stays the single + // source of truth. `enable_opts` carries a string value; every other + // parameter is an integer (`space.int_w` rejects a non-int value here). + if let Some(kw_dict) = kwds { + let mut parts: Vec = Vec::new(); + for (k, v) in unsafe { pyre_object::dictmultiobject::w_dict_items(kw_dict) } { + if !unsafe { pyre_object::is_str(k) } { + continue; + } + let key = unsafe { pyre_object::w_str_get_value(k) }; + if key == "__pyre_kw__" { + continue; + } + if key == "enable_opts" { + let value = crate::baseobjspace::text_w(v)?; + parts.push(format!("{key}={value}")); + } else { + let value = crate::baseobjspace::int_w(v)?; + parts.push(format!("{key}={value}")); + } + } + if !parts.is_empty() && crate::call::set_jit_param_string(&parts.join(",")).is_err() { + return Err(crate::PyError::new( + crate::PyErrorKind::ValueError, + "error in JIT parameters string".to_string(), + )); + } + } + + Ok(pyre_object::w_none()) +} + +crate::py_module! { + "pypyjit", + functions: { + "set_param" / * = |args| set_param(args), + } +} diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 3fd76caf8f3..70878b7659d 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -4739,6 +4739,7 @@ struct FbwStoreJournalRootArea { cell_stores: *const std::cell::RefCell>, sys_exc: *const std::cell::RefCell>, foriter: *const std::cell::RefCell>, + bridge_iter: *const std::cell::RefCell>, abort_resume: *const std::cell::RefCell>, active_session: *const std::cell::Cell<*const std::cell::RefCell>, escape_flush_undo: *const std::cell::RefCell>, @@ -4755,6 +4756,7 @@ thread_local! { cell_stores: FBW_CELL_STORE_JOURNAL.with(|value| value as *const _), sys_exc: FBW_SYS_EXC_JOURNAL.with(|value| value as *const _), foriter: FBW_FORITER_INFLIGHT.with(|value| value as *const _), + bridge_iter: FBW_BRIDGE_ITER_JOURNAL.with(|value| value as *const _), abort_resume: FBW_ABORT_CALL_RESUME.with(|value| value as *const _), active_session: ACTIVE_WALK_SESSION.with(|value| value as *const _), escape_flush_undo: escape_flush_undo_cell_ptr(), @@ -5039,6 +5041,17 @@ pub unsafe fn fbw_store_journal_root_walker_area( visitor(unsafe { &mut *(&mut latched.last_exc_value as *mut i64).cast() }); } } + // The bridge/retrace iterator cursor journal holds a range iterator across + // the rest of an authoritative bridge walk. When the parent compiled trace + // materialized that iterator via `NewWithVtable` (`CallMallocNursery`) it is + // nursery-resident, so a minor collection during the remaining walk moves it + // before the non-commit rollback restores its cursor — forward each iterator + // so `w_range_iter_set_cursor` writes through the live pointer, not a stale + // moved one. The `(pre_current, pre_remaining)` pair are plain scalars. + let bridge_iter = unsafe { &mut *(*area.bridge_iter).as_ptr() }; + for entry in bridge_iter.iter_mut() { + visitor(unsafe { &mut *(&mut entry.0 as *mut pyre_object::PyObjectRef).cast() }); + } // gh#467: the latched forward-flush operand stack (callable + args) is // nursery-resident across the abort unwind — the flush boxes Int/Float // locals, which can trigger a minor collection that moves these refs before diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 25d6666dae5..42add08b227 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -4699,6 +4699,16 @@ fn set_jit_param_via_warmstate(name: &str, value: i64) { .set_param(name, value); } +/// `pypyjit.set_param(str)` seam: apply a whole parameter string +/// (`"name=value,…"`, `"off"`, `"default"`) to the warmstate through the +/// authoritative parser, so the positional-string form shares one code path +/// with the `PYRE_JIT` env lever regardless of backend. +fn set_jit_param_string_via_warmstate(text: &str) -> Result<(), ()> { + let (driver, _) = driver_pair(); + let ws = driver.meta_interp_mut().warm_state_mut(); + apply_jit_param_string(ws, text) +} + /// WIP gate for jd1 (`unpackiterable_driver`) live-path residual execution. /// OFF by default: the merge-point hook stays inert so the second driver does /// not perturb jd0 until the full activation slice (blackhole entry + @@ -5054,6 +5064,7 @@ pub fn init_jit_hooks() { init_gc_subsystem(); pyre_interpreter::call::register_eval_override(eval_with_jit); pyre_interpreter::call::register_set_jit_param_hook(set_jit_param_via_warmstate); + pyre_interpreter::call::register_set_jit_param_string_hook(set_jit_param_string_via_warmstate); pyre_interpreter::call::register_unpack_merge_hook(unpack_merge_point_jit); // Install the dict key `eq_w` / `hash_w` / `compares_by_identity` // trampolines here, at boot, before any user statement runs. They are @@ -5732,6 +5743,7 @@ fn eval_with_jit_inner(frame: &mut PyFrame) -> PyResult { let code = unsafe { &*pyre_interpreter::pyframe_get_pycode(frame_root.frame()) }; pyre_interpreter::call::register_eval_override(eval_with_jit); pyre_interpreter::call::register_set_jit_param_hook(set_jit_param_via_warmstate); + pyre_interpreter::call::register_set_jit_param_string_hook(set_jit_param_string_via_warmstate); // The backend-agnostic registrations here — notably the JIT exception // raiser (`register_jit_exc_raiser`) that `jit_publish_exception` routes // residual-call raises through — are required on every backend; the