diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index 551a9e8fc03..4b6f27c4b6b 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -376,6 +376,9 @@ fn register_active_hooks(supports_guard_gc_type: bool) { supports_guard_gc_type, }); majit_gc::set_active_alloc_nursery_typed(Some(alloc_nursery_typed_via_active_runtime)); + majit_gc::set_active_alloc_nursery_headerless_no_collect(Some( + alloc_nursery_headerless_no_collect_via_active_runtime, + )); majit_gc::set_active_alloc_nursery_typed_with_placement(Some( alloc_nursery_typed_with_placement_via_active_runtime, )); @@ -1543,6 +1546,16 @@ fn alloc_nursery_typed_via_active_runtime(type_id: u32, size: usize) -> GcRef { with_cranelift_gc(|gc| gc.try_alloc_nursery_no_collect_typed(type_id, size)).unwrap_or(GcRef(0)) } +/// `majit_gc::AllocNurseryHeaderlessNoCollectFn` installed by +/// `set_gc_allocator`. The metainterp's jitcode tracer allocates a `NEW` on a +/// `headerless` descr through here so the object lands in the interpreter's +/// own collected pool rather than the host heap, where its collector could not +/// see it. Returns `GcRef(0)` when no GC is bound, leaving the caller on its +/// own path. +fn alloc_nursery_headerless_no_collect_via_active_runtime(size: usize) -> GcRef { + with_cranelift_gc(|gc| gc.alloc_nursery_headerless_no_collect(size)).unwrap_or(GcRef(0)) +} + /// Placement-reporting companion of /// [`alloc_nursery_typed_via_active_runtime`]. /// @@ -11670,36 +11683,164 @@ impl CraneliftBackend { &known_values, op.arg(0).to_opref(), ); - let result = emit_collecting_gc_call( - &mut builder, - ptr_type, - call_conv, - jf_ptr, - &ref_root_slots, - &defined_ref_vars, - &stale_ref_vars, - &demoted_failarg_slots, - ref_root_base_ofs, - per_call_gcmap, - gc_alloc_nursery_headerless_shim as *const () as usize, - &[size], - Some(cl_types::I64), - ) - .expect("headerless nursery allocation helper must return a value"); - jf_ptr = emit_reload_frame_if_necessary(&mut builder, ptr_type, call_conv); - builder.ins().set_pinned_reg(jf_ptr); - builder.def_var(var(vi), result); - // malloc_cond parity: the headerless slow path - // (gc_alloc_nursery_headerless_shim) returns NULL on - // host/bounded out-of-memory; propagate before the - // following stores dereference it. - emit_memory_error_check( - &mut builder, - ptr_type, - result, - propagate_exception_descr_ptr, - preamble_phase, - ); + // x86/assembler.py:2556-2565 malloc_cond parity, headerless + // variant — the same inline bump the `CallMallocNursery` + // arm below emits and the dynasm backends already emit for + // this opcode (`genop_call_malloc_nursery_headerless`). + // Three differences from the headered shape: the bump is by + // `size` alone (no `GcHeader::SIZE` reservation), no header + // word is zeroed, and the result is the old nursery base + // rather than `base + GcHeader::SIZE`. + // + // The raw bump is correct only while the active GC is + // headerless-aware, which is the same invariant + // `alloc_nursery_headerless`'s panicking default enforces on + // the overflow path — whoever declares `headerless_structs` + // upholds it. Without a bump surface (`nursery_free` / + // `nursery_top` reported as 0) the op stays on the helper. + let inline_bump = gc_nursery_addrs.filter(|&(nf, nt)| nf != 0 && nt != 0); + if let Some((nf_addr, nt_addr)) = inline_bump { + let flags = MemFlags::trusted(); + let nf_ptr = builder.ins().iconst(ptr_type, nf_addr as i64); + let nt_ptr = builder.ins().iconst(ptr_type, nt_addr as i64); + let free = builder.ins().load(ptr_type, flags, nf_ptr, 0); + let new_free = builder.ins().iadd(free, size); + let top = builder.ins().load(ptr_type, flags, nt_ptr, 0); + // `nursery_top` is one-past-last, so the region is exhausted + // exactly when the bumped free pointer runs past it — the + // dynasm emitters' `b.hi` / `ja` slow-path edge. + let fits = + builder + .ins() + .icmp(IntCC::UnsignedLessThanOrEqual, new_free, top); + + // Same block-param carry as the headered arm: only the slow + // path spills and reloads the ref roots, so the merge takes + // every live ref as a parameter instead of letting the two + // paths disagree on the variable's definition. + let live_refs: Vec<(u32, usize)> = ref_root_slots + .iter() + .filter(|(var_idx, _)| defined_ref_vars.contains(var_idx)) + .copied() + .collect(); + + let fast_block = builder.create_block(); + let slow_block = builder.create_block(); + let merge_block = builder.create_block(); + builder.append_block_param(merge_block, ptr_type); // result + builder.append_block_param(merge_block, ptr_type); // jf_ptr + for _ in &live_refs { + builder.append_block_param(merge_block, cl_types::I64); + } + builder.ins().brif(fits, fast_block, &[], slow_block, &[]); + + // fast: publish the bumped free pointer and hand back the + // old base. Nothing here can collect, so no gcmap is pushed + // and no ref root is spilled. + builder.switch_to_block(fast_block); + builder.seal_block(fast_block); + builder.ins().store(flags, new_free, nf_ptr, 0); + let mut fast_args: Vec = + vec![BlockArg::from(free), BlockArg::from(jf_ptr)]; + for &(var_idx, _) in &live_refs { + fast_args.push(BlockArg::from(builder.use_var(var(var_idx)))); + } + builder.ins().jump(merge_block, &fast_args); + + // slow: aarch64 `_build_malloc_slowpath` parity — spill the + // ref roots and install the gcmap, then let the helper + // collect and re-bump. + builder.switch_to_block(slow_block); + builder.seal_block(slow_block); + builder.set_cold_block(slow_block); + spill_ref_roots( + &mut builder, + jf_ptr, + &ref_root_slots, + &defined_ref_vars, + &stale_ref_vars, + &demoted_failarg_slots, + ref_root_base_ofs, + ); + emit_push_gcmap(&mut builder, jf_ptr, per_call_gcmap); + let slow_r = emit_host_call( + &mut builder, + ptr_type, + call_conv, + gc_alloc_nursery_headerless_shim as *const () as usize, + &[size], + Some(cl_types::I64), + ) + .expect("headerless nursery allocation helper must return a value"); + let jf_ptr_slow = + emit_reload_frame_if_necessary(&mut builder, ptr_type, call_conv); + emit_pop_gcmap(&mut builder, jf_ptr_slow, per_call_gcmap); + reload_ref_roots( + &mut builder, + jf_ptr_slow, + &ref_root_slots, + &defined_ref_vars, + &demoted_failarg_slots, + ref_root_base_ofs, + ); + let mut slow_args: Vec = + vec![BlockArg::from(slow_r), BlockArg::from(jf_ptr_slow)]; + for &(var_idx, _) in &live_refs { + slow_args.push(BlockArg::from(builder.use_var(var(var_idx)))); + } + builder.ins().jump(merge_block, &slow_args); + + builder.switch_to_block(merge_block); + builder.seal_block(merge_block); + let params = builder.block_params(merge_block).to_vec(); + let result = params[0]; + jf_ptr = params[1]; + builder.ins().set_pinned_reg(jf_ptr); + for (i, &(var_idx, _)) in live_refs.iter().enumerate() { + builder.def_var(var(var_idx), params[2 + i]); + } + builder.def_var(var(vi), result); + // malloc_cond parity: the headerless slow path + // (gc_alloc_nursery_headerless_shim) returns NULL on + // host/bounded out-of-memory; propagate before the + // following stores dereference it. The fast path cannot + // produce NULL, but the check is on the merged value so a + // slow-path NULL is caught on either edge. + emit_memory_error_check( + &mut builder, + ptr_type, + result, + propagate_exception_descr_ptr, + preamble_phase, + ); + } else { + let result = emit_collecting_gc_call( + &mut builder, + ptr_type, + call_conv, + jf_ptr, + &ref_root_slots, + &defined_ref_vars, + &stale_ref_vars, + &demoted_failarg_slots, + ref_root_base_ofs, + per_call_gcmap, + gc_alloc_nursery_headerless_shim as *const () as usize, + &[size], + Some(cl_types::I64), + ) + .expect("headerless nursery allocation helper must return a value"); + jf_ptr = emit_reload_frame_if_necessary(&mut builder, ptr_type, call_conv); + builder.ins().set_pinned_reg(jf_ptr); + builder.def_var(var(vi), result); + emit_memory_error_check( + &mut builder, + ptr_type, + result, + propagate_exception_descr_ptr, + preamble_phase, + ); + } } OpCode::CallMallocNursery => { // x86/assembler.py:2556-2565 malloc_cond parity. diff --git a/majit/majit-backend-dynasm/src/runner.rs b/majit/majit-backend-dynasm/src/runner.rs index 5c8e39c05e5..6c06fd42a39 100644 --- a/majit/majit-backend-dynasm/src/runner.rs +++ b/majit/majit-backend-dynasm/src/runner.rs @@ -180,6 +180,9 @@ fn register_active_hooks(supports_guard_gc_type: bool) { supports_guard_gc_type, }); majit_gc::set_active_alloc_nursery_typed(Some(dynasm_alloc_nursery_typed)); + majit_gc::set_active_alloc_nursery_headerless_no_collect(Some( + dynasm_alloc_nursery_headerless_no_collect, + )); majit_gc::set_active_alloc_nursery_typed_with_placement(Some( dynasm_alloc_nursery_typed_with_placement, )); @@ -357,6 +360,26 @@ pub(crate) extern "C" fn dynasm_new_alloc(size: usize) -> *mut u8 { }) } +/// Headerless no-collect nursery trampoline for backend-agnostic callers. +/// +/// The metainterp's jitcode tracer allocates a `NEW` on a `headerless` descr +/// through here so the object lands in the interpreter's own collected pool +/// rather than the host heap, where its collector could not see it. Returns +/// null when no GC is bound, leaving the caller on its own path. +fn dynasm_alloc_nursery_headerless_no_collect(size: usize) -> GcRef { + if let Some(r) = DYNASM_ACTIVE_GC.with(|c| { + c.borrow_mut() + .as_deref_mut() + .map(|g| g.alloc_nursery_headerless_no_collect(size)) + }) { + return r; + } + if majit_gc::gc_sync::is_initialized() { + return majit_gc::gc_sync::gc_op(|g| g.alloc_nursery_headerless_no_collect(size)); + } + GcRef::NULL +} + /// Host-side nursery allocation trampoline. Published via /// `majit_gc::set_active_alloc_nursery_typed` from `set_gc_allocator` /// so backend-agnostic callers (e.g. pyre-object `w_int_new`) can @@ -465,6 +488,49 @@ fn dynasm_alloc_oldgen_typed(type_id: u32, size: usize) -> GcRef { majit_gc::gc_sync::gc_op(|g| g.alloc_oldgen_typed(type_id, size)) } +/// Allocate the struct a `bh_new` / `bh_new_with_vtable` descr describes +/// (`llmodel.py:775-786`). +/// +/// A GC-managed struct (real `type_id`) MUST be allocated through the GC so the +/// collector can trace its pointer fields: a resume-materialized virtual (e.g. +/// an inlined-callee `PyFrame`) holds a `locals_cells_stack` ref to its arrays, +/// and a raw `libc::malloc` block is invisible to the GC, so a minor collection +/// during the blackhole forward run frees those arrays out from under the +/// frame. Allocate in the non-moving old generation (mark-sweep), mirroring +/// `w_int_new`/`w_float_new`: the blackhole register file and the deep forward +/// recursion capture raw pointers to the materialized struct that the resume +/// path does not re-root across the minor collections it triggers, so a moving +/// nursery object would leave those captures stale. Old-gen keeps every +/// materialized pointer stable for the lifetime of the resume. +/// +/// A headerless struct lives in the interpreter's own `headerless_structs` pool +/// and carries no `type_id` word at `ref - 8`, so it takes the headerless +/// nursery allocator instead: `alloc_oldgen_typed` returns +/// `base + GcHeader::SIZE`, which would shift every field offset the descr +/// carries. +/// +/// Non-GC descrs (`type_id == 0`, raw buffers) and a runtime with no allocator +/// hook installed (unit tests) keep the plain zeroed malloc. +fn bh_alloc_struct(sizedescr: &majit_translate::jitcode::BhDescr) -> *mut libc::c_void { + let size = sizedescr.as_size(); + let gc_ptr = if sizedescr.is_headerless() { + majit_gc::alloc_nursery_headerless_no_collect(size).0 + } else { + match sizedescr.resolve_gc_tid() { + 0 => 0, + type_id => dynasm_alloc_oldgen_typed(type_id, size).0, + } + }; + if gc_ptr != 0 { + return gc_ptr as *mut libc::c_void; + } + let ptr = unsafe { libc::malloc(size) }; + if !ptr.is_null() { + unsafe { libc::memset(ptr, 0, size) }; + } + ptr +} + /// User-level `gc.collect()` trampoline — drives `GcAllocator::collect_full` /// on the active dynasm-owned GC. PyPy's `pypy/module/gc/interp_gc.py:7-26` /// runs `rgc.collect()` from app-level `gc.collect`; this is the dynasm @@ -2962,40 +3028,12 @@ impl Backend for DynasmBackend { } fn bh_new(&self, sizedescr: &majit_translate::jitcode::BhDescr) -> i64 { - let size = sizedescr.as_size(); - let ptr = unsafe { libc::malloc(size) }; - if !ptr.is_null() { - unsafe { libc::memset(ptr, 0, size) }; - } - ptr as i64 + bh_alloc_struct(sizedescr) as i64 } fn bh_new_with_vtable(&self, sizedescr: &majit_translate::jitcode::BhDescr) -> i64 { - let size = sizedescr.as_size(); let vtable = sizedescr.get_vtable(); - // A GC-managed struct (real `type_id`) MUST be allocated through the GC - // so the collector can trace its pointer fields: a resume-materialized - // virtual (e.g. an inlined-callee `PyFrame`) holds a `locals_cells_stack` - // ref to its arrays, and a raw `libc::malloc` block is invisible to the - // GC, so a minor collection during the blackhole forward run frees those - // arrays out from under the frame. Allocate in the non-moving old - // generation (mark-sweep), mirroring `w_int_new`/`w_float_new`: the - // blackhole register file and the deep forward recursion capture raw - // pointers to the materialized struct that the resume path does not - // re-root across the minor collections it triggers, so a moving nursery - // object would leave those captures stale. Old-gen keeps every - // materialized pointer stable for the lifetime of the resume. Non-GC - // descrs (`type_id == 0`, raw buffers) keep the plain malloc. - let type_id = sizedescr.resolve_gc_tid(); - let ptr = if type_id != 0 { - dynasm_alloc_oldgen_typed(type_id, size).0 as *mut libc::c_void - } else { - let ptr = unsafe { libc::malloc(size) }; - if !ptr.is_null() { - unsafe { libc::memset(ptr, 0, size) }; - } - ptr - }; + let ptr = bh_alloc_struct(sizedescr); if !ptr.is_null() { unsafe { // llmodel.py:780-782: if self.vtable_offset is not None: diff --git a/majit/majit-backend-wasm/src/lib.rs b/majit/majit-backend-wasm/src/lib.rs index 79e65693158..87cac0b56a1 100644 --- a/majit/majit-backend-wasm/src/lib.rs +++ b/majit/majit-backend-wasm/src/lib.rs @@ -302,6 +302,9 @@ fn register_active_hooks(supports_guard_gc_type: bool) { supports_guard_gc_type, }); majit_gc::set_active_alloc_nursery_typed(Some(wasm_alloc_nursery_typed)); + majit_gc::set_active_alloc_nursery_headerless_no_collect(Some( + wasm_alloc_nursery_headerless_no_collect, + )); majit_gc::set_active_alloc_nursery_typed_with_placement(Some( wasm_alloc_nursery_typed_with_placement, )); @@ -520,6 +523,15 @@ fn wasm_alloc_nursery_typed(type_id: u32, size: usize) -> GcRef { .unwrap_or(GcRef(0)) } +/// `majit_gc::AllocNurseryHeaderlessNoCollectFn`. The metainterp's jitcode +/// tracer allocates a `NEW` on a `headerless` descr through here so the object +/// lands in the interpreter's own collected pool rather than the host heap, +/// where its collector could not see it. Returns `GcRef(0)` when no GC is +/// bound, leaving the caller on its own path. +fn wasm_alloc_nursery_headerless_no_collect(size: usize) -> GcRef { + with_wasm_active_gc_mut(|gc| gc.alloc_nursery_headerless_no_collect(size)).unwrap_or(GcRef(0)) +} + /// Placement-reporting companion of [`wasm_alloc_nursery_typed`]. /// /// # Safety diff --git a/majit/majit-gc/src/lib.rs b/majit/majit-gc/src/lib.rs index 56483555586..82cb02505c0 100644 --- a/majit/majit-gc/src/lib.rs +++ b/majit/majit-gc/src/lib.rs @@ -168,6 +168,16 @@ pub trait GcAllocator: Send { ); } + /// [`Self::alloc_nursery_headerless`] for a caller that cannot survive a + /// collection: the metainterp's jitcode tracer executes `NEW` while holding + /// raw object pointers in its own register bank, which is not part of any + /// root set, so a moving collection under this call would strand them. The + /// allocator must grow instead of evacuating. The default forwards to the + /// collecting form, which is correct for a non-moving collector. + fn alloc_nursery_headerless_no_collect(&mut self, size: usize) -> GcRef { + self.alloc_nursery_headerless(size) + } + /// Allocate a fixed-size object with a known GC type id. fn alloc_nursery_typed(&mut self, type_id: u32, size: usize) -> GcRef { let _ = type_id; @@ -1309,6 +1319,37 @@ pub fn alloc_nursery_typed(type_id: u32, payload_size: usize) -> GcRef { } } +/// Process-global callback for a headerless, non-collecting nursery +/// allocation. Returns `GcRef(0)` when no backend has installed a hook, so +/// the caller can keep its own non-GC path. +pub type AllocNurseryHeaderlessNoCollectFn = fn(size: usize) -> GcRef; + +global_hook!( + static ACTIVE_ALLOC_NURSERY_HEADERLESS_NO_COLLECT: AllocNurseryHeaderlessNoCollectFn +); + +/// Install the active backend's headerless no-collect allocator callback. +/// Pass `None` to clear. +pub fn set_active_alloc_nursery_headerless_no_collect( + hook: Option, +) { + ACTIVE_ALLOC_NURSERY_HEADERLESS_NO_COLLECT.set(hook); +} + +/// Allocate a headerless object through the active backend's GC without +/// letting it collect. Returns `GcRef(0)` when no backend installed a hook. +/// +/// The metainterp's jitcode tracer reaches its `NEW` allocation through here: +/// an interpreter that declares `headerless_structs` owns those objects in its +/// own collected pool, so a plain host-heap block there would be invisible to +/// its collector for as long as the object stays reachable. +pub fn alloc_nursery_headerless_no_collect(size: usize) -> GcRef { + match ACTIVE_ALLOC_NURSERY_HEADERLESS_NO_COLLECT.get() { + Some(f) => f(size), + None => GcRef(0), + } +} + /// Placement-reporting companion of [`AllocNurseryTypedFn`] for fresh-object /// initialization. The allocation remains no-collect; the out-parameter is /// `false` for a nursery result and `true` for an old-gen spill. @@ -1725,3 +1766,85 @@ pub fn gc_write_barrier(obj: GcRef) { f(obj) } } + +#[cfg(test)] +mod headerless_no_collect_tests { + use super::*; + + /// A `GcAllocator` that only implements the collecting headerless form, so + /// the default `alloc_nursery_headerless_no_collect` has to forward to it. + struct ForwardingGc { + headerless_calls: usize, + } + + impl GcAllocator for ForwardingGc { + fn alloc_nursery(&mut self, _size: usize) -> GcRef { + GcRef(0x1000) + } + fn alloc_nursery_headerless(&mut self, _size: usize) -> GcRef { + self.headerless_calls += 1; + GcRef(0x2000) + } + fn alloc_nursery_no_collect(&mut self, size: usize) -> GcRef { + self.alloc_nursery(size) + } + fn alloc_varsize(&mut self, base: usize, item: usize, len: usize) -> GcRef { + self.alloc_nursery(base + item * len) + } + fn alloc_varsize_no_collect(&mut self, base: usize, item: usize, len: usize) -> GcRef { + self.alloc_varsize(base, item, len) + } + fn write_barrier(&mut self, _obj: GcRef) {} + fn jit_remember_young_pointer_from_array(&mut self, _obj: GcRef) {} + fn remember_young_pointer_from_array2( + &mut self, + _obj: GcRef, + _index: usize, + _card_page_shift: u32, + ) { + } + fn collect_nursery(&mut self) {} + fn collect_full(&mut self) {} + fn nursery_free(&self) -> *mut u8 { + std::ptr::null_mut() + } + fn nursery_free_addr(&self) -> usize { + 0 + } + fn nursery_top(&self) -> *const u8 { + std::ptr::null() + } + fn nursery_top_addr(&self) -> usize { + 0 + } + fn max_nursery_object_size(&self) -> usize { + 0 + } + } + + #[test] + fn no_collect_default_forwards_to_the_collecting_headerless_form() { + let mut gc = ForwardingGc { + headerless_calls: 0, + }; + assert_eq!(gc.alloc_nursery_headerless_no_collect(16), GcRef(0x2000)); + assert_eq!(gc.headerless_calls, 1); + } + + fn stub_alloc(size: usize) -> GcRef { + GcRef(0x4000 + size) + } + + #[test] + fn active_hook_round_trips_and_is_null_when_absent() { + // Uninstalled: callers must see null so they can keep their own path. + set_active_alloc_nursery_headerless_no_collect(None); + assert_eq!(alloc_nursery_headerless_no_collect(16), GcRef(0)); + + set_active_alloc_nursery_headerless_no_collect(Some(stub_alloc)); + assert_eq!(alloc_nursery_headerless_no_collect(16), GcRef(0x4010)); + + set_active_alloc_nursery_headerless_no_collect(None); + assert_eq!(alloc_nursery_headerless_no_collect(16), GcRef(0)); + } +} diff --git a/majit/majit-ir/src/descr.rs b/majit/majit-ir/src/descr.rs index 0582adc5ad0..3cb10ffdbbb 100644 --- a/majit/majit-ir/src/descr.rs +++ b/majit/majit-ir/src/descr.rs @@ -98,6 +98,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::sync::Mutex; use std::sync::OnceLock; +use std::sync::RwLock; use std::sync::Weak; use std::sync::atomic::{AtomicI32, AtomicU32, AtomicU64, Ordering}; @@ -696,15 +697,7 @@ pub struct GcCache { _cache_interiorfield_order: Vec, /// Superseded SizeDescr Arcs retired by `register_keyed_size`'s - /// fuller-layout upgrade. PyPy's `_cache_size[STRUCT]` holds a single - /// canonical SizeDescr that is never freed, so field descrs' Weak parent - /// back-references stay valid for the process lifetime. pyre registers - /// struct layouts incrementally and replaces the cached SizeDescr with a - /// fuller one; without this vec the replaced Arc would drop while field - /// descrs already baked into recorded ops still hold Weak refs to it, - /// dangling their `get_parent_descr()`. Kept OUT of `_cache_size_order` - /// so `setup_descrs` descr_index assignment is unchanged — these are - /// retired, not re-registered; they exist only to keep the Weak alive. + /// fuller-layout upgrade. Kept out of `_cache_size_order`. _size_keepalive: Vec, /// `gctypelayout.py:301-357 TypeLayoutBuilder.get_type_id` analog — @@ -907,11 +900,36 @@ impl GcCache { is_immutable: bool, is_quasi_immutable: bool, flag: ArrayFlag, + index: u32, + virtualizable: bool, index_in_parent: usize, ) -> Arc { // descr.py:220-221: cache[STRUCT][fieldname] if let Some(inner) = self._cache_field.get(&struct_key) { if let Some(descr) = inner.get(field_name) { + debug_assert!( + descr.describes_same_field( + offset, + field_size, + field_type, + is_immutable, + is_quasi_immutable, + virtualizable, + index_in_parent, + ), + "get_field_descr cache hit for {field_name} disagrees with the caller: \ + cached (offset {}, size {}, type {:?}, immutable {}, quasi {}, vable {}, \ + index_in_parent {}) vs requested (offset {offset}, size {field_size}, \ + type {field_type:?}, immutable {is_immutable}, quasi {is_quasi_immutable}, \ + vable {virtualizable}, index_in_parent {index_in_parent})", + descr.offset, + descr.field_size, + descr.field_type, + descr.is_immutable, + descr.is_quasi_immutable(), + descr.virtualizable, + descr.index_in_parent, + ); return descr.clone(); } } @@ -925,14 +943,16 @@ impl GcCache { let parent = self._cache_size.get(&struct_key).cloned(); // descr.py:230-231: FieldDescr(name, offset, size, flag, index_in_parent, is_pure) let mut fd = SimpleFieldDescr::new_with_name( - u32::MAX, + index, offset, field_size, field_type, is_immutable, flag, name, + field_name.to_string(), ); + fd.virtualizable = virtualizable; // descr.py:228: index_in_parent (from heaptracker) fd.index_in_parent = index_in_parent; // descr.py:229 `is_quasi_immutable = '%s?' in STRUCT._hints.get( @@ -945,7 +965,7 @@ impl GcCache { fd = fd.with_quasi_immutable(is_quasi_immutable); // descr.py:238: fielddescr.parent_descr = get_size_descr(gccache, STRUCT, vtable) if let Some(ref p) = parent { - fd.parent_descr = Some(Arc::downgrade(p)); + fd.parent_descr = RwLock::new(Some(Arc::downgrade(p))); } let descr = Arc::new(fd); // descr.py:232-233: cachedict = cache.setdefault(STRUCT, {}) @@ -975,6 +995,7 @@ impl GcCache { false, ArrayFlag::Signed, // descr.py:264: get_type_flag(lltype.Signed) "len".to_string(), + "len".to_string(), )); // descr.py:265: result.parent_descr = None (no parent) self._cache_arraylen.insert(key, descr.clone()); @@ -1189,20 +1210,11 @@ impl GcCache { /// `descr.py:108-118 get_size_descr` cache-miss branch writes both /// the keyed map and the order Vec; this method mirrors that for /// mint sites that bypass `get_size_descr` (`make_simple_descr_group`, - /// runtime macro `__majit_register_descrs`). First-write wins — - /// subsequent calls with the same key keep the original Arc, matching - /// PyPy `cache[STRUCT] = sizedescr` semantics. + /// runtime macro `__majit_register_descrs`). pub fn register_keyed_size(&mut self, key: LLType, descr: DescrRef) { - // descr.py:108-118 `get_size_descr` populates `all_fielddescrs` - // from `heaptracker.all_fielddescrs(STRUCT)` — the COMPLETE field - // set — before caching. pyre's incremental `register_struct_layout` - // may produce multiple `make_simple_descr_group_keyed` calls for - // the same struct with progressively more fields. To match PyPy's - // invariant that the cached SizeDescr always carries the full known - // layout, replace an existing entry when the new descr has MORE - // fields. This prevents `StructPtrInfo.init_fields` from under- - // allocating slots, which causes cross-type forwards when different- - // typed fields map to the same slot. + // descr.py:108-118 caches the SizeDescr. Multiple pyre producers may + // report partial layouts, so the cached owner is upgraded when the + // incoming frozen list has more fields. let should_insert = match self._cache_size.get(&key) { None => true, Some(existing) => { @@ -1218,9 +1230,6 @@ impl GcCache { } }; if should_insert { - // PyPy never frees a cached SizeDescr; retire the superseded one - // into an immortal keepalive so any field descr already baked into - // a recorded op keeps a live Weak parent (get_parent_descr()). if let Some(old) = self._cache_size.get(&key) { if !arc_in_vec(&self._size_keepalive, old) { self._size_keepalive.push(old.clone()); @@ -1228,8 +1237,6 @@ impl GcCache { } self._cache_size.insert(key.clone(), descr.clone()); self._cache_size_order.retain(|d| { - // Remove the old entry for this key (if any) from the - // ordered vec so a stale orphan never appears. d.as_size_descr() .map(|sd| { sd.cache_key() != descr.as_size_descr().map(|s| s.cache_key()).unwrap_or(0) @@ -1237,7 +1244,12 @@ impl GcCache { .unwrap_or(true) }); if !arc_in_vec(&self._cache_size_order, &descr) { - self._cache_size_order.push(descr); + self._cache_size_order.push(descr.clone()); + } + if let Some(fields) = self._cache_field.get(&key) { + for field in fields.values() { + field.set_parent_descr(&descr); + } } } } @@ -1261,41 +1273,8 @@ impl GcCache { field_name: String, descr: Arc, ) { - // descr.py:218-239 `get_field_descr`: `cachedict[fieldname] = - // fielddescr` with `fielddescr.parent_descr = get_size_descr(STRUCT)`. - // In PyPy the parent SizeDescr is always the single canonical one - // (populated with ALL fields) so the Weak is always valid. - // - // pyre's incremental path may replace the cached SizeDescr with a - // fuller one (register_keyed_size upgrade). When that happens, - // existing field descrs' Weak parent back-references point to the - // OLD (now-dropped) SizeDescr → get_parent_descr() returns None. - // Detect this by checking whether the existing entry's parent_descr - // Weak can still upgrade; if not, replace it with the new descr - // whose parent Weak points to the current (upgraded) SizeDescr. let inner = self._cache_field.entry(struct_key).or_default(); - // PyPy's cached field descr always parents the single canonical (fullest) - // SizeDescr. pyre registers incrementally, so replace the cached field - // descr when the incoming one's parent carries MORE fields — this keeps - // future getfield recordings baking the fullest-layout parent. (The old - // Weak-liveness check is now inert: register_keyed_size retires superseded - // SizeDescrs into _size_keepalive instead of freeing them.) - let parent_field_count = |fd: &SimpleFieldDescr| -> usize { - fd.get_parent_descr() - .and_then(|p| p.as_size_descr().map(|sd| sd.all_fielddescrs().len())) - .unwrap_or(0) - }; - let should_replace = match inner.get(&field_name) { - None => true, // no entry yet -> insert - Some(existing) => parent_field_count(&descr) > parent_field_count(existing), - }; - if should_replace { - // Remove stale entry from _order if present. - if let Some(old) = inner.get(&field_name) { - let old_ref: DescrRef = old.clone() as DescrRef; - self._cache_field_order - .retain(|d| !Arc::ptr_eq(d, &old_ref)); - } + if !inner.contains_key(&field_name) { inner.insert(field_name, descr.clone()); let as_ref: DescrRef = descr as DescrRef; if !arc_in_vec(&self._cache_field_order, &as_ref) { @@ -2786,6 +2765,35 @@ fn flatten_vector_info(head: Option<&AccumInfo>) -> Vec { result } +/// Frontend decoder from a [`SizeDescr::vtable`] pointer to the canonical +/// `PyObject.w_class` object its instances carry (`get_instantiate(vtable)`). +/// +/// Pluggable for the same reason as [`crate::value::set_str_resolver`]: the +/// object model is the frontend's, and the IR layer holds no `PyType` layout. +/// Consulted live rather than cached on the descr — the type objects are +/// installed after the descrs are built, so a value snapshotted at +/// construction time would be null for every descr. +pub type WClassObjFn = fn(vtable: usize) -> Option; + +static W_CLASS_OBJ_RESOLVER: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// Register the frontend's [`WClassObjFn`]. First call wins, mirroring +/// `OnceLock::set`. The resolver only ever receives a `vtable` a +/// `SizeDescr` of this frontend reported, so it may assume its own type +/// layout. +pub fn set_w_class_obj_resolver(resolve: WClassObjFn) { + let _ = W_CLASS_OBJ_RESOLVER.set(resolve); +} + +/// Resolve `vtable` through the registered [`WClassObjFn`], or `None` when +/// the vtable is null or no frontend registered one. +pub fn resolve_w_class_obj(vtable: usize) -> Option { + if vtable == 0 { + return None; + } + W_CLASS_OBJ_RESOLVER.get().and_then(|resolve| resolve(vtable)) +} + /// Descriptor for a fixed-size struct/object allocation. /// /// Mirrors rpython/jit/backend/llsupport/descr.py SizeDescr. @@ -2854,7 +2862,9 @@ pub trait SizeDescr: Descr { /// builtin type inherits this value unless the trace stores an /// explicit `w_class` field. OptVirtualize folds `w_class` header /// reads to this constant. `None`/`0` for non-pyre size descrs or - /// before the type objects are initialised. + /// before the type objects are initialised. A frontend whose size + /// descrs are the generic [`SimpleSizeDescr`] supplies the answer + /// through [`set_w_class_obj_resolver`] instead of overriding this. fn w_class_obj(&self) -> Option { None } @@ -2987,6 +2997,12 @@ pub trait FieldDescr: Descr { "" } + /// descr.py:220-233 cache key (`fieldname`). Defaults to the + /// display name for descriptor implementations without a separate key. + fn field_key(&self) -> &str { + self.field_name() + } + /// heaptracker.py:66: `if name == 'typeptr': continue` /// /// RPython filters typeptr by raw field name BEFORE creating @@ -3666,6 +3682,9 @@ pub struct SimpleFieldDescr { ei_index: AtomicU32, /// RPython: FieldDescr.name — e.g. "MyStruct.field_name" name: String, + /// descr.py:220-233 cache key (`fieldname`), distinct from display + /// `name` built at descr.py:227. + field_key: String, offset: usize, field_size: usize, field_type: Type, @@ -3687,8 +3706,10 @@ pub struct SimpleFieldDescr { /// `OptContext::ensure_ptr_info_arg0` to dispatch Instance vs Struct /// PtrInfo per `optimizer.py:478-484`. Stored as `Weak` to break the /// SizeDescr → FieldDescr → SizeDescr Arc cycle introduced by - /// `make_simple_descr_group`. - pub parent_descr: Option>, + /// `make_simple_descr_group`. Interior-mutable because + /// `descr.py:238` updates the single parent backreference while the + /// field Arc is shared by `(STRUCT, fieldname)`. + pub parent_descr: RwLock>>, /// pyjitpl.py:1148-1149 `vinfo = fielddescr.get_vinfo()` — backref to /// the owning `VirtualizableInfo`. Stored as `Weak` /// because the vinfo Arc keeps the descriptor alive (via its @@ -3705,6 +3726,7 @@ impl Clone for SimpleFieldDescr { descr_index: AtomicI32::new(self.descr_index.load(Ordering::Relaxed)), ei_index: AtomicU32::new(self.ei_index.load(Ordering::Relaxed)), name: self.name.clone(), + field_key: self.field_key.clone(), offset: self.offset, field_size: self.field_size, field_type: self.field_type, @@ -3713,13 +3735,49 @@ impl Clone for SimpleFieldDescr { flag: self.flag, virtualizable: self.virtualizable, index_in_parent: self.index_in_parent, - parent_descr: self.parent_descr.clone(), + parent_descr: RwLock::new(self.parent_descr.read().unwrap().clone()), vinfo: self.vinfo.clone(), } } } impl SimpleFieldDescr { + /// Whether this descr already describes the field a `get_field_descr` + /// caller is asking for. + /// + /// `descr.py:218-239` derives every one of these from `(STRUCT, + /// fieldname)` itself — `symbolic.get_field_token`, `get_type_flag`, + /// `STRUCT._hints['_immutable_fields_']`, + /// `heaptracker.get_fielddescr_index_in` — so a cache hit there cannot + /// describe a different field than the caller means. Pyre takes them as + /// arguments instead, so two call sites *can* disagree, and the cache + /// silently keeps whichever minted first. That is never a legitimate + /// state: `heaptracker.py:76-101 get_fielddescr_index_in` is positional + /// and `optimizeopt/info.rs force_box` asserts on it, so a disagreeing + /// `index_in_parent` alone puts the two halves of the descr universe on + /// different slots of the same object. + /// + /// `index` is deliberately excluded — it is the per-trace codewriter slot + /// id, which the analyzer legitimately restamps onto a shared `Arc`. + pub fn describes_same_field( + &self, + offset: usize, + field_size: usize, + field_type: Type, + is_immutable: bool, + is_quasi_immutable: bool, + virtualizable: bool, + index_in_parent: usize, + ) -> bool { + self.offset == offset + && self.field_size == field_size + && self.field_type == field_type + && self.is_immutable == is_immutable + && self.is_quasi_immutable() == is_quasi_immutable + && self.virtualizable == virtualizable + && self.index_in_parent == index_in_parent + } + pub fn new( index: u32, offset: usize, @@ -3735,6 +3793,7 @@ impl SimpleFieldDescr { descr_index: AtomicI32::new(-1), ei_index: AtomicU32::new(u32::MAX), name: String::new(), + field_key: String::new(), offset, field_size, field_type, @@ -3743,7 +3802,7 @@ impl SimpleFieldDescr { flag, virtualizable: false, index_in_parent: 0, - parent_descr: None, + parent_descr: RwLock::new(None), vinfo: None, } } @@ -3759,12 +3818,14 @@ impl SimpleFieldDescr { is_immutable: bool, flag: ArrayFlag, name: String, + field_key: String, ) -> Self { SimpleFieldDescr { index: AtomicU32::new(index), descr_index: AtomicI32::new(-1), ei_index: AtomicU32::new(u32::MAX), name, + field_key, offset, field_size, field_type, @@ -3773,7 +3834,7 @@ impl SimpleFieldDescr { flag, virtualizable: false, index_in_parent: 0, - parent_descr: None, + parent_descr: RwLock::new(None), vinfo: None, } } @@ -3821,11 +3882,16 @@ impl SimpleFieldDescr { /// GETFIELD/SETFIELD/QUASIIMMUT_FIELD that flows through /// `ensure_ptr_info_arg0` (optimizer.py:478-484). pub fn with_parent_descr(mut self, parent: DescrRef, index_in_parent: usize) -> Self { - self.parent_descr = Some(Arc::downgrade(&parent)); + self.parent_descr = RwLock::new(Some(Arc::downgrade(&parent))); self.index_in_parent = index_in_parent; self } + /// descr.py:238 — update this field's single parent backreference. + pub fn set_parent_descr(&self, parent: &DescrRef) { + *self.parent_descr.write().unwrap() = Some(Arc::downgrade(parent)); + } + /// Builder: attach the owning `VirtualizableInfo` backreference that /// `FieldDescr::get_vinfo()` returns. `vinfo` is stored as a `Weak` /// reference; upgrades succeed for as long as the owning vinfo Arc @@ -3904,11 +3970,18 @@ impl FieldDescr for SimpleFieldDescr { fn field_name(&self) -> &str { &self.name } + fn field_key(&self) -> &str { + &self.field_key + } fn index_in_parent(&self) -> usize { self.index_in_parent } fn get_parent_descr(&self) -> Option { - self.parent_descr.as_ref().and_then(|p| p.upgrade()) + self.parent_descr + .read() + .unwrap() + .as_ref() + .and_then(|p| p.upgrade()) } fn get_vinfo(&self) -> Option> { self.vinfo.as_ref().and_then(|w| w.upgrade()) @@ -4046,6 +4119,24 @@ impl SimpleSizeDescr { self } + /// Add a GC edge that the positional `all_fielddescrs` list does not + /// name. `heaptracker.py:50-73 all_fielddescrs` recurses into the + /// inherited header so upstream's `gc_fielddescrs` covers it; pyre's + /// runtime object groups declare only the concrete payload, and the + /// allocation-clear census still has to see the embedded `PyObject` + /// pointer edge. Kept out of `all_fielddescrs` so the positional + /// indexing above is unaffected. + pub fn with_extra_gc_fielddescr(mut self, fd: Arc) -> Self { + if !self + .gc_fielddescrs + .iter() + .any(|old| old.offset() == fd.offset()) + { + self.gc_fielddescrs.push(fd); + } + self + } + /// gc.py:541: descr.tid = llop.combine_ushort(lltype.Signed, type_id, 0) /// Called by init_size_descr hook before Arc wrapping. pub fn set_type_id(&mut self, type_id: u32) { @@ -4102,11 +4193,23 @@ impl SizeDescr for SimpleSizeDescr { fn vtable(&self) -> usize { self.vtable } + /// The generic size descr has no object model of its own, so the + /// `w_class` identity comes from the frontend's registered + /// [`WClassObjFn`]. Without it `OptVirtualize` cannot fold a `w_class` + /// header read off a `new_with_vtable` virtual and forces the virtual + /// instead, which turns the read into a load of freshly allocated, + /// uninitialised memory and the guard on it into a per-iteration failure. + fn w_class_obj(&self) -> Option { + resolve_w_class_obj(self.vtable) + } } #[derive(Debug, Clone)] pub struct SimpleFieldDescrSpec { pub index: u32, + /// descr.py:220-233 cache key (`fieldname`). + pub field_key: String, + /// descr.py:227 display name. pub name: String, pub offset: usize, pub field_size: usize, @@ -4159,6 +4262,7 @@ pub fn make_simple_descr_group_keyed( is_gc_managed, false, field_specs, + &[], ) } @@ -4171,48 +4275,59 @@ pub fn make_simple_descr_group_keyed_with_headerless( is_gc_managed: bool, headerless: bool, field_specs: &[SimpleFieldDescrSpec], + extra_gc_fielddescrs: &[Arc], ) -> SimpleDescrGroup { - let group = make_simple_descr_group_inner( - index, - size, - type_id, - cache_key, - vtable, - is_gc_managed, - headerless, - field_specs, - ); let struct_key = LLType::struct_key(cache_key); - // `descr.py:108-118 get_size_descr` cache-miss `cache[STRUCT] = - // sizedescr` — for mint sites that bypass `get_size_descr` proper - // and call this factory, publish into the keyed map so - // analyzer-side `cc.fielddescrof` lookups via the same cache_key - // resolve to the same Arc. - crate::descr_registry::register_keyed_size( - struct_key.clone(), - group.size_descr.clone() as DescrRef, - ); - // `descr.py:225-235 get_field_descr` cache-miss - // `cachedict[fieldname] = fielddescr` — the inner-dict key at - // `descr.py:221 cache[STRUCT][fieldname]` is **bare** `fieldname`. - // `fd.name` carries the dotted display form - // (`'%s.%s' % (STRUCT._name, fieldname)`, `descr.py:227`); strip - // the `STRUCT._name` prefix to recover the bare `fieldname` key, - // matching the analyzer's bare-name `cc.fielddescrof_concrete` - // lookup (`call.rs` analyzer) and the runtime macro's bare-name - // `gc_cache.get_field_descr(__majit_key, fname_str, ...)` at - // `jit_struct.rs`. Both arms must publish at the same key so - // `cpu.fielddescrof(STRUCT, fieldname)` per-tuple Arc identity - // holds across analyzer / runtime / BhSize round-trip. - for fd in &group.field_descrs { - let bare_name = fd - .name - .rsplit_once('.') - .map(|(_, n)| n.to_string()) - .unwrap_or_else(|| fd.name.clone()); - crate::descr_registry::register_keyed_field(struct_key.clone(), bare_name, fd.clone()); + let mut gc = gc_cache().lock().unwrap(); + // descr.py:218-239 — cache-or-mint each FieldDescr by + // `(STRUCT, fieldname)` before freezing this producer's positional list. + let field_descrs: Vec> = field_specs + .iter() + .map(|spec| { + gc.get_field_descr( + struct_key.clone(), + &spec.field_key, + spec.offset, + spec.field_size, + spec.field_type, + spec.is_immutable, + spec.is_quasi_immutable, + spec.flag, + spec.index, + spec.virtualizable, + spec.index_in_parent, + ) + }) + .collect(); + let all_fielddescrs: Vec> = field_descrs + .iter() + .cloned() + .map(|field_descr| field_descr as Arc) + .collect(); + let mut sd = SimpleSizeDescr::with_vtable(index, size, type_id, vtable); + sd.set_cache_key(cache_key); + sd.set_gc_managed(is_gc_managed); + sd.set_headerless(headerless); + let mut sd = sd.with_all_fielddescrs(all_fielddescrs); + for fd in extra_gc_fielddescrs { + sd = sd.with_extra_gc_fielddescr(fd.clone()); + } + let size_descr = Arc::new(sd); + let size_ref = size_descr.clone() as DescrRef; + gc.register_keyed_size(struct_key.clone(), size_ref); + let parent = gc + ._cache_size + .get(&struct_key) + .cloned() + .unwrap_or_else(|| size_descr.clone() as DescrRef); + // descr.py:238 — each shared field reports the current cached SizeDescr. + for fd in &field_descrs { + fd.set_parent_descr(&parent); + } + SimpleDescrGroup { + size_descr, + field_descrs, } - group } /// Inner factory shared between [`make_simple_descr_group`] (no @@ -4241,6 +4356,7 @@ fn make_simple_descr_group_inner( descr_index: AtomicI32::new(-1), ei_index: AtomicU32::new(u32::MAX), name: spec.name.clone(), + field_key: spec.field_key.clone(), offset: spec.offset, field_size: spec.field_size, field_type: spec.field_type, @@ -4249,7 +4365,7 @@ fn make_simple_descr_group_inner( flag: spec.flag, virtualizable: spec.virtualizable, index_in_parent: spec.index_in_parent, - parent_descr: Some(parent_descr.clone()), + parent_descr: RwLock::new(Some(parent_descr.clone())), vinfo: None, }) }) @@ -4901,6 +5017,7 @@ pub fn make_vtable_field_descr() -> DescrRef { descr_index: AtomicI32::new(-1), ei_index: AtomicU32::new(u32::MAX), name: "object.typeptr".to_string(), + field_key: "typeptr".to_string(), offset: 0, field_size: std::mem::size_of::(), field_type: crate::Type::Int, @@ -4909,7 +5026,7 @@ pub fn make_vtable_field_descr() -> DescrRef { flag: ArrayFlag::Signed, virtualizable: false, index_in_parent: 0, - parent_descr: Some(parent_descr), + parent_descr: RwLock::new(Some(parent_descr)), vinfo: None, }); *field_descr_cell.borrow_mut() = Some(field_descr.clone()); @@ -5742,10 +5859,11 @@ pub fn make_field_descr_with_parent( field_type, false, flag, + name.clone(), name, ); fd.index_in_parent = index_in_parent; - fd.parent_descr = Some(parent_weak); + fd.parent_descr = RwLock::new(Some(parent_weak)); Arc::new(fd) } diff --git a/majit/majit-ir/src/descr_registry.rs b/majit/majit-ir/src/descr_registry.rs index 5fc95f3ee25..b0c2da7dcbc 100644 --- a/majit/majit-ir/src/descr_registry.rs +++ b/majit/majit-ir/src/descr_registry.rs @@ -151,6 +151,28 @@ pub fn snapshot_all() -> Vec { out } +/// `pyjitpl.py:2289 self.all_descrs = self.cpu.setup_descrs()` — the dense +/// list `descr_index` indexes into (`descr.py:28 v.descr_index = +/// len(all_descrs)`), and the list `bridgeopt.py:155 +/// metainterp_sd.all_descrs[descr_index]` reads back. +/// +/// Upstream stores it on `metainterp_sd` because there is exactly one, built +/// from exactly one `cpu.setup_descrs()`. Pyre's `GcCache` is instead +/// process-global and `set_descr_index` therefore stamps process-globally, so +/// the list those numbers index has to have the same scope: a front-end that +/// carries more than one `MetaInterpStaticData` (pyre keeps a second one for +/// the tracing walker) would otherwise number the descrs off one object's +/// list while `bridgeopt` indexes another object's empty one. +/// +/// `MetaInterpStaticData::all_descrs()` is the accessor; nothing else should +/// reach in here directly. +static ALL_DESCRS: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); + +/// Handle to the process-wide `all_descrs` list documented on [`ALL_DESCRS`]. +pub fn all_descrs() -> &'static std::sync::Mutex> { + &ALL_DESCRS +} + /// `descr.py:28-29 _cache_size` snapshot. pub fn snapshot_sizes() -> Vec { gc_cache().lock().unwrap().snapshot_sizes() diff --git a/majit/majit-ir/src/effectinfo.rs b/majit/majit-ir/src/effectinfo.rs index 173817b716a..e7ab8e6c538 100644 --- a/majit/majit-ir/src/effectinfo.rs +++ b/majit/majit-ir/src/effectinfo.rs @@ -25,6 +25,70 @@ impl std::fmt::Display for UnsupportedFieldExc { impl std::error::Error for UnsupportedFieldExc {} +/// Serializable projection of one member of the six raw EffectInfo descr +/// sets: the gccache key the analyzer minted the descr through. +/// +/// `Arc` cannot cross the `descrs.bin` process boundary, but the +/// key can — both halves of the split agree on it by construction, since +/// `cpu.fielddescrof(STRUCT, fieldname)` / `cpu.arraydescrof(ARRAY)` / +/// `cpu.interiorfielddescrof(ARRAY, fieldname)` are cache lookups on exactly +/// these tuples. The key alone is enough because the member is resolved by +/// lookup only, never by minting (see `descr_from_set_member` in +/// `pyre-jit-trace`). +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum DescrSetMember { + /// `descr.py:218-239 get_field_descr(gccache, STRUCT, fieldname)` — + /// `_cache_field[Struct(struct_id)][field_name]`. + Field { struct_id: u64, field_name: String }, + /// `descr.py:348-378 get_array_descr(gccache, ARRAY)` — + /// `_cache_array[Array(array_id)]`. + Array { array_id: u64 }, + /// `descr.py:404-437 get_interiorfield_descr(gccache, ARRAY, fieldname)` — + /// `_cache_interiorfield[(Array(array_id), name, "")]`. + InteriorField { array_id: u64, name: String }, +} + +/// `effectinfo.py:128-145 frozenset_or_none`: serializable projection of +/// the six raw EffectInfo descr sets. The vectors are kept in the same +/// canonical order as the raw `DescrRef` sets so deserialization can rebuild +/// the exact object graph before `compute_bitstrings`. +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub struct DescrSetKeys { + pub readonly_fields: Vec, + pub write_fields: Vec, + pub readonly_arrays: Vec, + pub write_arrays: Vec, + pub readonly_interiorfields: Vec, + pub write_interiorfields: Vec, +} + +impl DescrSetKeys { + /// The `frozenset_or_none(...)` image of six *empty* frozensets — an EI + /// the analyzer proved touches no field, array or interiorfield. + /// + /// Distinct from `descr_set_keys = None`, which is the + /// `EF_RANDOM_EFFECTS` wildcard: `effectinfo.py:149-162` makes the raw + /// sets `None` **iff** the EI is random-effects, and + /// `compute_bitstrings` (`effectinfo.py:484-489`) reads the two shapes + /// oppositely — empty sets keep the bitstrings, `None` clears them. A + /// concrete-but-empty EI serialized with `None` would therefore come + /// back from `descrs.bin` looking random-effects while its + /// `extraeffect` still says otherwise, and `check_readonly_descr_field` + /// would panic on the cleared bitstring. + /// + /// `const` so the `EffectInfo` associated constants can name it. + pub const fn const_empty() -> Self { + Self { + readonly_fields: Vec::new(), + write_fields: Vec::new(), + readonly_arrays: Vec::new(), + write_arrays: Vec::new(), + readonly_interiorfields: Vec::new(), + write_interiorfields: Vec::new(), + } + } +} + /// `EffectInfo` with setup-time interior mutability for the bitstring /// fields. /// @@ -267,6 +331,22 @@ pub struct EffectInfo { /// effectinfo.py:133 `_write_descrs_interiorfields`. #[serde(skip)] pub _write_descrs_interiorfields: Option>, + /// `descr.py:218-239 get_field_descr`, `descr.py:348-378 get_array_descr`, + /// `descr.py:404-437 get_interiorfield_descr`: serialized channel for + /// the six skipped raw descr sets above. + /// + /// Tracks the `Option`-ness of those sets exactly: `None` here **iff** + /// they are the `EF_RANDOM_EFFECTS` wildcard, `Some` (possibly with + /// empty vectors — [`DescrSetKeys::const_empty`]) whenever they are + /// concrete. `effectinfo.py:149-162` states that biconditional and + /// `compute_bitstrings` relies on it, so the two must not drift apart. + /// + /// Pure derivation of the raw sets, so it is deliberately excluded from + /// `PartialEq`, `Hash`, `LLType::func_key_with_release_gil_breaker`, and + /// `EffectInfoKey::from_effect_info`; including it would split call-descr + /// interning buckets on redundant data. + #[serde(default)] + pub descr_set_keys: Option, /// effectinfo.py:185 bitstring_readonly_descrs_fields. `None` = wildcard /// (effectinfo.py:488-489 sets the bitstring to `None` for `EF_RANDOM_EFFECTS`). pub readonly_descrs_fields: Option>, @@ -374,6 +454,7 @@ impl Default for EffectInfo { _write_descrs_arrays: Some(Vec::new()), _readonly_descrs_interiorfields: Some(Vec::new()), _write_descrs_interiorfields: Some(Vec::new()), + descr_set_keys: Some(DescrSetKeys::const_empty()), // effectinfo.py:175-181: empty frozenset for elidable, but `__new__` // requires a non-None value for non-RandomEffects EIs. Empty Vec // is the bitstring equivalent (no descrs touched). @@ -794,6 +875,7 @@ impl EffectInfo { _write_descrs_arrays: Some(Vec::new()), _readonly_descrs_interiorfields: Some(Vec::new()), _write_descrs_interiorfields: Some(Vec::new()), + descr_set_keys: Some(DescrSetKeys::const_empty()), readonly_descrs_fields: Some(Vec::new()), write_descrs_fields: Some(Vec::new()), readonly_descrs_arrays: Some(Vec::new()), @@ -856,6 +938,7 @@ impl EffectInfo { _write_descrs_arrays: None, _readonly_descrs_interiorfields: None, _write_descrs_interiorfields: None, + descr_set_keys: None, readonly_descrs_fields: None, write_descrs_fields: None, readonly_descrs_arrays: None, diff --git a/majit/majit-macros/src/jit_struct.rs b/majit/majit-macros/src/jit_struct.rs index 3d8f558e591..af32d11b07f 100644 --- a/majit/majit-macros/src/jit_struct.rs +++ b/majit/majit-macros/src/jit_struct.rs @@ -70,6 +70,8 @@ pub(crate) fn expand(_attr: TokenStream, item: TokenStream) -> TokenStream { false, false, #flag_tok, + u32::MAX, + false, #idx, ); } diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index 8a1655fc837..d434fb3f4ef 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -6701,7 +6701,9 @@ fn reject_symbolic_residual_call(bh: &mut BlackholeInterpreter, func: i64) -> Di if crate::majit_log_enabled() { eprintln!( "[bh] residual_call declined: funcptr {func:#x} is a symbolic path hash, not a code \ - address; register the callee's path in the host's fnaddr bindings" + address; register the callee's path in the host's fnaddr bindings (jitcode {:?} pos \ + {} last {})", + bh.jitcode.name, bh.position, bh.last_opcode_position, ); } bh.aborted = true; diff --git a/majit/majit-metainterp/src/call_descr.rs b/majit/majit-metainterp/src/call_descr.rs index 196f909c5a0..6b7c5e388cc 100644 --- a/majit/majit-metainterp/src/call_descr.rs +++ b/majit/majit-metainterp/src/call_descr.rs @@ -290,6 +290,7 @@ pub const CANNOT_RAISE_NO_HEAP_EFFECT_INFO: EffectInfo = EffectInfo { _write_descrs_arrays: Some(Vec::new()), _readonly_descrs_interiorfields: Some(Vec::new()), _write_descrs_interiorfields: Some(Vec::new()), + descr_set_keys: Some(majit_ir::effectinfo::DescrSetKeys::const_empty()), readonly_descrs_fields: Some(Vec::new()), write_descrs_fields: Some(Vec::new()), readonly_descrs_arrays: Some(Vec::new()), @@ -325,6 +326,7 @@ pub const INT_PY_DIV_EFFECT_INFO: EffectInfo = EffectInfo { _write_descrs_arrays: Some(Vec::new()), _readonly_descrs_interiorfields: Some(Vec::new()), _write_descrs_interiorfields: Some(Vec::new()), + descr_set_keys: Some(majit_ir::effectinfo::DescrSetKeys::const_empty()), readonly_descrs_fields: Some(Vec::new()), write_descrs_fields: Some(Vec::new()), readonly_descrs_arrays: Some(Vec::new()), @@ -353,6 +355,7 @@ pub const INT_PY_MOD_EFFECT_INFO: EffectInfo = EffectInfo { _write_descrs_arrays: Some(Vec::new()), _readonly_descrs_interiorfields: Some(Vec::new()), _write_descrs_interiorfields: Some(Vec::new()), + descr_set_keys: Some(majit_ir::effectinfo::DescrSetKeys::const_empty()), readonly_descrs_fields: Some(Vec::new()), write_descrs_fields: Some(Vec::new()), readonly_descrs_arrays: Some(Vec::new()), @@ -389,6 +392,7 @@ pub const UINT_PY_DIV_EFFECT_INFO: EffectInfo = EffectInfo { _write_descrs_arrays: Some(Vec::new()), _readonly_descrs_interiorfields: Some(Vec::new()), _write_descrs_interiorfields: Some(Vec::new()), + descr_set_keys: Some(majit_ir::effectinfo::DescrSetKeys::const_empty()), readonly_descrs_fields: Some(Vec::new()), write_descrs_fields: Some(Vec::new()), readonly_descrs_arrays: Some(Vec::new()), @@ -415,6 +419,7 @@ pub const UINT_PY_MOD_EFFECT_INFO: EffectInfo = EffectInfo { _write_descrs_arrays: Some(Vec::new()), _readonly_descrs_interiorfields: Some(Vec::new()), _write_descrs_interiorfields: Some(Vec::new()), + descr_set_keys: Some(majit_ir::effectinfo::DescrSetKeys::const_empty()), readonly_descrs_fields: Some(Vec::new()), write_descrs_fields: Some(Vec::new()), readonly_descrs_arrays: Some(Vec::new()), @@ -665,6 +670,25 @@ pub fn make_call_descr_void_word_abi(arg_types: &[Type], effect_info: EffectInfo make_call_descr_sized(arg_types, Type::Void, false, 8, effect_info) } +/// Sized variant of [`make_call_descr_with_effect`] for deserialized +/// `descrs.bin` call descriptors whose result size/sign were fixed by +/// `descr.py:650-665 getCallDescrClass`. +pub fn make_call_descr_sized_with_effect( + arg_types: &[Type], + result_type: Type, + result_signed: bool, + result_size: usize, + effect_info: EffectInfo, +) -> DescrRef { + make_call_descr_sized( + arg_types, + result_type, + result_signed, + result_size, + effect_info, + ) +} + fn make_call_descr_sized( arg_types: &[Type], result_type: Type, diff --git a/majit/majit-metainterp/src/jitcode/assembler.rs b/majit/majit-metainterp/src/jitcode/assembler.rs index b74c0830327..bd0e19e8330 100644 --- a/majit/majit-metainterp/src/jitcode/assembler.rs +++ b/majit/majit-metainterp/src/jitcode/assembler.rs @@ -8,9 +8,7 @@ use std::cmp::max; use majit_backend::JitCellToken; use majit_ir::OpCode; -use majit_translate::jitcode::{BhFieldSpec, BhSizeSpec}; - -const HEADERLESS_SIZE_OWNER_MARKER: &str = "__majit_headerless_size__"; +use majit_translate::jitcode::{BhFieldSpec, BhSizeSpec, HEADERLESS_SIZE_OWNER_MARKER}; use crate::jitcode; @@ -629,6 +627,7 @@ impl JitCodeBuilder { // runtime DescrCache mints one; index_in_parent carries // the structural slot the optimizer indexes by. index: u32::MAX, + field_key: name.to_string(), name: name.to_string(), offset, field_size: 8, diff --git a/majit/majit-metainterp/src/jitcode/mod.rs b/majit/majit-metainterp/src/jitcode/mod.rs index 3b1f7d4b06c..b5da411cb53 100644 --- a/majit/majit-metainterp/src/jitcode/mod.rs +++ b/majit/majit-metainterp/src/jitcode/mod.rs @@ -350,8 +350,15 @@ static GLOBAL_BUILD_DESCR_POOL: std::sync::OnceLock = std::sync /// Install the process-global build-time descr pool. Idempotent: the first /// call wins and later calls are ignored (the pool is a frozen build artifact, /// identical across callers). See [`GLOBAL_BUILD_DESCR_POOL`]. -pub fn set_global_build_descr_pool(pool: Vec) { - let _ = GLOBAL_BUILD_DESCR_POOL.set(GlobalDescrPool(pool)); +/// +/// `build` runs only on the call that installs the pool. It takes a closure +/// rather than a built `Vec` because the callers sit on hot paths — the jd1 +/// driver installs before every `_unpackiterable_unknown_length` walk — and +/// building the pool clones every `BhDescr` in the binary (including each call +/// descr's `EffectInfo` raw descr sets). Materializing that just to have +/// `OnceLock::set` drop it is the whole cost of the call. +pub fn init_global_build_descr_pool(build: impl FnOnce() -> Vec) { + GLOBAL_BUILD_DESCR_POOL.get_or_init(|| GlobalDescrPool(build())); } /// The installed global build-time descr pool, or `None` if the embedding diff --git a/majit/majit-metainterp/src/jitdriver.rs b/majit/majit-metainterp/src/jitdriver.rs index efa879b84ab..b0f39172ed4 100644 --- a/majit/majit-metainterp/src/jitdriver.rs +++ b/majit/majit-metainterp/src/jitdriver.rs @@ -1190,23 +1190,11 @@ pub struct JitDriver { + Send, >, >, - /// Dispatch JitCode singleton, registered at install time by - /// `__JitMeta::install_canonical_liveness`. `__trace_` invokes - /// this singleton; the resume-side `resolve_jitcode` closures - /// (`back_edge_internal`, `back_edge_or_run_compiled_internal`) - /// also clone it for the root frame of the multi-frame chain. - /// - /// RPython parity: `metainterp_sd.jitcodes[portal_jd.index]` global - /// registry slot, scoped to the per-`#[jit_interp]` driver. - dispatch_jitcode: Option>, - /// Flat global jitcode registry, indexed by each jitcode's absolute - /// index (`JitCode::index`). Slot 0 = the dispatch JitCode; slots - /// 1..N = every sub-JitCode reachable through the descr pools. Built - /// at `register_dispatch_jitcode`. resume.py:1050/1338 `jitcode = - /// jitcodes[jitcode_pos]` — every resume frame resolves its jitcode - /// from this table by the self-describing index the snapshot stamped, - /// with no parent-relative walk or root/sub bookkeeping. - jitcode_registry: Vec>, + /// jtransform.py:1704 `portal_jd.index` — the `jitdrivers_sd` slot this + /// driver's portal occupies. `call.py:147 jd.mainjitcode` holds the portal + /// JitCode itself, so this driver stores only the slot; `None` until + /// `register_dispatch_jitcode` runs. + portal_jd_index: Option, } thread_local! { @@ -1336,8 +1324,7 @@ impl JitDriver { _invalidation_thread: None, blackhole_allocator: None, portal_runner: None, - dispatch_jitcode: None, - jitcode_registry: Vec::new(), + portal_jd_index: None, shared_asm: std::sync::Arc::new(std::sync::Mutex::new( majit_translate::codewriter::assembler::Assembler::new(), )), @@ -1448,8 +1435,28 @@ impl JitDriver { } } } - self.dispatch_jitcode = Some(dispatch_arc); - self.jitcode_registry = registry.clone(); + // call.py:147 `jd.mainjitcode = self.get_jitcode(jd.portal_graph)` — the + // portal JitCode belongs to the driver's static data, not to the runtime + // driver. The back-pointer of call.py:148 has no counterpart here: the + // metainterp-side `JitCode` carries no `jitdriver_sd` slot (only the + // translate-side one does, via `set_jitdriver_sd`), so the slot index + // below is the link in that direction. + // `call.py:46-47 jd.index = idx` is this driver's own slot — the macro's + // install pipeline runs `ensure_descriptor_registered` before reaching + // here, so it is assigned. Fall back to the single-portal slot 0 only + // when a consumer skipped that step. + self.ensure_descriptor_registered(); + let portal_jd_index = self.index().unwrap_or(0); + self.meta + .jitdriver_sd_mut(portal_jd_index) + .expect("register_dispatch_jitcode: this driver's jitdrivers_sd slot is vacant") + .mainjitcode = Some(dispatch_arc); + self.portal_jd_index = Some(portal_jd_index); + // warmspot.py:281-282 `metainterp_sd.jitcodes = make_jitcodes()` — the + // drained list is owned by the staticdata, which is what `resume.py:1051` + // indexes. `Arc` identity carries over from the worklist above, so the + // `Arc::ptr_eq` dedup and the `set_index` stamps stay valid. + self.meta.install_jitcodes(registry.clone()); // Publish the registry + packed liveness for the stateless global // `frame_value_count` decode. `install_canonical_liveness` ran just // before this call, so staticdata carries the final liveness buffer. @@ -1458,10 +1465,12 @@ impl JitDriver { install_state_field_fvc(registry, all_liveness, op_live); } - /// Access the registered dispatch JitCode. Returns `None` until - /// `register_dispatch_jitcode` has been called. + /// pyjitpl.py:3294 `self.jitdriver_sd.mainjitcode` — this driver's portal + /// JitCode. Returns `None` until `register_dispatch_jitcode` has been + /// called; upstream has no such window, since `call.py:145-148` assigns + /// every driver's `mainjitcode` before the metainterp ever runs. pub fn dispatch_jitcode(&self) -> Option<&std::sync::Arc> { - self.dispatch_jitcode.as_ref() + self.meta.mainjitcode_of(self.portal_jd_index?) } /// Register a BlackholeAllocator for virtual materialization during @@ -2059,7 +2068,7 @@ impl JitDriver { // `last_compiled_key` is registered: on a cross-loop cut that // is the inner loop the close point belongs to, whereas // `loop_green_key` is the outer trace-start key. - if let Some(greens) = loop_close_greens.clone() { + if let Some(greens) = loop_close_greens { if crate::closedbg_enabled() { eprintln!("@@@CLOSE LOOP-GREENS key={} greens={greens:?}", k as i64); } @@ -3587,7 +3596,7 @@ impl JitDriver { // resolve every frame statelessly from the flat global // registry by its self-describing absolute index (no root/sub // branch, no parent-relative descrs walk, no last-frame state). - let jitcode_registry = self.jitcode_registry.clone(); + let jitcode_registry = self.meta.jitcodes().to_vec(); let resolve_jitcode = |jitcode_index: i32, pc: i32| -> Option { @@ -4967,15 +4976,13 @@ impl JitDriver { // a strong owner of every running loop, so releasing here // is the only path that drops those Arcs. self.meta.warm_state.memory_manager.release_all_loops(); - self.meta.compiled_loops.clear(); + self.meta.clear_compiled_loops(); } /// Invalidate compiled code for a specific trace_id, removing the /// compiled_loops entry whose root_trace_id matches. pub fn invalidate_compiled_trace(&mut self, trace_id: u64) { - self.meta - .compiled_loops - .retain(|_, entry| entry.root_trace_id != trace_id); + self.meta.invalidate_compiled_trace(trace_id); } /// warmspot.py:449 — set the per-driver result_type. @@ -5447,7 +5454,7 @@ impl JitDriver { // Scoped to state-field drivers by the `dispatch_jitcode` probe — a // JitState that overrides `setup_bridge_sym` with real multi-frame // support (pyre) registers no dispatch JitCode and is unaffected. - if self.dispatch_jitcode.is_some() + if self.dispatch_jitcode().is_some() && resume_data_result .as_ref() .is_some_and(|r| r.frames.len() > 1) @@ -5969,7 +5976,7 @@ impl JitDriver { // resolve every frame statelessly from the flat global // registry by its self-describing absolute index (no root/sub // branch, no parent-relative descrs walk, no last-frame state). - let jitcode_registry = self.jitcode_registry.clone(); + let jitcode_registry = self.meta.jitcodes().to_vec(); let resolve_jitcode = |jitcode_index: i32, pc: i32| -> Option { diff --git a/majit/majit-metainterp/src/lib.rs b/majit/majit-metainterp/src/lib.rs index 01a73990a3e..9e41622f586 100644 --- a/majit/majit-metainterp/src/lib.rs +++ b/majit/majit-metainterp/src/lib.rs @@ -109,7 +109,7 @@ pub use call_descr::{ INT_PY_MOD_EFFECT_INFO, LOOPINVARIANT_EFFECT_INFO, cannot_raise_effect_info, default_effect_info, effect_info_for_slot, forces_virtual_or_virtualizable_effect_info, make_call_assembler_descr, make_call_descr, make_call_descr_from_target_slot, - make_call_descr_with_effect, nursery_alloc_effect_info, + make_call_descr_sized_with_effect, make_call_descr_with_effect, nursery_alloc_effect_info, }; pub use compile::{ make_fail_descr, make_fail_descr_typed, make_finish_fail_descr_typed, @@ -125,7 +125,7 @@ pub use jit_state::{ }; pub use jitcode::{ BC_GOTO, JitArgKind, JitCallArg, JitCode, JitCodeBuilder, LivenessInfo, RuntimeBhDescr, insns, - live_slots_for_state_field_jit, set_global_build_descr_pool, + init_global_build_descr_pool, live_slots_for_state_field_jit, }; pub use jitdriver::{ DeclarativeJitDriver, JitDriver, JitDriverStaticData, MultiFrameBlackholeResult, @@ -141,6 +141,11 @@ pub use pyjitpl::{eval_binop_f, eval_binop_i, eval_float_cmp, eval_unary_f, eval // for `JitCode` / `BhDescr` re-exports above (`jitcode/mod.rs:4`). pub use majit_translate::codewriter::assembler::Assembler; pub use parity::{TraceParityCase, assert_trace_parity, normalize_ops, normalize_trace}; +/// The walker's own `getfield_gc` / `setfield_gc` descr resolution +/// (`blackhole.py:1432-1483` reads the descr straight out of the constant +/// pool). Exported so the descr-identity census can compare it against the +/// pool-side resolution without re-deriving a second copy of the logic. +pub use pyjitpl::dispatch::field_descr_ref_from_bh; pub use pyjitpl::{ BackEdgeAction, BridgeRetraceResult, ClosureRuntime, ClosureRuntimeWithResolver, CompileOutcome, CompiledExitLayout, CompiledTerminalExitLayout, CompiledTraceLayout, diff --git a/majit/majit-metainterp/src/opencoder.rs b/majit/majit-metainterp/src/opencoder.rs index a0e15323fbc..956fc8176d7 100644 --- a/majit/majit-metainterp/src/opencoder.rs +++ b/majit/majit-metainterp/src/opencoder.rs @@ -837,7 +837,7 @@ impl<'a> Iterator for ByteTraceIter<'a> { let resolved = if descr_index == 0 || opcode.is_guard() { None } else { - let all_descrs = self.trace.metainterp_sd.all_descrs.lock().unwrap(); + let all_descrs = self.trace.metainterp_sd.all_descrs().lock().unwrap(); let all_descr_len = all_descrs.len() as i64; if descr_index < all_descr_len + 1 { Some(all_descrs[(descr_index - 1) as usize].clone()) @@ -1479,7 +1479,7 @@ impl Trace { /// the global descriptor table length from the attached /// metainterp_sd. fn all_descrs_len(&self) -> u32 { - self.metainterp_sd.all_descrs.lock().unwrap().len() as u32 + self.metainterp_sd.all_descrs().lock().unwrap().len() as u32 } /// opencoder.py:503-508 set_inputargs(inputargs). @@ -3905,14 +3905,14 @@ mod tests { } } - let mut sd = crate::MetaInterpStaticData::new(); + let sd = crate::MetaInterpStaticData::new(); // Seed all_descrs with 7 dummies so the length drives the encoding. - for _ in 0..7 { - sd.all_descrs - .get_mut() - .unwrap() - .push(Arc::new(D { idx: 0 })); - } + // The list is process-wide (`descr_registry::all_descrs`), so set it + // rather than append — a sibling test's leftovers would otherwise + // shift the encoding this asserts on. + *sd.all_descrs().lock().unwrap() = (0..7) + .map(|_| Arc::new(D { idx: 0 }) as majit_ir::descr::DescrRef) + .collect(); let mut buf = TraceRecordBuffer::new(0, Arc::new(sd)); // Global descr returns `get_descr_index() + 1`. diff --git a/majit/majit-metainterp/src/optimizeopt/optimizer.rs b/majit/majit-metainterp/src/optimizeopt/optimizer.rs index a2ddd14d0d6..24bacbccec7 100644 --- a/majit/majit-metainterp/src/optimizeopt/optimizer.rs +++ b/majit/majit-metainterp/src/optimizeopt/optimizer.rs @@ -3183,7 +3183,28 @@ impl Optimizer { // preamble target (unroll.py:238-242). Only the // loop/peeled-loop path (optimize_peeled_loop // unroll.py:135-145) keeps this fatal. + // + // Neither arm is reachable today: the preview exports + // its state from `post_force_args` and re-matches that + // same list, so every `state[i]` was derived from + // `args[i]` and the walk is self-consistent. Probed + // with five virtual-carrying fixtures (escaping tuple, + // escaping instance, aliased list, varying-length + // array, nested virtual), two of which do compile + // bridges — zero hits, as with the aheui corpus and + // pyre/bench + pyre/extra_tests. Upstream matches + // against a *different* loop's stored state in + // `jump_to_existing_trace` (unroll.py:207); + // `export_state_re_matched_against_its_own_args_cannot_fail` + // (virtualstate.rs) pins the self-match, so moving the + // preview to the upstream shape breaks that test and + // flags this branch as newly live. if building_bridge { + if crate::bridge_debug_enabled() { + eprintln!( + "[bridgeB] preview virtual-state mismatch — leaving the export empty for the jump_to_existing_trace ladder" + ); + } break 'export None; } return Err(crate::optimize::InvalidLoop( @@ -5821,6 +5842,7 @@ mod tests { 0, &[majit_ir::descr::SimpleFieldDescrSpec { index: 91, + field_key: "CallResult.field".to_string(), name: "CallResult.field".to_string(), offset: 0, field_size: 8, @@ -5912,6 +5934,7 @@ mod tests { &[ majit_ir::descr::SimpleFieldDescrSpec { index: 101, + field_key: "CallResult.type".to_string(), name: "CallResult.type".to_string(), offset: 0, field_size: 8, @@ -5924,6 +5947,7 @@ mod tests { }, majit_ir::descr::SimpleFieldDescrSpec { index: 102, + field_key: "CallResult.value".to_string(), name: "CallResult.value".to_string(), offset: 8, field_size: 8, diff --git a/majit/majit-metainterp/src/optimizeopt/pure.rs b/majit/majit-metainterp/src/optimizeopt/pure.rs index 8685b410b94..6763a9b6c2f 100644 --- a/majit/majit-metainterp/src/optimizeopt/pure.rs +++ b/majit/majit-metainterp/src/optimizeopt/pure.rs @@ -2259,6 +2259,7 @@ mod tests { 0, &[majit_ir::descr::SimpleFieldDescrSpec { index, + field_key: format!("CseField{index}"), name: format!("CseField{index}"), offset: 0, field_size: 8, diff --git a/majit/majit-metainterp/src/optimizeopt/virtualize.rs b/majit/majit-metainterp/src/optimizeopt/virtualize.rs index 11ff75de4ce..adc0a33030b 100644 --- a/majit/majit-metainterp/src/optimizeopt/virtualize.rs +++ b/majit/majit-metainterp/src/optimizeopt/virtualize.rs @@ -3720,6 +3720,7 @@ mod tests { 0, &[majit_ir::descr::SimpleFieldDescrSpec { index: 10, + field_key: "Node.value".to_string(), name: "Node.value".to_string(), offset: 16, field_size: 8, @@ -5361,6 +5362,7 @@ mod tests { 0, &[majit_ir::descr::SimpleFieldDescrSpec { index: 10, + field_key: "Node.value".to_string(), name: "Node.value".to_string(), offset: 0, field_size: 8, diff --git a/majit/majit-metainterp/src/optimizeopt/virtualstate.rs b/majit/majit-metainterp/src/optimizeopt/virtualstate.rs index 9bdc3044db5..6257403c1fc 100644 --- a/majit/majit-metainterp/src/optimizeopt/virtualstate.rs +++ b/majit/majit-metainterp/src/optimizeopt/virtualstate.rs @@ -3255,6 +3255,56 @@ mod tests { ); } + /// The export preview in `optimize_with_constants_and_inputs_at` builds the + /// state from a list of oprefs and immediately re-matches it against that + /// SAME list, so `make_inputargs_and_virtuals` cannot raise + /// `VirtualStatesCantMatch` there: every `state[i]` was derived from + /// `args[i]`. + /// + /// That self-match is why the optimizer's `building_bridge` branch — which + /// keeps a preview mismatch non-fatal on the bridge path (unroll.py:193, + /// 207-210) — is unreachable today; probing it with five virtual-carrying + /// fixtures, two of which do compile bridges, produced zero hits. + /// + /// Upstream matches a preamble's exported state against a DIFFERENT loop's + /// stored state in `jump_to_existing_trace` (unroll.py:207). Whoever moves + /// the preview to that shape will break this test — and that is the signal + /// that the `building_bridge` branch has become live and needs its own + /// coverage. + #[test] + fn export_state_re_matched_against_its_own_args_cannot_fail() { + let descr = test_descr(10); + let mut ctx = OptContext::new(32); + let object = OpRef::ref_op(10); + let field = OpRef::int_op(11); + let scalar = OpRef::int_op(12); + ctx.materialize_operand_at(object); + ctx.materialize_operand_at(field); + ctx.materialize_operand_at(scalar); + + let object_box = ctx + .get_box_replacement_operand_opt(object) + .expect("object box is bound"); + let mut info = PtrInfo::virtual_obj(descr, None); + info.setfield( + 0, + crate::history::test_support::rooted_resop_operand(Type::Int, field.raw()), + ); + ctx.set_ptr_info(&object_box, info); + + let args = [object, field, scalar]; + let state = export_state(&args, &ctx); + let mut optimizer = crate::optimizeopt::optimizer::Optimizer::new(); + let matched = state.make_inputargs_and_virtuals(&args, &mut optimizer, &mut ctx, false); + + assert!( + matched.is_ok(), + "a state exported from `args` must re-match `args`; \ + a failure here means the preview is no longer a self-match and the \ + optimizer's `building_bridge` branch is now reachable" + ); + } + /// virtualstate.py:196 / 274 / 352 — `state.position > self.position` /// shared-substate dedup parity. When two top-level state entries /// reference the same `Rc` (an aliased nested box), diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index a234ae48568..618937d003c 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -1,4 +1,4 @@ -pub(crate) mod dispatch; +pub mod dispatch; mod frame; pub use dispatch::build_state_field_snapshot; @@ -1247,7 +1247,7 @@ pub struct MetaInterp { pending_preamble_tokens: indexmap::IndexMap>, // pyjitpl.py:2289 `self.staticdata.all_descrs = self.cpu.setup_descrs()` now // lives on MetaInterpStaticData (RPython `metainterp_sd.all_descrs`). - // Access via `self.staticdata.all_descrs` / `&mut self.staticdata.all_descrs`. + // Access via `self.staticdata.all_descrs()`. /// bridgeopt.py:124 frontend_boxes parity: runtime values from the /// guard failure DeadFrame. Saved by start_retrace_from_guard, used /// by compile_bridge for cls_of_box during deserialize_optimizer_knowledge. @@ -2633,6 +2633,47 @@ impl MetaInterp { staticdata.install_canonical_liveness(asm); } + /// warmspot.py:281-282 `self.metainterp_sd.jitcodes = jitcodes` — hand the + /// codewriter's drained jitcode list to the staticdata, the single table + /// `resume.py:1051` indexes. + /// + /// Same single-owner window as [`Self::install_canonical_liveness`], which + /// the install pipeline runs immediately before this. + pub fn install_jitcodes(&mut self, jitcodes: Vec>) { + let staticdata = std::sync::Arc::get_mut(&mut self.staticdata).expect( + "MetaInterp::install_jitcodes called after `staticdata` was cloned; \ + RPython warmspot.py:281-282 installs the table while the \ + MetaInterpStaticData still has a single owner", + ); + staticdata.install_jitcodes(jitcodes); + } + + /// resume.py:1051 `jitcode = metainterp.staticdata.jitcodes[jitcode_pos]`. + pub fn jitcodes(&self) -> &[std::sync::Arc] { + &self.staticdata.jitcodes + } + + /// Mutable borrow of one `jitdrivers_sd` slot, for the install-time wiring + /// `warmspot.py` performs on `jd` before the staticdata is shared. + pub fn jitdriver_sd_mut( + &mut self, + index: usize, + ) -> Option<&mut crate::jitdriver::JitDriverStaticData> { + std::sync::Arc::get_mut(&mut self.staticdata) + .expect("jitdriver_sd_mut: staticdata has other owners") + .jitdrivers_sd + .get_mut(index) + } + + /// call.py:147 `jd.mainjitcode` for a `jitdrivers_sd` slot. + pub fn mainjitcode_of(&self, index: usize) -> Option<&std::sync::Arc> { + self.staticdata + .jitdrivers_sd + .get(index)? + .mainjitcode + .as_ref() + } + /// Copy a freshly-snapshotted `all_liveness` /// byte stream into `staticdata.liveness_info` without re-running /// the full `install_canonical_liveness` insn-id seeding. @@ -2913,9 +2954,31 @@ impl MetaInterp { /// pyjitpl.py:2289 / descr.py:25-47 parity: take back all_descrs from /// optimizer after compilation. Optimizer.ensure_descr_index() assigns - /// sequential descr_index during collect_optimizer_knowledge_for_resume(). + /// sequential descr_index during collect_optimizer_knowledge_for_resume(), + /// and may append, so the optimizer's copy is written back wholesale. + /// + /// The optimizer is handed a **clone**, not `std::mem::take` of the slot: + /// `pyjitpl.py:2290` treats `metainterp_sd.all_descrs` as read-only after + /// `setup_descrs`, and emptying it for the duration of an optimize left any + /// reader that ran inside that window with a zero-length universe. + /// `bridgeopt.py:155 descr = metainterp_sd.all_descrs[descr_index]` is one + /// such reader (the bridge's `PendingBridgeRd` snapshot), and it indexes + /// blind. + /// + /// The list is **monotone**: `descr.py:25-47 setup_descrs` numbers it once + /// and `descr.py:28 v.descr_index = len(all_descrs); all_descrs.append(v)` + /// only ever appends, so a shorter write-back cannot be a legitimate new + /// universe. It comes from an optimizer that never received the seed — + /// `unroll.rs` hands `all_descrs` to each phase with `std::mem::take` and + /// restores it on the way out, so any early exit between the two leaves + /// the outer `UnrollOptimizer` holding an empty vector, which + /// `compile_loop` then publishes. Ignore those instead of invalidating + /// every `descr_index` already serialized into a compiled bridge. pub(crate) fn take_back_all_descrs(&mut self, all_descrs: Vec) { - *self.staticdata.all_descrs.lock().unwrap() = all_descrs; + let mut slot = self.staticdata.all_descrs().lock().unwrap(); + if all_descrs.len() >= slot.len() { + *slot = all_descrs; + } } /// Accessor for `pending_frontend_boxes` without consuming it. @@ -5578,7 +5641,7 @@ impl MetaInterp { let mut unroll_opt = crate::optimizeopt::unroll::UnrollOptimizer::new(); unroll_opt.compile_snapshot_root_slots = Some((&mut self.compile_snapshot_refs as *mut Vec) as usize); - unroll_opt.all_descrs = std::mem::take(&mut *self.staticdata.all_descrs.lock().unwrap()); + unroll_opt.all_descrs = self.staticdata.all_descrs().lock().unwrap().clone(); unroll_opt.target_tokens = prior_front_target_tokens.clone(); unroll_opt.retraced_count = prior_retraced_count_early; unroll_opt.retrace_limit = self.warm_state.retrace_limit(); @@ -5735,7 +5798,15 @@ impl MetaInterp { } else { Optimizer::default_pipeline() }; - simple_opt.all_descrs = std::mem::take(&mut unroll_opt.all_descrs); + // Clone rather than move: only the success arm below hands + // the list back, so a retry that aborts would otherwise + // leave `unroll_opt.all_descrs` empty, and the + // `take_back_all_descrs` at the end of compile_loop would + // blank `metainterp_sd.all_descrs` for the rest of the + // process. `pyjitpl.py:2288-2290` treats that list as + // read-only after `setup_descrs`; `bridgeopt.py:155` + // indexes it blind. + simple_opt.all_descrs = unroll_opt.all_descrs.clone(); // history.py:220/261/307: `Const.type` / // `InputArg.type` are intrinsic on the box; // no raw-u32 type side-table propagation is @@ -6988,7 +7059,7 @@ impl MetaInterp { let mut unroll_opt = crate::optimizeopt::unroll::UnrollOptimizer::new(); unroll_opt.compile_snapshot_root_slots = Some((&mut self.compile_snapshot_refs as *mut Vec) as usize); - unroll_opt.all_descrs = std::mem::take(&mut *self.staticdata.all_descrs.lock().unwrap()); + unroll_opt.all_descrs = self.staticdata.all_descrs().lock().unwrap().clone(); unroll_opt.target_tokens = prior_front_target_tokens.clone(); unroll_opt.retraced_count = self .compiled_loops @@ -7539,7 +7610,7 @@ impl MetaInterp { } else { Optimizer::default_pipeline() }; - optimizer.all_descrs = std::mem::take(&mut *self.staticdata.all_descrs.lock().unwrap()); + optimizer.all_descrs = self.staticdata.all_descrs().lock().unwrap().clone(); optimizer.call_pure_results = simple_data.call_pure_results.clone(); // history.py:_make_op parity: every InputArg carries its type // from the recorder. Propagate those raw recorder types to the @@ -7962,7 +8033,7 @@ impl MetaInterp { } else { Optimizer::default_pipeline() }; - optimizer.all_descrs = std::mem::take(&mut *self.staticdata.all_descrs.lock().unwrap()); + optimizer.all_descrs = self.staticdata.all_descrs().lock().unwrap().clone(); optimizer.call_pure_results = simple_data.call_pure_results.clone(); // history.py:220/261/307 — `Const.type` / `InputArg.type` are // intrinsic on the box itself (recovered via `OpRef::ty()` from @@ -9050,6 +9121,32 @@ impl MetaInterp { pub fn remove_compiled_loop(&mut self, green_key: u64) { self.compiled_loops.swap_remove(&green_key); self.pending_preamble_tokens.swap_remove(&green_key); + self.forget_loop_side_tables(green_key); + } + + /// Drop every compiled loop whose root trace is `trace_id`, with the + /// per-loop side tables that belong to it. + pub fn invalidate_compiled_trace(&mut self, trace_id: u64) { + let stale: Vec = self + .compiled_loops + .iter() + .filter(|(_, entry)| entry.root_trace_id == trace_id) + .map(|(green_key, _)| *green_key) + .collect(); + for green_key in stale { + self.compiled_loops.swap_remove(&green_key); + self.forget_loop_side_tables(green_key); + } + } + + /// Drop the per-loop side tables (`loop_header_pcs`, `loop_header_greens`) + /// when a loop is retired, so they cannot outlive `compiled_loops`. + /// `compiled_key_for_greens` already skips keys without compiled targets, + /// so a leftover entry could not mis-target a bridge — but keeping them + /// would grow both maps without bound over a long run. + fn forget_loop_side_tables(&mut self, green_key: u64) { + self.loop_header_pcs.swap_remove(&green_key); + self.loop_header_greens.swap_remove(&green_key); } /// rpython/rlib/rstack.py:75-90 `stack_almost_full` — delegates to @@ -9143,6 +9240,7 @@ impl MetaInterp { // entry (the merged `traces` map and the // previous_tokens Vec drop together). self.compiled_loops.swap_remove(&gk); + self.forget_loop_side_tables(gk); if crate::debug::have_debug_prints() { crate::debug::log_one( "jit-mem-collect", @@ -9672,8 +9770,15 @@ impl MetaInterp { /// Remove all compiled loops. Used when guard-fail recovery is /// unrecoverable (null Ref in resume data). + /// + /// Bulk form of `remove_compiled_loop`, so it drops the per-loop side + /// tables too — see `forget_loop_side_tables`. `pending_preamble_tokens` + /// is left alone: it is keyed by green key but holds tokens for a + /// recompile that has not happened yet, not for the loops being dropped. pub fn clear_compiled_loops(&mut self) { self.compiled_loops.clear(); + self.loop_header_pcs.clear(); + self.loop_header_greens.clear(); } /// warmstate.py:385 — whether this driver's portal returns a raw int. @@ -10188,7 +10293,7 @@ impl MetaInterp { let bridge_runtime_boxes = prepared.runtime_boxes.as_slice(); let mut optimizer = self.make_optimizer(); - optimizer.all_descrs = std::mem::take(&mut *self.staticdata.all_descrs.lock().unwrap()); + optimizer.all_descrs = self.staticdata.all_descrs().lock().unwrap().clone(); // history.py:220 box.type parity: promote the legacy `i64` pool // to a typed `Value` map for the optimizer's intrinsic Const // class identity. @@ -10702,7 +10807,7 @@ impl MetaInterp { frontend_boxes, liveboxes, livebox_types, - all_descrs: self.staticdata.all_descrs.lock().unwrap().clone(), + all_descrs: self.staticdata.all_descrs().lock().unwrap().clone(), cpu: self.cpu.clone(), }) }); @@ -10786,7 +10891,7 @@ impl MetaInterp { let bridge_runtime_boxes = prepared.runtime_boxes.as_slice(); let mut optimizer = self.make_optimizer(); - optimizer.all_descrs = std::mem::take(&mut *self.staticdata.all_descrs.lock().unwrap()); + optimizer.all_descrs = self.staticdata.all_descrs().lock().unwrap().clone(); if let Some(prd) = pending_bridge_rd.as_ref() { // bridgeopt.py:126 `assert len(frontend_boxes) == len(liveboxes)`. // The concrete values belong on the fresh bridge InputArg objects @@ -15309,6 +15414,20 @@ pub struct MetaInterpStaticData { /// `pyjitpl.py:2334-2342`), but pyre has not yet switched this /// storage edge over to the canonical codewriter `JitCode`. pub indirectcalltargets: Vec>, + /// warmspot.py:281-282 `metainterp_sd.jitcodes = codewriter.make_jitcodes()`. + /// + /// The one flat jitcode table. `codewriter.py:68` stamps each entry with + /// its position (`jitcode.index = len(all_jitcodes)` at the drain in + /// `codewriter.py:80`), and every resume frame carries that absolute index, + /// so `resume.py:1051 jitcode = metainterp.staticdata.jitcodes[jitcode_pos]` + /// resolves a frame with no per-driver or parent-relative bookkeeping. + /// + /// Upstream fills this once at translation, but the structure itself is a + /// growable memoized worklist — `call.py:155-172 get_jitcode` appends on a + /// miss and `codewriter.py:79-81` numbers each entry as it drains. Indices + /// are therefore append-only: published resume data bakes them + /// (`resume.py:250-252`), so an entry's index must never be reassigned. + pub jitcodes: Vec>, /// pyjitpl.py:2251-2253 `setup_list_of_addr2name(list_of_addr2name)`. /// Pair-list of (fnaddr, name) for debug introspection. pub _addr2name_keys: Vec, @@ -15390,26 +15509,6 @@ pub struct MetaInterpStaticData { /// default; Rust needs interior mutability for the same /// behavior. pub globaldata: std::sync::Mutex, - /// pyjitpl.py:2289 `self.staticdata.all_descrs = self.cpu.setup_descrs()`. - /// descr.py:25-47: dense list indexed by `descr_index`. - /// - /// RPython stores this on `metainterp_sd` (the static data object), - /// not on the live `MetaInterp` — opencoder / bridgeopt / optimizer - /// all read `metainterp_sd.all_descrs`. Pyre mirrors that location - /// so `opencoder::Trace` (which lives in this crate now) can read - /// the length directly via `self.metainterp_sd.all_descrs.lock().unwrap().len()` - /// from `_encode_descr` and the TraceIterator. - /// - /// TODO: wrapped in `Mutex` because - /// `MetaInterp.staticdata` is `Arc` and the - /// `TraceRecordBuffer` inside `TraceCtx` holds a clone of this Arc - /// (opencoder.py:471 `self.metainterp_sd = metainterp_sd` — - /// shared Python reference; lifts to Arc in Rust). With refcount ≥ 2, - /// `Arc::get_mut` fails, so `mem::take` at compile time and - /// `take_back_all_descrs` at post-optimize both route through a - /// Mutex lock. RPython's Python dicts are shared mutable references - /// by default; Rust needs interior mutability for the same behavior. - pub all_descrs: std::sync::Mutex>, /// `descr.py:20 GcCache._cache_array` parity for the dispatch JitCode /// trace-side `BC_GETARRAYITEM_GC_I` recorder. /// @@ -15570,6 +15669,19 @@ impl crate::compile::DescrContainer for MetaInterpStaticData { } impl MetaInterpStaticData { + /// pyjitpl.py:2289 `self.all_descrs = self.cpu.setup_descrs()` — + /// descr.py:25-47's dense list, indexed by `descr_index`. opencoder / + /// bridgeopt / optimizer all read `metainterp_sd.all_descrs`; this is + /// that slot. + /// + /// Upstream's is a plain attribute on the one `metainterp_sd`. Pyre's + /// backing store is `descr_registry::all_descrs()` because `descr_index` + /// is stamped off the process-wide `GcCache` — see that static's docs for + /// why the list cannot be per-instance. + pub fn all_descrs(&self) -> &'static std::sync::Mutex> { + majit_ir::descr_registry::all_descrs() + } + pub fn new() -> Self { // `pyjitpl.py:2222` `compile.make_and_attach_done_descrs([self, cpu])`. // RPython passes `[self, cpu]` — the same `Arc` @@ -15977,7 +16089,7 @@ impl MetaInterpStaticData { // Publish onto staticdata.all_descrs so opencoder / bridgeopt / // optimizer reads pick up the same length. - *self.all_descrs.lock().unwrap() = all_descrs.clone(); + *self.all_descrs().lock().unwrap() = all_descrs.clone(); // pyjitpl.py:2290 `effectinfo.compute_bitstrings(self.all_descrs)`. // Two-pass mutation: clone each call descr's EI for the algorithm @@ -16074,6 +16186,18 @@ impl MetaInterpStaticData { self.globaldata.lock().unwrap().indirectcall_dict = None; } + /// warmspot.py:281-282 `self.metainterp_sd.jitcodes = jitcodes` — install + /// a codewriter's drained jitcode list. Each entry must already carry its + /// absolute index (`codewriter.py:68`). + pub fn install_jitcodes(&mut self, jitcodes: Vec>) { + self.jitcodes = jitcodes; + } + + /// resume.py:1051 `jitcode = metainterp.staticdata.jitcodes[jitcode_pos]`. + pub fn jitcode_at(&self, index: usize) -> Option<&std::sync::Arc> { + self.jitcodes.get(index) + } + /// pyjitpl.py:2251-2253 `setup_list_of_addr2name(list_of_addr2name)`. pub fn setup_list_of_addr2name(&mut self, list_of_addr2name: Vec<(usize, String)>) { self._addr2name_keys = list_of_addr2name.iter().map(|(k, _)| *k).collect(); @@ -21837,3 +21961,78 @@ mod tests { assert!(hooks.on_compile_error.is_none()); } } + +/// Every path that retires a `compiled_loops` entry must also retire the +/// per-loop side tables keyed by the same green key, so they cannot outlive +/// the loop and grow without bound over a long run. +#[cfg(test)] +mod loop_side_table_tests { + use super::*; + + fn compiled_entry(root_trace_id: u64) -> CompiledEntry<()> { + CompiledEntry { + token: std::sync::Weak::new(), + meta: (), + front_target_tokens: Vec::new(), + front_target_source_positions: None, + root_trace_id, + traces: indexmap::IndexMap::new(), + previous_tokens: Vec::new(), + next_global_opref: 0, + } + } + + /// Register a loop at `green_key` with both side tables populated. + fn record_loop(meta: &mut MetaInterp<()>, green_key: u64, root_trace_id: u64) { + meta.compiled_loops + .insert(green_key, compiled_entry(root_trace_id)); + meta.record_loop_header_pc(green_key, green_key as usize); + meta.record_loop_header_greens(green_key, (vec![green_key as i64], vec![], vec![])); + } + + fn has_side_tables(meta: &MetaInterp<()>, green_key: u64) -> bool { + meta.loop_header_pcs.contains_key(&green_key) + || meta.loop_header_greens.contains_key(&green_key) + } + + #[test] + fn remove_compiled_loop_drops_the_side_tables() { + let mut meta = MetaInterp::<()>::new(1); + record_loop(&mut meta, 7, 100); + record_loop(&mut meta, 8, 101); + + meta.remove_compiled_loop(7); + + assert!(!has_side_tables(&meta, 7)); + assert!(has_side_tables(&meta, 8)); + } + + #[test] + fn clear_compiled_loops_drops_every_side_table() { + let mut meta = MetaInterp::<()>::new(1); + record_loop(&mut meta, 7, 100); + record_loop(&mut meta, 8, 101); + + meta.clear_compiled_loops(); + + assert!(meta.compiled_loops.is_empty()); + assert!(meta.loop_header_pcs.is_empty()); + assert!(meta.loop_header_greens.is_empty()); + } + + #[test] + fn invalidate_compiled_trace_drops_the_side_tables_of_the_matching_loops_only() { + let mut meta = MetaInterp::<()>::new(1); + record_loop(&mut meta, 7, 100); + record_loop(&mut meta, 8, 100); + record_loop(&mut meta, 9, 101); + + meta.invalidate_compiled_trace(100); + + assert_eq!(meta.compiled_loops.len(), 1); + assert!(meta.compiled_loops.contains_key(&9)); + assert!(!has_side_tables(&meta, 7)); + assert!(!has_side_tables(&meta, 8)); + assert!(has_side_tables(&meta, 9)); + } +} diff --git a/majit/majit-metainterp/src/pyjitpl/dispatch.rs b/majit/majit-metainterp/src/pyjitpl/dispatch.rs index 3ecf77c3101..87bf1215293 100644 --- a/majit/majit-metainterp/src/pyjitpl/dispatch.rs +++ b/majit/majit-metainterp/src/pyjitpl/dispatch.rs @@ -15,8 +15,6 @@ use crate::jitcode::insns::MAX_HOST_CALL_ARITY; use crate::jitcode::{self, JitArgKind, JitCallArg, JitCallTarget, JitCode, JitCodeRuntimeExt}; use crate::{TraceAction, TraceCtx}; -const HEADERLESS_SIZE_OWNER_MARKER: &str = "__majit_headerless_size__"; - /// Decode a virtualizable shadow Value (RPython Box concrete) back into the /// raw int/ref/float bit pattern that pyre stores in register shadows /// (`frame.int_values`, `frame.ref_values`, `frame.float_values`). @@ -54,6 +52,7 @@ fn field_spec_from_bh( ) -> majit_ir::descr::SimpleFieldDescrSpec { majit_ir::descr::SimpleFieldDescrSpec { index: f.index, + field_key: f.field_key().to_string(), name: f.name.clone(), offset: f.offset, field_size: f.field_size, @@ -77,11 +76,11 @@ fn field_spec_from_bh( /// A transient fieldless allocation carries only size + vtable + type /// identity, matching `bh_new`/`bh_new_with_vtable` dispatch descrs. fn size_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> majit_ir::DescrRef { + let headerless = descr.is_headerless(); if let crate::blackhole::BhDescr::Size { size, type_id, vtable, - owner, all_fielddescrs, is_gc_managed, .. @@ -96,8 +95,9 @@ fn size_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> majit_ir::DescrR *type_id, *vtable as usize, *is_gc_managed, - owner == HEADERLESS_SIZE_OWNER_MARKER, + headerless, &specs, + &[], ); let sd: majit_ir::DescrRef = group.size_descr; return sd; @@ -106,11 +106,6 @@ fn size_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> majit_ir::DescrR let size = descr.as_size(); let vtable = descr.get_vtable(); let type_id = descr.get_type_id() as u32; - let headerless = if let crate::blackhole::BhDescr::Size { owner, .. } = descr { - owner == HEADERLESS_SIZE_OWNER_MARKER - } else { - false - }; let mut sd = if vtable != 0 { majit_ir::descr::SimpleSizeDescr::with_vtable(u32::MAX, size, type_id, vtable) } else { @@ -131,7 +126,7 @@ fn size_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> majit_ir::DescrR /// (`optimizeopt/virtualize.rs:689`) requires the parent to virtualize /// the store. A parentless field (getfield round-trip / non-virtualized /// store) keeps the placeholder builder. -fn field_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> (usize, majit_ir::DescrRef) { +pub fn field_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> (usize, majit_ir::DescrRef) { match descr { crate::blackhole::BhDescr::Field { offset, @@ -141,6 +136,8 @@ fn field_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> (usize, majit_i index_in_parent, parent, name, + is_immutable, + is_quasi_immutable, .. } => { if let Some(p) = parent { @@ -163,6 +160,7 @@ fn field_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> (usize, majit_i p.is_gc_managed, p.headerless, &specs, + &[], ); let struct_key = majit_ir::descr::LLType::Struct(p.type_id); let cached = majit_ir::descr::gc_cache() @@ -176,35 +174,36 @@ fn field_descr_ref_from_bh(descr: &crate::blackhole::BhDescr) -> (usize, majit_i let fd: majit_ir::DescrRef = fd; return (*offset, fd); } - // Name-miss: the getfield names a Rust field (e.g. - // `strategy`) that the flattened layout represents under a - // different name — an inline enum's `__discriminant` at its - // sub-struct-relative offset — so the by-name lookup finds - // nothing. The getfield's own baked offset is the - // authoritative absolute offset; wire it to the containing - // struct's cache-owned SizeDescr so the FieldDescr satisfies - // `descr.py:238 get_parent_descr()` for - // `ensure_ptr_info_arg0` (`optimizer.py:478`) instead of - // falling to the parentless placeholder. - let parent_size = majit_ir::descr::gc_cache() - .lock() - .unwrap() - ._cache_size - .get(&struct_key) - .cloned(); - if let Some(parent_size) = parent_size { - return ( + // Name-miss: the getfield names an inline aggregate + // (`ob_header`, `int_items`, an enum's `__pos_0`) that the + // flattened layout only represents through its leaves, so + // the by-name lookup finds nothing. `heaptracker.py:68-69` + // recurses into a nested `lltype.Struct` without minting a + // descr for the container at all — `jtransform.py:942 + // rewrite_op_getsubstruct` lowers that access to + // `int_add(ptr, offset)`, no descr — so this whole branch + // exists only because pyre still emits a `getfield_gc` here. + // Until that lowering is ported, the container descr must at + // least obey the single-mint rule: go through + // `descr.py:218-239 get_field_descr` so the pool-side and + // walker-side resolutions land on one Arc instead of each + // minting a fresh one per resolution. + let mut gc = majit_ir::descr::gc_cache().lock().unwrap(); + if gc._cache_size.contains_key(&struct_key) { + let fd = gc.get_field_descr( + struct_key, + name, *offset, - majit_ir::descr::make_field_descr_with_parent( - *offset, - *field_size, - *field_type, - *field_flag, - *index_in_parent, - name.clone(), - &parent_size, - ), + *field_size, + *field_type, + *is_immutable, + *is_quasi_immutable, + *field_flag, + *index_in_parent as u32, + false, + *index_in_parent, ); + return (*offset, fd as majit_ir::DescrRef); } } } @@ -263,6 +262,7 @@ pub fn struct_fields_write_effect_info( }; majit_ir::descr::SimpleFieldDescrSpec { index: u32::MAX, + field_key: name.to_string(), name: name.to_string(), offset, field_size: 8, @@ -1526,6 +1526,7 @@ where false, majit_ir::descr::ArrayFlag::Signed, "len".to_string(), + "len".to_string(), )); d }); @@ -2896,7 +2897,7 @@ where // *records* New / NewWithVtable so the optimizer can virtualize // the struct away when it does not escape. let with_vtable = bytecode == jitcode::insns::BC_NEW_WITH_VTABLE; - let (size, vtable, descr, dest) = { + let (size, vtable, type_id, headerless, descr, dest) = { let frame = self.frames.current_mut(); let (descr_idx, dest) = frame.read_new(); let bh = frame.runtime_bh_descr(descr_idx).unwrap_or_else(|| { @@ -2905,16 +2906,57 @@ where ( bh.as_size(), bh.get_vtable(), + bh.resolve_gc_tid(), + bh.is_headerless(), size_descr_ref_from_bh(bh), dest, ) }; - // Mirror runner.rs bh_new / bh_new_with_vtable: malloc + zero, - // then write the vtable word at offset 0 (the OBJECTPTR typeptr - // slot) so a trace-time GuardClass reads the right class. - let layout = std::alloc::Layout::from_size_align(size.max(1), 8) - .expect("BC_NEW: invalid struct layout"); - let ptr = unsafe { std::alloc::alloc_zeroed(layout) } as i64; + // A `headerless` descr means the interpreter owns this struct in + // its own collected pool (`headerless_structs`), which is what + // compiled code allocates it from, through + // `call_malloc_nursery_headerless`. Putting it on the host heap + // instead hands the interpreter an object its collector cannot + // see: a moving collector range-checks its own pool, so it + // neither traces through the object nor forwards the references + // hanging off it, and the reachable graph below it is lost on + // the next collection. + // + // A headered GC-managed descr (real `type_id`) is the same + // problem one field deeper: a host-heap block carries no type + // word at `ref - 8`, so the collector never traces the struct + // and whatever its ref fields point at dies while the following + // `getfield` steps of this same trace still read them. It goes + // to the non-moving old generation, matching `runner.rs` + // bh_new / bh_new_with_vtable. + // + // The allocation must not collect. This runs mid-jitcode with + // raw object pointers live in the machine's own register bank — + // the `getfield` result feeding the `setfield` that follows this + // `new` — and that bank belongs to no root set, so a moving + // collection here would strand them. Both GC paths are + // no-collect, and old-gen is mark-sweep, so the pointer handed + // back to the register bank also survives later collections. + // + // A non-GC descr (`type_id == 0`, raw buffer) and an allocation + // the GC declines keep the host heap; the vtable word at offset + // 0 (the OBJECTPTR typeptr slot) is written either way so a + // trace-time GuardClass reads the right class. + let size = size.max(1); + let gc_ptr = if headerless { + majit_gc::alloc_nursery_headerless_no_collect(size).0 + } else if type_id != 0 { + majit_gc::alloc_oldgen_typed(type_id, size).0 + } else { + 0 + }; + let ptr = if gc_ptr != 0 { + gc_ptr as i64 + } else { + let layout = std::alloc::Layout::from_size_align(size, 8) + .expect("BC_NEW: invalid struct layout"); + unsafe { std::alloc::alloc_zeroed(layout) as i64 } + }; if with_vtable && vtable != 0 { unsafe { *(ptr as *mut usize) = vtable }; } @@ -2965,6 +3007,13 @@ where ); if struct_ptr != 0 { unsafe { *((struct_ptr as *mut u8).add(offset) as *mut i64) = concrete }; + // A ref store adds a heap edge struct→value; notify the GC + // on the container so a young value survives a minor + // collection triggered later in the walk (mirrors + // `bh_setfield_gc_r` and the setarrayitem case below). + if bytecode == jitcode::insns::BC_SETFIELD_GC_R { + majit_gc::gc_write_barrier(majit_ir::GcRef(struct_ptr as usize)); + } } } jitcode::insns::BC_RAW_STORE_I => { diff --git a/majit/majit-metainterp/src/resume_box_reader.rs b/majit/majit-metainterp/src/resume_box_reader.rs index 4e0dc337941..221ef43825e 100644 --- a/majit/majit-metainterp/src/resume_box_reader.rs +++ b/majit/majit-metainterp/src/resume_box_reader.rs @@ -744,7 +744,7 @@ pub fn materialize_bridge_virtual( entry.as_ref(), majit_ir::RdVirtualInfo::VUniConcatInfo { .. } ); - debug_assert_eq!( + assert_eq!( fieldnums.len(), 2, "VStr/VUniConcatInfo must have exactly 2 fieldnums (left, right)" @@ -792,7 +792,7 @@ pub fn materialize_bridge_virtual( entry.as_ref(), majit_ir::RdVirtualInfo::VUniSliceInfo { .. } ); - debug_assert_eq!( + assert_eq!( fieldnums.len(), 3, "VStr/VUniSliceInfo must have exactly 3 fieldnums (largerstr, start, length)" diff --git a/majit/majit-translate/src/codewriter/assembler.rs b/majit/majit-translate/src/codewriter/assembler.rs index a256ee7fcb1..ca7925ceb45 100644 --- a/majit/majit-translate/src/codewriter/assembler.rs +++ b/majit/majit-translate/src/codewriter/assembler.rs @@ -2993,6 +2993,7 @@ fn bh_field_spec_from_parts( ) -> crate::jitcode::BhFieldSpec { crate::jitcode::BhFieldSpec { index, + field_key: field_name.to_string(), name: bh_field_name(owner, field_name), offset, field_size, @@ -3062,7 +3063,7 @@ fn bh_all_field_specs_for_struct( owner: &str, ) -> Vec { let mut specs = Vec::new(); - bh_all_field_specs_for_struct_into(cc, owner, &mut specs); + bh_all_field_specs_for_struct_into(cc, owner, owner, "", 0, &mut specs); specs } @@ -3078,7 +3079,10 @@ fn bh_all_field_specs_for_struct( /// to recover the inner owner string before recursing. fn bh_all_field_specs_for_struct_into( cc: &CallControl, + root_owner: &str, owner: &str, + field_prefix: &str, + base_offset: usize, specs: &mut Vec, ) { if let Some(layout) = cc.struct_layout_for(owner) { @@ -3105,16 +3109,25 @@ fn bh_all_field_specs_for_struct_into( .find(|(name, _)| name == &fl.name) .map(|(_, ty)| ty.as_str()) { - bh_all_field_specs_for_struct_into(cc, inner_owner, specs); + let nested_prefix = format!("{field_prefix}{}.", fl.name); + bh_all_field_specs_for_struct_into( + cc, + root_owner, + inner_owner, + &nested_prefix, + base_offset + fl.offset, + specs, + ); } continue; } let index_in_parent = specs.len(); + let field_key = format!("{field_prefix}{}", fl.name); specs.push(bh_field_spec_from_parts( index_in_parent as u32, - owner, - &fl.name, - fl.offset, + root_owner, + &field_key, + base_offset + fl.offset, fl.size, fl.field_type, fl.flag, @@ -3153,15 +3166,24 @@ fn bh_all_field_specs_for_struct_into( // `heaptracker.py:68-69` recursive flatten for nested // structs. `field_type_str` is the inner owner name in // this textual path. - bh_all_field_specs_for_struct_into(cc, field_type_str, specs); + let nested_prefix = format!("{field_prefix}{field_name}."); + bh_all_field_specs_for_struct_into( + cc, + root_owner, + field_type_str, + &nested_prefix, + base_offset + offset, + specs, + ); } else { let index_in_parent = specs.len(); let rank = cc.field_immutability(Some(owner), field_name); + let field_key = format!("{field_prefix}{field_name}"); specs.push(bh_field_spec_from_parts( index_in_parent as u32, - owner, - field_name, - offset, + root_owner, + &field_key, + base_offset + offset, field_size, field_type, field_flag, @@ -3223,6 +3245,16 @@ fn fielddescrof( let mut is_quasi_immutable = false; let mut index_in_parent = 0usize; let mut parent = None; + let field_key = if let Some(owner) = field.owner_root.as_deref() { + let prefix = format!("{owner}."); + field + .name + .strip_prefix(&prefix) + .unwrap_or(&field.name) + .to_string() + } else { + field.name.clone() + }; if let (Some(cc), Some(owner)) = (callcontrol, field.owner_root.as_deref()) { parent = bh_size_spec_from_callcontrol(cc, owner); @@ -3269,7 +3301,7 @@ fn fielddescrof( is_field_signed = computed_signed; } - if let Some(rank) = cc.field_immutability(Some(owner), &field.name) { + if let Some(rank) = cc.field_immutability(Some(owner), &field_key) { is_immutable = rank.is_immutable(); is_quasi_immutable = rank.is_quasi_immutable(); } @@ -3285,7 +3317,7 @@ fn fielddescrof( is_quasi_immutable, index_in_parent, parent, - name: field.name.clone(), + name: field_key, owner: field.owner_root.clone().unwrap_or_default(), } } @@ -3353,6 +3385,7 @@ fn bh_field_spec_from_descr(fd: &dyn majit_ir::descr::FieldDescr) -> crate::jitc let field_flag = bh_field_flag_from_descr(fd); crate::jitcode::BhFieldSpec { index: fd.index(), + field_key: fd.field_key().to_string(), name: fd.field_name().to_string(), offset: fd.offset(), field_size: fd.field_size(), diff --git a/majit/majit-translate/src/codewriter/call.rs b/majit/majit-translate/src/codewriter/call.rs index d21db51f31c..fad76110615 100644 --- a/majit/majit-translate/src/codewriter/call.rs +++ b/majit/majit-translate/src/codewriter/call.rs @@ -97,26 +97,44 @@ pub struct WriteAnalysis { /// `effectinfo.py:294,301-305` `readonly_descrs_fields = []` /// populated via `add_struct → cpu.fielddescrof(T, fieldname)` from /// `("readstruct", T, fieldname)` tuples. - pub field_read_descrs: Vec, + pub field_read_descrs: Vec<( + majit_ir::descr::DescrRef, + Option, + )>, /// `effectinfo.py:297,301-305` `write_descrs_fields = []` from /// `("struct", T, fieldname)` tuples. - pub field_write_descrs: Vec, + pub field_write_descrs: Vec<( + majit_ir::descr::DescrRef, + Option, + )>, /// `effectinfo.py:296,313-325` `readonly_descrs_interiorfields = []` /// populated via `add_interiorfield → cpu.interiorfielddescrof(T, /// fieldname)` from `("readinteriorfield", T, fieldname)` tuples. - pub interior_read_descrs: Vec, + pub interior_read_descrs: Vec<( + majit_ir::descr::DescrRef, + Option, + )>, /// `effectinfo.py:299,313-325` `write_descrs_interiorfields = []` /// from `("interiorfield", T, fieldname)` tuples. - pub interior_write_descrs: Vec, + pub interior_write_descrs: Vec<( + majit_ir::descr::DescrRef, + Option, + )>, /// `effectinfo.py:295,307-311` `readonly_descrs_arrays = []` populated /// via `add_array → cpu.arraydescrof(ARRAY)` from `("readarray", T)` /// tuples (plus `("readinteriorfield", T, _)` tuples synthesised into /// `("readarray", T)` at `effectinfo.py:327-340`). - pub array_read_descrs: Vec, + pub array_read_descrs: Vec<( + majit_ir::descr::DescrRef, + Option, + )>, /// `effectinfo.py:298,307-311` `write_descrs_arrays = []` mirror of /// the read side, populated from `("array", T)` tuples (plus /// `("interiorfield", T, _)` synthesised into `("array", T)`). - pub array_write_descrs: Vec, + pub array_write_descrs: Vec<( + majit_ir::descr::DescrRef, + Option, + )>, /// RPython: `effects is top_set` — unanalyzable (random effects). pub is_top: bool, } @@ -1575,7 +1593,22 @@ impl CallControl { len_offset: Option, ) -> majit_ir::descr::DescrRef { self.arraydescrof_concrete(idx, array_type_id, ir_type, len_offset, Some(idx)) - as majit_ir::descr::DescrRef + .0 as majit_ir::descr::DescrRef + } + + pub fn arraydescrof_keyed( + &self, + idx: u32, + array_type_id: &Option, + ir_type: majit_ir::value::Type, + len_offset: Option, + ) -> ( + majit_ir::descr::DescrRef, + Option, + ) { + let (descr, key) = + self.arraydescrof_concrete(idx, array_type_id, ir_type, len_offset, Some(idx)); + (descr as majit_ir::descr::DescrRef, key) } /// Trait-typed sibling of [`Self::arraydescrof`] returning the cached @@ -1599,7 +1632,10 @@ impl CallControl { ir_type: majit_ir::value::Type, len_offset: Option, ei_publish: Option, - ) -> std::sync::Arc { + ) -> ( + std::sync::Arc, + Option, + ) { // RPython: ARRAY_INSIDE.OF — extract element type from full ARRAY type. let elem_name = array_type_id .as_deref() @@ -1663,8 +1699,10 @@ impl CallControl { // parity-correct response is to skip cache publish and mint // fresh per call so shape-coincident-but-logically-distinct // ARRAYs do not alias. - let ad_arc: std::sync::Arc = match array_type_id.as_deref() - { + let (ad_arc, key): ( + std::sync::Arc, + Option, + ) = match array_type_id.as_deref() { Some(atid) => { let path_hash_u64 = majit_ir::descr::path_hash(atid); let nolength = len_offset.is_none(); @@ -1730,7 +1768,10 @@ impl CallControl { } } } - ad_arc + let key = majit_ir::effectinfo::DescrSetMember::Array { + array_id: path_hash_u64, + }; + (ad_arc, Some(key)) } None => { // No identity carrier — local mint, no cache publish. @@ -1754,6 +1795,7 @@ impl CallControl { false, majit_ir::descr::ArrayFlag::Signed, "len".to_string(), + "len".to_string(), )) as majit_ir::descr::DescrRef }); let mut ad = majit_ir::descr::SimpleArrayDescr::with_flag( @@ -1770,7 +1812,7 @@ impl CallControl { let arc: std::sync::Arc = std::sync::Arc::new(ad); majit_ir::descr_registry::register_array(arc.clone() as majit_ir::descr::DescrRef); - arc as std::sync::Arc + (arc as std::sync::Arc, None) } }; // Per-trace codewriter id stamp — analyzer's @@ -1786,7 +1828,7 @@ impl CallControl { if let Some(arr_idx) = ei_publish { ad_arc.set_ei_index(arr_idx); } - ad_arc + (ad_arc, key) } /// RPython: `cpu.fielddescrof(STRUCT, fieldname)` — descr.py:215-247. @@ -1828,6 +1870,20 @@ impl CallControl { owner_id: Option, field_name: &str, ) -> Option { + self.fielddescrof_concrete(idx, owner_root, owner_id, field_name) + .map(|(descr, _)| descr) + } + + pub fn fielddescrof_keyed( + &self, + idx: u32, + owner_root: &str, + owner_id: Option, + field_name: &str, + ) -> Option<( + majit_ir::descr::DescrRef, + majit_ir::effectinfo::DescrSetMember, + )> { self.fielddescrof_concrete(idx, owner_root, owner_id, field_name) } @@ -1894,7 +1950,10 @@ impl CallControl { owner_root: &str, owner_id: Option, field_name: &str, - ) -> Option { + ) -> Option<( + majit_ir::descr::DescrRef, + majit_ir::effectinfo::DescrSetMember, + )> { use majit_ir::descr::{LLType, path_hash}; let fields = self.struct_fields.fields.get(owner_root)?; let mut offset: usize = 0; @@ -1988,6 +2047,10 @@ impl CallControl { owner_root, ))), }; + let struct_id = match struct_key { + LLType::Struct(id) => id, + _ => unreachable!("fielddescrof_concrete always builds a Struct key"), + }; // `descr.py:234-238 get_field_descr` always calls // `get_size_descr(gccache, STRUCT, vtable)` to bind // `fielddescr.parent_descr` before returning. Pyre's @@ -2001,6 +2064,19 @@ impl CallControl { // vtable on its PyreSizeDescr — cache-hit returns // *that* Arc here unchanged). let struct_size = compute_struct_size(self, owner_root); + let field_offset = owner_id + .or_else(|| majit_ir::descr::struct_id_for_name(owner_root)) + .and_then(|sid| self.struct_layouts.get(&sid)) + .and_then(|l| l.fields.iter().find(|f| f.name.as_str() == field_name)) + .map(|f| f.offset) + .unwrap_or(offset); + let rank = self.field_immutability(Some(owner_root), field_name); + let is_immutable = rank.map(|r| r.is_immutable()).unwrap_or(false); + let is_quasi_immutable = rank.map(|r| r.is_quasi_immutable()).unwrap_or(false); + let member = majit_ir::effectinfo::DescrSetMember::Field { + struct_id, + field_name: field_name.to_string(), + }; use majit_ir::descr::Descr; let size_descr_arc = { let mut gc = majit_ir::descr::gc_cache().lock().unwrap(); @@ -2024,7 +2100,7 @@ impl CallControl { let stored = fd.field_name(); if stored == field_name || stored.ends_with(&needle) { fd.set_index(idx); - return Some(fd.clone() as majit_ir::descr::DescrRef); + return Some((fd.clone() as majit_ir::descr::DescrRef, member)); } } } @@ -2044,15 +2120,6 @@ impl CallControl { // divergence that matters for enum variant payloads keyed // by `{enum_leaf}::{variant}`. Falls back to the // accumulator only for a struct absent from `struct_layouts`. - let field_offset = owner_id - .or_else(|| majit_ir::descr::struct_id_for_name(owner_root)) - .and_then(|sid| self.struct_layouts.get(&sid)) - .and_then(|l| l.fields.iter().find(|f| f.name.as_str() == field_name)) - .map(|f| f.offset) - .unwrap_or(offset); - let rank = self.field_immutability(Some(owner_root), field_name); - let is_immutable = rank.map(|r| r.is_immutable()).unwrap_or(false); - let is_quasi_immutable = rank.map(|r| r.is_quasi_immutable()).unwrap_or(false); let descr = majit_ir::descr::gc_cache().lock().unwrap().get_field_descr( struct_key, field_name, @@ -2062,10 +2129,12 @@ impl CallControl { is_immutable, is_quasi_immutable, flag, + u32::MAX, + false, field_pos, ); descr.set_index(idx); - return Some(descr as majit_ir::descr::DescrRef); + return Some((descr as majit_ir::descr::DescrRef, member)); } offset = offset.saturating_add(field_size); field_pos += 1; @@ -2097,6 +2166,19 @@ impl CallControl { array_type_id: &Option, field_name: &str, ) -> Option { + self.interiorfielddescrof_keyed(idx, array_type_id, field_name) + .map(|(descr, _)| descr) + } + + pub fn interiorfielddescrof_keyed( + &self, + idx: u32, + array_type_id: &Option, + field_name: &str, + ) -> Option<( + majit_ir::descr::DescrRef, + majit_ir::effectinfo::DescrSetMember, + )> { use majit_ir::descr::ArrayFlag; let array_str = array_type_id.as_deref()?; // ARRAY.OF.fieldname — extract the element type from the @@ -2159,6 +2241,9 @@ impl CallControl { // populated by either the runtime publish or the // analyzer-only mint. let struct_size = compute_struct_size(self, &elem_name); + let rank = self.field_immutability(Some(&elem_name), field_name); + let is_immutable = rank.map(|r| r.is_immutable()).unwrap_or(false); + let is_quasi_immutable = rank.map(|r| r.is_quasi_immutable()).unwrap_or(false); let size_descr_arc = { let mut gc = majit_ir::descr::gc_cache().lock().unwrap(); gc.get_size_descr(struct_key.clone(), struct_size, 0, false) @@ -2185,9 +2270,6 @@ impl CallControl { if found.is_none() { // No runtime publish for this `(STRUCT, fieldname)` — // analyzer-only mint. - let rank = self.field_immutability(Some(&elem_name), field_name); - let is_immutable = rank.map(|r| r.is_immutable()).unwrap_or(false); - let is_quasi_immutable = rank.map(|r| r.is_quasi_immutable()).unwrap_or(false); let mut gc = majit_ir::descr::gc_cache().lock().unwrap(); let mint = gc.get_field_descr( struct_key, @@ -2198,78 +2280,55 @@ impl CallControl { is_immutable, is_quasi_immutable, flag, + u32::MAX, + false, field_pos, ); found = Some(mint as std::sync::Arc); } - break; + let field_descr = found?; + let item_size = compute_struct_size(self, &elem_name); + let base_size = self.gc_typed_array_items_base(); + let array_id = majit_ir::descr::path_hash(array_str); + let member = majit_ir::effectinfo::DescrSetMember::InteriorField { + array_id, + name: field_name.to_string(), + }; + let array_key = majit_ir::descr::LLType::Array(array_id); + let cached: majit_ir::descr::DescrRef = { + let mut gc = majit_ir::descr::gc_cache().lock().unwrap(); + gc.get_array_descr( + array_key.clone(), + base_size, + item_size, + ArrayFlag::Struct, + majit_ir::value::Type::Ref, + false, // !nolength — length word at offset 0 + 0, // length_offset + false, // is_pure + '\x00', + ) + }; + let array_descr: std::sync::Arc = + majit_ir::descr::descr_arc_as_array_descr(cached) + .expect("gc_cache._cache_array slot held a non-ArrayDescr Arc"); + let descr = majit_ir::descr::gc_cache() + .lock() + .unwrap() + .get_interiorfield_descr( + array_key, + field_name.to_string(), + String::new(), + array_descr, + field_descr, + ); + descr.set_index(idx); + return Some((descr as majit_ir::descr::DescrRef, member)); } offset = offset.saturating_add(field_size); field_pos += 1; } - let field_descr = found?; - // `descr.py:430 arraydescr = get_array_descr(gc_ll_descr, ARRAY)`: - // PyPy `get_interiorfield_descr` reuses the per-ARRAY cached - // array descr. Pyre routes through `gc_cache.get_array_descr` - // cache-or-mint so analyzer's `arraydescrof` and - // `interiorfielddescrof` share Arc identity for the same - // `LLType::Array(path_hash(atid))` cache key. `try_downcast_arc` - // recovers `Arc` from the cache's - // `Arc` (the `Descr::as_any` override on - // `SimpleArrayDescr` makes the cast sound), then cast - // `as Arc` matches the trait-object field type - // on `SimpleInteriorFieldDescr.array_descr`. - let item_size = compute_struct_size(self, &elem_name); - // Interior-field struct arrays address `GcTypedArray`, whose items sit - // flat at the length word — NOT the element-aligned offset the list - // int/float storage (`TypedItemsBlock`) needs in `arraydescrof_concrete`. - let base_size = self.gc_typed_array_items_base(); - let array_key = majit_ir::descr::LLType::Array(majit_ir::descr::path_hash(array_str)); - let cached: majit_ir::descr::DescrRef = { - let mut gc = majit_ir::descr::gc_cache().lock().unwrap(); - gc.get_array_descr( - array_key.clone(), - base_size, - item_size, - ArrayFlag::Struct, - majit_ir::value::Type::Ref, - false, // !nolength — length word at offset 0 - 0, // length_offset - false, // is_pure - '\x00', - ) - }; - // `descr.py:348-378 get_array_descr` cache hit returns the - // existing `Arc` in the slot, upcast to the - // `ArrayDescr` trait object, keeping identity across runtime / - // analyzer paths per PyPy `cpu.arraydescrof(ARRAY)`. - let array_descr: std::sync::Arc = - majit_ir::descr::descr_arc_as_array_descr(cached) - .expect("gc_cache._cache_array slot held a non-ArrayDescr Arc"); - // `descr.py:423-438 get_interiorfield_descr` cache-or-mint: - // key is `(ARRAY, name, arrayfieldname=None)` — for the - // GcArray-of-Structs case (which is the only case pyre's - // analyzer mints) `arrayfieldname` is None per descr.py:431-432. - // Pyre encodes `None` as the empty string in the tuple key. - // Both arms of `make_simple_descr_group`'s array-of-struct - // population (Task D) and this analyzer mint must hit the same - // cache slot for `cpu.interiorfielddescrof` per-tuple identity. - // `field_descr` is already `Arc` (post B-4): - // either PyreFieldDescr from the runtime publish walk OR - // SimpleFieldDescr from the analyzer-only mint. Matches - // `SimpleInteriorFieldDescr::new` (descr.rs:3521) field type. - let descr = majit_ir::descr::gc_cache() - .lock() - .unwrap() - .get_interiorfield_descr( - array_key, - field_name.to_string(), - String::new(), - array_descr, - field_descr, - ); - descr.set_index(idx); - Some(descr as majit_ir::descr::DescrRef) + None } /// Insert into `function_graphs` and (if free function) the @@ -5520,6 +5579,21 @@ impl CallControl { } // RPython call.py:320-324 effectinfo assembly. + let effect_callee = match shape { + CallShape::Direct(target) => self + .target_to_path(target) + .map(|p| p.segments.join("::")) + .unwrap_or_else(|| format!("{target:?}")), + CallShape::Indirect(graphs) => format!( + "indirect[{}]", + graphs + .into_iter() + .flatten() + .map(|p| p.segments.join("::")) + .collect::>() + .join(",") + ), + }; let effects = match shape { CallShape::Direct(target) => { analyze_readwrite(target, &self.function_graphs, self, &self.descr_indices) @@ -5542,16 +5616,24 @@ impl CallControl { can_invalidate, can_collect, extradescrs, + &effect_callee, ); // RPython call.py:326-332 post-conditions on elidable / loopinvariant. if elidable || loopinvariant { - assert!( - effectinfo.extraeffect < ExtraEffect::ForcesVirtualOrVirtualizable, - "getcalldescr: elidable/loopinvariant call has effect {:?} \ - >= ForcesVirtualOrVirtualizable", - effectinfo.extraeffect - ); + // S4c degradation converts an unrepresentable concrete raw-set EI + // to `EF_RANDOM_EFFECTS`, the same conservative wildcard + // `effectinfo.py:285-292` uses when analysis is top. Preserve the + // upstream elidable/loopinvariant postcondition for every + // non-degraded EI. + if effectinfo.extraeffect != ExtraEffect::RandomEffects { + assert!( + effectinfo.extraeffect < ExtraEffect::ForcesVirtualOrVirtualizable, + "getcalldescr: elidable/loopinvariant call has effect {:?} \ + >= ForcesVirtualOrVirtualizable", + effectinfo.extraeffect + ); + } } // RPython call.py:334-335: @@ -5733,6 +5815,30 @@ fn analyze_readwrite_indirect_family( /// /// Takes pre-analyzed `effects` (from readwrite_analyzer) and `can_collect` /// (from collect_analyzer) and constructs an EffectInfo. +fn canonicalize_keyed_descrs( + mut pairs: Vec<( + majit_ir::descr::DescrRef, + Option, + )>, + exclude: Option<&std::collections::HashSet<*const ()>>, +) -> Option<( + Vec, + Vec, +)> { + if let Some(exclude) = exclude { + pairs.retain(|(descr, _)| !exclude.contains(&std::sync::Arc::as_ptr(descr).cast::<()>())); + } + pairs.sort_by_key(|(descr, _)| majit_ir::effectinfo::descr_ptr_id(descr)); + pairs.dedup_by(|(a, _), (b, _)| std::sync::Arc::ptr_eq(a, b)); + let mut descrs = Vec::with_capacity(pairs.len()); + let mut keys = Vec::with_capacity(pairs.len()); + for (descr, key) in pairs { + descrs.push(descr); + keys.push(key?); + } + Some((descrs, keys)) +} + pub fn effectinfo_from_writeanalyze( effects: WriteAnalysis, extraeffect: ExtraEffect, @@ -5740,6 +5846,7 @@ pub fn effectinfo_from_writeanalyze( can_invalidate: bool, can_collect: bool, extradescrs: Option>, + callee_path: &str, ) -> EffectInfo { // effectinfo.py:285: if effects is top_set or extraeffect == EF_RANDOM_EFFECTS: if effects.is_top || extraeffect == ExtraEffect::RandomEffects { @@ -5754,6 +5861,7 @@ pub fn effectinfo_from_writeanalyze( _write_descrs_arrays: None, _readonly_descrs_interiorfields: None, _write_descrs_interiorfields: None, + descr_set_keys: None, readonly_descrs_fields: None, write_descrs_fields: None, readonly_descrs_arrays: None, @@ -5816,11 +5924,11 @@ pub fn effectinfo_from_writeanalyze( // Snapshot the Arc-list before consumption — `single_write_descr_array` // takes ownership for its `.into_iter().next()` extract, but the EI's // `_write_descrs_arrays: Vec` raw set below also needs it. - let array_write_descrs_snapshot: Vec = array_write_descrs.clone(); + let array_write_descrs_snapshot = array_write_descrs.clone(); // effectinfo.py:201-206: single_write_descr_array let single_write_descr_array = if array_write_descrs.len() == 1 { - Some(array_write_descrs.into_iter().next().unwrap()) + Some(array_write_descrs.iter().next().unwrap().0.clone()) } else { None }; @@ -5894,45 +6002,70 @@ pub fn effectinfo_from_writeanalyze( // membership test. let field_write_ptr_set: std::collections::HashSet<*const ()> = field_write_descrs .iter() - .map(|d| std::sync::Arc::as_ptr(d).cast::<()>()) - .collect(); - let mut read_descrs_fields_arcs: Vec = field_read_descrs_raw - .into_iter() - .filter(|d| !field_write_ptr_set.contains(&std::sync::Arc::as_ptr(d).cast::<()>())) + .map(|d| std::sync::Arc::as_ptr(&d.0).cast::<()>()) .collect(); - read_descrs_fields_arcs.sort_by_key(majit_ir::effectinfo::descr_ptr_id); - read_descrs_fields_arcs.dedup_by(|a, b| std::sync::Arc::ptr_eq(a, b)); - let mut write_descrs_fields_arcs: Vec = field_write_descrs; - write_descrs_fields_arcs.sort_by_key(majit_ir::effectinfo::descr_ptr_id); - write_descrs_fields_arcs.dedup_by(|a, b| std::sync::Arc::ptr_eq(a, b)); + let read_fields_canon = + canonicalize_keyed_descrs(field_read_descrs_raw, Some(&field_write_ptr_set)); + let write_fields_canon = canonicalize_keyed_descrs(field_write_descrs, None); // Same `read \ write` subtract for interiorfield + array (PyPy // `effectinfo.py:351-360`), again by Arc identity. let interior_write_ptr_set: std::collections::HashSet<*const ()> = interior_write_descrs .iter() - .map(|d| std::sync::Arc::as_ptr(d).cast::<()>()) + .map(|d| std::sync::Arc::as_ptr(&d.0).cast::<()>()) .collect(); - let mut read_descrs_interior_arcs: Vec = interior_read_descrs_raw - .into_iter() - .filter(|d| !interior_write_ptr_set.contains(&std::sync::Arc::as_ptr(d).cast::<()>())) - .collect(); - read_descrs_interior_arcs.sort_by_key(majit_ir::effectinfo::descr_ptr_id); - read_descrs_interior_arcs.dedup_by(|a, b| std::sync::Arc::ptr_eq(a, b)); - let mut write_descrs_interior_arcs: Vec = interior_write_descrs; - write_descrs_interior_arcs.sort_by_key(majit_ir::effectinfo::descr_ptr_id); - write_descrs_interior_arcs.dedup_by(|a, b| std::sync::Arc::ptr_eq(a, b)); + let read_interior_canon = + canonicalize_keyed_descrs(interior_read_descrs_raw, Some(&interior_write_ptr_set)); + let write_interior_canon = canonicalize_keyed_descrs(interior_write_descrs, None); let array_write_ptr_set: std::collections::HashSet<*const ()> = array_write_descrs_snapshot .iter() - .map(|d| std::sync::Arc::as_ptr(d).cast::<()>()) - .collect(); - let mut read_descrs_arrays_arcs: Vec = array_read_descrs_raw - .into_iter() - .filter(|d| !array_write_ptr_set.contains(&std::sync::Arc::as_ptr(d).cast::<()>())) + .map(|d| std::sync::Arc::as_ptr(&d.0).cast::<()>()) .collect(); - read_descrs_arrays_arcs.sort_by_key(majit_ir::effectinfo::descr_ptr_id); - read_descrs_arrays_arcs.dedup_by(|a, b| std::sync::Arc::ptr_eq(a, b)); - let mut write_descrs_arrays_arcs: Vec = array_write_descrs_snapshot; - write_descrs_arrays_arcs.sort_by_key(majit_ir::effectinfo::descr_ptr_id); - write_descrs_arrays_arcs.dedup_by(|a, b| std::sync::Arc::ptr_eq(a, b)); + let read_arrays_canon = + canonicalize_keyed_descrs(array_read_descrs_raw, Some(&array_write_ptr_set)); + let write_arrays_canon = canonicalize_keyed_descrs(array_write_descrs_snapshot, None); + let ( + Some((read_descrs_fields_arcs, readonly_fields)), + Some((write_descrs_fields_arcs, write_fields)), + Some((read_descrs_arrays_arcs, readonly_arrays)), + Some((write_descrs_arrays_arcs, write_arrays)), + Some((read_descrs_interior_arcs, readonly_interiorfields)), + Some((write_descrs_interior_arcs, write_interiorfields)), + ) = ( + read_fields_canon, + write_fields_canon, + read_arrays_canon, + write_arrays_canon, + read_interior_canon, + write_interior_canon, + ) + else { + eprintln!( + "[s4c-degrade] {callee_path}: unrepresentable EffectInfo descr set member; using EF_RANDOM_EFFECTS" + ); + return EffectInfo { + extraeffect: ExtraEffect::RandomEffects, + oopspecindex, + pyre_helper: majit_ir::PyreHelperKind::None, + _readonly_descrs_fields: None, + _write_descrs_fields: None, + _readonly_descrs_arrays: None, + _write_descrs_arrays: None, + _readonly_descrs_interiorfields: None, + _write_descrs_interiorfields: None, + descr_set_keys: None, + readonly_descrs_fields: None, + write_descrs_fields: None, + readonly_descrs_arrays: None, + write_descrs_arrays: None, + readonly_descrs_interiorfields: None, + write_descrs_interiorfields: None, + single_write_descr_array: None, + extradescrs: extradescrs.clone(), + can_invalidate, + can_collect: true, + call_release_gil_target: EffectInfo::_NO_CALL_RELEASE_GIL_TARGET, + }; + }; EffectInfo { extraeffect, oopspecindex, @@ -5943,6 +6076,14 @@ pub fn effectinfo_from_writeanalyze( _write_descrs_arrays: Some(write_descrs_arrays_arcs), _readonly_descrs_interiorfields: Some(read_descrs_interior_arcs), _write_descrs_interiorfields: Some(write_descrs_interior_arcs), + descr_set_keys: Some(majit_ir::effectinfo::DescrSetKeys { + readonly_fields, + write_fields, + readonly_arrays, + write_arrays, + readonly_interiorfields, + write_interiorfields, + }), readonly_descrs_fields: Some(majit_ir::bitstring::make_bitstring(&readonly_descrs_fields)), write_descrs_fields: Some(majit_ir::bitstring::make_bitstring(&write_descrs_fields)), readonly_descrs_arrays: Some(majit_ir::bitstring::make_bitstring(&readonly_descrs_arrays)), @@ -6145,25 +6286,43 @@ fn collect_readwrite_effects( // effectinfo.py:294,301-305: `readonly_descrs_fields = []` populated // via `add_struct → cpu.fielddescrof(T, fieldname)` from // `("readstruct", T, fieldname)` tuples. - field_read_descrs: &mut Vec, + field_read_descrs: &mut Vec<( + majit_ir::descr::DescrRef, + Option, + )>, // effectinfo.py:297,301-305: `write_descrs_fields = []` from // `("struct", T, fieldname)` tuples. - field_write_descrs: &mut Vec, + field_write_descrs: &mut Vec<( + majit_ir::descr::DescrRef, + Option, + )>, // effectinfo.py:296,313-325: `readonly_descrs_interiorfields = []` // populated via `add_interiorfield → cpu.interiorfielddescrof(T, // fieldname)` from `("readinteriorfield", T, fieldname)` tuples. - interior_read_descrs: &mut Vec, + interior_read_descrs: &mut Vec<( + majit_ir::descr::DescrRef, + Option, + )>, // effectinfo.py:299,313-325: `write_descrs_interiorfields = []` // from `("interiorfield", T, fieldname)` tuples. - interior_write_descrs: &mut Vec, + interior_write_descrs: &mut Vec<( + majit_ir::descr::DescrRef, + Option, + )>, // effectinfo.py:295,307-311: `readonly_descrs_arrays = []` populated // via `add_array → cpu.arraydescrof(ARRAY)` from `("readarray", T)` // tuples (and `("readinteriorfield", T, _)` synthesised at // effectinfo.py:327-340). - array_read_descrs: &mut Vec, + array_read_descrs: &mut Vec<( + majit_ir::descr::DescrRef, + Option, + )>, // effectinfo.py:201-206,298,355-356: `write_descrs_arrays = []` — // also drives single_write_descr_array. - array_write_descrs: &mut Vec, + array_write_descrs: &mut Vec<( + majit_ir::descr::DescrRef, + Option, + )>, is_top: &mut bool, ) { if *is_top { @@ -6239,11 +6398,11 @@ fn collect_readwrite_effects( // (analyzer-unknown owner — matches PyPy's // `consider_struct=False` filter at effectinfo.py:380). if let Some(owner) = field.owner_root.as_deref() { - if !field_read_descrs.iter().any(|d| d.index() == idx) { + if !field_read_descrs.iter().any(|d| d.0.index() == idx) { if let Some(descr) = - cc.fielddescrof(idx, owner, field.owner_id, &field.name) + cc.fielddescrof_keyed(idx, owner, field.owner_id, &field.name) { - field_read_descrs.push(descr); + field_read_descrs.push((descr.0, Some(descr.1))); } } } @@ -6255,11 +6414,11 @@ fn collect_readwrite_effects( // RPython: effectinfo.py:301-305 — same as FieldRead's // implicit `add_struct` walk, just into `write_descrs_fields`. if let Some(owner) = field.owner_root.as_deref() { - if !field_write_descrs.iter().any(|d| d.index() == idx) { + if !field_write_descrs.iter().any(|d| d.0.index() == idx) { if let Some(descr) = - cc.fielddescrof(idx, owner, field.owner_id, &field.name) + cc.fielddescrof_keyed(idx, owner, field.owner_id, &field.name) { - field_write_descrs.push(descr); + field_write_descrs.push((descr.0, Some(descr.1))); } } } @@ -6296,7 +6455,7 @@ fn collect_readwrite_effects( // `cpu.arraydescrof(ARRAY)` and appends to // `readonly_descrs_arrays`. Dedup by descriptor index // (frozenset semantics, matching `ArrayWrite` handler). - if !array_read_descrs.iter().any(|d| d.index() == idx) { + if !array_read_descrs.iter().any(|d| d.0.index() == idx) { let ir_type = match item_ty { crate::model::ValueType::Int | crate::model::ValueType::Unsigned @@ -6314,7 +6473,7 @@ fn collect_readwrite_effects( ) } }; - array_read_descrs.push(cc.arraydescrof( + array_read_descrs.push(cc.arraydescrof_keyed( idx, &resolved_id, ir_type, @@ -6348,7 +6507,7 @@ fn collect_readwrite_effects( write_arrays.push(idx); // RPython: effectinfo.py:307-311 — cpu.arraydescrof(ARRAY). // Dedup by descriptor index (frozenset semantics). - if !array_write_descrs.iter().any(|d| d.index() == idx) { + if !array_write_descrs.iter().any(|d| d.0.index() == idx) { let ir_type = match item_ty { crate::model::ValueType::Int | crate::model::ValueType::Unsigned @@ -6372,7 +6531,7 @@ fn collect_readwrite_effects( // here so EffectInfo descrs match the same // `lendescr` shape `arraydescrof()` minted at the // emit-bytecode site (assembler.rs). - array_write_descrs.push(cc.arraydescrof( + array_write_descrs.push(cc.arraydescrof_keyed( idx, &resolved_id, ir_type, @@ -6409,11 +6568,14 @@ fn collect_readwrite_effects( // `cc.struct_fields` or the field is absent // (PyPy `effectinfo.py:316-324 consider_array` / // `Void` / `UnsupportedFieldExc` filters). - if !interior_read_descrs.iter().any(|d| d.index() == ifield_idx) { + if !interior_read_descrs + .iter() + .any(|d| d.0.index() == ifield_idx) + { if let Some(descr) = - cc.interiorfielddescrof(ifield_idx, &resolved_id, &field.name) + cc.interiorfielddescrof_keyed(ifield_idx, &resolved_id, &field.name) { - interior_read_descrs.push(descr); + interior_read_descrs.push((descr.0, Some(descr.1))); } } // effectinfo.py:327-340: synthesizes `("readarray", T)` @@ -6443,8 +6605,8 @@ fn collect_readwrite_effects( // appended to readonly_descrs_arrays via the synthesized // ("readarray", T) tuple. Dedup by descriptor index // (frozenset semantics). - if !array_read_descrs.iter().any(|d| d.index() == arr_idx) { - array_read_descrs.push(cc.arraydescrof( + if !array_read_descrs.iter().any(|d| d.0.index() == arr_idx) { + array_read_descrs.push(cc.arraydescrof_keyed( arr_idx, &resolved_id, majit_ir::value::Type::Ref, @@ -6479,12 +6641,12 @@ fn collect_readwrite_effects( // routed into `write_descrs_interiorfields`. if !interior_write_descrs .iter() - .any(|d| d.index() == ifield_idx) + .any(|d| d.0.index() == ifield_idx) { if let Some(descr) = - cc.interiorfielddescrof(ifield_idx, &resolved_id, &field.name) + cc.interiorfielddescrof_keyed(ifield_idx, &resolved_id, &field.name) { - interior_write_descrs.push(descr); + interior_write_descrs.push((descr.0, Some(descr.1))); } } // effectinfo.py:327-340: synthesizes `("array", T)` @@ -6514,8 +6676,8 @@ fn collect_readwrite_effects( // appended to write_descrs_arrays via the synthesized // ("array", T) tuple. Dedup by descriptor index // (frozenset semantics, matching ArrayWrite handler). - if !array_write_descrs.iter().any(|d| d.index() == arr_idx) { - array_write_descrs.push(cc.arraydescrof( + if !array_write_descrs.iter().any(|d| d.0.index() == arr_idx) { + array_write_descrs.push(cc.arraydescrof_keyed( arr_idx, &resolved_id, majit_ir::value::Type::Ref, @@ -6720,6 +6882,8 @@ fn all_interiorfielddescrs( *is_immutable, *is_quasi_immutable, *flag, + u32::MAX, + false, index_in_parent, ) } @@ -6850,6 +7014,8 @@ fn all_interiorfielddescrs( *is_immutable, *is_quasi_immutable, *flag, + u32::MAX, + false, index_in_parent, ) } diff --git a/majit/majit-translate/src/codewriter/jitcode.rs b/majit/majit-translate/src/codewriter/jitcode.rs index 5abb5fbf6c8..0926c5f7264 100644 --- a/majit/majit-translate/src/codewriter/jitcode.rs +++ b/majit/majit-translate/src/codewriter/jitcode.rs @@ -1009,6 +1009,8 @@ impl Default for BhCallDescr { #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct BhFieldSpec { pub index: u32, + #[serde(default)] + pub field_key: String, pub name: String, pub offset: usize, pub field_size: usize, @@ -1021,6 +1023,14 @@ pub struct BhFieldSpec { } impl BhFieldSpec { + pub fn field_key(&self) -> &str { + if self.field_key.is_empty() { + &self.name + } else { + &self.field_key + } + } + /// Mirror an `Arc` into the serializable /// `BhFieldSpec` shape so producers outside the codewriter /// (e.g. blackhole-allocator dispatch in `pyre-jit`) can build @@ -1040,6 +1050,7 @@ impl BhFieldSpec { }; Self { index: fd.index(), + field_key: fd.field_key().to_string(), name: fd.field_name().to_string(), offset: fd.offset(), field_size: fd.field_size(), @@ -1136,6 +1147,10 @@ fn ir_type_to_result_char(result_type: majit_ir::value::Type) -> char { } } +/// `owner` sentinel marking a [`BhDescr::Size`] as headerless. See +/// [`BhDescr::is_headerless`] for why the flag rides in the owner slot. +pub const HEADERLESS_SIZE_OWNER_MARKER: &str = "__majit_headerless_size__"; + #[derive(Debug, Clone, Serialize, Deserialize)] pub enum BhDescr { /// Field descriptor: for getfield/setfield. @@ -1352,6 +1367,21 @@ impl BhDescr { } } + /// Whether the described struct is headerless — allocated from the + /// interpreter's own `headerless_structs` pool with no `type_id` word at + /// `ref - 8`. A headerless struct must never be handed to a header-writing + /// allocator (`alloc_oldgen_typed`, `alloc_nursery_typed`): those return + /// `base + GcHeader::SIZE`, which shifts every field offset the descr + /// carries. The wire format has no dedicated flag, so the assembler stamps + /// [`HEADERLESS_SIZE_OWNER_MARKER`] into the `owner` slot it would + /// otherwise leave empty for a transient size descr. + pub fn is_headerless(&self) -> bool { + match self { + BhDescr::Size { owner, .. } => owner == HEADERLESS_SIZE_OWNER_MARKER, + _ => false, + } + } + /// Resolve the dense GC `tid` for a blackhole/resume header write from /// the identity this descr carries in [`get_type_id`]. Producers are /// mixed: `allocate_with_vtable` and the walker struct path widen the @@ -1531,7 +1561,7 @@ impl BhDescr { // fields only; the parent SizeDescr backref is not surfaced // by the live `FieldDescr` trait. parent: None, - name: spec.name, + name: spec.field_key, owner: String::new(), } } diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index 2a2a5320c5e..f18aa24fbab 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -1320,6 +1320,63 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { "pyre_object::list_write_barrier", pyre_object::list_write_barrier as *const (), ); + // The #171 fold descends `w_list_append` as a sub-jitcode walk, so a guard + // exit inside it is numbered against `w_list_append`'s own jitcode and is + // resumed there in the blackhole (`resume.py:1339 jitcodes[jitcode_pos]`). + // The resumed body then reaches the per-strategy store its arm selected — + // `W_ListObject::object_push` for the Object strategy, `IntArray::push` / + // `FloatArray::push` for the unwrapped ones — each a `residual_call`, and + // `blackhole.py:1230 bhimpl_residual_call_*` takes the funcptr straight to + // an indirect branch. Upstream never has to bind these: `call.py:181-183 + // getfunctionptr(graph)` resolves every callee in the same translation. + // pyre's codewriter runs in `build.rs`, so an unregistered callee keeps a + // `symbolic_fnaddr_for_path` hash, the blackhole aborts the frame, and the + // jd1 drain silently loses the in-flight `next()` item the resume was + // supposed to append. `fnaddr_for_target`'s `CallTarget::Method` fallback + // looks the address up as `CallPath::for_impl_method(receiver, name)`, i.e. + // the 2-segment `[receiver, method]` key `register_macro_helper_trace_fnaddr` + // derives by stripping the leading crate segment — hence the + // `pyre_object::::` spelling here. + let object_push: unsafe fn(&mut pyre_object::W_ListObject, pyre_object::PyObjectRef) = + pyre_object::W_ListObject::object_push; + push_fnaddr( + &mut entries, + "pyre_object::W_ListObject::object_push", + object_push as *const (), + ); + let int_array_push: fn(&mut pyre_object::IntArray, i64) = pyre_object::IntArray::push; + push_fnaddr( + &mut entries, + "pyre_object::IntArray::push", + int_array_push as *const (), + ); + let float_array_push: fn(&mut pyre_object::FloatArray, f64) = pyre_object::FloatArray::push; + push_fnaddr( + &mut entries, + "pyre_object::FloatArray::push", + float_array_push as *const (), + ); + // The same resume needs the jitcode *shells* it inline-calls to carry a + // real address: `blackhole.py:1300-1317 bhimpl_inline_call_*` calls + // `cpu.bh_call_*(adr2int(jitcode.fnaddr), ...)`, so a shell minted with + // `symbolic_fnaddr_for_path` is uncallable the same way. `w_list_append` + // is the fold's descended body and `w_list_len` its length probe. + let w_list_append: unsafe fn(pyre_object::PyObjectRef, pyre_object::PyObjectRef) = + pyre_object::listobject::w_list_append; + push_alias_pair( + &mut entries, + "pyre_object::listobject::w_list_append", + "pyre_object::w_list_append", + w_list_append as *const (), + ); + let w_list_len: unsafe fn(pyre_object::PyObjectRef) -> usize = + pyre_object::listobject::w_list_len; + push_alias_pair( + &mut entries, + "pyre_object::listobject::w_list_len", + "pyre_object::w_list_len", + w_list_len as *const (), + ); // The cold list strategy dehomogenization `switch_to_object_strategy` bulk // re-boxes typed int/float storage into an Object items block via // Vec/collect allocation the tracer cannot model. Register it so the hot diff --git a/pyre/pyre-jit-trace/build.rs b/pyre/pyre-jit-trace/build.rs index 2d3c58310de..6683e0d12b1 100644 --- a/pyre/pyre-jit-trace/build.rs +++ b/pyre/pyre-jit-trace/build.rs @@ -11,7 +11,7 @@ use walkdir::WalkDir; #[global_allocator] static ALLOC: mimalloc::MiMalloc = mimalloc::MiMalloc; -const CODEGEN_CACHE_VERSION: &str = "pyre-jit-trace-codegen-cache-v2"; +const CODEGEN_CACHE_VERSION: &str = "pyre-jit-trace-codegen-cache-v3"; /// Retained cache entries. Each is ~6 MB, and a handful covers the /// configurations one checkout switches between (native/wasm × release/dev). const CODEGEN_CACHE_MAX_ENTRIES: usize = 8; diff --git a/pyre/pyre-jit-trace/src/assembler.rs b/pyre/pyre-jit-trace/src/assembler.rs index 92d57a00ede..25e8ec3ef93 100644 --- a/pyre/pyre-jit-trace/src/assembler.rs +++ b/pyre/pyre-jit-trace/src/assembler.rs @@ -42,10 +42,34 @@ pub struct AssemblerState { impl AssemblerState { fn new() -> Self { + // `assembler.py:29-31` gives one `Assembler` one `all_liveness` + // buffer, and `codewriter.py:73-86 make_jitcodes` drains *every* + // pending graph through it, so upstream has exactly one offset space + // for the whole program. pyre splits the drain across two processes — + // `build.rs` assembles the extracted interpreter graphs, the runtime + // codewriter assembles Python-bytecode graphs — but the offsets baked + // into the build-time jitcodes are positions in that same buffer. + // Resuming from the build-time bytes is what makes it one buffer + // again: the baked offsets stay valid and the runtime's own + // `_encode_liveness` allocates strictly above them. + // + // `pyre_jit::Assembler::resuming_build_time_liveness` seeds the + // writer side the same way, so a `publish_state` wholesale replace + // never rewinds past this prefix. + // + // `assembler.py:20 self.insns = {}` continues the same way. The + // build-time jitcodes' `-live-` markers carry the canonical opcode + // byte, and `blackhole.py:55-61` recovers it as `asm.insns['live/']`; + // starting empty leaves `MetaInterpStaticData.op_live` at its unset + // sentinel, and `can_decode_live_vars` then hunts for that sentinel as + // a marker byte and declines every build-time resume. + let all_liveness = crate::jitcode_runtime::all_liveness().to_vec(); + let all_liveness_length = all_liveness.len(); + let insns = crate::jitcode_runtime::insns_opname_to_byte().clone(); Self { - insns: IndexMap::new(), - all_liveness: Vec::new(), - all_liveness_length: 0, + insns, + all_liveness, + all_liveness_length, all_liveness_positions: IndexMap::new(), num_liveness_ops: 0, } @@ -121,3 +145,38 @@ pub fn publish_state( }); crate::state::publish_liveness_info(all_liveness.to_vec()); } + +#[cfg(test)] +mod tests { + /// `assembler.py:29-31` — one `Assembler`, one `all_liveness` offset + /// space. The build-time drain's bytes are the prefix of that buffer, so + /// every offset baked into a build-time jitcode's `-live-` operand + /// addresses the same byte in `metainterp_sd.liveness_info` that + /// `resume.py:1022` reads. Losing the prefix would not fail loudly — a + /// baked offset would land inside an unrelated runtime triple and hand + /// back a mistyped value count — so pin it. + #[test] + fn assembler_state_resumes_the_build_time_liveness_prefix() { + let build_time = crate::jitcode_runtime::all_liveness(); + assert!( + !build_time.is_empty(), + "the build-time drain produced no liveness bytes; \ + `liveness.bin` is empty or failed to deserialize" + ); + super::ASSEMBLER_STATE.with(|r| { + let asm = r.borrow(); + assert_eq!( + asm.all_liveness_length, + asm.all_liveness.len(), + "`assembler.py:30 all_liveness_length` must track the buffer" + ); + assert!( + asm.all_liveness.starts_with(build_time), + "AssemblerState::new must resume the build-time buffer \ + (len {} vs build-time len {})", + asm.all_liveness.len(), + build_time.len(), + ); + }); + } +} diff --git a/pyre/pyre-jit-trace/src/descr.rs b/pyre/pyre-jit-trace/src/descr.rs index d99c450e681..d25e48f05ec 100644 --- a/pyre/pyre-jit-trace/src/descr.rs +++ b/pyre/pyre-jit-trace/src/descr.rs @@ -455,7 +455,14 @@ pub struct PyreSizeDescr { } struct PyreObjectDescrGroup { - size_descr: Arc, + size_descr: Arc, + /// This group's own fields, in the order its static table declared them. + /// + /// Static accessors index by their table position, so they must read this + /// list. `descr.py:218-239` still makes each Arc shared by + /// `(STRUCT, fieldname)`, while each SizeDescr keeps its own frozen + /// positional field list. + field_descrs: Vec>, } /// GC type id for the `rclass.OBJECT` root — pyre's static `INSTANCE_TYPE` @@ -625,13 +632,11 @@ pub use pyre_interpreter::pyframe::PYFRAME_GC_TYPE_ID; pub use pyre_interpreter::pyframe::{FRAME_BLOCK_GC_TYPE_ID, FRAME_DEBUG_DATA_GC_TYPE_ID}; fn field_descr_from_group(group: &PyreObjectDescrGroup, index: usize) -> DescrRef { - let field_descr = group - .size_descr - .all_fielddescrs + group + .field_descrs .get(index) .expect("field descriptor index out of bounds") - .clone(); - field_descr + .clone() as DescrRef } /// Build a SizeDescr group for a runtime PyObject layout and publish @@ -645,11 +650,8 @@ fn field_descr_from_group(group: &PyreObjectDescrGroup, index: usize) -> DescrRe /// alias for the future analyzer use-import resolver (B-5 follow-up): /// when that lands, analyzer's `owner_root` switches to qualified /// form and the SAME `Arc` is reachable via the -/// qualified hash. `register_keyed_size` is first-write-wins per -/// `descr.py:25-47 setup_descrs` cache-iteration invariant — the -/// second publish's losing Arc does NOT enter `_cache_size_order`, -/// so `all_descrs` enumerates exactly one entry per logical -/// SizeDescr (PyPy's per-tuple identity). +/// qualified hash. `register_keyed_size` keeps one `_cache_size_order` +/// entry per logical SizeDescr while allowing fuller-layout upgrades. /// /// `def_path` empty (or equal to `simple_name`) → single publish. fn build_object_descr_group_with_def_path( @@ -660,79 +662,55 @@ fn build_object_descr_group_with_def_path( simple_name: &str, def_path: &str, ) -> PyreObjectDescrGroup { - let size_descr = Arc::new_cyclic(|weak_size: &Weak| { - let parent_descr: Weak = weak_size.clone(); - let all_fielddescrs: Vec> = fields - .iter() - .enumerate() - .map( - |( - index_in_parent, - &(name, offset, field_size, field_type, signed, immutable, quasi_immutable), - )| { - Arc::new(PyreFieldDescr { - offset, - field_size, - field_type, - signed, - immutable, - quasi_immutable, - name, - index_in_parent, - parent_descr: Some(parent_descr.clone()), - ei_index: AtomicU32::new(u32::MAX), - }) as Arc + let cache_key = if !def_path.is_empty() { + majit_ir::descr::path_hash(def_path) + } else if !simple_name.is_empty() { + majit_ir::descr::path_hash(simple_name) + } else { + 0 + }; + let specs: Vec = fields + .iter() + .enumerate() + .map( + |( + index_in_parent, + &(field_key, offset, field_size, field_type, signed, immutable, quasi_immutable), + )| majit_ir::descr::SimpleFieldDescrSpec { + index: stable_field_index(offset, field_size, field_type, signed), + field_key: field_key.to_string(), + name: if simple_name.is_empty() { + field_key.to_string() + } else { + format!("{simple_name}.{field_key}") }, - ) - .collect(); - // descr.py:123-126 precompute both lists; `gc_fielddescrs` is - // `all_fielddescrs(only_gc=True)` per heaptracker.py:94-95. - let mut gc_fielddescrs: Vec> = all_fielddescrs - .iter() - .filter(|fd| fd.is_pointer_field()) - .cloned() - .collect(); - // descr.py:121-126 + heaptracker.py:94-95: gc_fielddescrs walks - // the complete GC struct, including inherited fields. Every - // runtime object group built here embeds PyObject, whose `w_class` - // is a GC reference even when the leaf field list only names the - // concrete payload (for example W_IntObject.intval). Keep the - // leaf-only all_fielddescrs indexing used by OptVirtualize, but add - // the inherited header edge to the allocation-clear census unless - // a group already declared that offset explicitly. - if !gc_fielddescrs - .iter() - .any(|fd| fd.offset() == pyre_object::pyobject::W_CLASS_OFFSET) - { - gc_fielddescrs.push(W_CLASS_FIELD_DESCR.clone()); - } - // `descr.py:108-118 get_size_descr` cache key — `path_hash`로 - // 만들어진 lltype-object identity. Prefer the canonical - // *def-path* qualifier (PyPy's `lltype.Struct` identity has - // a single module-path keyed slot); fall back to simple-name - // for legacy registrations without `def_path`. Both - // `path_hash(simple_name)` and `path_hash(def_path)` are still - // dual-published as Arc aliases so untransformed bare-name - // analyzer lookups (pending the use_imports lexical resolver, - // [[orthodox-6item-2026-05-17]]) still hit. Round-trip via - // `bh_size_spec_from_descr` lands on the canonical def-path - // slot. - let cache_key = if !def_path.is_empty() { - majit_ir::descr::path_hash(def_path) - } else if !simple_name.is_empty() { - majit_ir::descr::path_hash(simple_name) - } else { - 0 - }; - PyreSizeDescr { - obj_size, - type_id, - cache_key, - vtable, - all_fielddescrs, - gc_fielddescrs, - } - }); + offset, + field_size, + field_type, + is_immutable: immutable, + is_quasi_immutable: quasi_immutable, + flag: runtime_array_flag(field_type, signed), + virtualizable: false, + index_in_parent, + }, + ) + .collect(); + let group = majit_ir::descr::make_simple_descr_group_keyed_with_headerless( + SIZE_DESCR_TAG | (obj_size as u32 & 0x0FFF_FFFF), + obj_size, + type_id, + cache_key, + vtable, + true, + false, + &specs, + &[W_CLASS_FIELD_DESCR.clone()], + ); + let field_descrs = group.field_descrs; + let size_descr = group.size_descr; + // heaptracker.py:50-73 recurses into the inherited header, and + // heaptracker.py:70 includes the embedded `PyObject.w_class` GC edge. + // The factory keeps that extra edge out of the positional list. // Dual-publish: register under BOTH the simple-name slot AND // (when supplied) the crate-stripped def-path slot. // @@ -745,11 +723,8 @@ fn build_object_descr_group_with_def_path( // analyzer use-import resolver (B-5 follow-up): when that lands, // analyzer's `owner_root` switches to qualified form and the // SAME `Arc` is reachable via the qualified - // hash. `register_keyed_size` is first-write-wins per - // `descr.py:25-47 setup_descrs` cache-iteration invariant — the - // second registration's losing Arc does NOT enter - // `_cache_size_order`, so `all_descrs` enumerates exactly one - // entry per logical SizeDescr (PyPy's per-tuple identity). + // hash. `register_keyed_size` keeps one `_cache_size_order` entry + // per logical SizeDescr while allowing fuller-layout upgrades. if !simple_name.is_empty() { let key = majit_ir::descr::LLType::Struct(majit_ir::descr::path_hash(simple_name)); majit_ir::descr_registry::register_keyed_size( @@ -764,7 +739,10 @@ fn build_object_descr_group_with_def_path( size_descr.clone() as majit_ir::DescrRef, ); } - PyreObjectDescrGroup { size_descr } + PyreObjectDescrGroup { + size_descr, + field_descrs, + } } static W_INT_DESCR_GROUP: LazyLock = LazyLock::new(|| { @@ -772,15 +750,7 @@ static W_INT_DESCR_GROUP: LazyLock = LazyLock::new(|| { std::mem::size_of::(), W_INT_GC_TYPE_ID, &INT_TYPE as *const _ as usize, - &[( - "W_IntObject.intval", - INT_INTVAL_OFFSET, - 8, - Type::Int, - true, - true, - false, - )], + &[("intval", INT_INTVAL_OFFSET, 8, Type::Int, true, true, false)], "W_IntObject", "intobject::W_IntObject", ) @@ -792,7 +762,7 @@ static W_FLOAT_DESCR_GROUP: LazyLock = LazyLock::new(|| { W_FLOAT_GC_TYPE_ID, &FLOAT_TYPE as *const _ as usize, &[( - "W_FloatObject.floatval", + "floatval", FLOAT_FLOATVAL_OFFSET, 8, Type::Float, @@ -811,7 +781,7 @@ static W_LONG_DESCR_GROUP: LazyLock = LazyLock::new(|| { pyre_object::longobject::W_LONG_GC_TYPE_ID, &pyre_object::pyobject::LONG_TYPE as *const _ as usize, &[( - "W_LongObject.value", + "value", pyre_object::longobject::LONG_VALUE_OFFSET, 8, // The `value` slot is a gc-pointer to the BigInt payload, so it @@ -833,7 +803,7 @@ static W_BOOL_DESCR_GROUP: LazyLock = LazyLock::new(|| { W_BOOL_GC_TYPE_ID, &pyre_object::pyobject::BOOL_TYPE as *const _ as usize, &[( - "W_BoolObject.intval", + "intval", BOOL_INTVAL_OFFSET, 8, Type::Int, @@ -853,7 +823,7 @@ static RANGE_ITER_DESCR_GROUP: LazyLock = LazyLock::new(|| &pyre_object::functional::RANGE_ITER_TYPE as *const _ as usize, &[ ( - "W_IntRangeIterator.current", + "current", RANGE_ITER_CURRENT_OFFSET, 8, Type::Int, @@ -862,7 +832,7 @@ static RANGE_ITER_DESCR_GROUP: LazyLock = LazyLock::new(|| false, ), ( - "W_IntRangeIterator.remaining", + "remaining", RANGE_ITER_REMAINING_OFFSET, 8, Type::Int, @@ -871,7 +841,7 @@ static RANGE_ITER_DESCR_GROUP: LazyLock = LazyLock::new(|| false, ), ( - "W_IntRangeIterator.step", + "step", RANGE_ITER_STEP_OFFSET, 8, Type::Int, @@ -892,7 +862,7 @@ static RANGE_DESCR_GROUP: LazyLock = LazyLock::new(|| { &pyre_object::functional::RANGE_TYPE as *const _ as usize, &[ ( - "W_Range.start", + "start", RANGE_START_OFFSET, 8, Type::Ref, @@ -900,17 +870,9 @@ static RANGE_DESCR_GROUP: LazyLock = LazyLock::new(|| { true, false, ), + ("step", RANGE_STEP_OFFSET, 8, Type::Ref, false, true, false), ( - "W_Range.step", - RANGE_STEP_OFFSET, - 8, - Type::Ref, - false, - true, - false, - ), - ( - "W_Range.length", + "length", RANGE_LENGTH_OFFSET, 8, Type::Ref, @@ -945,7 +907,7 @@ static W_METHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { &pyre_object::function::METHOD_TYPE as *const _ as usize, &[ ( - "Method.w_function", + "w_function", METHOD_W_FUNCTION_OFFSET, 8, Type::Ref, @@ -954,7 +916,7 @@ static W_METHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "Method.w_self", + "w_self", METHOD_W_SELF_OFFSET, 8, Type::Ref, @@ -963,7 +925,7 @@ static W_METHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "Method.w_class", + "w_class", METHOD_W_CLASS_OFFSET, 8, Type::Ref, @@ -1007,7 +969,7 @@ static W_OBJECT_MUTABLE_CELL_DESCR_GROUP: LazyLock = LazyL W_OBJECT_MUTABLE_CELL_GC_TYPE_ID, &OBJECT_MUTABLE_CELL_TYPE as *const _ as usize, &[( - "ObjectMutableCell.w_value", + "w_value", W_OBJECT_MUTABLE_CELL_GC_PTR_OFFSETS[0], 8, Type::Ref, @@ -1040,7 +1002,7 @@ static W_LIST_DESCR_GROUP: LazyLock = LazyLock::new(|| { // out-of-bounds list access. Same fix as `str_len_descr`; the // `usize`/pointer fields below follow suit (the `Type::Ref` // fields are safe — read at pointer width regardless of size). - "W_ListObject.length", + "length", std::mem::offset_of!(W_ListObject, length), std::mem::size_of::(), Type::Int, @@ -1054,7 +1016,7 @@ static W_LIST_DESCR_GROUP: LazyLock = LazyLock::new(|| { // (`list.object_grow` → `grow_list_items_block`) or when the // strategy switches. ( - "W_ListObject.items", + "items", std::mem::offset_of!(W_ListObject, items), 8, Type::Ref, @@ -1076,7 +1038,7 @@ static W_LIST_DESCR_GROUP: LazyLock = LazyLock::new(|| { // pyre has no such hook yet, so `strategy` stays // plain-mutable. TODO — strategy split itself // is a pyre-only adaptation vs rlist.py. - "W_ListObject.strategy", + "strategy", std::mem::offset_of!(W_ListObject, strategy), 1, Type::Int, @@ -1090,7 +1052,7 @@ static W_LIST_DESCR_GROUP: LazyLock = LazyLock::new(|| { // the unwrap inline and doesn't add a separate backing // array). ( - "W_ListObject.int_items.len", + "int_items.len", std::mem::offset_of!(W_ListObject, int_items) + INT_ARRAY_LEN_OFFSET, std::mem::size_of::(), Type::Int, @@ -1100,7 +1062,7 @@ static W_LIST_DESCR_GROUP: LazyLock = LazyLock::new(|| { ), // Float-strategy typed storage. ( - "W_ListObject.float_items.len", + "float_items.len", std::mem::offset_of!(W_ListObject, float_items) + FLOAT_ARRAY_LEN_OFFSET, std::mem::size_of::(), Type::Int, @@ -1114,7 +1076,7 @@ static W_LIST_DESCR_GROUP: LazyLock = LazyLock::new(|| { // items[i] through the heap cache. Mutable: re-pointed on grow / // strategy switch (like `W_ListObject.items`). ( - "W_ListObject.int_items.block", + "int_items.block", std::mem::offset_of!(W_ListObject, int_items) + INT_ARRAY_BLOCK_OFFSET, 8, Type::Ref, @@ -1123,7 +1085,7 @@ static W_LIST_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "W_ListObject.float_items.block", + "float_items.block", std::mem::offset_of!(W_ListObject, float_items) + FLOAT_ARRAY_BLOCK_OFFSET, 8, Type::Ref, @@ -1165,7 +1127,7 @@ static W_TUPLE_DESCR_GROUP: LazyLock = LazyLock::new(|| { &[ // `Ptr(GcArray(OBJECTPTR))` — wrappeditems body. Immutable. ( - "W_TupleObject.wrappeditems", + "wrappeditems", std::mem::offset_of!(W_TupleObject, wrappeditems), 8, Type::Ref, @@ -1199,7 +1161,7 @@ static SPECIALISED_TUPLE_II_DESCR_GROUP: LazyLock = LazyLo &SPECIALISED_TUPLE_II_TYPE as *const _ as usize, &[ ( - "W_SpecialisedTupleObject_ii.value0", + "value0", SPECIALISED_TUPLE_II_VALUE0_OFFSET, 8, Type::Int, @@ -1208,7 +1170,7 @@ static SPECIALISED_TUPLE_II_DESCR_GROUP: LazyLock = LazyLo false, ), ( - "W_SpecialisedTupleObject_ii.value1", + "value1", SPECIALISED_TUPLE_II_VALUE1_OFFSET, 8, Type::Int, @@ -1239,7 +1201,7 @@ static SPECIALISED_TUPLE_FF_DESCR_GROUP: LazyLock = LazyLo &SPECIALISED_TUPLE_FF_TYPE as *const _ as usize, &[ ( - "W_SpecialisedTupleObject_ff.value0", + "value0", SPECIALISED_TUPLE_FF_VALUE0_OFFSET, 8, Type::Float, @@ -1248,7 +1210,7 @@ static SPECIALISED_TUPLE_FF_DESCR_GROUP: LazyLock = LazyLo false, ), ( - "W_SpecialisedTupleObject_ff.value1", + "value1", SPECIALISED_TUPLE_FF_VALUE1_OFFSET, 8, Type::Float, @@ -1279,7 +1241,7 @@ static SPECIALISED_TUPLE_OO_DESCR_GROUP: LazyLock = LazyLo &SPECIALISED_TUPLE_OO_TYPE as *const _ as usize, &[ ( - "W_SpecialisedTupleObject_oo.value0", + "value0", SPECIALISED_TUPLE_OO_VALUE0_OFFSET, 8, Type::Ref, @@ -1288,7 +1250,7 @@ static SPECIALISED_TUPLE_OO_DESCR_GROUP: LazyLock = LazyLo false, ), ( - "W_SpecialisedTupleObject_oo.value1", + "value1", SPECIALISED_TUPLE_OO_VALUE1_OFFSET, 8, Type::Ref, @@ -1317,7 +1279,7 @@ static ITEMS_BLOCK_DESCR_GROUP: LazyLock = LazyLock::new(| 0, 0, &[( - "ItemsBlock.capacity", + "capacity", pyre_object::object_array::ITEMS_BLOCK_LEN_OFFSET, std::mem::size_of::(), Type::Int, @@ -1344,7 +1306,7 @@ static W_SLICE_DESCR_GROUP: LazyLock = LazyLock::new(|| { &pyre_object::sliceobject::SLICE_TYPE as *const _ as usize, &[ ( - "W_SliceObject.w_start", + "w_start", SLICE_START_OFFSET, 8, Type::Ref, @@ -1353,7 +1315,7 @@ static W_SLICE_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "W_SliceObject.w_stop", + "w_stop", SLICE_STOP_OFFSET, 8, Type::Ref, @@ -1362,7 +1324,7 @@ static W_SLICE_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "W_SliceObject.w_step", + "w_step", SLICE_STEP_OFFSET, 8, Type::Ref, @@ -1387,7 +1349,7 @@ static PYFRAME_DESCR_GROUP: LazyLock = LazyLock::new(|| { &pyre_interpreter::pyframe::FRAME_TYPE as *const _ as usize, &[ ( - "PyFrame.locals_cells_stack_w", + "locals_cells_stack_w", crate::frame_layout::PYFRAME_LOCALS_CELLS_STACK_OFFSET, 8, Type::Ref, @@ -1396,7 +1358,7 @@ static PYFRAME_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "PyFrame.valuestackdepth", + "valuestackdepth", crate::frame_layout::PYFRAME_VALUESTACKDEPTH_OFFSET, 8, Type::Int, @@ -1405,7 +1367,7 @@ static PYFRAME_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "PyFrame.last_instr", + "last_instr", crate::frame_layout::PYFRAME_LAST_INSTR_OFFSET, 8, Type::Int, @@ -1414,7 +1376,7 @@ static PYFRAME_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, ), ( - "PyFrame.pycode", + "pycode", crate::frame_layout::PYFRAME_PYCODE_OFFSET, 8, Type::Ref, @@ -1422,6 +1384,8 @@ static PYFRAME_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, false, ), + // `pyframe.py:49 self.w_globals` — the slot the inline + // new-PyFrame helper populates from the function's globals dict. ( "PyFrame.w_globals", crate::frame_layout::PYFRAME_W_GLOBALS_OFFSET, @@ -1498,17 +1462,6 @@ static PYFRAME_DESCR_GROUP: LazyLock = LazyLock::new(|| { false, false, ), - // `pyframe.py:49 self.w_globals` parity. The inline new-PyFrame - // helper populates this slot from the function's globals dict. - ( - "PyFrame.w_globals", - crate::frame_layout::PYFRAME_W_GLOBALS_OFFSET, - 8, - Type::Ref, - false, - false, - false, - ), ], "PyFrame", "pyframe::PyFrame", @@ -1552,21 +1505,8 @@ impl SizeDescr for PyreSizeDescr { self.vtable } - /// The canonical `w_class` (Python class object) for instances of - /// this type — `get_instantiate(vtable_type)`. Read live (not cached - /// at construction) since the type objects are installed after the - /// descrs are built. `None` before `init_typeobjects()` runs. fn w_class_obj(&self) -> Option { - if self.vtable == 0 { - return None; - } - let tp = self.vtable as *const pyre_object::pyobject::PyType; - let w_class = unsafe { pyre_object::pyobject::get_instantiate(&*tp) }; - if w_class.is_null() { - None - } else { - Some(w_class as i64) - } + w_class_obj_for_vtable(self.vtable) } fn is_immutable(&self) -> bool { @@ -1806,6 +1746,30 @@ pub fn w_class_descr() -> DescrRef { W_CLASS_FIELD_DESCR.clone() as DescrRef } +/// The canonical `w_class` (Python class object) for instances of the type +/// `vtable` names — `get_instantiate(vtable_type)`. Read live (not cached at +/// construction) since the type objects are installed after the descrs are +/// built. `None` before `init_typeobjects()` runs. +/// +/// Registered as majit's [`majit_ir::descr::WClassObjFn`] so the generic +/// `SimpleSizeDescr` — which every runtime PyObject group and every +/// blackhole-dispatch size descr is built as — answers `w_class_obj` the same +/// way `PyreSizeDescr` does. Without it `OptVirtualize` cannot fold the +/// `w_class` header read off a `new_with_vtable` virtual and forces the +/// virtual instead. +pub fn w_class_obj_for_vtable(vtable: usize) -> Option { + if vtable == 0 { + return None; + } + let tp = vtable as *const pyre_object::pyobject::PyType; + let w_class = unsafe { pyre_object::pyobject::get_instantiate(&*tp) }; + if w_class.is_null() { + None + } else { + Some(w_class as i64) + } +} + /// Alias for backward compatibility — same as w_class_descr(). pub fn instance_w_type_descr() -> DescrRef { w_class_descr() @@ -1932,6 +1896,7 @@ pub fn int_mutable_cell_value_descr() -> DescrRef { false, majit_ir::descr::ArrayFlag::Signed, "IntMutableCell.intvalue".to_string(), + "intvalue".to_string(), )) }) .clone() @@ -2683,6 +2648,7 @@ pub fn ec_sys_exc_value_descr() -> DescrRef { 0, &[SimpleFieldDescrSpec { index: 0, + field_key: "sys_exc_value".to_string(), name: "ExecutionContext.sys_exc_value".to_string(), offset: pyre_interpreter::EC_SYS_EXC_VALUE_OFFSET, field_size: std::mem::size_of::(), @@ -2756,7 +2722,7 @@ pub fn pyframe_code_descr() -> DescrRef { /// PyObjectRef. `PyFrame.w_globals` is the single globals slot; /// the raw dict-storage accessor has been retired. pub fn pyframe_w_globals_obj_descr() -> DescrRef { - field_descr_from_group(&PYFRAME_DESCR_GROUP, 12) + field_descr_from_group(&PYFRAME_DESCR_GROUP, 4) } /// rewrite.py:665-695 handle_call_assembler scalar field read for the @@ -2965,6 +2931,7 @@ mod tests { all_fielddescrs: vec![ BhFieldSpec { index: 0, + field_key: "next".into(), name: "Cell.next".into(), offset: 8, field_size: 8, @@ -2977,6 +2944,7 @@ mod tests { }, BhFieldSpec { index: 1, + field_key: "value".into(), name: "Cell.value".into(), offset: 16, field_size: 8, @@ -3194,6 +3162,7 @@ mod tests { let fields = vec![ BhFieldSpec { index: 0, + field_key: "x".into(), name: "Point.x".into(), offset: 0, field_size: 8, @@ -3206,6 +3175,7 @@ mod tests { }, BhFieldSpec { index: 1, + field_key: "y".into(), name: "Point.y".into(), offset: 8, field_size: 8, @@ -3321,6 +3291,7 @@ fn simple_field_spec_from_bh( ) -> majit_ir::descr::SimpleFieldDescrSpec { majit_ir::descr::SimpleFieldDescrSpec { index: spec.index, + field_key: spec.field_key().to_string(), name: spec.name.clone(), offset: spec.offset, field_size: spec.field_size, @@ -3357,138 +3328,41 @@ fn simple_field_spec_from_bh( /// never aliases distinct STRUCTs; absent a real identity carrier, /// the closest orthodox behaviour is "each call is a distinct /// STRUCT" — mint fresh per call. -static SIMPLE_DESCR_GROUP_CACHE: std::sync::OnceLock< - std::sync::Mutex>, -> = std::sync::OnceLock::new(); - fn simple_descr_group_from_bh_size( spec: &majit_translate::jitcode::BhSizeSpec, ) -> majit_ir::descr::SimpleDescrGroup { - let mint = || -> majit_ir::descr::SimpleDescrGroup { - let field_specs: Vec<_> = spec - .all_fielddescrs - .iter() - .map(simple_field_spec_from_bh) - .collect(); - // `descr.py:108-118 get_size_descr` + `:218-239 get_field_descr` - // keyed publish: `spec.type_id` is the u64 `path_hash` cache - // key matching the runtime macro's `__majit_type_id`. Route - // through the keyed factory so analyzer-side `cc.fielddescrof` - // lookups (via `gc_cache.get_field_descr(LLType::Struct(key), - // name, ...)`) resolve to the same Arc this mint produces — - // restoring PyPy `cpu.fielddescrof` per-`(STRUCT, name)` - // identity. The u32 truncation for the SimpleSizeDescr's gc - // tid is a TODO (the tid is allocated by - // gc_cache.init_size_descr in the canonical path; this factory - // bypasses that, so the tid stays a path_hash-derived u32 with - // birthday-paradox collision risk around 2^16 distinct STRUCTs). - majit_ir::descr::make_simple_descr_group_keyed_with_headerless( - u32::MAX, - spec.size, - spec.type_id as u32, - spec.type_id, - spec.vtable as usize, - spec.is_gc_managed, - spec.headerless, - &field_specs, - ) - }; + let field_specs: Vec<_> = spec + .all_fielddescrs + .iter() + .map(simple_field_spec_from_bh) + .collect(); if spec.type_id == 0 { // No STRUCT-identity carrier — mint fresh per call so distinct // type_id-less STRUCTs don't collapse onto the first-inserted // descr group. Per-STRUCT caching kicks in only when callers // route through a real `type_id` source. - return mint(); - } - - let cache = - SIMPLE_DESCR_GROUP_CACHE.get_or_init(|| std::sync::Mutex::new(indexmap::IndexMap::new())); - { - let cache = cache.lock().unwrap(); - if let Some(group) = cache.get(&spec.type_id) { - return group.clone(); - } - } - let group = mint(); - let mut cache = cache.lock().unwrap(); - cache.entry_or_insert_with(spec.type_id, || group).clone() -} - -#[derive(Debug)] -struct ParentBackedFieldDescr { - field: Arc, - parent: Arc, -} - -impl Descr for ParentBackedFieldDescr { - fn index(&self) -> u32 { - self.field.index() - } - fn get_descr_index(&self) -> i32 { - self.field.get_descr_index() - } - fn set_descr_index(&self, index: i32) { - self.field.set_descr_index(index); - } - fn is_always_pure(&self) -> bool { - self.field.is_always_pure() - } - fn is_quasi_immutable(&self) -> bool { - self.field.is_quasi_immutable() - } - fn is_virtualizable(&self) -> bool { - self.field.is_virtualizable() - } - fn as_field_descr(&self) -> Option<&dyn FieldDescr> { - Some(self) - } - /// `effectinfo.py:526` `descr.ei_index = …` parity — delegate to - /// the inner `SimpleFieldDescr`'s atomic so `compute_bitstrings`'s - /// `set_ei_index` write reaches the same storage that - /// `heap.rs::field_effect_index` reads through any cloned wrapper. - fn get_ei_index(&self) -> u32 { - self.field.get_ei_index() - } - fn set_ei_index(&self, index: u32) { - self.field.set_ei_index(index); - } -} - -impl FieldDescr for ParentBackedFieldDescr { - fn offset(&self) -> usize { - self.field.offset() - } - fn field_size(&self) -> usize { - self.field.field_size() - } - fn field_type(&self) -> Type { - self.field.field_type() - } - fn is_pointer_field(&self) -> bool { - self.field.is_pointer_field() - } - fn is_float_field(&self) -> bool { - self.field.is_float_field() - } - fn is_field_signed(&self) -> bool { - self.field.is_field_signed() - } - fn is_immutable(&self) -> bool { - self.field.is_immutable() - } - fn field_name(&self) -> &str { - self.field.field_name() - } - fn index_in_parent(&self) -> usize { - self.field.index_in_parent() - } - fn get_parent_descr(&self) -> Option { - Some(self.parent.clone() as DescrRef) - } - fn get_vinfo(&self) -> Option> { - self.field.get_vinfo() + return majit_ir::descr::make_simple_descr_group( + u32::MAX, + spec.size, + spec.type_id as u32, + spec.vtable as usize, + &field_specs, + ); } + // `descr.py:108-118 get_size_descr` + `:218-239 get_field_descr` + // keyed publish: GcCache is the sole owner/cache for this STRUCT. + majit_ir::descr::make_simple_descr_group_keyed_with_headerless( + u32::MAX, + spec.size, + spec.type_id as u32, + spec.type_id, + spec.vtable as usize, + spec.is_gc_managed, + spec.headerless, + &field_specs, + &[], + ) } fn field_descr_from_bh_field( @@ -3502,42 +3376,54 @@ fn field_descr_from_bh_field( // runtime `PyreFieldDescr` (or analyzer-published // `SimpleFieldDescr`). Both back-reference the same parent // SizeDescr via `parent_descr` (descr.py:200), so the - // `ParentBackedFieldDescr` wrapper is unnecessary on this path - // — analyzer raw-set Arcs and runtime allocator descrs share + // an adapter wrapper is unnecessary on this path — analyzer + // raw-set Arcs and runtime allocator descrs share // one identity slot. if parent.type_id != 0 { let key = majit_ir::descr::LLType::Struct(parent.type_id); - let parent_size = majit_ir::descr::gc_cache() - .lock() - .unwrap() - ._cache_size + let field_key = field.field_key().to_string(); + let mut gc = majit_ir::descr::gc_cache().lock().unwrap(); + // `descr.py:220-221 cache[STRUCT][fieldname]` hit. + if let Some(fd) = gc + ._cache_field .get(&key) - .cloned(); - if let Some(parent_descr) = parent_size { - if let Some(parent_sd) = parent_descr.as_size_descr() { - for fd in parent_sd.all_fielddescrs() { - if fd.index_in_parent() == field.index_in_parent - && (fd.field_name() == field.name - || fd.field_name().ends_with(&format!(".{}", field.name))) - { - return fd.clone() as DescrRef; - } - } - } + .and_then(|inner| inner.get(&field_key)) + { + return fd.clone() as DescrRef; + } + // Miss with the parent STRUCT published: mint through + // `descr.py:225-238 get_field_descr` so this resolution and the + // walker's (`pyjitpl/dispatch.rs field_descr_ref_from_bh`) share + // one Arc. `descr.py:238 parent_descr = get_size_descr(STRUCT)` + // needs the `_cache_size` slot, so only take this route when it + // is populated. + if gc._cache_size.contains_key(&key) { + let fd = gc.get_field_descr( + key, + &field_key, + field.offset, + field.field_size, + field.field_type, + field.is_immutable, + field.is_quasi_immutable, + field.field_flag, + field.index, + false, + field.index_in_parent, + ); + return fd as DescrRef; } } - // Cache miss / non-keyed parent — fall back to the legacy - // SimpleDescrGroup mint + `ParentBackedFieldDescr` wrapper so - // the cyclic parent_descr Weak still binds correctly. + // Cache miss / non-keyed parent — fall back to the descr group + // field itself; the keyed path minted it through get_field_descr. let group = simple_descr_group_from_bh_size(parent); - if let Some((pos, _)) = parent.all_fielddescrs.iter().enumerate().find(|(_, spec)| { - spec.index_in_parent == field.index_in_parent && spec.name == field.name - }) { + if let Some((pos, _)) = + parent.all_fielddescrs.iter().enumerate().find(|(_, spec)| { + spec.offset == field.offset && spec.field_key() == field.field_key() + }) + { if let Some(descr) = group.field_descrs.get(pos) { - return Arc::new(ParentBackedFieldDescr { - field: descr.clone(), - parent: group.size_descr.clone(), - }); + return descr.clone() as DescrRef; } } } @@ -3550,6 +3436,7 @@ fn field_descr_from_bh_field( field.is_immutable, field.field_flag, field.name.clone(), + field.field_key().to_string(), ) .with_quasi_immutable(field.is_quasi_immutable); let arc: DescrRef = Arc::new(descr); @@ -3559,6 +3446,14 @@ fn field_descr_from_bh_field( arc } +fn bh_field_cache_key(owner: &str, name: &str) -> String { + if owner.is_empty() { + return name.to_string(); + } + let prefix = format!("{owner}."); + name.strip_prefix(&prefix).unwrap_or(name).to_string() +} + /// Keyed sibling: accepts the u64 `cache_key` (= `path_hash(array_type_id)`) /// so the freshly-minted `SimpleArrayDescr` lands in /// `gc_cache._cache_array[LLType::Array(cache_key)]` in addition to @@ -3911,6 +3806,7 @@ pub fn make_descr_from_bh(bh: &majit_translate::jitcode::BhDescr) -> DescrRef { owner, .. } => { + let field_key = bh_field_cache_key(owner, name); // #171 codewriter descr-bridge: `_handle_list_call` // (codewriter/jtransform.rs) lowers Integer-strategy list // ops to fields on the dotted nested names @@ -3923,6 +3819,20 @@ pub fn make_descr_from_bh(bh: &majit_translate::jitcode::BhDescr) -> DescrRef { // list specializations already use so an assembled codewriter // list body addresses `IntArray.{len,block}` rather // than the header. + // + // This runs BEFORE the parent-group lookup below, not after: when + // the codewriter DOES model the parent struct, that lookup answers + // with the parent group's own entry for the same offset, and the + // field ends up carrying two descrs — the parent group's for a + // codewriter-lowered body, `W_LIST_DESCR_GROUP`'s for the + // walker-native specializations. The heapcache and the optimizer's + // heap pass both key on descr identity, so the split silently + // breaks aliasing: the `w_list_append` sub-walk's + // `SetfieldGc(int_items.len)` does not invalidate the `len(xs)` + // read that follows it, which then folds to the pre-append length + // (one skipped `list.pop(0)` per compiled loop entry). One field is + // one descr — `metainterp_sd.all_descrs` has no second entry for a + // field just because a different interpreter reached it. if owner.as_str() == "W_ListObject" { match name.as_str() { "int_items.len" => return list_int_items_len_descr(), @@ -3951,6 +3861,30 @@ pub fn make_descr_from_bh(bh: &majit_translate::jitcode::BhDescr) -> DescrRef { _ => {} } } + if let Some(parent) = parent { + if parent.type_id != 0 { + let key = majit_ir::descr::LLType::Struct(parent.type_id); + if let Some(fd) = majit_ir::descr::gc_cache() + .lock() + .unwrap() + ._cache_field + .get(&key) + .and_then(|inner| inner.get(&field_key)) + { + return fd.clone() as DescrRef; + } + let group = simple_descr_group_from_bh_size(parent); + if let Some((pos, _)) = + parent.all_fielddescrs.iter().enumerate().find(|(_, spec)| { + spec.offset == *offset && spec.field_key() == field_key + }) + { + if let Some(descr) = group.field_descrs.get(pos) { + return descr.clone() as DescrRef; + } + } + } + } // #171 object-strategy capacity read: `list.obj_capacity` lowers // to getfield_gc_r(items) + getfield_gc_i(block.capacity). The // block's offset-0 GcArray length header IS the allocated @@ -3998,6 +3932,7 @@ pub fn make_descr_from_bh(bh: &majit_translate::jitcode::BhDescr) -> DescrRef { // upstream value rather than a `u32::MAX` sentinel. let field = majit_translate::jitcode::BhFieldSpec { index: *index_in_parent as u32, + field_key, name: full_name, offset: *offset, field_size: *field_size, @@ -4254,6 +4189,7 @@ pub fn make_descr_from_bh(bh: &majit_translate::jitcode::BhDescr) -> DescrRef { field_type, false, field_flag, + name.clone(), name, ) .with_index_in_parent(index_in_parent), @@ -4303,6 +4239,197 @@ pub fn make_descr_from_bh(bh: &majit_translate::jitcode::BhDescr) -> DescrRef { } } +/// Look a serialized raw-set member back up in the process-global gccache. +/// +/// **Lookup only — never mint.** `descr.py:218-239 get_field_descr` is +/// cache-or-mint because upstream calls it from `heaptracker.all_fielddescrs +/// (STRUCT)`, which always has the whole `STRUCT` in hand and therefore mints +/// the parent `SizeDescr` with its complete `all_fielddescrs` list +/// (`descr.py:188 init_size_descr`). A serialized member carries only its own +/// slot, so minting through it would publish a parent with an empty field +/// list, win `_cache_field` by first-write, and leave +/// `SizeDescr.all_fielddescrs` and `_cache_field` describing different +/// objects — breaking the positional invariant `heaptracker.py:76-101 +/// get_fielddescr_index_in` establishes and `optimizeopt/info.rs force_box` +/// asserts. +/// +/// Outcome of looking one serialized raw-set member back up. +enum SetMemberLookup { + /// The gccache holds the slot the analyzer minted through. + Resolved(majit_ir::DescrRef), + /// The *container* (`STRUCT` / `ARRAY`) is absent from this process's + /// descr universe entirely, so no operation recorded here can carry a + /// descr for it and the member cannot participate in any + /// `check_*_descr_*` answer. Dropping it is the faithful projection of + /// the analyzer's frozenset onto the runtime universe — the analyzer + /// walks the whole translated program, the runtime only registers what + /// it actually traces. Upstream has no counterpart because + /// `cpu.*descrof` and `compute_bitstrings` share one process. + AbsentContainer, + /// The container IS published but not under this member's key. A real + /// runtime descr for the field may exist under a different spelling, so + /// dropping the member would answer "not written" for a field that is; + /// the caller must fall back to the wildcard instead. + Ambiguous, +} + +/// Look a serialized raw-set member back up in the process-global gccache. +/// +/// **Lookup only — never mint.** `descr.py:218-239 get_field_descr` is +/// cache-or-mint because upstream calls it from `heaptracker.all_fielddescrs +/// (STRUCT)`, which always has the whole `STRUCT` in hand and therefore mints +/// the parent `SizeDescr` with its complete `all_fielddescrs` list +/// (`descr.py:188 init_size_descr`). A serialized member carries only its own +/// slot, so minting through it would publish a parent with an empty field +/// list, win `_cache_field` by first-write, and leave +/// `SizeDescr.all_fielddescrs` and `_cache_field` describing different +/// objects — breaking the positional invariant `heaptracker.py:76-101 +/// get_fielddescr_index_in` establishes and `optimizeopt/info.rs force_box` +/// asserts. +fn descr_from_set_member(m: &majit_ir::effectinfo::DescrSetMember) -> SetMemberLookup { + use majit_ir::descr::{LLType, gc_cache}; + + match m { + majit_ir::effectinfo::DescrSetMember::Field { + struct_id, + field_name, + .. + } => { + let struct_key = LLType::Struct(*struct_id); + let gc = gc_cache().lock().unwrap(); + match gc._cache_field.get(&struct_key) { + Some(inner) => match inner.get(field_name.as_str()) { + Some(fd) => SetMemberLookup::Resolved(fd.clone() as majit_ir::DescrRef), + None => SetMemberLookup::Ambiguous, + }, + // No field map and no size slot: nothing in this process + // ever named the struct. + None if !gc._cache_size.contains_key(&struct_key) => { + SetMemberLookup::AbsentContainer + } + None => SetMemberLookup::Ambiguous, + } + } + majit_ir::effectinfo::DescrSetMember::Array { array_id, .. } => { + match gc_cache() + .lock() + .unwrap() + ._cache_array + .get(&LLType::Array(*array_id)) + { + Some(ad) => SetMemberLookup::Resolved(ad.clone()), + None => SetMemberLookup::AbsentContainer, + } + } + majit_ir::effectinfo::DescrSetMember::InteriorField { array_id, name, .. } => { + let gc = gc_cache().lock().unwrap(); + let array_key = LLType::Array(*array_id); + match gc + ._cache_interiorfield + .get(&(array_key.clone(), name.clone(), String::new())) + { + Some(d) => SetMemberLookup::Resolved(d.clone()), + None if !gc._cache_array.contains_key(&array_key) => { + SetMemberLookup::AbsentContainer + } + None => SetMemberLookup::Ambiguous, + } + } + } +} + +/// Fill the six `_*_descrs_*` raw sets from `descr_set_keys`, canonicalising +/// exactly as `effectinfo.py:128-145 frozenset_or_none` / +/// `canonicalize_descr_set`. +/// +/// All six sets or none of them: `effectinfo.py:149-162` makes them `None` +/// **iff** the EI is `EF_RANDOM_EFFECTS`, and `compute_bitstrings` +/// (`effectinfo.py:484-489`) asserts that biconditional before deciding +/// whether to clear the bitstrings. A half-populated EI would clear them +/// while `extraeffect` still claims concrete effects, and the next +/// `check_readonly_descr_field` would then read a `None` bitstring. +pub fn rehydrate_effect_info(ei: &mut majit_ir::EffectInfo) { + // `effectinfo.py:285-292` wildcard: the shape to fall back to whenever + // the concrete sets cannot be rebuilt faithfully. Conservative in the + // sound direction — `has_random_effects()` makes every heap consumer + // assume the call touched everything. + fn degrade(ei: &mut majit_ir::EffectInfo) { + ei.extraeffect = majit_ir::ExtraEffect::RandomEffects; + // effectinfo.py:364-365 — the wildcard forces can_collect. + ei.can_collect = true; + ei._readonly_descrs_fields = None; + ei._write_descrs_fields = None; + ei._readonly_descrs_arrays = None; + ei._write_descrs_arrays = None; + ei._readonly_descrs_interiorfields = None; + ei._write_descrs_interiorfields = None; + ei.readonly_descrs_fields = None; + ei.write_descrs_fields = None; + ei.readonly_descrs_arrays = None; + ei.write_descrs_arrays = None; + ei.readonly_descrs_interiorfields = None; + ei.write_descrs_interiorfields = None; + ei.single_write_descr_array = None; + } + + let Some(keys) = ei.descr_set_keys.as_ref() else { + // No serialized key channel. For a build-time EI that means the + // codewriter already emitted the `EF_RANDOM_EFFECTS` wildcard, so + // the six raw sets are `None` and there is nothing to rebuild. Any + // other shape lost its sets somewhere the invariant above does not + // cover; restore the wildcard rather than leave a concrete + // `extraeffect` pointing at absent sets. + if ei.extraeffect != majit_ir::ExtraEffect::RandomEffects { + degrade(ei); + } + return; + }; + let resolve = |members: &[majit_ir::effectinfo::DescrSetMember]| { + let mut out = Vec::with_capacity(members.len()); + for m in members { + match descr_from_set_member(m) { + SetMemberLookup::Resolved(d) => out.push(d), + SetMemberLookup::AbsentContainer => {} + SetMemberLookup::Ambiguous => return None, + } + } + Some(majit_ir::effectinfo::canonicalize_descr_set(out)) + }; + let resolved = ( + resolve(&keys.readonly_fields), + resolve(&keys.write_fields), + resolve(&keys.readonly_arrays), + resolve(&keys.write_arrays), + resolve(&keys.readonly_interiorfields), + resolve(&keys.write_interiorfields), + ); + let ( + Some(readonly_fields), + Some(write_fields), + Some(readonly_arrays), + Some(write_arrays), + Some(readonly_interiorfields), + Some(write_interiorfields), + ) = resolved + else { + degrade(ei); + return; + }; + ei._readonly_descrs_fields = Some(readonly_fields); + ei._write_descrs_fields = Some(write_fields); + ei._readonly_descrs_arrays = Some(readonly_arrays); + // effectinfo.py:201-206 single_write_descr_array — also `serde(skip)`, + // and read in production by `heap.rs force_from_effectinfo`, so it is + // re-derived from the set that just came back rather than left `None`. + ei.single_write_descr_array = match write_arrays.as_slice() { + [only] => Some(only.clone()), + _ => None, + }; + ei._write_descrs_arrays = Some(write_arrays); + ei._readonly_descrs_interiorfields = Some(readonly_interiorfields); + ei._write_descrs_interiorfields = Some(write_interiorfields); +} + /// `BhCallDescr` -> `CallDescr` adapter. RPython parity: codewriter /// `Assembler.descrs` carries the same `CallDescr` instance the /// metainterp pulls during op recording. pyre keeps the codewriter-side diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 2624ceba48a..c88a5012b83 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -4743,10 +4743,27 @@ pub(crate) fn orthodox_list_append_commit( // trace). The descr-pool wiring above (strategy/header field descrs) is // exercised on the way in. - // Tracing is execution: apply the append + journal the rewind (the walker - // recorded the IR but did not mutate the concrete list). + // Tracing is execution: apply the append + journal the rewind. The + // journal entry is unconditional — it rewinds the receiver to + // `len_before` on an aborted walk, whichever side actually grew it. + // + // The sub-walk normally records the store as IR without touching the + // concrete list, so the append below is what applies it. It is not + // guaranteed to: the per-strategy store the descended arm reaches + // (`W_ListObject::object_push`, `IntArray::push`, `FloatArray::push`) is a + // `residual_call`, and a residual whose funcptr resolves to a real address + // is EXECUTED by `try_execute_residual_call_via_executor` rather than only + // recorded. Those three carry runtime bindings, so on a target where the + // arm keeps them as residuals the sub-walk has already appended, and + // appending again puts the value in twice — one extra element per compiled + // append, which is how it surfaces (`len(keep)` 20048 for 20000 + // iterations, a traceback name list with its last frame doubled). + // Re-read the length instead of assuming which side ran: it is the + // receiver's own state, so it answers for both. fbw_append_journal_push(inner_self, len_before); - unsafe { pyre_object::w_list_append(inner_self, value) }; + if unsafe { pyre_object::w_list_len(inner_self) } == len_before { + unsafe { pyre_object::w_list_append(inner_self, value) }; + } Ok(()) } diff --git a/pyre/pyre-jit-trace/src/jitcode_runtime.rs b/pyre/pyre-jit-trace/src/jitcode_runtime.rs index 2685afc8a07..abaf957494a 100644 --- a/pyre/pyre-jit-trace/src/jitcode_runtime.rs +++ b/pyre/pyre-jit-trace/src/jitcode_runtime.rs @@ -19,12 +19,15 @@ use std::cell::OnceCell; use std::collections::HashMap; -use std::sync::{Arc, LazyLock}; +use std::sync::{Arc, LazyLock, Mutex, Once}; use majit_ir::DescrRef; use majit_translate::CompiledJitDriver; use majit_translate::jitcode::{BhDescr, JitCode}; +static REHYDRATED_CALL_DESCR_REFS: LazyLock>>> = + LazyLock::new(|| Mutex::new(Vec::new())); + thread_local! { /// Per-thread cached `&'static` to the build-time `pipeline.jitcodes` /// table. Set on first access (see [`load_all_jitcodes`]). @@ -410,6 +413,73 @@ pub fn all_liveness() -> &'static [u8] { &ALL_LIVENESS } +fn call_descr_arg_types(arg_classes: &str) -> Vec { + arg_classes + .chars() + .filter_map(|c| match c { + 'i' | 'S' => Some(majit_ir::Type::Int), + 'r' => Some(majit_ir::Type::Ref), + 'f' | 'L' => Some(majit_ir::Type::Float), + _ => None, + }) + .collect() +} + +fn call_descr_result_type(result_type: char) -> majit_ir::Type { + match result_type { + 'i' | 'S' => majit_ir::Type::Int, + 'r' => majit_ir::Type::Ref, + 'f' | 'L' => majit_ir::Type::Float, + _ => majit_ir::Type::Void, + } +} + +fn rehydrated_call_descr_ref(bh: &majit_translate::jitcode::BhCallDescr) -> majit_ir::DescrRef { + let arg_types = call_descr_arg_types(&bh.arg_classes); + let result_type = call_descr_result_type(bh.result_type); + let mut effect_info = bh.extra_info.clone(); + crate::descr::rehydrate_effect_info(&mut effect_info); + majit_metainterp::make_call_descr_sized_with_effect( + &arg_types, + result_type, + bh.result_signed, + bh.result_size, + effect_info, + ) +} + +/// Rehydrate build-time EffectInfo raw descr sets before +/// `finish_setup_descrs`. `finish_setup_done` is per-thread, but the +/// rehydrated raw sets live in the process-global `GcCache`, so this guard is +/// process-global. +pub fn rehydrate_build_descr_raw_sets() { + static ONCE: Once = Once::new(); + ONCE.call_once(|| { + let all = all_descrs(); + // `descr.py:25-47 setup_descrs` group order — every non-call slot + // first. Each `Size` / `Field` entry publishes its parent's FULL + // `heaptracker.all_fielddescrs(STRUCT)` list into the gccache, and + // `descr_from_set_member` is lookup-only, so the raw-set members + // below can only land on slots that already carry their complete + // layout. Resolving in the other order would leave every member + // whose struct has not been published yet unresolvable. + for bh in all.iter() { + if !matches!(bh, BhDescr::Call { .. } | BhDescr::JitCode { .. }) { + crate::descr::make_descr_from_bh(bh); + } + } + let mut refs = vec![None; all.len()]; + for (i, bh) in all.iter().enumerate() { + let calldescr = match bh { + BhDescr::Call { calldescr } | BhDescr::JitCode { calldescr, .. } => calldescr, + _ => continue, + }; + refs[i] = Some(rehydrated_call_descr_ref(calldescr)); + } + *REHYDRATED_CALL_DESCR_REFS.lock().unwrap() = refs; + }); +} + /// Pool of `DescrRef`s indexed alongside [`all_descrs`] so the /// trace-side jitcode walker /// ([`crate::jitcode_dispatch::dispatch_via_miframe`]) can resolve each @@ -443,12 +513,195 @@ pub fn all_liveness() -> &'static [u8] { /// `Arc` instance still build their own at the call site /// until the by-index identity factories land. static ALL_DESCR_REFS: LazyLock> = LazyLock::new(|| { - all_descrs() + let refs: Vec = all_descrs() .iter() - .map(crate::descr::make_descr_from_bh) - .collect() + .enumerate() + .map(|(i, bh)| match bh { + BhDescr::Call { .. } => REHYDRATED_CALL_DESCR_REFS + .lock() + .unwrap() + .get(i) + .and_then(Clone::clone) + .unwrap_or_else(|| crate::descr::make_descr_from_bh(bh)), + _ => crate::descr::make_descr_from_bh(bh), + }) + .collect(); + if std::env::var_os("PYRE_FIELD_IDENTITY_CENSUS").is_some() { + field_descr_identity_census(&refs); + } + refs }); +/// S4c prerequisite measurement — how many build-time `BhDescr::Field` slots +/// resolve to the SAME `Arc` the runtime `descr.py:218-239 get_field_descr` +/// cache holds for their `(STRUCT, fieldname)` key. +/// +/// `effectinfo.py:465-547 compute_bitstrings` partitions descrs by object +/// identity, so an `EffectInfo` raw set rehydrated from `descrs.bin` is only +/// meaningful if each member lands on the descr the trace itself caches. A +/// slot that mints a fresh `Arc` instead is a silent mis-partition, which is +/// strictly worse than the missing raw set it would replace — hence this runs +/// before the format change, not after. +pub fn field_descr_identity_census_now() { + field_descr_identity_census(all_descr_refs()); +} + +/// Same-Arc test for two `DescrRef`s. `Arc::ptr_eq` on `Arc` +/// compares the fat pointer (data + vtable); two upcasts of the same +/// allocation through the same concrete type agree on both halves, and an +/// upcast of a *different* concrete type must not compare equal anyway. +fn same_arc(a: &DescrRef, b: &DescrRef) -> bool { + std::sync::Arc::as_ptr(a) as *const () == std::sync::Arc::as_ptr(b) as *const () +} + +/// Why a build-time `Field` slot fails to land on the canonical +/// `_cache_field[STRUCT][fieldname]` Arc. `descr.py:218-239 get_field_descr` +/// admits exactly one outcome — cache hit or cache-miss mint — so every +/// class below except `Converged` marks a place where pyre mints a second +/// FieldDescr for a `(STRUCT, fieldname)` PyPy keeps single. +#[derive(PartialEq, Eq, Hash, Clone, Copy, PartialOrd, Ord)] +enum FieldIdentityClass { + Converged, + /// `_cache_size[STRUCT]` empty — the parent was never published, so + /// `get_field_descr` could not have set `parent_descr` either. + NoParentSlot, + /// Parent published, but `_cache_field` has no inner map for it: + /// `heaptracker.all_fielddescrs` never ran through `get_field_descr`. + NoFieldMap, + /// Inner map exists but not under this spelling — pyre's cache key and + /// its lookup key disagree (PyPy keys on `fieldname`, displays + /// `'%s.%s' % (STRUCT._name, fieldname)`; pyre conflates the two). + NameMiss, + /// Entry exists and is a different Arc: two mint points for one field. + DoubleMint, +} + +impl FieldIdentityClass { + fn label(self) -> &'static str { + match self { + Self::Converged => "converged", + Self::NoParentSlot => "no _cache_size[STRUCT]", + Self::NoFieldMap => "no _cache_field[STRUCT]", + Self::NameMiss => "name not in _cache_field[STRUCT]", + Self::DoubleMint => "different Arc (double mint)", + } + } +} + +fn field_descr_identity_census(refs: &[DescrRef]) { + use std::collections::BTreeMap; + + let (mut fields, mut keyed, mut parentless) = (0usize, 0usize, 0usize); + // Pool-side (`field_descr_from_bh_field`) vs `_cache_field`. + let mut pool_classes: BTreeMap = BTreeMap::new(); + // Walker-side (`field_descr_ref_from_bh`, the Arc actually baked into + // recorded getfield/setfield ops) vs `_cache_field`. + let mut walker_classes: BTreeMap = BTreeMap::new(); + // Pool-side vs walker-side: the split that makes an `EffectInfo` raw set + // rehydrated from `descrs.bin` unable to reach the recorded op's descr. + let mut pool_vs_walker_same = 0usize; + // How often the pool Arc is one of the parent SizeDescr's + // `all_fielddescrs()` (the second mint point). + let mut pool_from_all_fielddescrs = 0usize; + let mut samples: Vec = Vec::new(); + + for (i, bh) in all_descrs().iter().enumerate() { + let BhDescr::Field { + parent, + name, + owner, + index_in_parent, + .. + } = bh + else { + continue; + }; + fields += 1; + let Some(parent) = parent.as_ref().filter(|p| p.type_id != 0) else { + parentless += 1; + continue; + }; + keyed += 1; + let key = majit_ir::descr::LLType::Struct(parent.type_id); + let (parent_size, cached, field_keys) = { + let gc = majit_ir::descr::gc_cache().lock().unwrap(); + let inner = gc._cache_field.get(&key); + ( + gc._cache_size.get(&key).cloned(), + inner.and_then(|m| m.get(name.as_str())).cloned(), + inner.map(|m| { + let mut ks: Vec = m.keys().cloned().collect(); + ks.sort(); + ks + }), + ) + }; + let cached_ref: Option = cached.map(|fd| fd as DescrRef); + let classify = |candidate: &DescrRef| -> FieldIdentityClass { + match (&parent_size, &cached_ref, &field_keys) { + (None, _, _) => FieldIdentityClass::NoParentSlot, + (_, Some(c), _) if same_arc(c, candidate) => FieldIdentityClass::Converged, + (_, Some(_), _) => FieldIdentityClass::DoubleMint, + (_, None, None) => FieldIdentityClass::NoFieldMap, + (_, None, Some(_)) => FieldIdentityClass::NameMiss, + } + }; + + let pool = &refs[i]; + let (_, walker) = majit_metainterp::field_descr_ref_from_bh(bh); + let pool_class = classify(pool); + let walker_class = classify(&walker); + *pool_classes.entry(pool_class).or_default() += 1; + *walker_classes.entry(walker_class).or_default() += 1; + if same_arc(pool, &walker) { + pool_vs_walker_same += 1; + } + if let Some(sd) = parent_size.as_ref().and_then(|p| p.as_size_descr()) { + if sd + .all_fielddescrs() + .iter() + .any(|fd| same_arc(&(fd.clone() as DescrRef), pool)) + { + pool_from_all_fielddescrs += 1; + } + } + + if pool_class != FieldIdentityClass::Converged && samples.len() < 25 { + let n_all = parent_size + .as_ref() + .and_then(|p| p.as_size_descr()) + .map(|sd| sd.all_fielddescrs().len()); + samples.push(format!( + "{owner}.{name}[{index_in_parent}] T{:#x} pool={} walker={} \ + all_fielddescrs={n_all:?} _cache_field keys={:?}", + parent.type_id, + pool_class.label(), + walker_class.label(), + field_keys.as_deref().unwrap_or(&[]), + )); + } + } + + eprintln!("[field-identity] {fields} Field slots: {keyed} keyed, {parentless} parentless"); + for (label, classes) in [("pool", &pool_classes), ("walker", &walker_classes)] { + let rendered: Vec = classes + .iter() + .map(|(c, n)| format!("{}={n}", c.label())) + .collect(); + eprintln!( + "[field-identity] {label} vs _cache_field: {}", + rendered.join(", ") + ); + } + eprintln!( + "[field-identity] pool==walker: {pool_vs_walker_same}/{keyed}; \ + pool Arc came from parent.all_fielddescrs(): {pool_from_all_fielddescrs}/{keyed}" + ); + for s in &samples { + eprintln!("[field-identity] {s}"); + } +} + /// `&'static [DescrRef]` view over [`ALL_DESCR_REFS`] for the walker's /// `WalkContext::descr_refs` parameter. pub fn all_descr_refs() -> &'static [DescrRef] { @@ -493,22 +746,30 @@ pub fn build_time_field_offset(owner: &str, name: &str) -> Option { /// `as_bh_descr()`. /// /// Idempotent: the metainterp `OnceLock` keeps the first pool, so repeated -/// calls (harness + production init) are safe. +/// calls (harness + production init) are safe. The pool is built inside the +/// `OnceLock` initializer, so a repeat call costs a load and nothing else — +/// `drive_unpack_iterable_trace` reaches here once per +/// `_unpackiterable_unknown_length`, and cloning every `BhDescr` (each call +/// descr carrying its `EffectInfo` raw descr sets) only to drop it is the +/// dominant cost of an unpack-heavy program. pub fn install_global_build_descr_pool() { use majit_metainterp::RuntimeBhDescr; - let pool: Vec = all_descrs() - .iter() - .map(|bh| match bh { - BhDescr::JitCode { jitcode_index, .. } => match get_jitcode_by_index(*jitcode_index) { - Some(canonical) => RuntimeBhDescr::JitCode(Arc::new( - majit_metainterp::JitCode::from_canonical((*canonical).clone()), - )), - None => RuntimeBhDescr::Descr(bh.clone()), - }, - other => RuntimeBhDescr::Descr(other.clone()), - }) - .collect(); - majit_metainterp::set_global_build_descr_pool(pool); + majit_metainterp::init_global_build_descr_pool(|| { + all_descrs() + .iter() + .map(|bh| match bh { + BhDescr::JitCode { jitcode_index, .. } => { + match get_jitcode_by_index(*jitcode_index) { + Some(canonical) => RuntimeBhDescr::JitCode(Arc::new( + majit_metainterp::JitCode::from_canonical((*canonical).clone()), + )), + None => RuntimeBhDescr::Descr(bh.clone()), + } + } + other => RuntimeBhDescr::Descr(other.clone()), + }) + .collect() + }); } /// Build a `BlackholeInterpBuilder` pre-configured for this binary's diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 29642db4af9..0dd7c6a3e81 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -99,6 +99,12 @@ struct MetaInterpStaticData { /// cached opcode ids and liveness bytes in place. finish_setup_done: bool, + /// `Assembler.insns` length the cached `op_*` ids above were read off. + /// Paired with `liveness_info.len()` it decides whether a refresh would + /// observe anything new — both writer-side buffers are append-only + /// (`assembler.py:29-31`), so equal lengths mean equal contents. + insns_len: usize, + // pyjitpl.py:2236-2243 opcode number cache filled by `setup_insns`. // RPython stores every field even when the runtime currently does // not read them, so the structural parity is preserved. Sentinel @@ -139,6 +145,7 @@ impl MetaInterpStaticData { jitcodes: Vec::new(), liveness_info: std::sync::Arc::<[u8]>::from(Vec::::new().into_boxed_slice()), finish_setup_done: false, + insns_len: 0, op_live: u8::MAX, op_goto: u8::MAX, op_catch_exception: u8::MAX, @@ -186,6 +193,19 @@ impl MetaInterpStaticData { self.op_ref_return = insns.get("ref_return/r").copied().unwrap_or(u8::MAX); self.op_float_return = insns.get("float_return/f").copied().unwrap_or(u8::MAX); self.op_void_return = insns.get("void_return/").copied().unwrap_or(u8::MAX); + self.insns_len = insns.len(); + } + + /// Whether a `finish_setup_if_needed` call with these writer-side buffer + /// lengths would republish anything. `ensure_finish_setup` runs on every + /// `jitcode_for`, and building its arguments copies both buffers whole; + /// `assembler.py:29-31` seeds them from the build-time tables, so that + /// copy is proportional to the whole opcode and liveness universe rather + /// than to what the call would change. + fn finish_setup_would_republish(&self, insns_len: usize, all_liveness_len: usize) -> bool { + !self.finish_setup_done + || insns_len != self.insns_len + || all_liveness_len != self.liveness_info.len() } /// pyjitpl.py:2255-2264 `finish_setup`: wire the assembler's opcode table @@ -209,6 +229,12 @@ impl MetaInterpStaticData { // descr's `ei_index` slot via `effectinfo::compute_bitstrings` // (`effectinfo.py:526 descr.ei_index = …`); no process-global // side table. + // + // This staticdata is not the one the tracing `MetaInterp` owns — + // pyre carries two — so the `all_descrs` list this publishes has to + // be the process-wide one `descr_index` is stamped against + // (`MetaInterpStaticData::all_descrs`), or `bridgeopt.py:155 + // metainterp_sd.all_descrs[descr_index]` indexes an empty list. if !was_done { self.canonical.finish_setup_descrs(); } @@ -580,10 +606,18 @@ fn ensure_finish_setup() { FRAME_VALUE_COUNT_INIT.call_once(|| { majit_ir::resumedata::set_frame_value_count_fn(frame_value_count_at); }); - let (insns, all_liveness) = ASSEMBLER_STATE.with(|a| { + let snapshot = ASSEMBLER_STATE.with(|a| { let asm = a.borrow(); - (asm.insns.clone(), asm.all_liveness.clone()) + let republishes = METAINTERP_SD.with(|r| { + r.borrow() + .finish_setup_would_republish(asm.insns.len(), asm.all_liveness.len()) + }); + republishes.then(|| (asm.insns.clone(), asm.all_liveness.clone())) }); + let Some((insns, all_liveness)) = snapshot else { + return; + }; + crate::jitcode_runtime::rehydrate_build_descr_raw_sets(); METAINTERP_SD.with(|r| { r.borrow_mut().finish_setup_if_needed(&insns, all_liveness); }); @@ -1169,20 +1203,23 @@ pub fn frame_value_count_at(jitcode_index: i32, pc: i32) -> usize { /// /// pyre has two jitcode numbering spaces. jd0 (`pyframe_driver`) numbers /// Python-bytecode jitcodes into `MetaInterpStaticData.jitcodes`, keyed by -/// CodeObject, and interns their `-live-` triples into -/// `metainterp_sd.liveness_info` (`pyjitpl.py:2264`) as tracing discovers them. -/// A novable driver over an extracted interpreter body — jd1 +/// CodeObject. A novable driver over an extracted interpreter body — jd1 /// `unpackiterable_driver`, whose jitcode is the /// `_unpackiterable_unknown_length` graph plus its inlined build-time callees — -/// numbers against `jitcode_runtime::all_jitcodes()`, with `-live-` offsets -/// baked at extraction into `jitcode_runtime::all_liveness()`. +/// numbers against `jitcode_runtime::all_jitcodes()`. /// -/// Decoding one space's coordinate against the other's tables does not fail -/// loudly, which is why the store has to be picked per driver rather than tried -/// and retried: the runtime store's low indices hold unrelated PyCode jitcodes -/// that decode at the same pc and hand back a mistyped count (the drain's 2 refs +/// Decoding one space's index against the other's table does not fail loudly, +/// which is why the store has to be picked per driver rather than tried and +/// retried: the runtime store's low indices hold unrelated PyCode jitcodes that +/// decode at the same pc and hand back a mistyped count (the drain's 2 refs /// read as ints → `Const::getint on Ref`). Same split, and same reasoning, as -/// the `novable` arms of `call_jit.rs`'s `blackhole_resume_via_rd_numb`. +/// the `novable` arm of `resolve_jitcode` in `call_jit.rs`. +/// +/// The `-live-` *offsets* are no longer split: the build-time byte stream is +/// the prefix of `metainterp_sd.liveness_info` +/// (`Assembler::resuming_build_time_liveness`), so this reads the one pool +/// `resume.py:1022` reads, exactly like [`frame_value_count_at`]. Only the +/// jitcode table below is still per-space. /// /// Installed on jd1's `JitDriverStaticData::frame_value_count_fn`, so only that /// driver's guard metadata decodes here. @@ -1203,7 +1240,7 @@ pub fn build_time_frame_value_count_at(jitcode_index: i32, pc: i32) -> usize { Some(jc) => jc, None => return 0, }; - let all_liveness = crate::jitcode_runtime::all_liveness(); + let all_liveness = liveness_info_snapshot(); if pc >= 0 && jitcode.can_decode_live_vars(pc as usize, op_live) { let off = jitcode.get_live_vars_info(pc as usize, op_live); if off + 2 < all_liveness.len() { @@ -5605,6 +5642,17 @@ impl PyreJitState { Some("frame"), ); descriptor.is_recursive = true; + // The portal's frames are numbered in the CodeObject-keyed runtime + // store this crate grows, so name that decoder on the driver rather + // than leaning on the process-global slot. The global has one writer + // per store and no arbitration between them + // (`ensure_finish_setup` here, `install_state_field_fvc` in + // majit-metainterp), so a driver that leaves this `None` decodes + // against whichever store registered last — and the wrong store + // returns a mistyped count instead of failing (see the field's doc). + // jd1 names its build-time store the same way + // (`unpackiterable_driver_descriptor`). + descriptor.frame_value_count_fn = Some(frame_value_count_at); descriptor } diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 6542e14b275..081f1ab3398 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -1371,6 +1371,12 @@ pub fn install_jit_call_bridge() { majit_ir::value::default_str_eq, majit_ir::value::default_unicode_hash, ); + // Same frontend-owns-its-object-model split for the `w_class` + // header identity `OptVirtualize` folds `new_with_vtable` reads + // to: `SimpleSizeDescr` carries pyre's `ob_type` in its `vtable` + // slot but knows nothing about `PyType`, so pyre registers the + // `get_instantiate` decoder here. + majit_ir::descr::set_w_class_obj_resolver(pyre_jit_trace::descr::w_class_obj_for_vtable); register_jit_function_caller(jit_call_user_function_from_frame); register_jit_exc_raiser(jit_exc_raise_shim); // compile.py:1090 `memory_error = MemoryError()` parity — give @@ -2016,21 +2022,14 @@ pub fn blackhole_resume_via_rd_numb( if novable { None } else { Some(vinfo_dyn) }; let vrefinfo_dyn: &dyn resume::VRefInfo = driver.meta_interp().virtualref_info(); let allocator = crate::eval::PyreBlackholeAllocator; - // pyjitpl.py:2264: metainterp_sd.liveness_info — single shared pool. - // Snapshot once per call so the slice outlives ResumeDataDirectReader. - let runtime_liveness = pyre_jit_trace::state::liveness_info_snapshot(); - // A novable drain frame's `-live-` markers carry 2-byte offsets baked at - // extraction into the build-time `ALL_LIVENESS` byte stream, NOT the - // runtime-accumulated `metainterp_sd.liveness_info` that jd0 tracing grows - // via `intern_liveness`. Decoding a baked offset against the runtime buffer - // reads an unrelated jd0 frame's liveness triple (mistyping the drain's 2 - // refs as ints → `Const::getint on Ref`). Decode against the same build-time - // table the drain jitcode was resolved from. - let all_liveness: &[u8] = if novable { - pyre_jit_trace::jitcode_runtime::all_liveness() - } else { - &runtime_liveness - }; + // resume.py:1022 `self.metainterp_sd.liveness_info` — the one pool, for + // every driver. A novable drain frame's `-live-` markers carry offsets + // baked at extraction into the build-time byte stream; those bytes are the + // prefix of this buffer (`Assembler::resuming_build_time_liveness`), so a + // baked offset and a runtime-interned one address the same space and no + // per-driver pick is needed. Snapshot once per call so the slice outlives + // ResumeDataDirectReader. + let all_liveness = pyre_jit_trace::state::liveness_info_snapshot(); // Scope the &mut to chain construction; the run() loop below uses // release_bh_rd to drop and re-acquire the borrow. let bh = BH_BUILDER_RD.with(|cell| unsafe { @@ -2041,7 +2040,7 @@ pub fn blackhole_resume_via_rd_numb( &resolve_jitcode, rd_numb, rd_consts, - all_liveness, + &all_liveness, deadframe, deadframe_types, // deadframe_types: decode_ref boxes TAGBOX ints rd_virtuals_slice, // rd_virtuals diff --git a/pyre/pyre-jit/src/jit/assembler.rs b/pyre/pyre-jit/src/jit/assembler.rs index b40798db3d1..71d3f5579da 100644 --- a/pyre/pyre-jit/src/jit/assembler.rs +++ b/pyre/pyre-jit/src/jit/assembler.rs @@ -122,6 +122,53 @@ impl Assembler { Self::default() } + /// `assembler.py:19-32` `Assembler()` as `CodeWriter.__init__` builds it + /// (codewriter.py:21), for a codewriter that continues the build-time + /// drain rather than starting a fresh one. + /// + /// Upstream has one `Assembler` per program and `codewriter.py:73-86 + /// make_jitcodes` runs every pending graph through it, so `all_liveness` + /// is a single offset space. pyre assembles the extracted interpreter + /// graphs in `build.rs` and the Python-bytecode graphs at runtime; the + /// build-time offsets are baked into those jitcodes' `-live-` operands, + /// and a resume decodes them against `metainterp_sd.liveness_info` + /// (`resume.py:1022`). Seeding the buffer with the build-time bytes is + /// what keeps that one offset space: the baked offsets stay addressable + /// and `_encode_liveness` hands out positions strictly above them. + /// + /// The reader-side mirror seeds identically + /// (`pyre_jit_trace::assembler::AssemblerState::new`), so the wholesale + /// replace in `publish_state` never rewinds past this prefix. + /// `assembler.py:20 self.insns = {}` is the other half of the same + /// continuation, and it is load-bearing for the same reason. The build-time + /// jitcodes' `-live-` markers are written with the canonical opcode byte, + /// and `blackhole.py:55-61 BlackholeInterpBuilder.__init__` recovers it as + /// `asm.insns['live/']` — one dict, because upstream has one assembler. + /// A runtime `insns` that starts empty leaves + /// `MetaInterpStaticData.op_live` at its unset sentinel until some Python + /// bytecode graph happens to assemble a `-live-`, and until then + /// `JitCode::can_decode_live_vars` looks for the sentinel as a marker byte, + /// finds none, and declines every resume against a build-time jitcode. + /// That decline is silent and lossy: a jd1 drain guard that has already run + /// its `self.next(w_iterator)` cannot be re-executed, so the fetched item + /// is dropped instead of appended. + /// + /// Seeding is sound because the bytes are not allocated here — they come + /// from the fixed `wellknown_bh_insns` / `pyre_extension_insns` tables, so + /// the build-time and runtime spellings of an opname already agree on a + /// byte; the map only records which opnames exist. + pub fn resuming_build_time_liveness() -> Self { + let all_liveness = pyre_jit_trace::jitcode_runtime::all_liveness().to_vec(); + let all_liveness_length = all_liveness.len(); + let insns = pyre_jit_trace::jitcode_runtime::insns_opname_to_byte().clone(); + Self { + insns, + all_liveness, + all_liveness_length, + ..Self::default() + } + } + /// `assembler.py:29` accessor. pub fn all_liveness(&self) -> &[u8] { &self.all_liveness @@ -376,9 +423,17 @@ impl Assembler { let pos = if let Some(&cached) = self.all_liveness_positions.get(&key) { cached } else { + // The 2-byte `-live-` operand caps the buffer at 64 KiB, upstream + // included. pyre spends the low end of that budget on the + // build-time drain's bytes (`resuming_build_time_liveness`), so + // name both halves — an overflow here is "the shared pool filled + // up", not "the runtime codewriter alone ran away". assert!( self.all_liveness_length <= u16::MAX as usize, - "all_liveness offset overflow" + "all_liveness offset overflow at {} bytes ({} of them the \ + build-time drain's prefix); a `-live-` operand is 2 bytes", + self.all_liveness_length, + pyre_jit_trace::jitcode_runtime::all_liveness().len(), ); assert!( live_i.len() <= u8::MAX as usize diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index a513e35a3d5..817e2cfd1cf 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -5147,7 +5147,7 @@ impl CodeWriter { // pattern (`self.cpu`) still works. let cpu = super::cpu::Cpu::new(); Self { - assembler: RefCell::new(Assembler::new()), + assembler: RefCell::new(Assembler::resuming_build_time_liveness()), callcontrol: UnsafeCell::new(super::call::CallControl::new(cpu, Vec::new())), call_descr_stub_cache: Mutex::new(HashMap::new()), } diff --git a/pyre/pyre-jit/src/lib.rs b/pyre/pyre-jit/src/lib.rs index d5f3e9f1344..19faded9254 100644 --- a/pyre/pyre-jit/src/lib.rs +++ b/pyre/pyre-jit/src/lib.rs @@ -54,6 +54,7 @@ pub mod jit; mod trace_verify; // Re-export auto-generated trace functions from pyre-jit-trace +pub use pyre_jit_trace::jitcode_runtime::field_descr_identity_census_now; pub use pyre_jit_trace::{ trace_box_float, trace_box_int, trace_float_binop, trace_float_compare, trace_int_binop, trace_int_binop_ovf, trace_int_compare, trace_unbox_float, trace_unbox_int, diff --git a/pyre/pyre-object/src/listobject.rs b/pyre/pyre-object/src/listobject.rs index e7bf89480b6..7ced6ae8338 100644 --- a/pyre/pyre-object/src/listobject.rs +++ b/pyre/pyre-object/src/listobject.rs @@ -173,7 +173,15 @@ impl W_ListObject { /// Upstream list.append equivalent for the object strategy. /// (listobject.py:1695 `AbstractUnwrappedStrategy.append` for the /// Object case: no unwrap, just append.) - unsafe fn object_push(&mut self, value: PyObjectRef) { + /// `pub` because the blackhole calls it by address: the #171 fold descends + /// `w_list_append`, so a guard exit inside that body resumes in its jitcode + /// and re-executes this store as a `residual_call`, whose funcptr comes + /// from the `jit_fnaddr.rs` binding (pyre's stand-in for `call.py:181-183 + /// getfunctionptr(graph)`). + /// + /// # Safety + /// `self` must be an Object-strategy list. + pub unsafe fn object_push(&mut self, value: PyObjectRef) { // At capacity, route the grow through the `dont_look_inside` // boundary: it roots `value` across the (collecting) resize and // returns it relocated. The in-place store below stays outside the diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index 679bca4bda7..6bd3d4eb7f7 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -423,7 +423,12 @@ pub fn main_entry(binary_name: &'static str) { pyre_interpreter::module::signal::signalstate::block_async_signals_on_origin_thread(); std::thread::Builder::new() .stack_size(256 * 1024 * 1024) - .spawn(|| real_main(binary_name)) + .spawn(|| { + real_main(binary_name); + if std::env::var_os("PYRE_FIELD_IDENTITY_CENSUS").is_some() { + pyre_jit::field_descr_identity_census_now(); + } + }) .expect("spawn main thread") .join() .unwrap();