From f4edbac1c32aa8b1e35924e81f5718399faa595f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Fri, 21 Aug 2026 20:44:49 +0900 Subject: [PATCH 01/14] gc: report a storage box's raw payload as memory pressure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gc_alloc_storage_box` registers `size_of::()` with the collector, so a `Vec` payload counted as the 24 bytes of its container rather than the bytes it holds. Add `add_storage_memory_pressure`, backed by a new `majit_gc::add_memory_pressure_estimate` — `rgc.py`'s object-less `add_memory_pressure(estimate)` form — and call it from `w_bytes_from_bytes` and `w_bytearray_alloc` once the payload is held by a live object, the point `buffer.py RawByteBuffer.__init__` reports at. Both allocators now build the object body once and write that body into either the GC-stable arm or the `malloc_typed` arm. Assisted-by: Claude --- majit/majit-gc/src/lib.rs | 6 ++++ pyre/pyre-object/src/bytearrayobject.rs | 44 +++++++++++-------------- pyre/pyre-object/src/bytesobject.rs | 40 +++++++++++----------- pyre/pyre-object/src/gc_storage.rs | 20 +++++++++++ 4 files changed, 64 insertions(+), 46 deletions(-) diff --git a/majit/majit-gc/src/lib.rs b/majit/majit-gc/src/lib.rs index 73d710a0577..e679b606be7 100644 --- a/majit/majit-gc/src/lib.rs +++ b/majit/majit-gc/src/lib.rs @@ -2681,6 +2681,12 @@ pub fn add_memory_pressure(size: isize, object: GcRef) { } } +/// `rgc.py add_memory_pressure(estimate)` — the form that names no object, +/// for a caller whose raw allocation hangs off no single translated field. +pub fn add_memory_pressure_estimate(size: isize) { + add_memory_pressure(size, GcRef::NULL); +} + pub fn total_memory_pressure() -> isize { ACTIVE_TOTAL_MEMORY_PRESSURE.get().map_or(0, |hook| hook()) } diff --git a/pyre/pyre-object/src/bytearrayobject.rs b/pyre/pyre-object/src/bytearrayobject.rs index fce06f8c0e6..ecbd9b752f9 100644 --- a/pyre/pyre-object/src/bytearrayobject.rs +++ b/pyre/pyre-object/src/bytearrayobject.rs @@ -88,43 +88,37 @@ impl crate::lltype::GcType for W_BytearrayObject { fn w_bytearray_alloc(buf: Vec) -> PyObjectRef { let length = buf.len(); let alloc = if buf.is_empty() { 0 } else { buf.len() + 1 }; + let payload = buf.capacity(); let data = crate::gc_storage::gc_alloc_storage_box(buf, crate::bytesobject::bytes_data_gc_type_id()); let header = PyObject { ob_type: &BYTEARRAY_TYPE as *const PyType, w_class: get_instantiate(&BYTEARRAY_TYPE), }; + let body = W_BytearrayObject { + ob_header: header, + data, + length, + alloc, + logical_offset: 0, + exports: 0, + w_dict: PY_NULL, + w_weakreflifeline: PY_NULL, + }; let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_BYTEARRAY_GC_TYPE_ID, W_BYTEARRAY_OBJECT_SIZE); - if !raw.is_null() { + let w_bytearray = if !raw.is_null() { unsafe { - std::ptr::write( - raw as *mut W_BytearrayObject, - W_BytearrayObject { - ob_header: header, - data, - length, - alloc, - logical_offset: 0, - exports: 0, - w_dict: PY_NULL, - w_weakreflifeline: PY_NULL, - }, - ); + std::ptr::write(raw as *mut W_BytearrayObject, body); } raw as PyObjectRef } else { - crate::lltype::malloc_typed(W_BytearrayObject { - ob_header: header, - data, - length, - alloc, - logical_offset: 0, - exports: 0, - w_dict: PY_NULL, - w_weakreflifeline: PY_NULL, - }) as PyObjectRef - } + crate::lltype::malloc_typed(body) as PyObjectRef + }; + // `buffer.py RawByteBuffer.__init__` reports only once the payload is held + // by a live object; see `w_bytes_from_bytes`. + crate::gc_storage::add_storage_memory_pressure(payload); + w_bytearray } /// Allocate a new bytearray filled with zeros. diff --git a/pyre/pyre-object/src/bytesobject.rs b/pyre/pyre-object/src/bytesobject.rs index 2708b98b779..1f3f19042ee 100644 --- a/pyre/pyre-object/src/bytesobject.rs +++ b/pyre/pyre-object/src/bytesobject.rs @@ -105,32 +105,30 @@ pub fn w_bytes_from_bytes(bytes: &[u8]) -> PyObjectRef { ob_type: &BYTES_TYPE as *const PyType, w_class: get_instantiate(&BYTES_TYPE), }; + let body = W_BytesObject { + ob_header: header, + data, + len, + ctypes_keepalive_refs: 0, + w_dict: PY_NULL, + w_weakreflifeline: PY_NULL, + }; let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_BYTES_GC_TYPE_ID, W_BYTES_OBJECT_SIZE); - if !raw.is_null() { + let w_bytes = if !raw.is_null() { unsafe { - std::ptr::write( - raw as *mut W_BytesObject, - W_BytesObject { - ob_header: header, - data, - len, - ctypes_keepalive_refs: 0, - w_dict: PY_NULL, - w_weakreflifeline: PY_NULL, - }, - ); + std::ptr::write(raw as *mut W_BytesObject, body); } raw as PyObjectRef } else { - crate::lltype::malloc_typed(W_BytesObject { - ob_header: header, - data, - len, - ctypes_keepalive_refs: 0, - w_dict: PY_NULL, - w_weakreflifeline: PY_NULL, - }) as PyObjectRef - } + crate::lltype::malloc_typed(body) as PyObjectRef + }; + // `buffer.py RawByteBuffer.__init__` reports only once `self._buf` holds the + // raw allocation: the report arms the next allocation to collect, so a + // payload still reachable from nothing but a local would be swept out from + // under the object being built. Nothing allocates between here and the + // return. + crate::gc_storage::add_storage_memory_pressure(len); + w_bytes } /// Allocate a bytes-subclass instance in the managed heap. PyPy's diff --git a/pyre/pyre-object/src/gc_storage.rs b/pyre/pyre-object/src/gc_storage.rs index 92e10a0cb42..377988ef37f 100644 --- a/pyre/pyre-object/src/gc_storage.rs +++ b/pyre/pyre-object/src/gc_storage.rs @@ -40,6 +40,26 @@ pub fn gc_alloc_storage_box(value: T, tid: u32) -> *mut T { crate::lltype::malloc_raw(value) } +/// `buffer.py RawByteBuffer.__init__` — report a raw payload's bytes +/// to the collector, the line upstream writes right after the raw malloc. +/// +/// Upstream keeps two buffer representations. `ByteBuffer` (`buffer.py`) +/// holds `['\0'] * n`, a GC-heap list the collector allocates and therefore +/// counts by itself. `RawByteBuffer` puts the same bytes in raw memory and +/// pairs the allocation with `rgc.add_memory_pressure(length)`; +/// `rawstorage.alloc_raw_storage` spells it as `add_memory_pressure=True`. +/// A storage box is the raw representation — the collector registers +/// `size_of::()` and never sees the container's own allocation — so it +/// takes the same report. Without it a `BufferedReader` buffer counts as the +/// 24 bytes of its `Vec` rather than the 128KB it holds, and a heap of them +/// never moves the major-collection threshold. +/// +/// `size` is the payload alone. `incminimark.py raw_malloc_memory_pressure`'s +/// per-allocation term is added by the collector. +pub fn add_storage_memory_pressure(size: usize) { + majit_gc::add_memory_pressure_estimate(size as isize); +} + /// GC-sweep destructor for a storage box built by /// [`gc_alloc_storage_box::`]. /// From 19662ffc313af2186ae5850ddd4f394f229b3246 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Fri, 21 Aug 2026 20:45:01 +0900 Subject: [PATCH 02/14] jit: trace IMPORT_NAME through the __import__ gateway MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMPORT_NAME carries the interned `co_names_w` entry instead of building a fresh string per execution: `execute_import_name` passes the name index down, `import_name` takes the name object, and the codewriter emits the cached `w_code_getname_w_or_new` constant — the object `pyopcode.py`'s `IMPORT_NAME` gets from `getname_w`. `check_sys_modules` loses `dont_look_inside`; the mutable `sys.modules` dict is the invalidation boundary, and the attribute turned the cached-import fast path into an `EF_RANDOM_EFFECTS` residual whose dictionary read the optimizer never saw. The builtin half lowers as `PyreHelperKind::LoadImport` for the `builtins.__import__` lookup plus an ordinary `CallFn` invocation, and the gateway wrapper is published in `jit_trace_fnaddrs` beside the other `BuiltinCode.func` wrappers. `builtin_kwargs` binding takes `Arguments._match_signature`'s `unroll_safe` shape and moves its unexpected-keyword message into a cold `builtin_unexpected_keyword_failure` residual; `front::mir` resolves `split_builtin_kwargs`'s RangeTo receiver and stop through their block-link aliases so the slice lowers to `getslice_minusone`. `I::ImportName` joins the FOR_ITER body gate, with a unit test and the `import_name_cached_name_and_rebind_jit` parity fixture. Assisted-by: Claude --- majit/majit-ir/src/effectinfo.rs | 4 + majit/majit-translate/src/front/mir.rs | 64 ++++- .../majit-translate/src/front/slice_index.rs | 13 +- .../import_name_cached_name_and_rebind_jit.py | 51 ++++ pyre/pyre-interpreter/src/builtins.rs | 153 +++++++--- pyre/pyre-interpreter/src/eval.rs | 8 +- pyre/pyre-interpreter/src/importing.rs | 16 +- pyre/pyre-interpreter/src/jit_fnaddr.rs | 6 + pyre/pyre-interpreter/src/pyopcode.rs | 4 +- pyre/pyre-jit/src/call_jit.rs | 100 +++---- pyre/pyre-jit/src/eval.rs | 16 ++ pyre/pyre-jit/src/jit/codewriter.rs | 136 +++++---- pyre/pyre-jit/src/jit/cpu.rs | 12 +- pyre/pyre-jit/src/jit/flatten.rs | 270 +++++++----------- 14 files changed, 503 insertions(+), 350 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py diff --git a/majit/majit-ir/src/effectinfo.rs b/majit/majit-ir/src/effectinfo.rs index fbb8346c397..bba5b61d02b 100644 --- a/majit/majit-ir/src/effectinfo.rs +++ b/majit/majit-ir/src/effectinfo.rs @@ -901,6 +901,10 @@ pub enum PyreHelperKind { /// helper (`pyopcode.py:866-870`). Same standing as /// [`PyreHelperKind::LoadLocals`]. LoadBuildClass, + /// `bh_load_import_fn(frame)` — the builtin lookup half of IMPORT_NAME. + /// The following invocation is emitted through [`PyreHelperKind::CallFn`] + /// so gateway builtins retain their ordinary meta-traceable call shape. + LoadImport, /// `bh_call_fn_N(callable, null_or_self, args...)` — the CALL-family /// Python-call helper. `null_or_self` (arg index 1) is a sentinel /// the helper checks before use (a non-null receiver is prepended as diff --git a/majit/majit-translate/src/front/mir.rs b/majit/majit-translate/src/front/mir.rs index 060e4f35c63..cbb49f94cfb 100644 --- a/majit/majit-translate/src/front/mir.rs +++ b/majit/majit-translate/src/front/mir.rs @@ -10347,9 +10347,12 @@ impl<'a> Lowering<'a> { /// indexing. RPython represents the same operation directly as /// `getitem` / `setitem`; the Rust trait shim is opaque in Charon and must /// not survive as a residual call. Range implementations share the same - /// name, so accept only the literal `usize` second argument; RangeFrom / - /// RangeTo continue through `front::slice_index`'s bounded getslice - /// rewrites. + /// name, so accept only an integer-bank second argument. `usize` is the + /// ordinary spelling, but Charon may preserve an integer alias instead of + /// the literal atom; [`vec_index_type_is_scalar`] performs the same + /// representation test used by `Vec::index`. RangeFrom / RangeTo remain + /// Ref-bank values and continue through `front::slice_index`'s bounded + /// getslice rewrites. fn is_slice_scalar_index_call(&self, reg: &RegularCall, index_ty: Option<&TyRef>) -> bool { let CallKind::Fun(FunId::Regular { id }) = ®.kind else { return false; @@ -10360,7 +10363,16 @@ impl<'a> Lowering<'a> { "core::slice::index::::index" | "core::slice::index::::index_mut" ) }); - is_index && index_ty.and_then(|ty| self.tyref_literal_uint_atom(ty)) == Some("Usize") + let callsite_index_is_scalar = index_ty + .is_some_and(|ty| vec_index_type_is_scalar(ty, self.llbc)) + || reg + .generics + .get("types") + .and_then(serde_json::Value::as_array) + .and_then(|types| types.get(1)) + .and_then(|ty| serde_json::from_value::(ty.clone()).ok()) + .is_some_and(|ty| vec_index_type_is_scalar(&ty, self.llbc)); + is_index && callsite_index_is_scalar } fn is_slice_scalar_index_mut_call(&self, reg: &RegularCall) -> bool { @@ -27306,4 +27318,48 @@ mod tests { "From for usize should use RPython's cast_bool_to_uint path" ); } + + /// `split_builtin_kwargs` returns `&args[..args.len() - 1]` after proving + /// the slice non-empty. MIR carries both the stop and receiver through + /// block-link aliases, so the RangeTo proof must resolve those aliases + /// before recognizing the orthodox `getslice_minusone` shape. + #[test] + #[ignore] + fn split_builtin_kwargs_rangeto_aliases_lower_to_getslice() { + use crate::model::{CallTarget, OpKind}; + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../build/llbc/pyre-interpreter.ullbc" + ); + let llbc = Llbc::load(path).expect("load real LLBC"); + let graph = super::lower_function(&llbc, "split_builtin_kwargs") + .expect("lower split_builtin_kwargs"); + let calls_path = |want: &[&str]| -> usize { + let want: Vec = want.iter().map(|part| part.to_string()).collect(); + graph + .blocks + .iter() + .flat_map(|block| &block.operations) + .filter(|op| { + matches!( + &op.kind, + OpKind::Call { + target: CallTarget::FunctionPath { segments }, + .. + } if segments == &want + ) + }) + .count() + }; + assert_eq!( + calls_path(&["core", "slice", "index", "", "index"]), + 0, + "the proven RangeTo index must not remain a core residual" + ); + assert_eq!( + calls_path(&["__getslice_minusone"]), + 1, + "the marker strip must use the len-minus-one slice helper" + ); + } } diff --git a/majit/majit-translate/src/front/slice_index.rs b/majit/majit-translate/src/front/slice_index.rs index f1b647a019c..edf39fb4061 100644 --- a/majit/majit-translate/src/front/slice_index.rs +++ b/majit/majit-translate/src/front/slice_index.rs @@ -980,6 +980,12 @@ fn rangeto_static_length_bound_matches( /// required wraparound semantics. `ArrayLen` and plain `sub` are the measured /// post-lowering forms (`front/mir.rs`). fn minus_one_end_matches(graph: &FunctionGraph, end: &Variable, slice: &Variable) -> bool { + let Some(end) = resolve_block_alias(graph, end) else { + return false; + }; + let Some(slice) = resolve_block_alias(graph, slice) else { + return false; + }; let Some((lhs, rhs)) = graph .blocks .iter() @@ -993,15 +999,18 @@ fn minus_one_end_matches(graph: &FunctionGraph, end: &Variable, slice: &Variable rhs, result_ty: ValueType::Unsigned, }, - ) if result == end && op == "sub" => Some((lhs.clone(), rhs.clone())), + ) if result == &end && op == "sub" => Some((lhs.clone(), rhs.clone())), _ => None, }) else { return false; }; + let lhs = resolve_block_alias(graph, &lhs).unwrap_or(lhs); + let rhs = resolve_block_alias(graph, &rhs).unwrap_or(rhs); let has_len = graph.blocks.iter().flat_map(|b| &b.operations).any(|op| { op.result.as_ref() == Some(&lhs) - && matches!(&op.kind, OpKind::ArrayLen { base, .. } if base == slice) + && matches!(&op.kind, OpKind::ArrayLen { base, .. } + if resolve_block_alias(graph, base).as_ref() == Some(&slice)) }); let has_one = graph .blocks diff --git a/pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py b/pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py new file mode 100644 index 00000000000..9dab7cb1b9c --- /dev/null +++ b/pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py @@ -0,0 +1,51 @@ +# CPython-suite gap: import-hook tests do not combine co_names identity with a +# hot loop that rebinds builtins.__import__ after IMPORT_NAME has been traced. +# parity-tests reason: IMPORT_NAME must keep PyPy's co_names_w object and live +# builtin lookup when its call is exposed to the meta-tracer. + +import builtins + + +old_import = builtins.__import__ +os_module = old_import("os") +calls = [0, 0] +N = 40000 +SWITCH = N // 2 + + +def run(): + names = run.__code__.co_names + expected_name = names[names.index("os")] + expected_globals = globals() + + def first(name, globals_arg, locals_arg, fromlist, level): + assert name is expected_name + assert globals_arg is expected_globals + assert locals_arg is None + calls[0] += 1 + return os_module + + def second(name, globals_arg, locals_arg, fromlist, level): + assert name is expected_name + assert globals_arg is expected_globals + assert locals_arg is None + calls[1] += 1 + return os_module + + builtins.__import__ = first + try: + i = 0 + while i < N: + import os + + assert os is os_module + if i == SWITCH: + builtins.__import__ = second + i += 1 + finally: + builtins.__import__ = old_import + + +run() +assert calls == [SWITCH + 1, N - SWITCH - 1], calls +print("OK") diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 0a147815095..427db732f49 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -3715,7 +3715,8 @@ pub fn install_default_builtins(ns: PyObjectRef) { // `moduledef.py:78-87 startup` — "Copy our __import__ to builtins". // `baseobjspace.py:730` keeps that same object as // `space.w_default_importlib_import`. - let w_import = make_module_builtin_function("__import__", builtin_dunder_import); + let w_import = + make_module_builtin_function("__import__", __pyre_wrap_builtin_dunder_import); crate::importing::set_default_importlib_import(w_import); w_import }); @@ -5040,6 +5041,11 @@ pub(crate) fn clinic_arity( /// keywords by parameter name without a per-function `Signature`; the /// `#[pyre_function]` wrapper supplies the name/required tables it knows /// at expansion time. +// PyPy: `Arguments._match_signature` (`pypy/interpreter/argument.py`) +// is `@jit.unroll_safe`. The loops below are bounded by the builtin's static +// signature and argument count in the same way; without the hint the JIT +// policy residualizes this gateway step and cannot descend into the builtin. +#[majit_macros::unroll_safe] pub(crate) fn bind_builtin_kwargs( args: &[PyObjectRef], names: &[&str], @@ -5052,12 +5058,27 @@ pub(crate) fn bind_builtin_kwargs( // leaves an omitted one `PY_NULL`; a positional-only registration hands // the body just the arguments the call made. Reading a null slot as an // argument that was not passed makes the two registrations bind alike. - let supplied = positional.iter().filter(|v| !v.is_null()).count(); + let mut supplied = 0; + let mut positional_index = 0; + while positional_index < positional.len() { + if !positional[positional_index].is_null() { + supplied += 1; + } + positional_index += 1; + } + let mut required_count = 0; + let mut required_index = 0; + while required_index < required.len() { + if required[required_index] { + required_count += 1; + } + required_index += 1; + } clinic_arity( fn_name, supplied, real_kwarg_count(kwargs), - required.iter().filter(|r| **r).count(), + required_count, names.len(), 0, )?; @@ -5066,18 +5087,43 @@ pub(crate) fn bind_builtin_kwargs( // `argument.py` keys the message off `space.text_w(keyword_names_w[i])`, // the keyword's own storage, so a name carrying a lone surrogate reaches // `e.args[0]` intact. Keep the WTF-8 rather than a lossy `String`. - let mut unknown: Option = None; - for (i, &v) in positional.iter().enumerate() { - scope[i] = v; - filled[i] = !v.is_null(); - } - if let Some(dict) = kwargs { - let entries = unsafe { pyre_object::w_dict_str_entries_wtf8(dict) }; - for (key, val) in entries.iter() { - if key.as_str() == Ok("__pyre_kw__") { + let keyword_entries = kwargs.map(|dict| unsafe { pyre_object::w_dict_str_entries_wtf8(dict) }); + let mut unknown: Option = None; + // PyPy `_match_signature` copies positional values with + // `for i in range(take)` so the constant signature bounds let the JIT + // unroll ordinary indexed reads. Keep that storage shape instead of Rust + // iterator adapters, whose `Filter`/`Enumerate` state has no RPython + // counterpart. + let mut positional_index = 0; + while positional_index < positional.len() { + let value = positional[positional_index]; + scope[positional_index] = value; + filled[positional_index] = !value.is_null(); + positional_index += 1; + } + if let Some(entries) = keyword_entries.as_ref() { + let mut entry_index = 0; + while entry_index < entries.len() { + let (key, val) = &entries[entry_index]; + let key_str = if unsafe { pyre_object::dictmultiobject::wtf8_key_is_utf8(key) } { + Some(unsafe { pyre_object::dictmultiobject::wtf8_key_as_str_unchecked(key) }) + } else { + None + }; + if key_str == Some("__pyre_kw__") { + entry_index += 1; continue; } - match names.iter().position(|n| key.as_str() == Ok(*n)) { + let mut matched_index = None; + let mut name_index = 0; + while name_index < names.len() { + if key_str == Some(names[name_index]) { + matched_index = Some(name_index); + break; + } + name_index += 1; + } + match matched_index { Some(idx) => { if filled[idx] { return Err(crate::PyError::type_error(format!( @@ -5093,29 +5139,49 @@ pub(crate) fn bind_builtin_kwargs( // so a call that misses a required argument is reported // against that argument even when it also passed a keyword // the function does not know. - None => unknown = Some(key.to_wtf8_buf()), + None => unknown = Some(entry_index), } + entry_index += 1; } } - for i in 0..names.len() { - if !filled[i] && required[i] { + let mut name_index = 0; + while name_index < names.len() { + if !filled[name_index] && required[name_index] { return Err(crate::PyError::type_error(format!( "{fn_name}() missing required argument '{}' (pos {})", - names[i], - i + 1, + names[name_index], + name_index + 1, ))); } + name_index += 1; } - if let Some(key) = unknown { - let mut msg = - Wtf8Buf::from_string(format!("{fn_name}() got an unexpected keyword argument '")); - msg.push_wtf8(&key); - msg.push_str("'"); - return Err(crate::PyError::type_error(msg)); + if let Some(entry_index) = unknown { + let entries = keyword_entries + .as_ref() + .expect("an unknown keyword index requires keyword entries"); + return builtin_unexpected_keyword_failure(fn_name, &entries[entry_index].0); } Ok(scope) } +/// Cold `Arguments._match_signature` unexpected-keyword formatter. +/// +/// PyPy retains an `ArgErrUnknownKwds` until the gateway converts it to the +/// final `TypeError`, so accepted calls never trace WTF-8 string assembly. +/// Keep the same boundary here; the hot binder carries only the offending +/// entry index and reaches this residual helper after missing-required checks. +#[cold] +#[majit_macros::dont_look_inside] +pub(crate) fn builtin_unexpected_keyword_failure( + fn_name: &str, + key: &rustpython_wtf8::Wtf8, +) -> Result, crate::PyError> { + let mut msg = Wtf8Buf::from_string(format!("{fn_name}() got an unexpected keyword argument '")); + msg.push_wtf8(key); + msg.push_str("'"); + Err(crate::PyError::type_error(msg)) +} + /// Resolve a builtin with a single required positional-or-keyword parameter /// through the gateway `parse_into_scope`, so the argument binds by name and /// the trailing `__pyre_kw__` marker dict never leaks as a value. Mirrors an @@ -19192,14 +19258,13 @@ fn builtin_dunder_import(args: &[PyObjectRef]) -> Result() - } else { - unsafe { (*frame).execution_context } - } - }); + // PyPy's `interp___import__` receives `space` and any slow native-import + // fallback reaches the execution context through + // `space.getexecutioncontext()`. Do not recover it from pyre's + // portal-level CURRENT_FRAME TLS: an inlined callee has its own red frame, + // while that anchor can still name the caller. The established + // object-space analogue owns the shared execution context directly. + let exec_ctx = crate::call::getexecutioncontext(); // The native importer keys every lookup by `&str`, so a name that has no // such spelling goes straight to the app-level bootstrap. Re-read the // name through its root: `space_index_w` above may have moved it. @@ -19212,6 +19277,30 @@ fn builtin_dunder_import(args: &[PyObjectRef]) -> Result Result { + builtin_dunder_import(args) +} + +#[cfg(not(target_arch = "wasm32"))] +#[linkme::distributed_slice(crate::gateway::BUILTIN_WRAPPER_DESCRIPTORS)] +#[allow(non_upper_case_globals)] +static __pyre_wrap_builtin_dunder_import_target: crate::gateway::BuiltinWrapperDescriptor = + crate::gateway::BuiltinWrapperDescriptor { + path: concat!(module_path!(), "::", "__pyre_wrap_builtin_dunder_import"), + func: __pyre_wrap_builtin_dunder_import, + }; + #[cfg(test)] mod tests { use super::*; diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index 7346b9f35e6..1b9b246579a 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -4020,11 +4020,15 @@ impl OpcodeStepExecutor for PyFrame { Self::push_anchored(&anchor, result) } - fn import_name(&mut self, name: &str) -> Result<(), PyError> { + fn import_name(&mut self, name: &str, nameindex: usize) -> Result<(), PyError> { let w_fromlist = self.pop(); let w_flag = self.pop(); let anchor = FrameAnchor::new(self); - let w_obj = crate::importing::import_name(self, name, w_fromlist, w_flag)?; + // PyPy pyopcode.py `w_modulename = self.getname_w(nameindex)`. + let w_modulename = unsafe { + crate::pycode::w_code_getname_w_or_new(self.pycode as PyObjectRef, nameindex, name) + }; + let w_obj = crate::importing::import_name(self, w_modulename, w_fromlist, w_flag)?; Self::push_anchored(&anchor, w_obj) } diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index dfeb9c8fa0c..82f44c07baa 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -2738,13 +2738,11 @@ pub fn add_sys_path_0() { // ── check_sys_modules ──────────────────────────────────────────────── // PyPy equivalent: importing.py `check_sys_modules(space, w_modulename)` -/// Reads the process-owned `SYS_MODULES` registry (and the runtime-stamped -/// `sys.modules` dict through `sys_modules_dict`), neither a build-time -/// constant, so the JIT residualizes the call rather than folding a stale -/// `sys.modules` snapshot (`@dont_look_inside`, the `sys_modules_dict` / -/// `lookup_exc_class` shape). The `Option` return fits one word -/// and the `&str` argument matches `lookup_exc_class`. -#[majit_macros::dont_look_inside] +/// PyPy `importing.py:check_sys_modules` is an ordinary traceable lookup. +/// The mutable `sys.modules` dictionary supplies the invalidation boundary; +/// hiding this whole function behind `dont_look_inside` turns the cached +/// import fast path into an `EF_RANDOM_EFFECTS` residual and prevents the +/// optimizer from seeing the dictionary read at all. pub(crate) fn check_sys_modules(name: &str) -> Option { // Once installed, the Python-visible dict is the sole semantic module // cache. PyPy's `check_sys_modules` reads `space.sys.get('modules')` and @@ -4546,7 +4544,7 @@ fn absolute_import( /// PyPy equivalent: pyopcode.py `IMPORT_NAME`. pub fn import_name( frame: &mut PyFrame, - name: &str, + w_modulename: PyObjectRef, w_fromlist: PyObjectRef, w_flag: PyObjectRef, ) -> Result { @@ -4568,8 +4566,6 @@ pub fn import_name( _ => pyre_object::w_none(), }; let w_globals = frame.get_w_globals(); - let w_modulename = pyre_object::w_str_new(name); - crate::call::call_callable( frame, w_import, diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index af4186e6434..e65b07e33c7 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -395,6 +395,12 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { "builtins::builtin_kwargs_marker_dict", crate::builtins::builtin_kwargs_marker_dict as *const (), ); + push_alias_pair( + &mut entries, + "pyre_interpreter::builtins::builtin_unexpected_keyword_failure", + "builtins::builtin_unexpected_keyword_failure", + crate::builtins::builtin_unexpected_keyword_failure as *const (), + ); // RPython annotator PBC parity for `BuiltinCode.func`: every generated // interp2app wrapper is a possible value of the indirect function-pointer diff --git a/pyre/pyre-interpreter/src/pyopcode.rs b/pyre/pyre-interpreter/src/pyopcode.rs index 1f3e00efde1..607141b1c35 100644 --- a/pyre/pyre-interpreter/src/pyopcode.rs +++ b/pyre/pyre-interpreter/src/pyopcode.rs @@ -1259,7 +1259,7 @@ pub trait OpcodeStepExecutor: SharedOpcodeHandler { } // ── Import ── - fn import_name(&mut self, _name: &str) -> Result<(), PyError> { + fn import_name(&mut self, _name: &str, _nameindex: usize) -> Result<(), PyError> { Err(crate::PyError::type_error("import_name not implemented")) } fn import_from(&mut self, _name: &str) -> Result<(), PyError> { @@ -3386,7 +3386,7 @@ pub fn execute_import_name( unreachable!() }; let name_idx = u32_as_usize(namei.get(op_arg)); - executor.import_name(code.names[name_idx].as_ref())?; + executor.import_name(code.names[name_idx].as_ref(), name_idx)?; Ok(StepResult::Continue) } diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 7568e3a163c..f13540d4d6f 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -5917,65 +5917,6 @@ pub extern "C" fn bh_delete_attr_fn(obj: i64, w_code_ptr: i64, name_idx: i64) -> 0 } -/// IMPORT_NAME residual (`import_name` HLOp → `residual_call_ir_r`). -/// Resolves the module name from the jitcode's own code object via -/// `name_idx` (same `co_names` invariant as `bh_load_attr_fn`), fetches -/// `__import__` from the threaded frame's builtins, and calls it with the -/// frame's globals and locals. Importing a module may run its -/// top-level Python (`MayForce`); on error the exception is published -/// through `BH_LAST_EXC_VALUE` for the trailing `GuardNoException` and the -/// call returns 0. `fromlist` and `level` are the two popped operands -/// (`eval.rs import_name`: `fromlist = pop()`, `level = pop()`). -pub extern "C" fn bh_import_name_fn( - fromlist: i64, - level: i64, - w_code_ptr: i64, - frame_ptr: i64, - name_idx: i64, -) -> i64 { - let w_code = w_code_ptr as pyre_object::PyObjectRef; - let code = unsafe { - &*(pyre_interpreter::w_code_get_ptr(w_code) as *const pyre_interpreter::CodeObject) - }; - let idx = name_idx as usize; - debug_assert!( - idx < code.names.len(), - "bh_import_name_fn name_idx {idx} out of range ({} names) — codegen invariant", - code.names.len() - ); - if idx >= code.names.len() { - return 0; - } - let name = code.names[idx].as_ref(); - let frame = frame_ptr as *mut PyFrame; - debug_assert!(!frame.is_null(), "IMPORT_NAME requires a live frame"); - if frame.is_null() { - // IMPORT_NAME produces a module or raises; it never yields a null - // result. A null frame cannot honour that, so fail closed by - // publishing an exception for the trailing `GuardNoException` - // instead of returning a bare 0 the guard would accept. - let mut err = pyre_interpreter::PyError::new( - pyre_interpreter::PyErrorKind::SystemError, - "IMPORT_NAME residual received a null frame", - ); - publish_residual_call_exception(err.to_exc_object() as i64); - return 0; - } - match pyre_interpreter::importing::import_name( - unsafe { &mut *frame }, - name, - fromlist as pyre_object::PyObjectRef, - level as pyre_object::PyObjectRef, - ) { - Ok(module) => module as i64, - Err(mut err) => { - let exc_obj = err.to_exc_object(); - publish_residual_call_exception(exc_obj as i64); - 0 - } - } -} - /// IMPORT_FROM residual (`import_from` HLOp → `residual_call_ir_r`). /// Resolves the attribute name from the jitcode's own code object via /// `name_idx` (same `co_names` invariant as `bh_load_attr_fn`) and runs @@ -6531,6 +6472,47 @@ pub extern "C" fn bh_load_build_class_fn(frame_ptr: i64) -> i64 { } } +/// IMPORT_NAME's builtin lookup, split from the subsequent Python call just +/// like PyPy's `pyopcode.py:IMPORT_NAME` (`get_builtin().__import__`, then +/// `space.call_function`). Keeping this lookup as a small residual lets the +/// ordinary `CallFn` path descend through a gateway `BuiltinCode.func` rather +/// than hiding the whole importer behind one opaque residual. +pub extern "C" fn bh_load_import_fn(frame_ptr: i64) -> i64 { + let frame = frame_ptr as *mut PyFrame; + debug_assert!( + !frame.is_null(), + "bh_load_import_fn requires a non-null PyFrame" + ); + if frame.is_null() { + let mut err = pyre_interpreter::PyError::new( + pyre_interpreter::PyErrorKind::SystemError, + "IMPORT_NAME received a null frame", + ); + publish_residual_call_exception(err.to_exc_object() as i64); + return 0; + } + let w_builtin = unsafe { (*frame).get_builtin() }; + if !w_builtin.is_null() && unsafe { pyre_object::is_module(w_builtin) } { + let w_dict = unsafe { pyre_object::w_module_get_w_dict(w_builtin) }; + if !w_dict.is_null() { + match pyre_interpreter::baseobjspace::finditem_str(w_dict, "__import__") { + Ok(Some(value)) => return value as i64, + Ok(None) => {} + Err(mut err) => { + publish_residual_call_exception(err.to_exc_object() as i64); + return 0; + } + } + } + } + let mut err = pyre_interpreter::PyError::new( + pyre_interpreter::PyErrorKind::ImportError, + "__import__ not found", + ); + publish_residual_call_exception(err.to_exc_object() as i64); + 0 +} + /// DELETE_GLOBAL residual using the frame receiver and interned-name ABI. /// pyopcode.py DELETE_GLOBAL deletes directly from `w_globals`. pub extern "C" fn bh_delete_global_fn(frame_ptr: i64, w_name: i64) -> i64 { diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index d6ef0aa1985..35cfa744cb6 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -7376,6 +7376,12 @@ fn for_iter_body_op_is_jit_safe(instr: pyre_interpreter::Instruction) -> bool { // here added no safety beyond the Layer 2 defense above. | I::CallFunctionEx | I::LoadGlobal { .. } + // IMPORT_NAME is the same Python-call boundary as CALL: it + // resolves builtins.__import__ and invokes it. The Layer 2 + // effect journal above is the replay-safety authority for both; + // rejecting only the opcode spelling kept otherwise identical + // `for` loops interpreted while PyPy traces them. + | I::ImportName { .. } | I::Resume { .. } // container builders: produce new heap objects but do not mutate // existing ones; walk-abort just drops the incomplete object @@ -14451,6 +14457,16 @@ mod tests { assert_eq!(unsupported_jit_shape_of(&code), UnsupportedJitShape::None); } + #[test] + fn for_iter_cached_import_body_is_jit_safe() { + use pyre_interpreter::compile_exec; + let module = compile_exec("def f(n):\n for _ in range(n):\n import os\n") + .expect("test code should compile"); + let code = function_code_from_module(&module, "f"); + assert!(function_entry_trace_is_jit_safe(&code)); + assert_eq!(unsupported_jit_shape_of(&code), UnsupportedJitShape::None); + } + #[test] fn for_iter_single_level_binaryop_mutation_body_is_jit_safe() { // single-level `s += t` (in-place list extend via BINARY_OP) recovers on diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index cdc99a22c4f..74c1864d265 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -3075,32 +3075,6 @@ fn emit_frontend_is_op( ) } -fn emit_frontend_import_name( - graph: &mut super::flow::FunctionGraph, - block: &super::flow::BlockRef, - fromlist: super::flow::FlowValue, - level: super::flow::FlowValue, - code: super::flow::FlowValue, - frame: super::flow::FlowValue, - name_idx: super::flow::FlowValue, - offset: i64, -) -> super::flow::Variable { - emit_graph_op_with_result( - graph, - block, - "import_name", - vec![ - fromlist.into(), - level.into(), - code.into(), - frame.into(), - name_idx.into(), - ], - Kind::Ref, - offset, - ) -} - fn emit_frontend_import_from( graph: &mut super::flow::FunctionGraph, block: &super::flow::BlockRef, @@ -3575,7 +3549,6 @@ struct FnPtrIndices { format_with_spec_fn: HelperHandle, build_string_from_array_fn: HelperHandle, convert_value_fn: HelperHandle, - import_name_fn: HelperHandle, import_from_fn: HelperHandle, load_super_attr_fn: HelperHandle, super_attr_unwrap_fn: HelperHandle, @@ -3596,6 +3569,7 @@ struct FnPtrIndices { match_class_fn: HelperHandle, load_locals_fn: HelperHandle, load_build_class_fn: HelperHandle, + load_import_fn: HelperHandle, load_from_dict_or_globals_fn: HelperHandle, call_function_ex_fn: HelperHandle, unary_not_fn: HelperHandle, @@ -3952,13 +3926,6 @@ fn register_helper_fn_pointers( cpu.convert_value_fn as *const (), CallFlavor::MayForce, ); - // `bh_import_name_fn` runs `__import__` (module top-level Python may run) - // → `MayForce`. Appended last to preserve fn_ptr indices. - let import_name_fn = bind( - assembler, - cpu.import_name_fn as *const (), - CallFlavor::MayForce, - ); // `bh_import_from_fn` runs `importing::import_from` (a submodule-import // fallback may run module top-level Python) → `MayForce`. let import_from_fn = bind( @@ -4324,6 +4291,14 @@ fn register_helper_fn_pointers( cpu.load_build_class_fn as *const (), CallFlavor::Plain, ); + // IMPORT_NAME performs this builtin lookup and then uses the ordinary + // CallFn path for the actual invocation. Bind after the existing helpers + // so their pool indices remain stable. + let load_import_fn = bind( + assembler, + cpu.load_import_fn as *const (), + CallFlavor::Plain, + ); // The hand-written PUSH_EXC_INFO lowering must complete the interpreter's // caught-exception ownership transfer. Bind last so every existing // helper index remains stable. @@ -4394,7 +4369,6 @@ fn register_helper_fn_pointers( format_with_spec_fn, build_string_from_array_fn, convert_value_fn, - import_name_fn, import_from_fn, load_super_attr_fn, super_attr_unwrap_fn, @@ -4427,6 +4401,7 @@ fn register_helper_fn_pointers( match_class_fn, load_locals_fn, load_build_class_fn, + load_import_fn, load_from_dict_or_globals_fn, call_function_ex_fn, call_kw_fn_0, @@ -6348,11 +6323,6 @@ impl CodeWriter { idx: convert_value_fn_idx, flavor: _convert_value_fn_flavor, }, - import_name_fn: - HelperHandle { - idx: import_name_fn_idx, - flavor: _import_name_fn_flavor, - }, import_from_fn: HelperHandle { idx: import_from_fn_idx, @@ -6443,6 +6413,11 @@ impl CodeWriter { idx: load_build_class_fn_idx, flavor: _load_build_class_fn_flavor, }, + load_import_fn: + HelperHandle { + idx: load_import_fn_idx, + flavor: _load_import_fn_flavor, + }, load_from_dict_or_globals_fn: HelperHandle { idx: load_from_dict_or_globals_fn_idx, @@ -6699,7 +6674,6 @@ impl CodeWriter { format_with_spec_fn_idx, build_string_from_array_fn_idx, convert_value_fn_idx, - import_name_fn_idx, import_from_fn_idx, load_super_attr_fn_idx, super_attr_unwrap_fn_idx, @@ -6720,6 +6694,7 @@ impl CodeWriter { match_class_fn_idx, load_locals_fn_idx, load_build_class_fn_idx, + load_import_fn_idx, load_from_dict_or_globals_fn_idx, call_function_ex_fn_idx, unary_not_fn_idx, @@ -12300,37 +12275,72 @@ impl CodeWriter { emit_abort_permanent!(py_pc); } - // ImportName: pops 2 (fromlist=TOS, level=TOS1), pushes - // 1 module. Net: -1. `import_name(fromlist, level, code, - // name_idx)` HLOp → `residual_call_ir_r(import_name_fn, - // ListI[name_idx], ListR[fromlist, level, code])`. The - // jitcode's own PyCode travels as a post-rtype - // `Signed(ptr) + Kind::Ref` constant and the `co_names` - // index the helper resolves the module name with — the - // same surrogate-operand shape as the LoadAttr arm. - // `bh_import_name_fn` runs `__import__` through the - // TLS-pinned execution context (MayForce). + // PyPy pyopcode.py IMPORT_NAME: resolve + // `get_builtin().__import__`, then invoke it through + // the ordinary Python call path with + // `(name, globals, None, fromlist, level)`. Keeping + // the lookup and CallFn separate is load-bearing: the + // latter can descend through BuiltinCode.func and + // trace `_gcd_import`; the old monolithic + // one monolithic residual hid the entire importer. Instruction::ImportName { namei } => { let name_idx = namei.get(op_arg) as usize; - let code_const: super::flow::FlowValue = super::flow::Constant::new( - super::flow::ConstantValue::Signed(w_code as i64), - Some(Kind::Ref), - ) - .into(); - let name_idx_const: super::flow::FlowValue = - super::flow::Constant::signed(name_idx as i64).into(); let _ = emit_popvalue_ref!(current_depth, py_pc); let fromlist_value = pop_ref_or_fresh(&mut current_state, &mut graph); let _ = emit_popvalue_ref!(current_depth, py_pc); let level_value = pop_ref_or_fresh(&mut current_state, &mut graph); - let result_value = emit_frontend_import_name( + + let callable = emit_frontend_frame_only_ref( &mut graph, ¤t_block.block(), - fromlist_value, - level_value, - code_const, + "load_import", frame_var.into(), - name_idx_const, + py_pc as i64, + ); + + let name = code + .names + .get(name_idx) + .expect("IMPORT_NAME co_names index is validated by the compiler"); + // PyPy pyopcode.py `IMPORT_NAME`'s `getname_w` uses the exact interned + // object in this PyCode's `co_names_w` table. + let w_name = unsafe { + pyre_interpreter::pycode::w_code_getname_w_or_new( + w_code as pyre_object::PyObjectRef, + name_idx, + name.as_ref(), + ) + }; + let name_value = pyobject_const_ref_value(w_name); + // Every per-code jitcode now carries its own red + // frame. Read that frame's live w_globals exactly + // as PyPy's `self.get_w_globals()` does; an inlined + // callee must never inherit the caller's namespace + // or a code-wrapper constant in its place. + let globals_value: super::flow::FlowValue = emit_graph_op_with_result( + &mut graph, + ¤t_block.block(), + "getfield_vable_r", + vable_getfield_ref_graph_args( + frame_var.into(), + VABLE_NAMESPACE_FIELD_IDX, + ), + Kind::Ref, + py_pc as i64, + ) + .into(); + let result_value = emit_frontend_simple_call( + &mut graph, + ¤t_block.block(), + callable.into(), + super::flow::Constant::none().into(), + vec![ + name_value, + globals_value, + pyobject_const_ref_value(pyre_object::w_none()), + fromlist_value, + level_value, + ], py_pc as i64, ); push_and_bump!(result_value.into(), py_pc); diff --git a/pyre/pyre-jit/src/jit/cpu.rs b/pyre/pyre-jit/src/jit/cpu.rs index c00a4d4e503..3153060f7fb 100644 --- a/pyre/pyre-jit/src/jit/cpu.rs +++ b/pyre/pyre-jit/src/jit/cpu.rs @@ -210,12 +210,6 @@ pub struct Cpu { /// `conv` is a `runtime_ops::convert_value_code`; user `__str__` / /// `__repr__` may run Python (fallible). pub convert_value_fn: extern "C" fn(i64, i64) -> i64, - /// `bh_import_name_fn(fromlist, level, code, frame, name_idx)` — - /// IMPORT_NAME `__import__` residual; resolves the module name from the - /// code object, reads `__name__`/`__package__` for relative imports from - /// the threaded `frame`, and imports through the TLS-pinned execution - /// context (may run module top-level Python → fallible). - pub import_name_fn: extern "C" fn(i64, i64, i64, i64, i64) -> i64, /// `bh_import_from_fn(module, code, name_idx)` — IMPORT_FROM residual; /// resolves the attribute name from the code object and runs /// `importing::import_from` on the peeked module (namespace lookup, then a @@ -345,6 +339,10 @@ pub struct Cpu { /// `LOAD_BUILD_CLASS` (`pyopcode.py`); reads `__build_class__` out of /// the frame's builtin mapping. pub load_build_class_fn: extern "C" fn(i64) -> i64, + /// Load `builtins.__import__` for IMPORT_NAME. Kept separate from the + /// call itself so the generated jitcode has the same ordinary Python + /// call boundary as PyPy's `IMPORT_NAME` implementation. + pub load_import_fn: extern "C" fn(i64) -> i64, /// `newtuple(list_w)` (`objspace.py:332`) — (ref array) → new tuple. /// The array is the forced `popvalues` list; length travels inside /// the array, so any arity fits. @@ -507,7 +505,6 @@ impl Cpu { format_simple_fn: crate::call_jit::bh_format_simple_fn, format_with_spec_fn: crate::call_jit::bh_format_with_spec_fn, convert_value_fn: crate::call_jit::bh_convert_value_fn, - import_name_fn: crate::call_jit::bh_import_name_fn, import_from_fn: crate::call_jit::bh_import_from_fn, load_super_attr_fn: crate::call_jit::bh_load_super_attr_fn, super_attr_unwrap_fn: crate::call_jit::bh_super_attr_unwrap_fn, @@ -546,6 +543,7 @@ impl Cpu { delete_global_fn: crate::call_jit::bh_delete_global_fn, load_locals_fn: crate::call_jit::bh_load_locals_fn, load_build_class_fn: crate::call_jit::bh_load_build_class_fn, + load_import_fn: crate::call_jit::bh_load_import_fn, newtuple_from_array_fn: crate::call_jit::bh_newtuple_from_array, build_map_from_array_fn: crate::call_jit::bh_build_map_from_array, build_set_from_array_fn: crate::call_jit::bh_build_set_from_array, diff --git a/pyre/pyre-jit/src/jit/flatten.rs b/pyre/pyre-jit/src/jit/flatten.rs index de13fe952f3..49ea365b2a5 100644 --- a/pyre/pyre-jit/src/jit/flatten.rs +++ b/pyre/pyre-jit/src/jit/flatten.rs @@ -2640,6 +2640,7 @@ pub fn graph_op_can_raise(op: &super::flow::SpaceOperation) -> bool { | "delete_name" | "delete_global" | "load_build_class" + | "load_import" | "simple_call" | "getattr" | "load_special" @@ -2666,7 +2667,6 @@ pub fn graph_op_can_raise(op: &super::flow::SpaceOperation) -> bool { | "match_keys" | "match_class" | "not_" - | "import_name" | "import_from" | "load_from_dict_or_globals" | "load_super_attr" @@ -3331,6 +3331,9 @@ pub struct LoweringContext { /// `load_build_class_fn` descrs-pool index. LOAD_BUILD_CLASS lowers to the /// same one-Ref shape as [`Self::load_locals_fn_idx`]. pub load_build_class_fn_idx: u16, + /// `load_import_fn` descrs-pool index. This is the builtin-lookup half of + /// IMPORT_NAME; the subsequent invocation uses the ordinary CallFn path. + pub load_import_fn_idx: u16, /// `bind(assembler, cpu.newtuple_from_array_fn as *const (), /// CallFlavor::Plain)` descrs-pool index for the production /// source. BUILD_TUPLE records the rtyped `pyopcode.py` @@ -3499,16 +3502,6 @@ pub struct LoweringContext { /// `bh_convert_value_fn(value, conv)` runs str/repr/ascii (a user /// `__str__` / `__repr__` may force → `MayForce`). pub convert_value_fn_idx: u16, - /// `import_name_fn` descrs-pool index — see codewriter.rs - /// `register_helper_fn_pointers`. IMPORT_NAME records the - /// `import_name(fromlist, level, code, name_idx)` HLOp (code = the - /// jitcode's own PyCode as a `Signed(ptr) + Kind::Ref` constant, - /// name_idx = `co_names` index) lowered to `residual_call_ir_r( - /// ConstInt(fn_idx), ListI([name_idx]), ListR([fromlist, level, code]), - /// Descr) → reg` via [`lower_import_name_hlop_to_insn`]; - /// `bh_import_name_fn` runs `__import__` (module top-level Python may - /// run → `MayForce`). - pub import_name_fn_idx: u16, /// `import_from_fn` descrs-pool index. IMPORT_FROM records the /// `import_from(module, code, name_idx)` HLOp (code = the jitcode's own /// PyCode as a `Signed(ptr) + Kind::Ref` constant, name_idx = @@ -4487,6 +4480,41 @@ where ) } +/// Lower the builtin-lookup half of pyopcode.py IMPORT_NAME to a one-Ref +/// residual. Its result becomes the callable of a separate `simple_call`. +pub fn lower_load_import_hlop_to_insn( + op: &super::flow::SpaceOperation, + ctx: &LoweringContext, + get_register: &mut F, + lower_constant: &mut LC, +) -> Option +where + F: FnMut(super::flow::Variable) -> Register, + LC: FnMut(&Constant) -> Operand, +{ + if op.opname != "load_import" || op.args.len() != 1 { + return None; + } + let frame_operand = flatten_arg_with_lowering(&op.args[0], get_register, lower_constant); + let dst_reg = match &op.result { + Some(super::flow::FlowValue::Variable(var)) => get_register(*var), + _ => return None, + }; + // `get_builtin().getdictvalue('__import__')` is an analyzed, non-elidable + // lookup: it can raise ImportError but neither runs Python nor writes the + // GC heap. Use EF_CAN_RAISE with concrete-empty effect sets. The generic + // Plain flavor means “no graph was analyzed” and becomes RANDOM_EFFECTS / + // CALL_MAY_FORCE, contradicting PyPy's zero-forcing IMPORT_NAME trace. + let mut effect_info = majit_ir::EffectInfo::default(); + effect_info.pyre_helper = majit_ir::PyreHelperKind::LoadImport; + Some(build_residual_call_r_r_insn_with_effect_info( + ctx.load_import_fn_idx, + vec![frame_operand], + effect_info, + dst_reg, + )) +} + /// Lower pyopcode.py DELETE_GLOBAL to a void two-Ref residual call. pub fn lower_delete_global_hlop_to_insn( op: &super::flow::SpaceOperation, @@ -4873,7 +4901,6 @@ pub fn build_residual_call_r_r_insn_from_operands( pyre_helper: majit_ir::PyreHelperKind, dst_reg: Register, ) -> Insn { - let arg_kinds = vec![Kind::Ref; ref_operands.len()]; // `bh_call_fn_N` dispatches the callable supplied at runtime. Its target // is therefore the indirect-call top set, not the empty effect set of the // helper wrapper itself: user code can mutate any escaped heap location. @@ -4887,6 +4914,16 @@ pub fn build_residual_call_r_r_insn_from_operands( effect_info_for_call_flavor(flavor) }; effect_info.pyre_helper = pyre_helper; + build_residual_call_r_r_insn_with_effect_info(fn_idx, ref_operands, effect_info, dst_reg) +} + +fn build_residual_call_r_r_insn_with_effect_info( + fn_idx: u16, + ref_operands: Vec, + effect_info: majit_ir::EffectInfo, + dst_reg: Register, +) -> Insn { + let arg_kinds = vec![Kind::Ref; ref_operands.len()]; let descr_operand = Operand::descr(DescrOperand::CallDescrStub(CallDescrStub { effect_info, arg_kinds, @@ -5426,6 +5463,9 @@ where if let Some(insn) = lower_load_build_class_hlop_to_insn(op, ctx, get_register, lower_constant) { return Some(insn); } + if let Some(insn) = lower_load_import_hlop_to_insn(op, ctx, get_register, lower_constant) { + return Some(insn); + } if let Some(insn) = lower_tuple_build_hlop_to_insn(op, ctx, get_register, lower_constant) { return Some(insn); } @@ -5478,9 +5518,6 @@ where if let Some(insn) = lower_convert_value_hlop_to_insn(op, ctx, get_register, lower_constant) { return Some(insn); } - if let Some(insn) = lower_import_name_hlop_to_insn(op, ctx, get_register, lower_constant) { - return Some(insn); - } if let Some(insn) = lower_import_from_hlop_to_insn(op, ctx, get_register, lower_constant) { return Some(insn); } @@ -6689,67 +6726,6 @@ where )) } -/// Lower the IMPORT_NAME pyre HLOp `import_name(fromlist, level, code, frame, -/// name_idx)` → `result: Ref` to `residual_call_ir_r(ConstInt( -/// import_name_fn_idx), ListI([name_idx]), ListR([fromlist, level, code, -/// frame]), Descr) → reg`, the four-Ref sibling of -/// [`lower_getattr_hlop_to_insn`] (same `(refs.., int) → ref` marshalling the -/// void STORE_ATTR residual already proves with `[obj, value, code]`). -/// `bh_import_name_fn` resolves the module name from the code object and runs -/// `__import__` (module top-level Python may force → `MayForce`); `frame` is -/// the live red frame the residual reads `__name__`/`__package__` from for -/// relative-import resolution, mirroring `bh_load_global_fn`'s threaded frame -/// pointer instead of `getexecutioncontext().gettopframe()`. -/// -/// Returns `None` for non-`import_name` opnames so the caller can fall -/// through to other lowering arms. -pub fn lower_import_name_hlop_to_insn( - op: &super::flow::SpaceOperation, - ctx: &LoweringContext, - get_register: &mut F, - lower_constant: &mut LC, -) -> Option -where - F: FnMut(super::flow::Variable) -> Register, - LC: FnMut(&Constant) -> Operand, -{ - if op.opname != "import_name" || op.args.len() != 5 { - return None; - } - let fromlist = operand_for_value_arg(&op.args[0], get_register, lower_constant)?; - let level = operand_for_value_arg(&op.args[1], get_register, lower_constant)?; - let code = operand_for_value_arg(&op.args[2], get_register, lower_constant)?; - let frame = operand_for_value_arg(&op.args[3], get_register, lower_constant)?; - let name_idx = const_int_for_value_arg(&op.args[4])?; - let dst_reg = match &op.result { - Some(super::flow::FlowValue::Variable(var)) => get_register(*var), - _ => return None, - }; - let effect_info = effect_info_for_call_flavor(CallFlavor::MayForce); - let descr_operand = Operand::descr(DescrOperand::CallDescrStub(CallDescrStub { - effect_info, - arg_kinds: vec![Kind::Ref, Kind::Ref, Kind::Ref, Kind::Ref, Kind::Int], - result_kind: Some(Kind::Ref), - void_word_abi: false, - })); - Some(Insn::op_with_result( - "residual_call_ir_r", - vec![ - Operand::ConstInt(ctx.import_name_fn_idx as i64), - Operand::ListOfKind(ListOfKind::new( - Kind::Int, - vec![Operand::ConstInt(name_idx)], - )), - Operand::ListOfKind(ListOfKind::new( - Kind::Ref, - vec![fromlist, level, code, frame], - )), - descr_operand, - ], - dst_reg, - )) -} - /// Lower the IMPORT_FROM pyre HLOp `import_from(module, code, name_idx)` → /// `result: Ref` to `residual_call_ir_r(ConstInt(import_from_fn_idx), /// ListI([name_idx]), ListR([module, code]), Descr) → reg` — the same @@ -11738,7 +11714,6 @@ mod tests { format_simple_fn_idx: 101, format_with_spec_fn_idx: 102, convert_value_fn_idx: 104, - import_name_fn_idx: 105, import_from_fn_idx: 117, load_super_attr_fn_idx: 106, super_attr_unwrap_fn_idx: 107, @@ -13553,112 +13528,69 @@ mod tests { } #[test] - fn lower_import_name_hlop_emits_import_name_fn_residual() { - // `import_name(fromlist, level, code, frame, name_idx)` → - // `residual_call_ir_r(ConstInt(import_name_fn_idx), ListI([name_idx]), - // ListR([fromlist, level, code, frame]), Descr) → reg` (MayForce — - // module top-level Python may run). Four Ref operands plus one Int; - // `frame` is the live red frame the residual reads - // `__name__`/`__package__` from for relative-import resolution. - let fromlist_var = Variable::new(VariableId(8), Kind::Ref); - let level_var = Variable::new(VariableId(10), Kind::Ref); - let frame_var = Variable::new(VariableId(11), Kind::Ref); - let result_var = Variable::new(VariableId(9), Kind::Ref); - let (ctx, code_const, name_idx_const) = load_attr_lowering_fixture(); - let op = super::super::flow::SpaceOperation::new( - "import_name", - vec![ - fromlist_var.into(), - level_var.into(), - code_const.into(), - frame_var.into(), - name_idx_const.into(), - ], - Some(result_var.into()), - 0, - ); - let mut get_register = |var: Variable| match var.id { - VariableId(8) => Register { - kind: Kind::Ref, - index: 101, - }, - VariableId(10) => Register { - kind: Kind::Ref, - index: 103, - }, - VariableId(11) => Register { - kind: Kind::Ref, - index: 104, - }, - VariableId(9) => Register { - kind: Kind::Ref, - index: 102, - }, - _ => panic!("unexpected var id {:?}", var.id), + fn lower_load_import_hlop_emits_builtin_lookup_residual() { + // PyPy IMPORT_NAME performs the builtin lookup separately from its + // ordinary Python call. The lookup therefore has the same one-frame + // analyzed EF_CAN_RAISE residual shape, while the following + // `simple_call` remains visible to the meta-tracer. + let frame = Variable::new(VariableId(8), Kind::Ref); + let result = Variable::new(VariableId(9), Kind::Ref); + let op = SpaceOperation::new("load_import", vec![frame.into()], Some(result.into()), 0); + let ctx = LoweringContext { + load_import_fn_idx: 134, + ..Default::default() }; - let mut lower_constant = super::flatten_constant_operand_for_test; - let insn = super::lower_import_name_hlop_to_insn( - &op, - &ctx, - &mut get_register, - &mut lower_constant, - ) - .expect("5-arg import_name lowering must succeed"); + let mut get_register = identity_register_mapper(); + let mut lower_constant = test_constant_lowering(); + let insn = + lower_load_import_hlop_to_insn(&op, &ctx, &mut get_register, &mut lower_constant) + .expect("load_import lowering must succeed"); + match insn { Insn::Op { opname, args, - result, + result: Some(dst), } => { - assert_eq!(opname, "residual_call_ir_r"); - assert!( - matches!(args[0], Operand::ConstInt(105)), - "import_name_fn pool index, got {:?}", - args[0] - ); + assert_eq!(opname, "residual_call_r_r"); + assert!(matches!(args[0], Operand::ConstInt(134))); match &args[1] { Operand::ListOfKind(list) => { - assert_eq!(list.kind, Kind::Int); - assert!( - matches!(&list.content[..], [Operand::ConstInt(5)]), - "ListI = [name_idx], got {:?}", - list.content - ); + assert_eq!(list.kind, Kind::Ref); + assert!(matches!( + &list.content[..], + [Operand::Register(Register { + kind: Kind::Ref, + index: 8 + })] + )); } - other => panic!("expected ListI, got {other:?}"), + other => panic!("expected one-Ref ListR, got {other:?}"), } + assert_eq!(dst, Register::new(Kind::Ref, 9)); match &args[2] { - Operand::ListOfKind(list) => { - assert_eq!(list.kind, Kind::Ref); - match &list.content[..] { - [ - Operand::Register(fl), - Operand::Register(lv), - Operand::ConstRef(0x2000), - Operand::Register(fr), - ] => { - assert_eq!(fl.index, 101, "leading Ref operand must be fromlist"); - assert_eq!(lv.index, 103, "second Ref operand must be level"); - assert_eq!(fr.index, 104, "fourth Ref operand must be frame"); - } - other => { - panic!( - "ListR must be [fromlist, level, code, frame], got {other:?}" - ) - } + Operand::Descr(descr) => match &**descr { + DescrOperand::CallDescrStub(stub) => { + assert_eq!( + stub.effect_info.pyre_helper, + majit_ir::PyreHelperKind::LoadImport + ); + assert_eq!( + stub.effect_info.extraeffect, + majit_ir::ExtraEffect::CanRaise + ); + assert_eq!( + dispatch_kind_for_effect_info(&stub.effect_info), + CallFlavor::Plain + ); + assert!(!stub.effect_info.has_random_effects()); } - } - other => panic!("expected ListR, got {other:?}"), + other => panic!("expected CallDescrStub, got {other:?}"), + }, + other => panic!("expected call descr, got {other:?}"), } - assert_eq!( - result, - Some(Register { - kind: Kind::Ref, - index: 102 - }), - ); } - _ => panic!("expected Insn::Op, got {insn:?}"), + other => panic!("expected residual_call_r_r, got {other:?}"), } } From 43a467689b01d57b105c3de8ec60ca0dd39727e9 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 22 Aug 2026 07:02:32 +0900 Subject: [PATCH 03/14] gc: hold the bytes payload in a varsize block instead of a storage box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rstr.py:1226-1228` — `STR.become(GcStruct('rpy_string', ('hash', Signed), ('chars', Array(Char, ...))))`: the byte payload is a varsize GcArray inside the managed heap. A storage box holds a `Vec` whose bytes live in the Rust heap, so `alloc_in_oldgen` sizes the payload as the 24 bytes of the container and `bytes_made_old_since_cycle` never learns about the buffer. `BytesBlock` is that `Array(Char)`: a length header followed by the chars, with `BYTES_BLOCK_TOKEN` carrying the `get_array_token` triple and a `TypeInfo::varsize` registration at the tail of the tid chain. `W_BytesObject.data` points at it, allocated through the same stable hook the storage box used, so the address stays put under both collection kinds and `w_bytes_data`'s slice stays valid. The `data` edge and `bytes_object_custom_trace` are unchanged; only the object that edge names is different. Drop `add_storage_memory_pressure` and `majit_gc::add_memory_pressure_estimate` with their last callers. `bytearray` keeps the `Vec` box and its own tid, and no longer reports pressure: reporting it on every allocation moved collection timing enough that `_pyio.py FileIO.readall`'s temporary `memoryview(result)[bytes_read:]` still held a buffer export when `result.resize(bytes_read)` ran, and `test.test_file` `PyOtherFileTests.testIteration` failed with BufferError on all three CI hosts. Assisted-by: Claude --- majit/majit-gc/src/lib.rs | 6 - pyre/pyre-jit-trace/src/descr.rs | 2 +- pyre/pyre-jit/src/eval.rs | 25 +++- pyre/pyre-object/src/bytearrayobject.rs | 4 - pyre/pyre-object/src/bytesobject.rs | 148 ++++++++++++++++++++---- pyre/pyre-object/src/gc_storage.rs | 20 ---- 6 files changed, 149 insertions(+), 56 deletions(-) diff --git a/majit/majit-gc/src/lib.rs b/majit/majit-gc/src/lib.rs index e679b606be7..73d710a0577 100644 --- a/majit/majit-gc/src/lib.rs +++ b/majit/majit-gc/src/lib.rs @@ -2681,12 +2681,6 @@ pub fn add_memory_pressure(size: isize, object: GcRef) { } } -/// `rgc.py add_memory_pressure(estimate)` — the form that names no object, -/// for a caller whose raw allocation hangs off no single translated field. -pub fn add_memory_pressure_estimate(size: isize) { - add_memory_pressure(size, GcRef::NULL); -} - pub fn total_memory_pressure() -> isize { ACTIVE_TOTAL_MEMORY_PRESSURE.get().map_or(0, |hook| hook()) } diff --git a/pyre/pyre-jit-trace/src/descr.rs b/pyre/pyre-jit-trace/src/descr.rs index 5af43cf547c..1ac68bf678f 100644 --- a/pyre/pyre-jit-trace/src/descr.rs +++ b/pyre/pyre-jit-trace/src/descr.rs @@ -1276,7 +1276,7 @@ static W_BYTES_DESCR_GROUP: LazyLock = LazyLock::new(|| { ( "data", pyre_object::bytesobject::BYTES_DATA_OFFSET, - std::mem::size_of::<*const Vec>(), + std::mem::size_of::<*const pyre_object::bytesobject::BytesBlock>(), Type::Ref, false, true, diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 35cfa744cb6..686eafaff7c 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -4223,16 +4223,31 @@ fn build_gc() -> Box { pyre_object::gc_storage::storage_box_destructor::, pyre_object::celldict::set_module_dict_strategy_gc_type_id, ); - // bytes / bytearray `data` storage box (off-GC storage). A leaf - // `Vec` (no inner refs) shared by both types; `bytes_object_custom_trace` - // / `bytearray_object_custom_trace` grey it through the `data` field slot and - // the box tid's drop glue reclaims the buffer on sweep. Keep this id at the - // absolute registration tail. + // bytearray `data` storage box (off-GC storage). A leaf `Vec` (no inner + // refs); `bytearray_object_custom_trace` greys it through the `data` field + // slot and the box tid's drop glue reclaims the buffer on sweep. Keep this + // id at the absolute registration tail. register_leaf_storage_box::( &mut gc, pyre_object::gc_storage::storage_box_destructor::, pyre_object::bytesobject::set_bytes_data_gc_type_id, ); + // `bytes` `data` block — `rstr.py:1226-1228`'s `STR.chars`, an + // `Array(Char)`. A varsize GcArray of bytes with no inner refs, so it + // registers with the shape `get_array_token` reads off that one ARRAY and + // no destructor: the payload is inside the block, so the sweep reclaims it + // with the block and the collector sizes it from the block's own length + // header. `bytes_object_custom_trace` greys it through the `data` field + // slot, the same edge the storage box was reached by. + let bytes_block_token = &pyre_object::bytesobject::BYTES_BLOCK_TOKEN; + let bytes_block_tid = gc.register_type(TypeInfo::varsize( + bytes_block_token.base_size, + bytes_block_token.item_size, + bytes_block_token.len_offset, + false, + Vec::new(), + )); + pyre_object::bytesobject::set_bytes_block_gc_type_id(bytes_block_tid); // Mortal (subclass) `str` `value` WTF-8 buffer storage box (off-GC storage // epic S5). A leaf `Wtf8Buf` (no inner refs); the W_UnicodeObject `value` // gc-pointer edge greys it and the box tid's drop glue reclaims the buffer diff --git a/pyre/pyre-object/src/bytearrayobject.rs b/pyre/pyre-object/src/bytearrayobject.rs index ecbd9b752f9..9e2dfb661cf 100644 --- a/pyre/pyre-object/src/bytearrayobject.rs +++ b/pyre/pyre-object/src/bytearrayobject.rs @@ -88,7 +88,6 @@ impl crate::lltype::GcType for W_BytearrayObject { fn w_bytearray_alloc(buf: Vec) -> PyObjectRef { let length = buf.len(); let alloc = if buf.is_empty() { 0 } else { buf.len() + 1 }; - let payload = buf.capacity(); let data = crate::gc_storage::gc_alloc_storage_box(buf, crate::bytesobject::bytes_data_gc_type_id()); let header = PyObject { @@ -115,9 +114,6 @@ fn w_bytearray_alloc(buf: Vec) -> PyObjectRef { } else { crate::lltype::malloc_typed(body) as PyObjectRef }; - // `buffer.py RawByteBuffer.__init__` reports only once the payload is held - // by a live object; see `w_bytes_from_bytes`. - crate::gc_storage::add_storage_memory_pressure(payload); w_bytearray } diff --git a/pyre/pyre-object/src/bytesobject.rs b/pyre/pyre-object/src/bytesobject.rs index 1f3f19042ee..91ceb710944 100644 --- a/pyre/pyre-object/src/bytesobject.rs +++ b/pyre/pyre-object/src/bytesobject.rs @@ -9,10 +9,11 @@ use crate::pyobject::*; pub static BYTES_TYPE: PyType = crate::pyobject::new_pytype("bytes"); -/// GC-managed byte buffer shared by `bytes` and `bytearray` bodies. +/// GC-managed byte buffer behind a `bytearray` body. /// /// The `Vec` is a leaf (no inner `PyObjectRef`s); its GC box carries only -/// drop glue that reclaims the buffer on sweep. +/// drop glue that reclaims the buffer on sweep. `bytes` holds its payload +/// inline instead, in a [`BytesBlock`]. pub type BytesDataStorage = Vec; /// Runtime-assigned GC type id for [`BytesDataStorage`]. Like the set-items @@ -31,16 +32,130 @@ pub fn bytes_data_gc_type_id() -> u32 { BYTES_DATA_GC_TYPE_ID.load(std::sync::atomic::Ordering::Relaxed) } +/// `rstr.py:1226-1228` — `STR.become(GcStruct('rpy_string', ('hash', Signed), +/// ('chars', Array(Char, ...))))`: the byte payload is a varsize `GcArray` +/// inside the managed heap, not a pointer to memory the collector cannot see. +/// +/// A storage box holds a `Vec` whose bytes live in the Rust heap, so the +/// collector sizes the payload as the 24 bytes of the container and its +/// major-collection threshold never learns about the buffer. The bytes here sit +/// after the length header, so `encode_type_shape`'s varsize rule +/// (`gctypelayout.py`) sizes the block from its own contents and the +/// threshold moves by what was actually allocated. +/// +/// Same block shape as [`crate::object_array::TypedItemsBlock`], with `Char` +/// items instead of words. +#[repr(C)] +pub struct BytesBlock { + /// The GcArray length header — the collector's `ofstolength`. + pub length: usize, + /// `chars` inline after the header; size known only at allocation time. + chars: [u8; 0], +} + +/// Offset of `chars[0]` — the collector's `ofstovar`. +pub const BYTES_BLOCK_CHARS_OFFSET: usize = std::mem::offset_of!(BytesBlock, chars); + +/// Offset of the length header the collector reads as the GcArray length. +pub const BYTES_BLOCK_LEN_OFFSET: usize = std::mem::offset_of!(BytesBlock, length); + +/// `get_array_token(Array(Char))` — the one triple every consumer of this +/// block's shape reads, as `encode_type_shape` reads all three from the one +/// ARRAY. +pub const BYTES_BLOCK_TOKEN: crate::object_array::ArrayToken = crate::object_array::ArrayToken { + base_size: BYTES_BLOCK_CHARS_OFFSET, + item_size: std::mem::size_of::(), + len_offset: BYTES_BLOCK_LEN_OFFSET, +}; + +/// Runtime-assigned GC type id for [`BytesBlock`], published by +/// `pyre-jit::eval` with the other tail registrations. +static BYTES_BLOCK_GC_TYPE_ID: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + +/// Record the GC type id registered for [`BytesBlock`]. +pub fn set_bytes_block_gc_type_id(id: u32) { + BYTES_BLOCK_GC_TYPE_ID.store(id, std::sync::atomic::Ordering::Relaxed); +} + +/// Read the runtime-assigned GC type id for [`BytesBlock`]. +#[majit_macros::dont_look_inside] +pub fn bytes_block_gc_type_id() -> u32 { + BYTES_BLOCK_GC_TYPE_ID.load(std::sync::atomic::Ordering::Relaxed) +} + +/// Allocate a [`BytesBlock`] holding `bytes`. +/// +/// The tier is the stable (old-gen) one the storage box already used: a +/// caller holds the returned block on the unrooted Rust stack while it +/// allocates the object body, and `try_gc_alloc_stable_raw` is the hook whose +/// address survives both collection kinds. It also keeps +/// [`w_bytes_data`]'s `&'static [u8]` pointing at bytes that never move. +/// +/// Falls back to a plain allocation before the GC is up or in a unit test, +/// where the block is immortal, as the storage box's `malloc_raw` fallback is. +pub fn alloc_bytes_block(bytes: &[u8]) -> *mut BytesBlock { + let size = BYTES_BLOCK_CHARS_OFFSET + bytes.len(); + let tid = bytes_block_gc_type_id(); + let raw = if tid != 0 { + crate::gc_hook::try_gc_alloc_stable_raw(tid, size) + } else { + std::ptr::null_mut() + }; + let block = if raw.is_null() { + let layout = bytes_block_layout(bytes.len()); + // SAFETY: the layout is non-zero — the header alone occupies a word. + unsafe { std::alloc::alloc(layout) } + } else { + raw + }; + if block.is_null() { + std::alloc::handle_alloc_error(bytes_block_layout(bytes.len())); + } + // SAFETY: `block` names `size` writable bytes, which is the header + // followed by `bytes.len()` char slots. + unsafe { + let block = block as *mut BytesBlock; + (*block).length = bytes.len(); + std::ptr::copy_nonoverlapping( + bytes.as_ptr(), + std::ptr::addr_of_mut!((*block).chars) as *mut u8, + bytes.len(), + ); + block + } +} + +/// The `[header | chars]` layout of a block holding `len` bytes. +fn bytes_block_layout(len: usize) -> std::alloc::Layout { + std::alloc::Layout::from_size_align( + BYTES_BLOCK_CHARS_OFFSET + len, + std::mem::align_of::(), + ) + .expect("bytes block layout") +} + +/// The `chars` of a block, as `rstr.py`'s `ll_chars` reads them. +/// +/// # Safety +/// `block` must name a live [`BytesBlock`]. +pub unsafe fn bytes_block_chars(block: *const BytesBlock) -> &'static [u8] { + unsafe { + std::slice::from_raw_parts( + std::ptr::addr_of!((*block).chars) as *const u8, + (*block).length, + ) + } +} + /// Python bytes object — immutable byte sequence. /// -/// PyPy: W_BytesObject stores `_value` (RPython string). -/// pyre: stores a heap-allocated `Vec` in a GC-managed non-moving storage -/// box (off-GC storage), same layout as W_BytearrayObject but without -/// setitem/extend. +/// `W_BytesObject._value` is an RPython string, whose `chars` array +/// (`rstr.py:1226-1228`) lives in the managed heap; [`BytesBlock`] is that +/// array. Same layout as W_BytearrayObject but without setitem/extend. #[repr(C)] pub struct W_BytesObject { pub ob_header: PyObject, - pub data: *const Vec, + pub data: *const BytesBlock, pub len: usize, /// Strong references owned by ctypes `_objects` dictionaries. Pyre is a /// tracing-GC runtime, so it has no CPython `ob_refcnt`; this trailing @@ -56,7 +171,7 @@ pub struct W_BytesObject { pub w_weakreflifeline: PyObjectRef, } -/// `W_BytesObject.data` — the pointer to the heap-allocated byte buffer. +/// `W_BytesObject.data` — the pointer to the block holding the bytes. pub const BYTES_DATA_OFFSET: usize = std::mem::offset_of!(W_BytesObject, data); /// `W_BytesObject.len` — the byte count, the analogue of the `strlen` PyPy @@ -89,8 +204,8 @@ impl crate::lltype::GcType for W_BytesObject { /// Allocate a new bytes object from a byte slice. /// -/// The `data` buffer lives in a GC-managed non-moving storage box; the sweep -/// reclaims it through the box tid's drop glue. The `W_BytesObject` body is +/// The `data` block is a varsize GcArray in the managed heap, so the sweep +/// reclaims it with no drop glue to run. The `W_BytesObject` body is /// allocated in GC old-gen (`try_gc_alloc_stable_raw`) so the collector traces /// through it and greys the box, mirroring `w_list_new`/`w_set_new`. Falls back /// to `malloc_typed`/`malloc_raw` when no GC hook is installed (unit tests). @@ -100,7 +215,7 @@ impl crate::lltype::GcType for W_BytesObject { #[majit_macros::dont_look_inside] pub fn w_bytes_from_bytes(bytes: &[u8]) -> PyObjectRef { let len = bytes.len(); - let data = crate::gc_storage::gc_alloc_storage_box(bytes.to_vec(), bytes_data_gc_type_id()); + let data = alloc_bytes_block(bytes); let header = PyObject { ob_type: &BYTES_TYPE as *const PyType, w_class: get_instantiate(&BYTES_TYPE), @@ -122,12 +237,6 @@ pub fn w_bytes_from_bytes(bytes: &[u8]) -> PyObjectRef { } else { crate::lltype::malloc_typed(body) as PyObjectRef }; - // `buffer.py RawByteBuffer.__init__` reports only once `self._buf` holds the - // raw allocation: the report arms the next allocation to collect, so a - // payload still reachable from nothing but a local would be swept out from - // under the object being built. Nothing allocates between here and the - // return. - crate::gc_storage::add_storage_memory_pressure(len); w_bytes } @@ -148,7 +257,7 @@ pub fn w_bytes_subclass_from_bytes(bytes: &[u8], w_class: PyObjectRef) -> PyObje ob_type: &BYTES_TYPE as *const PyType, w_class: crate::gc_roots::shadow_stack_get(root_base), }, - data: crate::lltype::malloc_raw(bytes.to_vec()), + data: alloc_bytes_block(bytes), len: bytes.len(), ctypes_keepalive_refs: 0, w_dict: PY_NULL, @@ -260,8 +369,7 @@ pub unsafe fn w_bytes_getitem(obj: PyObjectRef, index: usize) -> u8 { pub unsafe fn w_bytes_data(obj: PyObjectRef) -> &'static [u8] { unsafe { let b = obj as *const W_BytesObject; - let data_ref: &Vec = &*(*b).data; - data_ref.as_slice() + bytes_block_chars((*b).data) } } diff --git a/pyre/pyre-object/src/gc_storage.rs b/pyre/pyre-object/src/gc_storage.rs index 377988ef37f..92e10a0cb42 100644 --- a/pyre/pyre-object/src/gc_storage.rs +++ b/pyre/pyre-object/src/gc_storage.rs @@ -40,26 +40,6 @@ pub fn gc_alloc_storage_box(value: T, tid: u32) -> *mut T { crate::lltype::malloc_raw(value) } -/// `buffer.py RawByteBuffer.__init__` — report a raw payload's bytes -/// to the collector, the line upstream writes right after the raw malloc. -/// -/// Upstream keeps two buffer representations. `ByteBuffer` (`buffer.py`) -/// holds `['\0'] * n`, a GC-heap list the collector allocates and therefore -/// counts by itself. `RawByteBuffer` puts the same bytes in raw memory and -/// pairs the allocation with `rgc.add_memory_pressure(length)`; -/// `rawstorage.alloc_raw_storage` spells it as `add_memory_pressure=True`. -/// A storage box is the raw representation — the collector registers -/// `size_of::()` and never sees the container's own allocation — so it -/// takes the same report. Without it a `BufferedReader` buffer counts as the -/// 24 bytes of its `Vec` rather than the 128KB it holds, and a heap of them -/// never moves the major-collection threshold. -/// -/// `size` is the payload alone. `incminimark.py raw_malloc_memory_pressure`'s -/// per-allocation term is added by the collector. -pub fn add_storage_memory_pressure(size: usize) { - majit_gc::add_memory_pressure_estimate(size as isize); -} - /// GC-sweep destructor for a storage box built by /// [`gc_alloc_storage_box::`]. /// From 032c0a8b2e9989534dc35bcf081a0167fcd7d939 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 22 Aug 2026 11:36:49 +0900 Subject: [PATCH 04/14] =?UTF-8?q?design:=20add=20=C2=A73.7=20on=20the=20wa?= =?UTF-8?q?rmspot=20split=20and=20the=20profiled-JIT=20outage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Names which half of `apply_jit` runs at build time, which half is unported, and the second codewriter that runs over user code objects at runtime. Records the measured cost of installing any profiler or tracer (1168-2836x, against PyPy 7.3.20's 1.1-4.6x) and attributes it to the JIT dispatch body lacking `execute_frame`'s activation bracket rather than to the folded `is_being_profiled` green, with the falsification that would overturn that reading. Assisted-by: Claude --- pyre/design.md | 94 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/pyre/design.md b/pyre/design.md index bef12dacd9f..f879633a5a4 100644 --- a/pyre/design.md +++ b/pyre/design.md @@ -32,7 +32,8 @@ Rust, and a PyPy-equivalent (pyre) on top of that.** |---|---|---| | RPython the language | **Rust** | The host language is no longer a Python subset; it is a real language with a real type system. See §3.1. | | RPython translator (flowspace → annotator → rtyper) | **majit-translate** (`front/ast` → `flowspace/` → `annotator/` → `rtyper/`) over **Charon LLBC** artifacts | Same pipeline, same module names, run at `cargo build` time over extracted `.ullbc` instead of live bytecode. | -| `jtransform`/codewriter → JitCode | **codewriter/** → JitCode | Identical role. | +| `jtransform`/codewriter → JitCode | **majit-translate `codewriter/`** → JitCode | Same role, at `cargo build` time. pyre additionally runs a *second*, hand-written codewriter over user `CodeObject`s at runtime; see §3.7. | +| `warmspot` — translation-time portal generator | **split**: build-time derivation in majit-translate, hand-written warm entry in pyre-jit | Not a port. `apply_jit` is unwired and `warmspot.rs` is a `pub use` namespace; see §3.7. | | metainterp, optimizer, resume, blackhole | **majit-metainterp / majit-trace** | Line-by-line port of the *tracing* JIT (pyjitpl5 lineage), not the 2007 PE JIT. | | x86/ARM/… hand-written backends (~300k LOC) | **majit-backend-dynasm / -cranelift / -wasm** | Three thin backends behind one trait, current primary dynasm; see §3.4. | | incminimark GC | **majit-gc** (nursery + oldgen + incremental + card marking) | Port of the winner, not of Boehm/refcount/mark-sweep. | @@ -300,6 +301,97 @@ configuration. pyre keeps: --- +### 3.7 The portal boundary: warmspot split in two + +Upstream mints the portal at translation time. `warmspot.apply_jit` — the body +of the translator task literally named "JIT compiler generation" +(`task_pyjitpl_lltype`) — derives each driver's green/red specification +(`make_args_specification`), rewrites the `jit_merge_point` and `can_enter_jit` +markers into calls (`rewrite_jit_merge_point`, `rewrite_can_enter_jits`), and +fills the fields `JitDriverStaticData` declares but never computes. Upstream's +`jitdriver.py` is an attribute container with two executable statements +precisely because warmspot writes the rest. pyre splits that pipeline across two +layers, unevenly: + +- **The derivation and the marker erasure do run at build time**, over Charon + LLBC, in majit-translate's `jtransform` and `CallControl::setup_jitdriver`, + driven from `pyre-jit-trace/build.rs`. The derived green/red layout is + asserted against the real MIR operands and a mismatch fails the build. This + half is at the right layer and is not debt. +- **`apply_jit` itself is unported.** `task_pyjitpl_lltype` assembles every + upstream-shaped argument and then returns `TaskError`, because majit-translate + does not depend on majit-metainterp; `warmspot.rs` is a `pub use` namespace, + not an implementation. Seven `missing_task_leaf` sites exist across that + driver, so the stub is not unique — it is named here because the fields it + would fill are instead written by hand from consumer source. +- **A second codewriter runs at runtime.** majit-translate's + `transform_graph_to_jitcode` consumes a `FunctionGraph` once per build; + pyre-jit's `transform_graph_to_jitcode` consumes a user `CodeObject`, is + fallible, and runs unboundedly. Upstream has one, over the interpreter's own + graphs. This is the A1 debt in this area — it is written, it carries Python + opcode semantics, and it has already produced a wrong answer of exactly the + class N3 names: in a chained blackhole resume `portal_frame_reg` aliased the + caller frame, so an inlined callee's `LOAD_GLOBAL` indexed the caller's + `names` table. A1 is **not** weakened to accommodate it; it stands as a + tracked generation defect whose convergence target is majit-translate's + codewriter. + +**Measured cost, 2026-08-22.** Installing `sys.setprofile`, `sys.settrace` or +`cProfile` costs **1168–2836×** on a hot loop where PyPy 7.3.20 pays +**1.1–4.6×** and stays compiled. It is a total outage, not a reuse failure: +warming *under* the profiler never compiles at all. Event counts match CPython +exactly, so this is a cliff and not a wrong answer. Stated plainly: **a profile +taken on pyre measures the interpreter, not the JIT**, and pdb and coverage.py +are in the same position. + +**The root is a missing bracket, not the folded green.** Upstream brackets the +portal itself: `PyFrame.execute_frame` wraps `dispatch` — the function that +carries the merge point and nothing else — in `ExecutionContext.enter`, +`call_trace`, then `return_trace` and `leave` in `finally` clauses. pyre put +that bracket *inside* the plain dispatch body (`eval_frame_plain_with_resume`) +and left the JIT dispatch body bare: `eval_with_jit_inner` substitutes +`install_current_frame`, which performs only `enter`'s topframeref/f_backref +half, and `CurrentFrameGuard`'s drop, which performs only `leave`'s +topframeref half. Neither emits an event, and `pyre-jit` contains no +`call_trace` or `return_trace` call at all. + +A JIT-activated frame therefore emits no `call` and no `return` event, and the +only thing hiding that is the refusal itself — `frame_tracing_active` sends +every traced or profiled frame down `execute_frame_plain`, which is the +bracketed path. `run_with_jit` states the dependency in the affirmative: it +routes non-JIT-eligible frames through `execute_frame` "so `call_trace` / +`return_trace` frame events still fire". **The gate is not a performance +concession; it is the whole implementation of frame events for JIT-eligible +frames**, and the measured event parity above is produced by it. Restoring the +bracket above the portal is a prerequisite for touching the gate, and it needs +no green. + +**What upstream does not do.** It does not fold the tracing state away. +`ExecutionContext` declares `_immutable_fields_` with `profilefunc?` and +`w_tracefunc?`, yet the recorded traces read both as ordinary fields and guard +them, and the comment directly above that declaration says so: the fields +"should be known to a constant … but they're not". They are cheap because +they sit on the entry bridge, once per frame activation — not because they +disappear. Nor would the declaration help here: `quasi_immut_descr` requires a +constant struct operand, and pyre's `ec` is a portal red (`PYPYJIT_RED_VARS`), +so it is never one. The per-opcode half is a different mechanism again — +`dispatch_bytecode`'s explicit `we_are_jitted()` arm tests the *per-frame* +`w_f_trace` through the virtualizable `debugdata`, not the global tracefunc. + +Where the green does pay is the profiled-call dispatch: `call_valuestack` and +its keyword/ex siblings branch on `get_is_being_profiled()` before +`call_args_and_c_profile`, and a real green folds those branches to nothing in +the unprofiled trace while giving the profiled state its own cell, counter and +procedure token. That is the last step of the repair, not the first. + +**Falsification.** Restoring the activation bracket should leave event counts +unchanged with the gate still in place, and should let the gate's +`profilefunc`/global-tracefunc disjuncts be dropped without losing events. If +events go missing once the bracket is above the portal, the bracket is not what +the gate was standing in for and this entry is wrong. + +--- + ## 4. Norms (operating rules) **N1 — Layering.** majit never depends on pyre. pyre-interpreter stays From d0d98a50a1525fdf06104e63e4a812152902e194 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 22 Aug 2026 22:41:25 +0900 Subject: [PATCH 05/14] _ssl: convert a certificate path with the filesystem converter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `path_string` rejects None and bool and then hands the object to `fspath_buf`, which reads it as a `str`. Its own error message names `bytes` and `os.PathLike` too, and `load_cert_chain(certfile=b'...')` reached `w_str_get_wtf8` holding a `W_BytesObject`, taking its length word out of the middle of the path text. Route through `fsencode_bytes_w` — the `PyUnicode_FSConverter` spelling, which resolves all three — and build the host name from the bytes it returns. `fspath_buf`'s three other callers establish `is_str` first. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/_ssl/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pyre/pyre-interpreter/src/module/_ssl/mod.rs b/pyre/pyre-interpreter/src/module/_ssl/mod.rs index 0333f5fb19b..8645e3f4557 100644 --- a/pyre/pyre-interpreter/src/module/_ssl/mod.rs +++ b/pyre/pyre-interpreter/src/module/_ssl/mod.rs @@ -222,7 +222,13 @@ fn path_string(obj: PyObjectRef) -> Result { "path should be string, bytes, os.PathLike or integer, not NoneType", )); } - Ok(crate::gateway::fspath_buf(obj)? + // `PyUnicode_FSConverter`: a path is named by a `str`, by the filesystem + // `bytes` that `str` encodes to, or by an `os.PathLike`, and all three + // reach the host as those bytes. `fspath_buf` takes a `str` its caller has + // already established — reading a `bytes` through it decodes the payload + // as text and takes a length word out of the middle of the path. + let bytes = crate::gateway::fsencode_bytes_w(obj)?; + Ok(crate::gateway::os_string_from_fs_bytes(&bytes) .to_string_lossy() .into_owned()) } From 5a6d5b5fb2213f16cea1276fa8dd11448574f8c5 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 22 Aug 2026 22:41:25 +0900 Subject: [PATCH 06/14] majit-translate: list bind_builtin_kwargs in REVIEWED_UNROLL_SAFE `bind_builtin_kwargs` took `unroll_safe` when the `__import__` gateway landed. `argument.py:172` carries `@jit.unroll_safe` on `_match_signature`, the keyword-binding loop it mirrors. Assisted-by: Claude --- majit/majit-translate/tests/test_unroll_safe_inventory.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/majit/majit-translate/tests/test_unroll_safe_inventory.rs b/majit/majit-translate/tests/test_unroll_safe_inventory.rs index 71b00366e67..94e20347381 100644 --- a/majit/majit-translate/tests/test_unroll_safe_inventory.rs +++ b/majit/majit-translate/tests/test_unroll_safe_inventory.rs @@ -49,6 +49,10 @@ const REVIEWED_UNROLL_SAFE: &[(&str, &str)] = &[ // No upstream counterpart by name; the loop is a bounded scan of a // fixed-size argument slice. ("leading_non_null_count", "flat builtin-keyword ABI scan"), + // `argument.py:172` carries `@jit.unroll_safe` on `_match_signature`, the + // keyword-binding loop this one mirrors; both are bounded by a signature + // fixed at the callee rather than by the call's arguments. + ("bind_builtin_kwargs", "argument.py _match_signature"), ]; /// `builtins::leading_non_null_count` has carried its own `unroll_safe` From ec68b58e1a55fe2ae8ba2f312bd329ed9e32d4c8 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 22 Aug 2026 22:41:25 +0900 Subject: [PATCH 07/14] gc: register the bytes block tid after every other type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tid is a position in the registration chain, and pyre-object spells several of them as literals — `W_BYTES_GC_TYPE_ID` is 27, `W_LIST_GC_TYPE_ID` is 7 — paired with their registration by `debug_assert_eq!` alone, which a release build drops. Registering the varsize block last moves no existing number. Assisted-by: Claude --- pyre/pyre-jit/src/eval.rs | 43 ++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 686eafaff7c..af86baae824 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -4232,22 +4232,6 @@ fn build_gc() -> Box { pyre_object::gc_storage::storage_box_destructor::, pyre_object::bytesobject::set_bytes_data_gc_type_id, ); - // `bytes` `data` block — `rstr.py:1226-1228`'s `STR.chars`, an - // `Array(Char)`. A varsize GcArray of bytes with no inner refs, so it - // registers with the shape `get_array_token` reads off that one ARRAY and - // no destructor: the payload is inside the block, so the sweep reclaims it - // with the block and the collector sizes it from the block's own length - // header. `bytes_object_custom_trace` greys it through the `data` field - // slot, the same edge the storage box was reached by. - let bytes_block_token = &pyre_object::bytesobject::BYTES_BLOCK_TOKEN; - let bytes_block_tid = gc.register_type(TypeInfo::varsize( - bytes_block_token.base_size, - bytes_block_token.item_size, - bytes_block_token.len_offset, - false, - Vec::new(), - )); - pyre_object::bytesobject::set_bytes_block_gc_type_id(bytes_block_tid); // Mortal (subclass) `str` `value` WTF-8 buffer storage box (off-GC storage // epic S5). A leaf `Wtf8Buf` (no inner refs); the W_UnicodeObject `value` // gc-pointer edge greys it and the box tid's drop glue reclaims the buffer @@ -4425,6 +4409,33 @@ fn build_gc() -> Box { gc.register_type(TypeInfo::with_gc_ptrs(size, offsets)) }); + // `bytes` `data` block — `rstr.py:1226-1228`'s `STR.chars`, an + // `Array(Char)`. A varsize GcArray of bytes with no inner refs, so it + // registers with the shape `get_array_token` reads off that one ARRAY and + // no destructor: the payload is inside the block, so the sweep reclaims it + // with the block and the collector sizes it from the block's own length + // header. `bytes_object_custom_trace` greys it through the `data` field + // slot, the same edge the storage box was reached by. + // + // Registered after every other type, synthetic structs included. A tid is a + // position in this chain, and the interpreter spells many of them as + // literals — `W_BYTES_GC_TYPE_ID` is 27, `W_LIST_GC_TYPE_ID` is 7 — so an + // insertion anywhere earlier renumbers every registration below it while the + // `debug_assert_eq!`s that pair each literal with its registration are + // compiled out of a release build. Allocations made before this line read a + // zero tid and take `alloc_bytes_block`'s plain-allocation arm; the trace + // skips those blocks on `try_gc_owns_object`, as it did for the storage box + // from its own later registration point. + let bytes_block_token = &pyre_object::bytesobject::BYTES_BLOCK_TOKEN; + let bytes_block_tid = gc.register_type(TypeInfo::varsize( + bytes_block_token.base_size, + bytes_block_token.item_size, + bytes_block_token.len_offset, + false, + Vec::new(), + )); + pyre_object::bytesobject::set_bytes_block_gc_type_id(bytes_block_tid); + // ── GC-root registration completeness oracle ───────────────────────── // Every `#[pyre_class]` type appends its descriptor to the whole-program // `PYRE_CLASS_DESCRIPTORS` slice. A type with inline managed children must From de20424bb6e8f973bbba2c04e2d7a856d3fba388 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 23 Aug 2026 01:07:54 +0900 Subject: [PATCH 08/14] Bound the builtin positional copy, unpublish a fat-pointer helper, and share the __import__ lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bind_builtin_kwargs` wrote `scope[i]` for every entry in `positional`, but `scope` and `filled` are `names.len()` long and `clinic_arity` bounds the count of non-null entries rather than the slice, so a null-padded slice longer than the signature indexed out of bounds. `_match_signature` takes `take = min(num_args, co_argcount - upfront)` for the same reason its comment gives — "take is always smaller than co_argcount" is what makes the unrolled loop safe. `builtin_unexpected_keyword_failure` no longer publishes its address: its `&str` and `&Wtf8` arguments are two-word aggregates and its `Result, PyError>` return is multiword, so the one-word residual-call ABI would pass and return the wrong number of words. `bind_builtin_kwargs` is `unroll_safe`, so the codewriter descends into it and reaches that `#[cold]` call as a residual; with no address it falls back to the symbolic hash, the way `stack_underflow_error` and `drain_collect_items` already do. `bh_load_import_fn` and `importing::import_name` each carried their own copy of the builtin `__import__` lookup; both now call `importing::lookup_dunder_import`. Assisted-by: Claude --- pyre/pyre-interpreter/src/builtins.rs | 14 ++++++++----- pyre/pyre-interpreter/src/importing.rs | 26 +++++++++++++++++-------- pyre/pyre-interpreter/src/jit_fnaddr.rs | 14 +++++++------ pyre/pyre-jit/src/call_jit.rs | 23 +++++----------------- 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 427db732f49..06a440029b3 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -5090,12 +5090,16 @@ pub(crate) fn bind_builtin_kwargs( let keyword_entries = kwargs.map(|dict| unsafe { pyre_object::w_dict_str_entries_wtf8(dict) }); let mut unknown: Option = None; // PyPy `_match_signature` copies positional values with - // `for i in range(take)` so the constant signature bounds let the JIT - // unroll ordinary indexed reads. Keep that storage shape instead of Rust - // iterator adapters, whose `Filter`/`Enumerate` state has no RPython - // counterpart. + // `take = min(num_args, co_argcount - upfront)` and `for i in range(take)`, + // so the constant signature bounds let the JIT unroll ordinary indexed + // reads. Keep that storage shape instead of Rust iterator adapters, whose + // `Filter`/`Enumerate` state has no RPython counterpart — and keep the + // `min`: `scope` and `filled` are `names.len()` long, while `clinic_arity` + // bounds the count of non-null entries rather than the slice itself, so a + // null-padded `positional` longer than the signature reaches here. + let take = positional.len().min(names.len()); let mut positional_index = 0; - while positional_index < positional.len() { + while positional_index < take { let value = positional[positional_index]; scope[positional_index] = value; filled[positional_index] = !value.is_null(); diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 82f44c07baa..8dbbed0ddf7 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -4542,12 +4542,12 @@ fn absolute_import( // ── IMPORT_NAME ────────────────────────────────────────────────────── /// PyPy equivalent: pyopcode.py `IMPORT_NAME`. -pub fn import_name( - frame: &mut PyFrame, - w_modulename: PyObjectRef, - w_fromlist: PyObjectRef, - w_flag: PyObjectRef, -) -> Result { +/// `pyopcode.py` `IMPORT_NAME`'s `self.get_builtin().getdictvalue(space, +/// '__import__')`, on its own so the interpreter and the JIT's `LoadImport` +/// residual read the importer the same way. The `is_module` test has no +/// upstream counterpart: `get_builtin` can hand back a plain mapping, which +/// carries no module dict to look the name up in. +pub fn lookup_dunder_import(frame: &PyFrame) -> Result { let w_builtin = frame.get_builtin(); let w_import = if !w_builtin.is_null() && unsafe { is_module(w_builtin) } { let w_dict = unsafe { pyre_object::w_module_get_w_dict(w_builtin) }; @@ -4558,8 +4558,18 @@ pub fn import_name( } } else { None - } - .ok_or_else(|| crate::PyError::new(crate::PyErrorKind::ImportError, "__import__ not found"))?; + }; + w_import + .ok_or_else(|| crate::PyError::new(crate::PyErrorKind::ImportError, "__import__ not found")) +} + +pub fn import_name( + frame: &mut PyFrame, + w_modulename: PyObjectRef, + w_fromlist: PyObjectRef, + w_flag: PyObjectRef, +) -> Result { + let w_import = lookup_dunder_import(frame)?; let w_locals = match frame.getdebug() { Some(d) if !d.w_locals.is_null() => d.w_locals, diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index e65b07e33c7..2aaa3bc3ec2 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -395,12 +395,14 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { "builtins::builtin_kwargs_marker_dict", crate::builtins::builtin_kwargs_marker_dict as *const (), ); - push_alias_pair( - &mut entries, - "pyre_interpreter::builtins::builtin_unexpected_keyword_failure", - "builtins::builtin_unexpected_keyword_failure", - crate::builtins::builtin_unexpected_keyword_failure as *const (), - ); + // `builtin_unexpected_keyword_failure` deliberately remains unpublished: + // its `&str` and `&Wtf8` arguments are two-word aggregates and its + // `Result, PyError>` return is multiword, neither of + // which the one-word residual-call ABI carries. `bind_builtin_kwargs` is + // `unroll_safe`, so the codewriter descends into it and reaches this + // `#[cold]` `#[dont_look_inside]` call as a residual; without an address + // it falls back to the symbolic hash instead of passing and returning the + // wrong number of words. // RPython annotator PBC parity for `BuiltinCode.func`: every generated // interp2app wrapper is a possible value of the indirect function-pointer diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index f13540d4d6f..4b7f1c34b81 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -6491,26 +6491,13 @@ pub extern "C" fn bh_load_import_fn(frame_ptr: i64) -> i64 { publish_residual_call_exception(err.to_exc_object() as i64); return 0; } - let w_builtin = unsafe { (*frame).get_builtin() }; - if !w_builtin.is_null() && unsafe { pyre_object::is_module(w_builtin) } { - let w_dict = unsafe { pyre_object::w_module_get_w_dict(w_builtin) }; - if !w_dict.is_null() { - match pyre_interpreter::baseobjspace::finditem_str(w_dict, "__import__") { - Ok(Some(value)) => return value as i64, - Ok(None) => {} - Err(mut err) => { - publish_residual_call_exception(err.to_exc_object() as i64); - return 0; - } - } + match pyre_interpreter::importing::lookup_dunder_import(unsafe { &*frame }) { + Ok(w_import) => w_import as i64, + Err(mut err) => { + publish_residual_call_exception(err.to_exc_object() as i64); + 0 } } - let mut err = pyre_interpreter::PyError::new( - pyre_interpreter::PyErrorKind::ImportError, - "__import__ not found", - ); - publish_residual_call_exception(err.to_exc_object() as i64); - 0 } /// DELETE_GLOBAL residual using the frame receiver and interned-name ABI. From 763d8c78284fa580e1259ff40419dcf97a52e6ee Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 23 Aug 2026 07:15:02 +0900 Subject: [PATCH 09/14] gc: root the bytes block across the allocations that follow it `build_list_storage` states the rule: old-gen is mark-sweep, so a block with no heap edge yet is sweepable rather than merely immobile, and it has to stay rooted across every later GC operation. `w_bytes_from_bytes` held the block in a bare local across `get_instantiate` and `try_gc_alloc_stable_raw`, both of which allocate; `w_bytes_subclass_from_bytes` had the inverse, evaluating `alloc_bytes_block` inside the struct literal that is built after `try_gc_alloc_stable_raw` has already produced an unrooted body. Both now pin on the shadow stack and read the value back from its slot once the last allocation is behind them. `BytesBlock` gains a layout assertion: the collector sizes the block from `len_offset` and `bytes_block_chars` reads the payload at `base_size`, so a new field would move both silently. Assisted-by: Claude --- pyre/pyre-object/src/bytesobject.rs | 44 +++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/pyre/pyre-object/src/bytesobject.rs b/pyre/pyre-object/src/bytesobject.rs index 91ceb710944..58021a48cd3 100644 --- a/pyre/pyre-object/src/bytesobject.rs +++ b/pyre/pyre-object/src/bytesobject.rs @@ -68,6 +68,16 @@ pub const BYTES_BLOCK_TOKEN: crate::object_array::ArrayToken = crate::object_arr len_offset: BYTES_BLOCK_LEN_OFFSET, }; +// The collector sizes a block as `base_size + item_size * length` read at +// `len_offset`, and `bytes_block_chars` reads the payload at `base_size`. A +// field added to `BytesBlock` moves both silently, so pin the shape here: the +// length header first, the chars one word in, and one byte per item. +const _: () = { + assert!(BYTES_BLOCK_LEN_OFFSET == 0); + assert!(BYTES_BLOCK_CHARS_OFFSET == std::mem::size_of::()); + assert!(BYTES_BLOCK_TOKEN.item_size == 1); +}; + /// Runtime-assigned GC type id for [`BytesBlock`], published by /// `pyre-jit::eval` with the other tail registrations. static BYTES_BLOCK_GC_TYPE_ID: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); @@ -215,20 +225,31 @@ impl crate::lltype::GcType for W_BytesObject { #[majit_macros::dont_look_inside] pub fn w_bytes_from_bytes(bytes: &[u8]) -> PyObjectRef { let len = bytes.len(); + // `build_list_storage` (listobject.rs) states the rule the block obeys: + // old-gen is mark-sweep, so a block with no heap edge yet is sweepable + // rather than merely immobile, and it has to be rooted across every later + // GC operation. `try_gc_alloc_stable_raw` is one of them + // (`IntArray::pin_block`), and `get_instantiate` allocates as well, so both + // the block and the class travel on the shadow stack and are read back from + // their slots once the last allocation is behind them. + let _roots = crate::gc_roots::push_roots(); + let data_slot = crate::gc_roots::shadow_stack_len(); let data = alloc_bytes_block(bytes); - let header = PyObject { - ob_type: &BYTES_TYPE as *const PyType, - w_class: get_instantiate(&BYTES_TYPE), - }; + let _ = crate::gc_roots::pin_root(data as PyObjectRef); + let class_slot = crate::gc_roots::shadow_stack_len(); + let _ = crate::gc_roots::pin_root(get_instantiate(&BYTES_TYPE)); + let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_BYTES_GC_TYPE_ID, W_BYTES_OBJECT_SIZE); let body = W_BytesObject { - ob_header: header, - data, + ob_header: PyObject { + ob_type: &BYTES_TYPE as *const PyType, + w_class: crate::gc_roots::shadow_stack_get(class_slot), + }, + data: crate::gc_roots::shadow_stack_get(data_slot) as *const BytesBlock, len, ctypes_keepalive_refs: 0, w_dict: PY_NULL, w_weakreflifeline: PY_NULL, }; - let raw = crate::gc_hook::try_gc_alloc_stable_raw(W_BYTES_GC_TYPE_ID, W_BYTES_OBJECT_SIZE); let w_bytes = if !raw.is_null() { unsafe { std::ptr::write(raw as *mut W_BytesObject, body); @@ -248,6 +269,13 @@ pub fn w_bytes_subclass_from_bytes(bytes: &[u8], w_class: PyObjectRef) -> PyObje let _roots = crate::gc_roots::push_roots(); let root_base = crate::gc_roots::shadow_stack_len(); let _ = crate::gc_roots::pin_root(w_class); + // The block is allocated before the body and rooted across it, not built + // inside the struct literal: the literal is evaluated after + // `try_gc_alloc_stable_raw` has already produced `raw`, which leaves that + // fresh body unrooted across the block's own allocation. + let data_slot = crate::gc_roots::shadow_stack_len(); + let data = alloc_bytes_block(bytes); + let _ = crate::gc_roots::pin_root(data as PyObjectRef); let raw = crate::gc_hook::try_gc_alloc_stable_raw( ::type_id(), ::SIZE, @@ -257,7 +285,7 @@ pub fn w_bytes_subclass_from_bytes(bytes: &[u8], w_class: PyObjectRef) -> PyObje ob_type: &BYTES_TYPE as *const PyType, w_class: crate::gc_roots::shadow_stack_get(root_base), }, - data: alloc_bytes_block(bytes), + data: crate::gc_roots::shadow_stack_get(data_slot) as *const BytesBlock, len: bytes.len(), ctypes_keepalive_refs: 0, w_dict: PY_NULL, From 5e026774991886eaf067d989a4f0acdff898d6c4 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 23 Aug 2026 07:18:52 +0900 Subject: [PATCH 10/14] jit: repair a garbled sentence in the IMPORT_NAME comment Assisted-by: Claude --- pyre/pyre-jit/src/jit/codewriter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index 74c1864d265..b2f247f059c 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -12281,8 +12281,8 @@ impl CodeWriter { // `(name, globals, None, fromlist, level)`. Keeping // the lookup and CallFn separate is load-bearing: the // latter can descend through BuiltinCode.func and - // trace `_gcd_import`; the old monolithic - // one monolithic residual hid the entire importer. + // trace `_gcd_import`; one monolithic residual hid + // the entire importer. Instruction::ImportName { namei } => { let name_idx = namei.get(op_arg) as usize; let _ = emit_popvalue_ref!(current_depth, py_pc); From b2625aed5337d0bf22a9034d34101980ba3566c3 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 23 Aug 2026 07:25:14 +0900 Subject: [PATCH 11/14] _ssl: carry certificate paths as filesystem paths, not text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `path_string` decoded the filesystem bytes it had just produced with `to_string_lossy`, and `set_default_verify_paths` did the same to `SSL_CERT_FILE`/`SSL_CERT_DIR` with `String::from_utf8_lossy`. A name the filesystem accepts but UTF-8 does not collapses onto a `U+FFFD` spelling that names no file, and two such names collapse onto the same one. The converter is now `fs_path` and returns `PathBuf`; the four native entry points that consume one — `context_load_cert_chain`, `context_load_verify_file`, `context_add_verify_dir`, `certificate_decode_file` — take `&Path`. Each of them already did nothing with the argument but hand it to `std::fs::read` or `PathBuf::from`. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/_ssl/mod.rs | 40 ++++++++++---------- pyre/pyre-native/src/ssl.rs | 15 +++++--- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/pyre/pyre-interpreter/src/module/_ssl/mod.rs b/pyre/pyre-interpreter/src/module/_ssl/mod.rs index 8645e3f4557..9aabfe019d8 100644 --- a/pyre/pyre-interpreter/src/module/_ssl/mod.rs +++ b/pyre/pyre-interpreter/src/module/_ssl/mod.rs @@ -216,7 +216,7 @@ fn tls_result(result: pyre_native::ssl::TlsResult) -> Result Result { +fn fs_path(obj: PyObjectRef) -> Result { if obj.is_null() || unsafe { is_none(obj) || is_bool(obj) } { return Err(crate::PyError::type_error( "path should be string, bytes, os.PathLike or integer, not NoneType", @@ -227,10 +227,12 @@ fn path_string(obj: PyObjectRef) -> Result { // reach the host as those bytes. `fspath_buf` takes a `str` its caller has // already established — reading a `bytes` through it decodes the payload // as text and takes a length word out of the middle of the path. + // Those bytes stay a path all the way to the host call. Decoding them to + // text is lossy on a name the filesystem accepts but UTF-8 does not: two + // distinct files collapse onto one `U+FFFD` spelling, and re-encoding that + // spelling names neither of them. let bytes = crate::gateway::fsencode_bytes_w(obj)?; - Ok(crate::gateway::os_string_from_fs_bytes(&bytes) - .to_string_lossy() - .into_owned()) + Ok(crate::gateway::os_string_from_fs_bytes(&bytes).into()) } fn password_bytes(obj: PyObjectRef) -> Result>, crate::PyError> { @@ -824,9 +826,9 @@ mod context_methods { let password = crate::builtins::bind_pos_or_kw(user, kwargs, 2, "password", "load_cert_chain", 3)?; crate::builtins::kwarg_reject_unknown(kwargs, KEYWORDS, "load_cert_chain")?; - let cert_path = path_string(cert)?; + let cert_path = fs_path(cert)?; let key_path = match key { - Some(value) if !unsafe { is_none(value) } => path_string(value)?, + Some(value) if !unsafe { is_none(value) } => fs_path(value)?, _ => cert_path.clone(), }; // OpenSSL asks its callback only after parsing discovers an @@ -897,14 +899,14 @@ mod context_methods { )); } if let Some(cafile) = cafile { - let path = path_string(cafile)?; + let path = fs_path(cafile)?; native_result(unsafe { pyre_native::ssl::context_load_verify_file(self.backend, &path) })?; } if let Some(capath) = capath { - let path = path_string(capath)?; - if !std::path::Path::new(&path).is_dir() { + let path = fs_path(capath)?; + if !path.is_dir() { return Err(crate::PyError::os_error_with_errno( libc::ENOENT, "CA directory does not exist", @@ -954,14 +956,14 @@ mod context_methods { /// the first and `SSL_CERT_DIR` only the second, so neither variable /// may suppress the other's source. fn set_default_verify_paths(&mut self) -> Result<(), crate::PyError> { + // `SSL_CERT_FILE` and `SSL_CERT_DIR` name files, so their bytes + // reach the host as a path for the same reason `fs_path`'s do. let env_path = |name: &[u8]| { - crate::host_seam::getenv(name) - .ok() - .flatten() - .map(|value| String::from_utf8_lossy(&value).into_owned()) + crate::host_seam::getenv(name).ok().flatten().map(|value| { + std::path::PathBuf::from(crate::gateway::os_string_from_fs_bytes(&value)) + }) }; - let cert_file = - env_path(b"SSL_CERT_FILE").filter(|path| std::path::Path::new(path).is_file()); + let cert_file = env_path(b"SSL_CERT_FILE").filter(|path| path.is_file()); match cert_file { Some(path) => native_result(unsafe { pyre_native::ssl::context_load_verify_file(self.backend, &path) @@ -973,8 +975,8 @@ mod context_methods { .map(|_| ())?, } let (_, default_dir) = pyre_native::ssl::default_verify_paths(); - let cert_dir = env_path(b"SSL_CERT_DIR").unwrap_or(default_dir); - if std::path::Path::new(&cert_dir).is_dir() { + let cert_dir = env_path(b"SSL_CERT_DIR").unwrap_or_else(|| default_dir.into()); + if cert_dir.is_dir() { // OpenSSL defers hashed directory loading until chain lookup, // so it does not contribute to cert_store_stats here. unsafe { pyre_native::ssl::context_add_verify_dir(self.backend, &cert_dir) }; @@ -1080,7 +1082,7 @@ mod context_methods { "load_dh_params() missing required path argument", )); } - let path = path_string(path)?; + let path = fs_path(path)?; let data = std::fs::read(&path).map_err(|error| { crate::PyError::os_error_with_errno( error.raw_os_error().unwrap_or(libc::EIO), @@ -2592,7 +2594,7 @@ mod cert_store { } fn test_decode_cert(args: &[PyObjectRef]) -> Result { - let path = path_string(args[0])?; + let path = fs_path(args[0])?; let cert = native_result(pyre_native::ssl::certificate_decode_file(&path))?; Ok(decoded_certificate_dict(cert)) } diff --git a/pyre/pyre-native/src/ssl.rs b/pyre/pyre-native/src/ssl.rs index efea179bd14..617ad9914b7 100644 --- a/pyre/pyre-native/src/ssl.rs +++ b/pyre/pyre-native/src/ssl.rs @@ -533,8 +533,8 @@ fn read_private_key(data: &[u8], password: Option<&[u8]>) -> NativeResult, ) -> NativeResult { ensure_provider(); @@ -588,7 +588,10 @@ fn parse_concatenated_der(mut data: &[u8]) -> NativeResult>> { /// # Safety /// `context` must point to a live [`Context`]. #[inline(never)] -pub unsafe fn context_load_verify_file(context: *mut Context, path: &str) -> NativeResult { +pub unsafe fn context_load_verify_file( + context: *mut Context, + path: &std::path::Path, +) -> NativeResult { let data = std::fs::read(path).map_err(io_error)?; let items = rustls_pemfile::read_all(&mut Cursor::new(data)) .collect::, _>>() @@ -627,9 +630,9 @@ pub unsafe fn context_load_verify_file(context: *mut Context, path: &str) -> Nat /// # Safety /// `context` must point to a live [`Context`]. #[inline(never)] -pub unsafe fn context_add_verify_dir(context: *mut Context, path: &str) { +pub unsafe fn context_add_verify_dir(context: *mut Context, path: &std::path::Path) { let context = unsafe { &mut *context }; - let path = std::path::PathBuf::from(path); + let path = path.to_path_buf(); if !context.capaths.iter().any(|known| known == &path) { context.capaths.push(path); *context @@ -988,7 +991,7 @@ pub fn certificate_decode_der(der: &[u8]) -> NativeResult<*mut DecodedCertificat } #[inline(never)] -pub fn certificate_decode_file(path: &str) -> NativeResult<*mut DecodedCertificate> { +pub fn certificate_decode_file(path: &std::path::Path) -> NativeResult<*mut DecodedCertificate> { let data = std::fs::read(path).map_err(io_error)?; let certs = read_pem_certificates(&data)?; certificate_decode_der(certs[0].as_ref()) From b7eeedba747faa5f87753636d8bfd2069ad621c4 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 23 Aug 2026 10:21:49 +0900 Subject: [PATCH 12/14] jit: pass IMPORT_NAME's locals argument instead of a baked None MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pyopcode.py:1119-1125` reads the frame's debug locals and substitutes `None` only when the frame has none; `importing.rs` import_name does the same. The traced IMPORT_NAME arm passed `None` unconditionally. Measured on a class body, whose locals mapping is a real dict: CPython hands the namespace to `__import__` on all 40000 iterations, and this tree handed it over for the first 1040 and then `None` for the remaining 38959 — the switch happens at the loop-compile threshold, so the interpreted iterations were right and every compiled one was wrong. A function frame has no locals mapping and a module frame's is its globals, so neither shape shows the difference; the new parity test uses the class body. `load_import_locals` is the frame-receiver residual for the read, alongside `load_import` for the builtin lookup, and shares `importing::import_locals` with the interpreter. `bh_load_locals_fn` could not be reused: LOAD_LOCALS spells `getorcreatedebug()`, which would create a mapping where upstream passes `None`. Assisted-by: Claude --- majit/majit-ir/src/effectinfo.rs | 4 + .../import_name_cached_name_and_rebind_jit.py | 10 ++ .../import_name_class_body_locals_jit.py | 38 +++++++ pyre/pyre-interpreter/src/importing.rs | 17 +++- pyre/pyre-jit/src/call_jit.rs | 14 +++ pyre/pyre-jit/src/jit/codewriter.rs | 26 ++++- pyre/pyre-jit/src/jit/cpu.rs | 5 + pyre/pyre-jit/src/jit/flatten.rs | 98 +++++++++++++++++++ 8 files changed, 207 insertions(+), 5 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/import_name_class_body_locals_jit.py diff --git a/majit/majit-ir/src/effectinfo.rs b/majit/majit-ir/src/effectinfo.rs index bba5b61d02b..3dcbd28a3ec 100644 --- a/majit/majit-ir/src/effectinfo.rs +++ b/majit/majit-ir/src/effectinfo.rs @@ -905,6 +905,10 @@ pub enum PyreHelperKind { /// The following invocation is emitted through [`PyreHelperKind::CallFn`] /// so gateway builtins retain their ordinary meta-traceable call shape. LoadImport, + /// `bh_load_import_locals_fn(frame)` — IMPORT_NAME's locals argument + /// (`pyopcode.py:1119-1125`). Infallible, same standing as + /// [`PyreHelperKind::LoadLocals`]. + LoadImportLocals, /// `bh_call_fn_N(callable, null_or_self, args...)` — the CALL-family /// Python-call helper. `null_or_self` (arg index 1) is a sentinel /// the helper checks before use (a non-null receiver is prepended as diff --git a/pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py b/pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py index 9dab7cb1b9c..c0a1d3c28c8 100644 --- a/pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py +++ b/pyre/extra_tests/parity_tests/import_name_cached_name_and_rebind_jit.py @@ -22,6 +22,11 @@ def first(name, globals_arg, locals_arg, fromlist, level): assert name is expected_name assert globals_arg is expected_globals assert locals_arg is None + # `import os` compiles to IMPORT_NAME with no fromlist and an absolute + # level. Both hooks return os_module regardless, so a wrong value + # reaches nothing that would fail unless it is asserted here. + assert fromlist is None + assert level == 0 calls[0] += 1 return os_module @@ -29,6 +34,11 @@ def second(name, globals_arg, locals_arg, fromlist, level): assert name is expected_name assert globals_arg is expected_globals assert locals_arg is None + # `import os` compiles to IMPORT_NAME with no fromlist and an absolute + # level. Both hooks return os_module regardless, so a wrong value + # reaches nothing that would fail unless it is asserted here. + assert fromlist is None + assert level == 0 calls[1] += 1 return os_module diff --git a/pyre/extra_tests/parity_tests/import_name_class_body_locals_jit.py b/pyre/extra_tests/parity_tests/import_name_class_body_locals_jit.py new file mode 100644 index 00000000000..806d64fa102 --- /dev/null +++ b/pyre/extra_tests/parity_tests/import_name_class_body_locals_jit.py @@ -0,0 +1,38 @@ +# CPython-suite gap: the import-hook tests never put IMPORT_NAME inside a class +# body whose loop runs hot enough to compile, so nothing covers the one frame +# kind whose locals mapping is a real dict rather than None. +# parity-tests reason: `pyopcode.py:1119-1125` reads the frame's debug locals +# and substitutes None only when the frame has none. A class body has one, so +# a traced IMPORT_NAME that bakes None is visible to any custom __import__. + +import builtins + +old_import = builtins.__import__ +os_module = old_import("os") +seen = [] + + +def hook(name, globals_arg, locals_arg, fromlist, level): + seen.append(locals_arg is None) + return os_module + + +N = 40000 +builtins.__import__ = hook +try: + + class C: + i = 0 + while i < N: + import os + + i += 1 + +finally: + builtins.__import__ = old_import + +assert len(seen) == N, len(seen) +# A baked None shows up only once the loop compiles, so report the iteration +# it starts at rather than the whole list. +assert not any(seen), seen.index(True) +print("OK") diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 8dbbed0ddf7..5dd6f7ae37a 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -4563,6 +4563,18 @@ pub fn lookup_dunder_import(frame: &PyFrame) -> Result PyObjectRef { + match frame.getdebug() { + Some(d) if !d.w_locals.is_null() => d.w_locals, + _ => pyre_object::w_none(), + } +} + pub fn import_name( frame: &mut PyFrame, w_modulename: PyObjectRef, @@ -4571,10 +4583,7 @@ pub fn import_name( ) -> Result { let w_import = lookup_dunder_import(frame)?; - let w_locals = match frame.getdebug() { - Some(d) if !d.w_locals.is_null() => d.w_locals, - _ => pyre_object::w_none(), - }; + let w_locals = import_locals(frame); let w_globals = frame.get_w_globals(); crate::call::call_callable( frame, diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 4b7f1c34b81..1eb8a1a1dad 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -6500,6 +6500,20 @@ pub extern "C" fn bh_load_import_fn(frame_ptr: i64) -> i64 { } } +/// IMPORT_NAME's locals argument, split from the call for the same reason as +/// [`bh_load_import_fn`]. Infallible — it peeks the debug slot rather than +/// creating one — so like `bh_load_locals_fn` it has no exception-publishing +/// arm. +pub extern "C" fn bh_load_import_locals_fn(frame_ptr: i64) -> i64 { + assert!( + frame_ptr != 0, + "bh_load_import_locals_fn requires a non-null PyFrame; every IMPORT_NAME \ + emit site must thread portal_frame_reg as its ref operand" + ); + let frame = unsafe { &*(frame_ptr as *mut PyFrame) }; + pyre_interpreter::importing::import_locals(frame) as i64 +} + /// DELETE_GLOBAL residual using the frame receiver and interned-name ABI. /// pyopcode.py DELETE_GLOBAL deletes directly from `w_globals`. pub extern "C" fn bh_delete_global_fn(frame_ptr: i64, w_name: i64) -> i64 { diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index b2f247f059c..3b9bd05092f 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -3570,6 +3570,7 @@ struct FnPtrIndices { load_locals_fn: HelperHandle, load_build_class_fn: HelperHandle, load_import_fn: HelperHandle, + load_import_locals_fn: HelperHandle, load_from_dict_or_globals_fn: HelperHandle, call_function_ex_fn: HelperHandle, unary_not_fn: HelperHandle, @@ -4299,6 +4300,11 @@ fn register_helper_fn_pointers( cpu.load_import_fn as *const (), CallFlavor::Plain, ); + let load_import_locals_fn = bind( + assembler, + cpu.load_import_locals_fn as *const (), + CallFlavor::Plain, + ); // The hand-written PUSH_EXC_INFO lowering must complete the interpreter's // caught-exception ownership transfer. Bind last so every existing // helper index remains stable. @@ -4402,6 +4408,7 @@ fn register_helper_fn_pointers( load_locals_fn, load_build_class_fn, load_import_fn, + load_import_locals_fn, load_from_dict_or_globals_fn, call_function_ex_fn, call_kw_fn_0, @@ -6418,6 +6425,11 @@ impl CodeWriter { idx: load_import_fn_idx, flavor: _load_import_fn_flavor, }, + load_import_locals_fn: + HelperHandle { + idx: load_import_locals_fn_idx, + flavor: _load_import_locals_fn_flavor, + }, load_from_dict_or_globals_fn: HelperHandle { idx: load_from_dict_or_globals_fn_idx, @@ -6695,6 +6707,7 @@ impl CodeWriter { load_locals_fn_idx, load_build_class_fn_idx, load_import_fn_idx, + load_import_locals_fn_idx, load_from_dict_or_globals_fn_idx, call_function_ex_fn_idx, unary_not_fn_idx, @@ -12329,6 +12342,17 @@ impl CodeWriter { py_pc as i64, ) .into(); + // `pyopcode.py:1119-1125` reads the frame's debug + // locals and substitutes `None` only when the frame + // has none. A baked `None` here would hide a + // materialized mapping from a custom `__import__`. + let locals_value = emit_frontend_frame_only_ref( + &mut graph, + ¤t_block.block(), + "load_import_locals", + frame_var.into(), + py_pc as i64, + ); let result_value = emit_frontend_simple_call( &mut graph, ¤t_block.block(), @@ -12337,7 +12361,7 @@ impl CodeWriter { vec![ name_value, globals_value, - pyobject_const_ref_value(pyre_object::w_none()), + locals_value.into(), fromlist_value, level_value, ], diff --git a/pyre/pyre-jit/src/jit/cpu.rs b/pyre/pyre-jit/src/jit/cpu.rs index 3153060f7fb..bfdaac4fb90 100644 --- a/pyre/pyre-jit/src/jit/cpu.rs +++ b/pyre/pyre-jit/src/jit/cpu.rs @@ -343,6 +343,10 @@ pub struct Cpu { /// call itself so the generated jitcode has the same ordinary Python /// call boundary as PyPy's `IMPORT_NAME` implementation. pub load_import_fn: extern "C" fn(i64) -> i64, + /// Load IMPORT_NAME's locals argument. Separate from `load_import_fn` + /// because `pyopcode.py:1119-1125` reads two independent things off the + /// frame before the call. + pub load_import_locals_fn: extern "C" fn(i64) -> i64, /// `newtuple(list_w)` (`objspace.py:332`) — (ref array) → new tuple. /// The array is the forced `popvalues` list; length travels inside /// the array, so any arity fits. @@ -544,6 +548,7 @@ impl Cpu { load_locals_fn: crate::call_jit::bh_load_locals_fn, load_build_class_fn: crate::call_jit::bh_load_build_class_fn, load_import_fn: crate::call_jit::bh_load_import_fn, + load_import_locals_fn: crate::call_jit::bh_load_import_locals_fn, newtuple_from_array_fn: crate::call_jit::bh_newtuple_from_array, build_map_from_array_fn: crate::call_jit::bh_build_map_from_array, build_set_from_array_fn: crate::call_jit::bh_build_set_from_array, diff --git a/pyre/pyre-jit/src/jit/flatten.rs b/pyre/pyre-jit/src/jit/flatten.rs index 49ea365b2a5..b00e8f7338c 100644 --- a/pyre/pyre-jit/src/jit/flatten.rs +++ b/pyre/pyre-jit/src/jit/flatten.rs @@ -2641,6 +2641,7 @@ pub fn graph_op_can_raise(op: &super::flow::SpaceOperation) -> bool { | "delete_global" | "load_build_class" | "load_import" + | "load_import_locals" | "simple_call" | "getattr" | "load_special" @@ -3334,6 +3335,10 @@ pub struct LoweringContext { /// `load_import_fn` descrs-pool index. This is the builtin-lookup half of /// IMPORT_NAME; the subsequent invocation uses the ordinary CallFn path. pub load_import_fn_idx: u16, + /// `load_import_locals_fn` descrs-pool index. IMPORT_NAME's locals + /// argument lowers to the same one-Ref shape as + /// [`Self::load_locals_fn_idx`]. + pub load_import_locals_fn_idx: u16, /// `bind(assembler, cpu.newtuple_from_array_fn as *const (), /// CallFlavor::Plain)` descrs-pool index for the production /// source. BUILD_TUPLE records the rtyped `pyopcode.py` @@ -4515,6 +4520,27 @@ where )) } +/// Lower IMPORT_NAME's locals argument to a one-Ref residual call. +pub fn lower_load_import_locals_hlop_to_insn( + op: &super::flow::SpaceOperation, + ctx: &LoweringContext, + get_register: &mut F, + lower_constant: &mut LC, +) -> Option +where + F: FnMut(super::flow::Variable) -> Register, + LC: FnMut(&Constant) -> Operand, +{ + lower_frame_only_ref_hlop_to_insn( + op, + "load_import_locals", + ctx.load_import_locals_fn_idx, + majit_ir::PyreHelperKind::LoadImportLocals, + get_register, + lower_constant, + ) +} + /// Lower pyopcode.py DELETE_GLOBAL to a void two-Ref residual call. pub fn lower_delete_global_hlop_to_insn( op: &super::flow::SpaceOperation, @@ -5466,6 +5492,10 @@ where if let Some(insn) = lower_load_import_hlop_to_insn(op, ctx, get_register, lower_constant) { return Some(insn); } + if let Some(insn) = lower_load_import_locals_hlop_to_insn(op, ctx, get_register, lower_constant) + { + return Some(insn); + } if let Some(insn) = lower_tuple_build_hlop_to_insn(op, ctx, get_register, lower_constant) { return Some(insn); } @@ -13527,6 +13557,74 @@ mod tests { } } + #[test] + fn lower_load_import_locals_hlop_emits_infallible_frame_residual() { + // `pyopcode.py:1119-1125` reads the frame's debug locals before the + // call. Peeking that slot creates nothing and cannot raise, so this + // is the plain one-Ref frame-receiver shape, not the CanRaise shape + // the sibling `load_import` lookup carries. + let frame = Variable::new(VariableId(8), Kind::Ref); + let result = Variable::new(VariableId(9), Kind::Ref); + let op = SpaceOperation::new( + "load_import_locals", + vec![frame.into()], + Some(result.into()), + 0, + ); + let ctx = LoweringContext { + load_import_locals_fn_idx: 135, + ..Default::default() + }; + let mut get_register = identity_register_mapper(); + let mut lower_constant = test_constant_lowering(); + let insn = lower_load_import_locals_hlop_to_insn( + &op, + &ctx, + &mut get_register, + &mut lower_constant, + ) + .expect("load_import_locals lowering must succeed"); + + match insn { + Insn::Op { + opname, + args, + result: Some(dst), + } => { + assert_eq!(opname, "residual_call_r_r"); + assert!(matches!(args[0], Operand::ConstInt(135))); + match &args[1] { + Operand::ListOfKind(list) => { + assert_eq!(list.kind, Kind::Ref); + assert!(matches!( + &list.content[..], + [Operand::Register(Register { + kind: Kind::Ref, + index: 8 + })] + )); + } + other => panic!("expected one-Ref ListR, got {other:?}"), + } + assert_eq!(dst, Register::new(Kind::Ref, 9)); + match &args[2] { + Operand::Descr(descr) => match &**descr { + DescrOperand::CallDescrStub(stub) => { + assert_eq!( + stub.effect_info.pyre_helper, + majit_ir::PyreHelperKind::LoadImportLocals + ); + assert!(!stub.effect_info.has_random_effects()); + } + other => panic!("expected CallDescrStub, got {other:?}"), + }, + other => panic!("expected call descr, got {other:?}"), + } + } + other => panic!("expected Insn::Op, got {other:?}"), + } + } + #[test] fn lower_load_import_hlop_emits_builtin_lookup_residual() { // PyPy IMPORT_NAME performs the builtin lookup separately from its From 29513dbb251ae1572bfa5adbf9befe824a47ce5d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 23 Aug 2026 14:48:37 +0900 Subject: [PATCH 13/14] jit: give the IMPORT_NAME locals residual analyzed effects The shared frame-only lowering helper takes the Plain "no graph was analyzed" path, which becomes RANDOM_EFFECTS / CALL_MAY_FORCE. `load_import` is hand-written rather than routed through that helper for exactly this reason, and the locals half needs the same treatment: as written, one IMPORT_NAME forced the frame where the traced opcode should force it not at all. Build the effect info the way the sibling lookup does, and assert the flavor and extraeffect in the lowering test rather than only the absence of random effects. The test failed on the previous commit and is what caught this. Assisted-by: Claude --- pyre/pyre-jit/src/jit/flatten.rs | 36 +++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/pyre/pyre-jit/src/jit/flatten.rs b/pyre/pyre-jit/src/jit/flatten.rs index b00e8f7338c..dc4ef45307c 100644 --- a/pyre/pyre-jit/src/jit/flatten.rs +++ b/pyre/pyre-jit/src/jit/flatten.rs @@ -4531,14 +4531,28 @@ where F: FnMut(super::flow::Variable) -> Register, LC: FnMut(&Constant) -> Operand, { - lower_frame_only_ref_hlop_to_insn( - op, - "load_import_locals", + if op.opname != "load_import_locals" || op.args.len() != 1 { + return None; + } + let frame_operand = flatten_arg_with_lowering(&op.args[0], get_register, lower_constant); + let dst_reg = match &op.result { + Some(super::flow::FlowValue::Variable(var)) => get_register(*var), + _ => return None, + }; + // Analyzed with concrete-empty effect sets, exactly as the sibling + // `load_import` lookup is: peeking the debug slot neither runs Python nor + // writes the GC heap. Routing this through the generic frame-only helper + // instead would take the Plain "no graph was analyzed" path and become + // RANDOM_EFFECTS / CALL_MAY_FORCE -- forcing the frame twice per + // IMPORT_NAME, against the zero-forcing trace the other half preserves. + let mut effect_info = majit_ir::EffectInfo::default(); + effect_info.pyre_helper = majit_ir::PyreHelperKind::LoadImportLocals; + Some(build_residual_call_r_r_insn_with_effect_info( ctx.load_import_locals_fn_idx, - majit_ir::PyreHelperKind::LoadImportLocals, - get_register, - lower_constant, - ) + vec![frame_operand], + effect_info, + dst_reg, + )) } /// Lower pyopcode.py DELETE_GLOBAL to a void two-Ref residual call. @@ -13614,6 +13628,14 @@ mod tests { stub.effect_info.pyre_helper, majit_ir::PyreHelperKind::LoadImportLocals ); + assert_eq!( + stub.effect_info.extraeffect, + majit_ir::ExtraEffect::CanRaise + ); + assert_eq!( + dispatch_kind_for_effect_info(&stub.effect_info), + CallFlavor::Plain + ); assert!(!stub.effect_info.has_random_effects()); } other => panic!("expected CallDescrStub, got {other:?}"), From 34cb643a06deec5aef9db8fbc3375c467b59a2b0 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 23 Aug 2026 15:50:20 +0900 Subject: [PATCH 14/14] jit: bind the IMPORT_NAME locals residual PlainCannotRaise `import_locals` reads the frame's debug slot and returns what is already there. It cannot raise, so `CanRaise` was costing a `GUARD_NO_EXCEPTION` after every IMPORT_NAME and misdescribing the operation. It does read GC heap state (`PyFrame.debugdata`, then `w_locals`), so the flavor is `PlainCannotRaise` and not the no-heap one -- the distinction `for_iter_exception_match_fn` is bound under, three lines further down the same function. Ops that lower `PlainCannotRaise` must be absent from `graph_op_can_raise`, as its comment records for `load_method_self` and its neighbours, so drop `load_import_locals` from that list and name it alongside them. Also records why this helper asserts on a null frame where its sibling publishes a SystemError: with the guard dropped, a published exception here would have nothing left to observe it. Assisted-by: Claude --- pyre/pyre-jit/src/call_jit.rs | 9 ++++++++ pyre/pyre-jit/src/jit/codewriter.rs | 5 ++++- pyre/pyre-jit/src/jit/flatten.rs | 32 +++++++++++++++-------------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 1eb8a1a1dad..da2af38004b 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -6504,6 +6504,15 @@ pub extern "C" fn bh_load_import_fn(frame_ptr: i64) -> i64 { /// [`bh_load_import_fn`]. Infallible — it peeks the debug slot rather than /// creating one — so like `bh_load_locals_fn` it has no exception-publishing /// arm. +/// +/// The asymmetry with [`bh_load_import_fn`] is deliberate rather than an +/// oversight. That one resolves `__import__` and genuinely raises ImportError +/// when the name is absent, so it carries the publish-and-return-0 machinery +/// and is bound `CanRaise`. This one is bound `PlainCannotRaise`, which makes +/// `do_residual_call` drop the trailing `GUARD_NO_EXCEPTION` -- so a published +/// exception here would have nothing to observe it. A null frame is a wiring +/// bug in an emit site, not a runtime condition, and takes the same assert +/// `bh_load_locals_fn` uses for the same reason. pub extern "C" fn bh_load_import_locals_fn(frame_ptr: i64) -> i64 { assert!( frame_ptr != 0, diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index 3b9bd05092f..59799f79bbf 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -4300,10 +4300,13 @@ fn register_helper_fn_pointers( cpu.load_import_fn as *const (), CallFlavor::Plain, ); + // The locals half only reads the frame's debug slot: it cannot raise and + // cannot collect, but it does read GC heap state, so it takes + // `PlainCannotRaise` rather than the no-heap flavor. let load_import_locals_fn = bind( assembler, cpu.load_import_locals_fn as *const (), - CallFlavor::Plain, + CallFlavor::PlainCannotRaise, ); // The hand-written PUSH_EXC_INFO lowering must complete the interpreter's // caught-exception ownership transfer. Bind last so every existing diff --git a/pyre/pyre-jit/src/jit/flatten.rs b/pyre/pyre-jit/src/jit/flatten.rs index dc4ef45307c..0d5a2b69d76 100644 --- a/pyre/pyre-jit/src/jit/flatten.rs +++ b/pyre/pyre-jit/src/jit/flatten.rs @@ -2627,8 +2627,9 @@ pub fn graph_op_can_raise(op: &super::flow::SpaceOperation) -> bool { if binary_op_tag_for_opname(opname).is_some() || compare_op_tag_for_opname(opname).is_some() { return true; } - // `load_method_self`, `store_deref_value`, `unbound_local_error` - // lower with `PlainCannotRaise` and are intentionally absent. + // `load_method_self`, `store_deref_value`, `unbound_local_error`, + // `load_import_locals` lower with `PlainCannotRaise` and are intentionally + // absent. matches!( opname, "bool" @@ -2641,7 +2642,6 @@ pub fn graph_op_can_raise(op: &super::flow::SpaceOperation) -> bool { | "delete_global" | "load_build_class" | "load_import" - | "load_import_locals" | "simple_call" | "getattr" | "load_special" @@ -4539,18 +4539,20 @@ where Some(super::flow::FlowValue::Variable(var)) => get_register(*var), _ => return None, }; - // Analyzed with concrete-empty effect sets, exactly as the sibling - // `load_import` lookup is: peeking the debug slot neither runs Python nor - // writes the GC heap. Routing this through the generic frame-only helper - // instead would take the Plain "no graph was analyzed" path and become - // RANDOM_EFFECTS / CALL_MAY_FORCE -- forcing the frame twice per - // IMPORT_NAME, against the zero-forcing trace the other half preserves. - let mut effect_info = majit_ir::EffectInfo::default(); - effect_info.pyre_helper = majit_ir::PyreHelperKind::LoadImportLocals; - Some(build_residual_call_r_r_insn_with_effect_info( + // Reads the frame's debug slot and returns what is already there, so it + // cannot raise and cannot collect -- `do_residual_call` then drops the + // trailing `GUARD_NO_EXCEPTION` the sibling `load_import` lookup needs. + // It does read GC heap state (`PyFrame.debugdata`, then `w_locals`), so + // this is `PlainCannotRaise` and not the no-heap flavor, the same + // distinction `for_iter_exception_match_fn` is bound under. The generic + // frame-only helper would instead take the Plain "no graph was analyzed" + // path and become RANDOM_EFFECTS / CALL_MAY_FORCE, forcing the frame that + // this opcode should not force at all. + Some(build_residual_call_r_r_insn_from_operands( ctx.load_import_locals_fn_idx, vec![frame_operand], - effect_info, + CallFlavor::PlainCannotRaise, + majit_ir::PyreHelperKind::LoadImportLocals, dst_reg, )) } @@ -13630,11 +13632,11 @@ mod tests { ); assert_eq!( stub.effect_info.extraeffect, - majit_ir::ExtraEffect::CanRaise + majit_ir::ExtraEffect::CannotRaise ); assert_eq!( dispatch_kind_for_effect_info(&stub.effect_info), - CallFlavor::Plain + CallFlavor::PlainCannotRaise ); assert!(!stub.effect_info.has_random_effects()); }