diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index 45863f70edd..fca2a9ece0e 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -4745,6 +4745,25 @@ fn build_known_values_set(inputargs: &[InputArg], ops: &[Op]) -> IndexSet { known } +fn build_used_vars_set(ops: &[Op]) -> IndexSet { + let mut used = IndexSet::new(); + for op in ops { + for arg in op.getarglist() { + if !arg.is_none() && !arg.is_constant() { + used.insert(arg.to_opref().raw()); + } + } + if let Some(fail_args) = op.getfailargs() { + for arg in fail_args { + if !arg.is_none() && !arg.is_constant() { + used.insert(arg.to_opref().raw()); + } + } + } + } + used +} + fn build_force_token_set(_inputargs: &[InputArg], _ops: &[Op]) -> IndexSet { // FORCE_TOKEN is a GCREF to the active JITFRAME // (`virtualizable.py:315-318`, `resoperation.py:1090`). Keep its in-frame @@ -5393,7 +5412,8 @@ fn resolve_failarg_opref( fn resolve_local_jump_arg( builder: &mut FunctionBuilder, constants: &indexmap::IndexMap, - jf_ptr: CValue, + ptr_type: cl_types::Type, + cached_jf_ptr: &mut Option, demoted_failarg_slots: &IndexMap, opref: OpRef, ) -> CValue { @@ -5401,6 +5421,7 @@ fn resolve_local_jump_arg( && !opref.is_constant() && let Some(offset) = demoted_failarg_offset(demoted_failarg_slots, opref.raw()) { + let jf_ptr = cached_pinned_reg(builder, ptr_type, cached_jf_ptr); return builder .ins() .load(cl_types::I64, MemFlags::trusted(), jf_ptr, offset); @@ -5748,7 +5769,8 @@ fn reload_ref_roots( fn sync_ref_root_var( builder: &mut FunctionBuilder, - jf_ptr: CValue, + ptr_type: cl_types::Type, + cached_jf_ptr: &mut Option, ref_root_slots: &[(u32, usize)], var_idx: u32, value: CValue, @@ -5757,11 +5779,26 @@ fn sync_ref_root_var( ) { if let Some((_, slot)) = ref_root_slots.iter().find(|(idx, _)| *idx == var_idx) { let offset = ref_root_base_ofs + (*slot as i32) * 8; + let jf_ptr = cached_pinned_reg(builder, ptr_type, cached_jf_ptr); builder.ins().store(MemFlags::new(), value, jf_ptr, offset); synced_ref_vars.insert(var_idx); } } +fn cached_pinned_reg( + builder: &mut FunctionBuilder, + ptr_type: cl_types::Type, + cached_jf_ptr: &mut Option, +) -> CValue { + if let Some(jf_ptr) = *cached_jf_ptr { + jf_ptr + } else { + let jf_ptr = builder.ins().get_pinned_reg(ptr_type); + *cached_jf_ptr = Some(jf_ptr); + jf_ptr + } +} + fn mark_ref_roots_synced(synced_ref_vars: &mut IndexSet, ref_root_slots: &[(u32, usize)]) { for &(var_idx, _) in ref_root_slots { synced_ref_vars.insert(var_idx); @@ -8753,6 +8790,7 @@ impl CraneliftBackend { let num_inputs = inputargs.len(); let known_values = build_known_values_set(inputargs, ops); + let used_vars = build_used_vars_set(ops); let type_index = OpTypeIndex::new(inputargs, ops); let (type_overrides, _op_def_positions) = build_type_overrides(ops, &type_index); let ref_root_slots = @@ -9368,6 +9406,7 @@ impl CraneliftBackend { // loader paths reach a LABEL block that re-syncs its carried roots with // no GC in between. let mut deferred_entry_root_syncs: Vec<(u32, CValue)> = Vec::new(); + let mut entry_sync_jf_ptr = None; let has_labels = !label_indices.is_empty(); for (i, val) in entry_input_vals.iter().copied().enumerate() { let slot = inputargs[i].index; @@ -9377,7 +9416,8 @@ impl CraneliftBackend { } else { sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut entry_sync_jf_ptr, &ref_root_slots, slot, val, @@ -9594,11 +9634,12 @@ impl CraneliftBackend { // the br_table — see the deferral note at the entry-block sync. The // loaders read the dense carried-value slots, which can overlap this // root region, so the sync must not precede the dispatch. - let cur_jf = builder.ins().get_pinned_reg(ptr_type); + let mut deferred_sync_jf_ptr = None; for &(slot, val) in &deferred_entry_root_syncs { sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut deferred_sync_jf_ptr, &ref_root_slots, slot, val, @@ -9620,13 +9661,14 @@ impl CraneliftBackend { let args = block_args_to(&mut builder, loop_block, &vals); builder.ins().jump(loop_block, &args); builder.switch_to_block(loop_block); + let mut loop_param_sync_jf_ptr = None; for i in 0..loop_param_count { let param = builder.block_params(loop_block)[i]; builder.def_var(var(i as u32), param); - let cur_jf = builder.ins().get_pinned_reg(ptr_type); sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut loop_param_sync_jf_ptr, &ref_root_slots, i as u32, param, @@ -9719,6 +9761,7 @@ impl CraneliftBackend { // must not reload it every iteration. Its home is seeded on // the fall-through/loader edges and read directly by guards. let mut param_idx = 0usize; + let mut label_param_sync_jf_ptr = None; for (i, arg_ref) in ops[op_idx].getarglist().iter().enumerate() { if loop_phi_keep.is_some_and(|keep| !keep[i]) { // Frame-resident: no block param or per-iteration @@ -9737,10 +9780,10 @@ impl CraneliftBackend { && !constants.contains_key(&arg_ref.to_opref().raw()) { builder.def_var(var(arg_ref.to_opref().raw()), param); - let cur_jf = builder.ins().get_pinned_reg(ptr_type); sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut label_param_sync_jf_ptr, &ref_root_slots, arg_ref.to_opref().raw(), param, @@ -9758,14 +9801,33 @@ impl CraneliftBackend { let op = &ops[op_idx]; let vi = op_var_index(op, op_idx, num_inputs) as u32; - // RPython parity: ebp is the live register at every instruction - // boundary. Refresh the cached jf_ptr CValue from - // the Cranelift Variable so the FunctionBuilder threads the - // correct value through any merge blocks introduced by the - // previous opcode (LABEL, brif, etc.). Without this, opcode - // handlers that emit IR directly using the cached locals can - // reference an SSA value defined in a non-dominating block. - jf_ptr = builder.ins().get_pinned_reg(ptr_type); + let needs_jf_ptr = matches!( + op.opcode, + OpCode::GuardNotForced + | OpCode::CallI + | OpCode::CallR + | OpCode::CallF + | OpCode::CallN + | OpCode::CallPureI + | OpCode::CallPureR + | OpCode::CallPureF + | OpCode::CallPureN + | OpCode::CallLoopinvariantI + | OpCode::CallLoopinvariantR + | OpCode::CallLoopinvariantF + | OpCode::CallLoopinvariantN + | OpCode::CallMallocNurseryHeaderless + | OpCode::CallMallocNursery + | OpCode::CallMallocNurseryVarsize + | OpCode::CallMallocNurseryVarsizeFrame + | OpCode::NewArray + | OpCode::NewArrayClear + | OpCode::Newstr + | OpCode::Newunicode + ); + if needs_jf_ptr { + jf_ptr = builder.ins().get_pinned_reg(ptr_type); + } // regalloc.py:1089-1106 get_gcmap: per-call-site gcmap // marking only alive ref root slots at this position. @@ -9991,10 +10053,10 @@ impl CraneliftBackend { let a = coerce_ty(&mut builder, a, want); builder.def_var(var(vi), a); if op.opcode == OpCode::SameAsR { - let cur_jf = builder.ins().get_pinned_reg(ptr_type); sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut None, &ref_root_slots, vi, a, @@ -11052,7 +11114,8 @@ impl CraneliftBackend { if op.result_type() == Type::Ref && !force_tokens.contains(&vi) { sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -11498,10 +11561,10 @@ impl CraneliftBackend { if op.result_type() != Type::Void { builder.def_var(var(vi), merged_result); if op.result_type() == Type::Ref && !force_tokens.contains(&vi) { - let cur_jf = builder.ins().get_pinned_reg(ptr_type); sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut None, &ref_root_slots, vi, merged_result, @@ -11568,6 +11631,7 @@ impl CraneliftBackend { if info.gcmap != 0 { emit_jitframe_write_barrier(&mut builder, ptr_type, call_conv, cur_jf); } + let call_jf = builder.ins().get_pinned_reg(ptr_type); // x86/assembler.py:2236: self._genop_call(op, arglocs, result_loc) let descr = op.getdescr().expect("call op must have a descriptor"); @@ -11586,7 +11650,7 @@ impl CraneliftBackend { call_descr, call_conv, ptr_type, - jf_ptr, + call_jf, &ref_root_slots, &defined_ref_vars, &stale_ref_vars, @@ -11647,6 +11711,7 @@ impl CraneliftBackend { if info.gcmap != 0 { emit_jitframe_write_barrier(&mut builder, ptr_type, call_conv, cur_jf); } + let call_jf = builder.ins().get_pinned_reg(ptr_type); let descr = op .getdescr() @@ -11658,14 +11723,14 @@ impl CraneliftBackend { // Spill GC roots before the call spill_ref_roots( &mut builder, - jf_ptr, + call_jf, &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); + emit_push_gcmap(&mut builder, call_jf, per_call_gcmap); // Release GIL (call the pre-hook) let _ = emit_host_call( @@ -11790,6 +11855,7 @@ impl CraneliftBackend { if let Some(descr) = op.getdescr() { if let Some(call_descr) = descr.as_call_descr() { + let call_jf = builder.ins().get_pinned_reg(ptr_type); let _ = emit_indirect_call_from_parts( &mut builder, &constants, @@ -11801,7 +11867,7 @@ impl CraneliftBackend { call_descr, call_conv, ptr_type, - jf_ptr, + call_jf, &ref_root_slots, &defined_ref_vars, &stale_ref_vars, @@ -11849,6 +11915,7 @@ impl CraneliftBackend { let mut call_result = cond; // fallback if let Some(descr) = op.getdescr() { if let Some(call_descr) = descr.as_call_descr() { + let call_jf = builder.ins().get_pinned_reg(ptr_type); if let Some(result) = emit_indirect_call_from_parts( &mut builder, &constants, @@ -11860,7 +11927,7 @@ impl CraneliftBackend { call_descr, call_conv, ptr_type, - jf_ptr, + call_jf, &ref_root_slots, &defined_ref_vars, &stale_ref_vars, @@ -12572,10 +12639,10 @@ impl CraneliftBackend { )?; builder.def_var(var(vi), result); if value_type == Type::Ref { - let cur_jf = builder.ins().get_pinned_reg(ptr_type); sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -12630,10 +12697,10 @@ impl CraneliftBackend { )?; builder.def_var(var(vi), result); if value_type == Type::Ref { - let cur_jf = builder.ins().get_pinned_reg(ptr_type); sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -13311,6 +13378,7 @@ impl CraneliftBackend { .iter() .find(|(_, block)| *block == target_block) .and_then(|(label_idx, _)| loop_phi_keep_by_label.get(label_idx)); + let mut jump_jf_ptr = None; let vals: Vec = op .getarglist() .iter() @@ -13320,7 +13388,8 @@ impl CraneliftBackend { resolve_local_jump_arg( &mut builder, &constants, - jf_ptr, + ptr_type, + &mut jump_jf_ptr, &demoted_failarg_slots, r.to_opref(), ) @@ -13476,8 +13545,10 @@ impl CraneliftBackend { // x86/assembler.py genop_force_token: mov resloc, ebp // FORCE_TOKEN returns the JitFrame pointer itself. // resoperation.py:1090: "returns the jitframe" - let cur_jf = builder.ins().get_pinned_reg(ptr_type); - builder.def_var(var(vi), cur_jf); + if used_vars.contains(&vi) { + let cur_jf = builder.ins().get_pinned_reg(ptr_type); + builder.def_var(var(vi), cur_jf); + } } // ── VirtualRef operations ── @@ -13488,10 +13559,10 @@ impl CraneliftBackend { let obj = resolve_opref(&mut builder, &constants, op.arg(0).to_opref()); builder.def_var(var(vi), obj); if op.opcode == OpCode::VirtualRefR { - let cur_jf = builder.ins().get_pinned_reg(ptr_type); sync_ref_root_var( &mut builder, - cur_jf, + ptr_type, + &mut None, &ref_root_slots, vi, obj, @@ -14108,7 +14179,8 @@ impl CraneliftBackend { builder.def_var(var(vi), result); sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -14142,7 +14214,8 @@ impl CraneliftBackend { builder.def_var(var(vi), result); sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -14195,7 +14268,8 @@ impl CraneliftBackend { builder.def_var(var(vi), result); sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -14265,7 +14339,8 @@ impl CraneliftBackend { builder.def_var(var(vi), result); sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -14291,7 +14366,8 @@ impl CraneliftBackend { builder.def_var(var(vi), result); sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut None, &ref_root_slots, vi, result, @@ -14378,7 +14454,8 @@ impl CraneliftBackend { builder.def_var(var(vi), result); sync_ref_root_var( &mut builder, - jf_ptr, + ptr_type, + &mut None, &ref_root_slots, vi, result, diff --git a/majit/majit-gc/src/rewrite.rs b/majit/majit-gc/src/rewrite.rs index aaac188f013..494423063f9 100644 --- a/majit/majit-gc/src/rewrite.rs +++ b/majit/majit-gc/src/rewrite.rs @@ -220,13 +220,12 @@ pub struct GcRewriterImpl { pub supports_load_effective_address: bool, /// llsupport/gc.py:30-34 `malloc_zero_filled` parity. /// - /// `true` when the allocator zero-fills payload bytes on - /// allocation. pyre's `Nursery` uses `alloc_zeroed` (nursery.rs:68) - /// and `reset()` memsets to zero on recycle (nursery.rs:105-110), - /// so production is always `true`. Gates `clear_gc_fields` per - /// rewrite.py:499-500; a future non-zero-fill allocator path would - /// flip this to `false` and let the existing plumbing emit - /// explicit NULL-pointer stores at flush time + /// `true` when the allocation path itself guarantees zero-filled + /// payload bytes. Production backends set this to `false` whenever a + /// real collector is installed and to `true` for the Boehm/raw-calloc + /// fallback (compiler.rs:8303, runner.rs:1704). Gates + /// `clear_gc_fields` per rewrite.py:499-500; the non-zero-fill path + /// emits explicit NULL-pointer stores at flush time /// (rewrite.py:761-766). pub malloc_zero_filled: bool, /// llsupport/gc.py:39 `self.memcpy_fn = memcpy_fn` cast to a Signed @@ -1205,24 +1204,18 @@ impl GcRewriterImpl { self.gen_initialize_vtable(obj_ref.clone(), vtable, vtable_fd_ref, st); } } - // Upstream rewrite.py:479-484 rewrites NEW_WITH_VTABLE into allocation - // plus full header initialization. Pyre's object layout carries a - // separate `w_class` Python-class pointer alongside the vtable; - // interpreter, blackhole, and deopt-materialize paths all write it, - // so compiled allocations must too or trace-time GuardValue(w_class) - // folds fail deterministically on trace-made objects. - if let Some(w_class) = descr.w_class_obj() { - if w_class != 0 { - if let Some(w_class_fd) = - descr.gc_fielddescrs().iter().find(|fd| fd.is_w_class()) - { - self.gen_initialize_w_class( - obj_ref.clone(), - w_class, - w_class_fd.as_ref(), - st, - ); - } + } + // Upstream rewrite.py:479-484 rewrites NEW_WITH_VTABLE into allocation + // plus full header initialization. Pyre's object layout carries a + // separate `w_class` Python-class pointer alongside the vtable. Honor + // that descriptor invariant for both fixed-size allocation opcodes: + // clear_gc_fields handles both, and the optimizer's force path may + // materialize either Virtual or VirtualStruct without a duplicate + // SETFIELD_GC for this header slot. + if let Some(w_class) = descr.w_class_obj() { + if w_class != 0 { + if let Some(w_class_fd) = descr.gc_fielddescrs().iter().find(|fd| fd.is_w_class()) { + self.gen_initialize_w_class(obj_ref.clone(), w_class, w_class_fd.as_ref(), st); } } } @@ -1958,12 +1951,11 @@ impl GcRewriterImpl { /// (rewrite.py:761-766) does not re-zero a slot that this explicit /// SETFIELD_GC is about to overwrite. /// - /// Under pyre's default zero-fill nursery configuration - /// (`malloc_zero_filled = true`), `clear_gc_fields` skips its - /// insertion path, so this is effectively a no-op. The body is - /// wired for parity so that a non-zero-fill allocator automatically - /// activates the delayed-zero tracking without further callsite - /// changes. + /// Under the Boehm/raw-calloc fallback (`malloc_zero_filled = true`), + /// `clear_gc_fields` skips its insertion path, so this is effectively a + /// no-op. With a real collector, production backends set the flag to + /// false (compiler.rs:8303, runner.rs:1704), activating the delayed-zero + /// tracking. fn consider_setfield_gc(&self, op: &Op, st: &mut RewriteState) { let Some(descr) = op.getdescr() else { return }; let Some(fd) = descr.as_field_descr() else { @@ -4595,6 +4587,66 @@ mod tests { ); } + #[test] + fn test_new_with_vtable_eagerly_initializes_w_class_without_trace_store() { + let mut rw = make_rewriter(); + rw.fielddescr_vtable = None; + let w_class = 0xD00D; + let ops = vec![Op::with_descr( + OpCode::NewWithVtable, + &[], + size_descr_with_w_class(48, 3, 0, Some(w_class), vec![w_class_field_descr_at(8)]), + )]; + + let (result, _constants, gcrefs) = rw.rewrite_for_gc_with_constants(&ops, &ConstMap::new()); + + assert_eq!(gcrefs, vec![GcRef(w_class as usize)]); + assert_eq!( + result + .iter() + .filter(|op| { + op.opcode == OpCode::GcStore + && op.arg(1).to_opref().inline_const_bits() == Some(8) + && op.arg(2).to_opref().inline_const_bits() != Some(0) + }) + .count(), + 1, + "allocation lowering must remain the sole w_class writer: {result:?}" + ); + } + + #[test] + fn test_new_initializes_w_class_before_clear_gc_fields() { + let mut rw = make_rewriter(); + rw.fielddescr_vtable = None; + rw.malloc_zero_filled = false; + let w_class = 0xD00D; + let ops = vec![ + Op::with_descr( + OpCode::New, + &[], + size_descr_with_w_class(48, 3, 0, Some(w_class), vec![w_class_field_descr_at(8)]), + ), + Op::new(OpCode::Jump, &[]), + ]; + + let (result, _constants, gcrefs) = rw.rewrite_for_gc_with_constants(&ops, &ConstMap::new()); + + assert_eq!(gcrefs, vec![GcRef(w_class as usize)]); + let w_class_stores: Vec<_> = result + .iter() + .filter(|op| { + op.opcode == OpCode::GcStore && op.arg(1).to_opref().inline_const_bits() == Some(8) + }) + .collect(); + assert_eq!(w_class_stores.len(), 1, "{result:?}"); + assert_ne!( + w_class_stores[0].arg(2).to_opref().inline_const_bits(), + Some(0), + "plain NEW must receive the eager class value, not delayed NULL" + ); + } + #[test] fn test_clear_gc_fields_zeros_w_class_without_init_value() { let mut rw = make_rewriter(); diff --git a/majit/majit-ir/src/descr.rs b/majit/majit-ir/src/descr.rs index 66b392210c5..666093f0ca5 100644 --- a/majit/majit-ir/src/descr.rs +++ b/majit/majit-ir/src/descr.rs @@ -4247,11 +4247,11 @@ pub fn vable_array_descr(idx: u16) -> DescrRef { /// (after `v_inst`) and `setfield_vable_` (after `v_inst, /// v_value`). /// -/// Pyre's `PyFrame._virtualizable_` declaration (see -/// `pyre-interpreter/src/pyframe.rs:406` and `interp_jit.py:25-31`) -/// has 6 static fields in fixed order: `[last_instr, pycode, -/// valuestackdepth, debugdata, lastblock, w_globals]`, so legitimate -/// `idx` values are `0..=5`. The struct stores only the per-field +/// `interp_jit.py:25-30` has 5 scalar fields in fixed order: +/// `[last_instr, pycode, valuestackdepth, debugdata, w_globals]`, so +/// legitimate `idx` values are `0..=4`. The canonical table is +/// `pyre-jit-trace/src/virtualizable_spec.rs::PYFRAME_VABLE_FIELDS`. +/// The struct stores only the per-field /// index; bytecode emission and runtime field access still go /// through the field-idx-to-offset table maintained by /// `virtualizable_spec.rs`. @@ -4277,14 +4277,16 @@ impl Descr for VableStaticFieldDescr { /// Number of `OnceLock` slots reserved for /// `vable_static_field_descr(idx)` singletons. Matches the exact -/// scalar-field count of pyre's PyFrame virtualizable -/// (`interp_jit.py:25-31`: `last_instr, pycode, valuestackdepth, -/// debugdata, lastblock, w_globals`), mirroring upstream +/// scalar-field count of PyFrame's virtualizable +/// (`interp_jit.py:25-30`: `last_instr, pycode, valuestackdepth, +/// debugdata, w_globals`), with the canonical table at +/// `pyre-jit-trace/src/virtualizable_spec.rs::PYFRAME_VABLE_FIELDS`. +/// This mirrors /// `rpython/jit/metainterp/virtualizable.py:71`'s /// `static_field_descrs = [... for name in static_fields]` which /// is sized exactly to `len(static_fields)`. Bump this when the /// PyFrame `_virtualizable_` declaration grows. -const VABLE_STATIC_FIELD_DESCR_SLOTS: usize = 6; +const VABLE_STATIC_FIELD_DESCR_SLOTS: usize = 5; /// Singleton accessor for `static_field_descrs[idx]`. /// @@ -4302,14 +4304,13 @@ pub fn vable_static_field_descr(idx: u16) -> DescrRef { OnceLock::new(), OnceLock::new(), OnceLock::new(), - OnceLock::new(), ]; let i = idx as usize; assert!( i < VABLE_STATIC_FIELD_DESCR_SLOTS, "vable_static_field_descr: idx={} exceeds VABLE_STATIC_FIELD_DESCR_SLOTS={}; \ pyre's PyFrame _virtualizable_ declares only {} static fields \ - (interp_jit.py:25-31)", + (interp_jit.py:25-30)", idx, VABLE_STATIC_FIELD_DESCR_SLOTS, VABLE_STATIC_FIELD_DESCR_SLOTS, diff --git a/majit/majit-macros/src/virtualizable/derive.rs b/majit/majit-macros/src/virtualizable/derive.rs index 3dd4f822851..11539b83938 100644 --- a/majit/majit-macros/src/virtualizable/derive.rs +++ b/majit/majit-macros/src/virtualizable/derive.rs @@ -489,7 +489,7 @@ pub fn expand_sym(input: DeriveInput) -> TokenStream { /// Flush virtualizable static fields from concrete values. /// /// `values` is `[last_instr, pycode, valuestackdepth, ...]` - /// in VirtualizableInfo declared field order (interp_jit.py:25-31). + /// in VirtualizableInfo declared field order (interp_jit.py:25-30). pub fn flush_vable_fields( &mut self, ctx: &mut majit_metainterp::TraceCtx, diff --git a/majit/majit-macros/src/virtualizable/mod.rs b/majit/majit-macros/src/virtualizable/mod.rs index 8e1f3a85495..9a1e7e5b432 100644 --- a/majit/majit-macros/src/virtualizable/mod.rs +++ b/majit/majit-macros/src/virtualizable/mod.rs @@ -40,7 +40,7 @@ pub(crate) struct VirtualizableMacroInput { /// Frame pointer field name in the state struct (e.g., `frame`). frame_field: Option, /// Vable scalar fields (= RPython `_virtualizable_` static fields, - /// `interp_jit.py:25-31`). Read/written via getfield_vable on the + /// `interp_jit.py:25-30`). Read/written via getfield_vable on the /// vable heap object; included in extract_live / jump_args. inputargs: Vec, /// Extra red inputargs that are NOT vable scalar fields (= RPython @@ -486,7 +486,7 @@ fn generate_layout_helpers( /// /// TODO: codegen-time constant equivalent to /// `len(VABLEINFO.static_field_descrs) + 1` (frame ptr + N - /// `_virtualizable_` scalars from `interp_jit.py:25-31`). RPython + /// `_virtualizable_` scalars from `interp_jit.py:25-30`). RPython /// derives the count dynamically by iterating /// `range(len(self.static_field_descrs))` (`virtualizable.py:86`); /// pyre crystallises it at proc-macro expansion time so the flat diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index 7abf2ad780e..bb9a4e9e879 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -207,9 +207,11 @@ pub struct BlackholeInterpreter { /// of `AbstractDescr` objects carrying field offsets, array item sizes, /// etc. In pyre, we store raw offsets (usize) as a simplification — /// descriptor-index argcode ('d', 2 bytes) indexes into this table. - /// RPython `blackhole.py:288` `self.descrs = builder.descrs`. - /// Descriptor table — heterogeneous like RPython AbstractDescr list. - pub descrs: Vec, + /// `blackhole.py:288` binds `builder.descrs` by reference and :102-103 + /// stores the assembler list itself, so the table is shared and never + /// copied; :154 is its only consumer and only reads. This has the same + /// lifetime shape as the sibling `cpu: Option<&'static dyn Backend>`. + pub descrs: &'static [BhDescr], /// RPython `blackhole.py:289` `self.op_catch_exception = builder.op_catch_exception`. pub op_catch_exception: u8, /// RPython `blackhole.py:290` `self.op_rvmprof_code = builder.op_rvmprof_code`. @@ -392,7 +394,8 @@ impl Default for BlackholeInterpreter { fn default() -> Self { Self { cpu: None, - descrs: Vec::new(), + // blackhole.py:280 `EMPTY_LIST_I = [] # shared`. + descrs: &[], // RPython blackhole.py:289 — copied from builder in `acquire_interp`. // Sentinel `u8::MAX` matches RPython's `insns.get('…', -1)` fallback. op_catch_exception: u8::MAX, @@ -546,7 +549,7 @@ impl BlackholeInterpreter { // Six builder-shared fields per `BlackholeInterpBuilder::acquire_interp` // (`blackhole.rs:3825-3842`). self.cpu = parent.cpu; - self.descrs = parent.descrs.clone(); + self.descrs = parent.descrs; self.op_catch_exception = parent.op_catch_exception; self.op_rvmprof_code = parent.op_rvmprof_code; self.op_live = parent.op_live; @@ -595,55 +598,6 @@ impl BlackholeInterpreter { (fnptr, jd.mainjitcode_calldescr.clone()) } - /// Resolve field descriptor offsets in this interpreter's descrs table. - /// Delegates to the same logic as BlackholeInterpBuilder::resolve_field_offsets. - pub fn resolve_field_offsets(&mut self, resolver: impl Fn(&str, &str) -> usize) { - for descr in &mut self.descrs { - if let BhDescr::Field { - offset, - name, - owner, - parent, - .. - } = descr - { - if *offset == 0 && !name.is_empty() { - *offset = resolver(owner, name); - if let Some(parent) = parent { - let full_name = if owner.is_empty() || name.contains('.') { - name.clone() - } else { - format!("{owner}.{name}") - }; - if let Some(field) = parent - .all_fielddescrs - .iter_mut() - .find(|field| field.name == full_name) - { - field.offset = *offset; - } - } - } - } - } - } - - /// Resolve JitCode fnaddr values in this interpreter's descrs table. - pub fn resolve_jitcode_fnaddrs(&mut self, resolver: impl Fn(usize) -> i64) { - for descr in &mut self.descrs { - if let BhDescr::JitCode { - jitcode_index, - fnaddr, - .. - } = descr - { - if *fnaddr == 0 { - *fnaddr = resolver(*jitcode_index); - } - } - } - } - /// `blackhole.py:1102-1132` — the four `bhimpl_recursive_call_{i,r,f,v}` /// share one prologue: resolve the driver's portal runner, then merge /// greens and reds per kind (`greens_i + reds_i`, ...) into the @@ -2016,7 +1970,7 @@ pub struct BlackholeInterpBuilder { pub op_rvmprof_code: u8, /// RPython `blackhole.py:103` `self.descrs`. /// Populated by `setup_descrs()` from the assembler's descriptor table. - pub descrs: Vec, + pub descrs: &'static [BhDescr], /// Dispatch table: opcode byte → handler fn pointer. /// RPython builds `dispatch_loop` closure via `unrolling_iterable`; /// Rust uses indirect call through this table. @@ -2052,7 +2006,8 @@ impl BlackholeInterpBuilder { op_live: u8::MAX, op_catch_exception: u8::MAX, op_rvmprof_code: u8::MAX, - descrs: Vec::new(), + // blackhole.py:280 `EMPTY_LIST_I = [] # shared`. + descrs: &[], dispatch_table: std::sync::Arc::new(Vec::new()), jitdrivers_sd: Vec::new(), } @@ -2194,7 +2149,7 @@ impl BlackholeInterpBuilder { } /// RPython `blackhole.py:102-103` `setup_descrs(descrs)`. - pub fn setup_descrs(&mut self, descrs: Vec) { + pub fn setup_descrs(&mut self, descrs: &'static [BhDescr]) { self.descrs = descrs; } @@ -2209,60 +2164,6 @@ impl BlackholeInterpBuilder { self.jitdrivers_sd = jitdrivers_sd; } - /// Resolve JitCode fnaddr values from a mapping function. - /// RPython: fnaddr is already set on JitCode objects when they're stored in descrs. - /// pyre: fnaddr is 0 at assembly time, resolved here after compilation. - /// `resolver(jitcode_index) -> fnaddr`. - pub fn resolve_jitcode_fnaddrs(&mut self, resolver: impl Fn(usize) -> i64) { - for descr in &mut self.descrs { - if let BhDescr::JitCode { - jitcode_index, - fnaddr, - .. - } = descr - { - if *fnaddr == 0 { - *fnaddr = resolver(*jitcode_index); - } - } - } - } - - /// Resolve field descriptor offsets from a mapping function. - /// RPython: FieldDescr carries actual byte offset from rtyper. - /// pyre: offset is 0 at assembly time, resolved here from runtime layout. - /// `resolver(owner, field_name) -> byte_offset`. - pub fn resolve_field_offsets(&mut self, resolver: impl Fn(&str, &str) -> usize) { - for descr in &mut self.descrs { - if let BhDescr::Field { - offset, - name, - owner, - parent, - .. - } = descr - { - if *offset == 0 && !name.is_empty() { - *offset = resolver(owner, name); - if let Some(parent) = parent { - let full_name = if owner.is_empty() || name.contains('.') { - name.clone() - } else { - format!("{owner}.{name}") - }; - if let Some(field) = parent - .all_fielddescrs - .iter_mut() - .find(|field| field.name == full_name) - { - field.offset = *offset; - } - } - } - } - } - } - /// RPython `blackhole.py:83-100` `dispatch_loop(self, code, position)`. /// /// Runs the codewriter-orthodox bytecode dispatch loop. Each iteration @@ -2407,7 +2308,7 @@ impl BlackholeInterpBuilder { // self.op_rvmprof_code = builder.op_rvmprof_code bh.cpu = self.cpu; // RPython blackhole.py:288: self.descrs = builder.descrs - bh.descrs = self.descrs.clone(); + bh.descrs = self.descrs; bh.op_catch_exception = self.op_catch_exception; bh.op_rvmprof_code = self.op_rvmprof_code; // self.op_live = builder.op_live @@ -4655,6 +4556,14 @@ mod tests { fn test_clone_context_from_mirrors_acquire_interp_fields() { let mut builder = super::build_inline_call_only_bh_builder(); let mut parent = builder.acquire_interp(); + let table: &'static [BhDescr] = Box::leak( + vec![ + BhDescr::VableField { index: 1 }, + BhDescr::VableArray { index: 2 }, + ] + .into_boxed_slice(), + ); + parent.descrs = table; // Make the parent's vable / jitdriver state non-default so // the assertion below distinguishes "copied" from // "callee-default". @@ -4671,7 +4580,10 @@ mod tests { assert_eq!(callee.op_catch_exception, parent.op_catch_exception); assert_eq!(callee.op_rvmprof_code, parent.op_rvmprof_code); assert_eq!(callee.op_live, parent.op_live); - assert_eq!(callee.descrs.len(), parent.descrs.len()); + assert!( + std::ptr::eq(callee.descrs.as_ptr(), parent.descrs.as_ptr()), + "clone_context_from must alias the parent table" + ); assert_eq!(callee.virtualizable_ptr, parent.virtualizable_ptr); assert_eq!( callee.virtualizable_stack_base, diff --git a/majit/majit-metainterp/src/optimizeopt/info.rs b/majit/majit-metainterp/src/optimizeopt/info.rs index cc927683033..096e567d475 100644 --- a/majit/majit-metainterp/src/optimizeopt/info.rs +++ b/majit/majit-metainterp/src/optimizeopt/info.rs @@ -14,6 +14,41 @@ fn lookup_field_descr(field_descrs: &[DescrRef], field_idx: u32) -> Option bool { + let Some(field) = field_descr.as_field_descr().filter(|fd| fd.is_w_class()) else { + return false; + }; + let Some(size) = size_descr.as_size_descr() else { + return false; + }; + let Some(w_class) = size.w_class_obj().filter(|&w| w != 0) else { + return false; + }; + let Some(init_field) = size.gc_fielddescrs().iter().find(|fd| fd.is_w_class()) else { + return false; + }; + if init_field.offset() != field.offset() || init_field.field_size() != field.field_size() { + return false; + } + matches!( + ctx.resolve_operand_operand_opt(value) + .and_then(|resolved| resolved.const_value()), + Some(Value::Ref(value)) if value == GcRef(w_class as usize) + ) +} + pub use majit_ir::field_entry::{FieldEntry, PreambleOp}; pub use majit_ir::op_info::{EmptyInfo, FloatConstInfo, OpInfo}; pub use majit_ir::ptr_info::reasonable_array_index; @@ -1122,6 +1157,9 @@ fn force_box_impl( let descr = descr.expect( "force_box: field_idx must resolve through descr.get_all_fielddescrs()[i]", ); + if w_class_store_is_covered_by_alloc(&vinfo.descr, &descr, &value_ref, ctx) { + continue; + } let arg_alloc = ctx.materialize_operand_at(alloc_ref); let arg_value = ctx.resolve_operand_operand(&value_ref); let mut set_op = @@ -1170,6 +1208,9 @@ fn force_box_impl( let descr = descr.expect( "force_box: field_idx must resolve through descr.get_all_fielddescrs()[i]", ); + if w_class_store_is_covered_by_alloc(&vinfo.descr, &descr, &value_ref, ctx) { + continue; + } let arg_alloc = ctx.materialize_operand_at(alloc_ref); let arg_value = ctx.resolve_operand_operand(&value_ref); let mut set_op = @@ -1625,13 +1666,138 @@ pub use majit_ir::ptr_info::{ mod tests { use super::*; use crate::optimizeopt::OptContext; - use majit_ir::{Descr, OpCode, Value}; + use majit_ir::{Descr, FieldDescr, OpCode, SizeDescr, Value}; use std::sync::Arc; #[derive(Debug)] struct TestDescr; impl Descr for TestDescr {} + #[derive(Debug)] + struct ForceFieldDescr { + offset: usize, + field_size: usize, + field_type: Type, + name: &'static str, + } + + impl Descr for ForceFieldDescr { + fn as_field_descr(&self) -> Option<&dyn FieldDescr> { + Some(self) + } + } + + impl FieldDescr for ForceFieldDescr { + fn offset(&self) -> usize { + self.offset + } + fn field_size(&self) -> usize { + self.field_size + } + fn field_type(&self) -> Type { + self.field_type + } + fn is_pointer_field(&self) -> bool { + self.field_type == Type::Ref + } + fn field_name(&self) -> &str { + self.name + } + } + + #[derive(Debug)] + struct ForceSizeDescr { + all_fields: Vec>, + gc_fields: Vec>, + w_class: i64, + } + + impl Descr for ForceSizeDescr { + fn as_size_descr(&self) -> Option<&dyn SizeDescr> { + Some(self) + } + } + + impl SizeDescr for ForceSizeDescr { + fn size(&self) -> usize { + 32 + } + fn type_id(&self) -> u32 { + 7 + } + fn is_immutable(&self) -> bool { + false + } + fn all_fielddescrs(&self) -> &[Arc] { + &self.all_fields + } + fn gc_fielddescrs(&self) -> &[Arc] { + &self.gc_fields + } + fn w_class_obj(&self) -> Option { + Some(self.w_class) + } + } + + fn force_virtual_with_w_class( + stored_w_class: usize, + stored_offset: usize, + init_offset: usize, + ) -> Vec { + let stored_field: Arc = Arc::new(ForceFieldDescr { + offset: stored_offset, + field_size: 8, + field_type: Type::Ref, + name: "PyObject.w_class", + }); + let other_field: Arc = Arc::new(ForceFieldDescr { + offset: 16, + field_size: 8, + field_type: Type::Int, + name: "Object.payload", + }); + // Deliberately use an independent descriptor for the allocation-side + // lookup: name equality alone must not establish byte identity. + let init_field: Arc = Arc::new(ForceFieldDescr { + offset: init_offset, + field_size: 8, + field_type: Type::Ref, + name: "w_class", + }); + let w_class = 0xCAFEusize; + let size_descr: DescrRef = Arc::new(ForceSizeDescr { + all_fields: vec![stored_field, other_field], + gc_fields: vec![init_field], + w_class: w_class as i64, + }); + let mut info = PtrInfo::virtual_obj(size_descr, Some(0xDEAD)); + info.setfield( + 0, + Operand::const_from_value(Value::Ref(GcRef(stored_w_class))), + ); + info.setfield(1, Operand::const_from_value(Value::Int(42))); + + let mut ctx = OptContext::new(8); + ctx.in_final_emission = true; + let virtual_box = field_op(Type::Ref, 10); + info.force_box(&virtual_box, &mut ctx); + ctx.new_operations + .iter() + .map(|op| op.as_ref().clone()) + .collect() + } + + fn setfield_offsets(ops: &[Op]) -> Vec { + ops.iter() + .filter(|op| op.opcode == OpCode::SetfieldGc) + .map(|op| { + op.getdescr() + .and_then(|descr| descr.as_field_descr().map(|fd| fd.offset())) + .expect("SETFIELD_GC must carry a field descriptor") + }) + .collect() + } + /// Bound-producer `Operand` at position `int_op(pos)` / `ref_op(pos)`, /// the field-value analog of the old `from_opref` test stand-ins. fn field_op(tp: Type, pos: u32) -> Operand { @@ -1671,6 +1837,30 @@ mod tests { assert!(virtual_struct.is_virtual()); } + #[test] + fn test_force_box_elides_alloc_covered_w_class_store() { + let ops = force_virtual_with_w_class(0xCAFE, 8, 8); + + assert_eq!(ops[0].opcode, OpCode::NewWithVtable); + assert_eq!(setfield_offsets(&ops), vec![16]); + } + + #[test] + fn test_force_box_keeps_reassigned_w_class_store() { + let ops = force_virtual_with_w_class(0xBEEF, 8, 8); + + assert_eq!(ops[0].opcode, OpCode::NewWithVtable); + assert_eq!(setfield_offsets(&ops), vec![8, 16]); + } + + #[test] + fn test_force_box_keeps_w_class_store_at_different_offset() { + let ops = force_virtual_with_w_class(0xCAFE, 24, 8); + + assert_eq!(ops[0].opcode, OpCode::NewWithVtable); + assert_eq!(setfield_offsets(&ops), vec![24, 16]); + } + #[test] fn test_const_ptr_info_getlenbound_returns_none_at_base() { // The base `PtrInfo::getlenbound` returns None for `PtrInfo::Constant` diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 64c785c7684..4e306b85613 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -4431,8 +4431,8 @@ impl MetaInterp { return input_types; }; // `extract_live_values()` still emits the expanded - // `[frame, last_instr, pycode, valuestackdepth, debugdata, lastblock, - // w_globals, locals..., stack...]` shape, so the trace's inputarg + // `[frame, last_instr, pycode, valuestackdepth, debugdata, w_globals, + // locals..., stack...]` shape, so the trace's inputarg // types do NOT carry the reds in the leading `num_reds` slots — // truncating to `num_reds` here would register a bogus // `[Ref(frame), Int(last_instr)]` ABI when reds is `[frame, ec]`. diff --git a/majit/majit-translate/src/codewriter/assembler.rs b/majit/majit-translate/src/codewriter/assembler.rs index 941bc092919..a54cfb0748d 100644 --- a/majit/majit-translate/src/codewriter/assembler.rs +++ b/majit/majit-translate/src/codewriter/assembler.rs @@ -3335,6 +3335,63 @@ fn bh_field_name(owner: &str, field_name: &str) -> String { } } +enum BhFieldLookup { + Parent(crate::jitcode::BhFieldSpec), + Layout(crate::codewriter::call::StructFieldLayout), + Missing, +} + +fn bh_field_lookup(cc: &CallControl, field: &crate::model::FieldDescriptor) -> BhFieldLookup { + let Some(owner) = field.owner_root.as_deref() else { + return BhFieldLookup::Missing; + }; + if let Some(parent_spec) = bh_size_spec_from_callcontrol(cc, owner) { + let full_name = bh_field_name(owner, &field.name); + // `all_fielddescrs` is flattened depth-first, so a nested + // `Outer.inner.x` precedes the `Outer.x` the caller named. A + // single `find` over the disjunction would let that nested row + // win on the dotted-suffix arm alone; run the exact spellings to + // exhaustion first and keep the suffix match as the fallback. + let dotted_suffix = format!(".{}", field.name); + let matched = parent_spec + .all_fielddescrs + .iter() + .find(|spec| spec.name == full_name || spec.field_key() == field.name) + .or_else(|| { + parent_spec + .all_fielddescrs + .iter() + .find(|spec| spec.name.ends_with(&dotted_suffix)) + }); + if let Some(spec) = matched { + return BhFieldLookup::Parent(spec.clone()); + } + } + cc.struct_layout_for(owner) + .and_then(|layout| layout.fields.iter().find(|row| row.name == field.name)) + .cloned() + .map(BhFieldLookup::Layout) + .unwrap_or(BhFieldLookup::Missing) +} + +/// Return the byte offset of a by-value substructure that has no flattened +/// field descriptor of its own. +/// +/// This deliberately shares `fielddescrof`'s lookup: an exact parent +/// `all_fielddescrs` match wins, and only a layout-row fallback carrying the +/// struct flag denotes the `getsubstruct` shape. +pub(crate) fn inline_substruct_field_offset( + cc: &crate::codewriter::call::CallControl, + field: &crate::model::FieldDescriptor, +) -> Option { + match bh_field_lookup(cc, field) { + BhFieldLookup::Layout(row) if row.flag == majit_ir::descr::ArrayFlag::Struct => { + Some(row.offset) + } + BhFieldLookup::Parent(_) | BhFieldLookup::Layout(_) | BhFieldLookup::Missing => None, + } +} + fn bh_field_spec_from_parts( index: u32, owner: &str, @@ -3784,26 +3841,10 @@ fn fielddescrof( { parent_spec.type_id = owner_id.as_u64(); } - let mut found_parent_field = false; - if let Some(parent_spec) = parent.as_ref() { - let full_name = bh_field_name(owner, &field.name); - // `all_fielddescrs` is flattened depth-first, so a nested - // `Outer.inner.x` precedes the `Outer.x` the caller named. A - // single `find` over the disjunction would let that nested row - // win on the dotted-suffix arm alone; run the exact spellings to - // exhaustion first and keep the suffix match as the fallback. - let dotted_suffix = format!(".{}", field.name); - let matched = parent_spec - .all_fielddescrs - .iter() - .find(|spec| spec.name == full_name || spec.field_key() == field.name) - .or_else(|| { - parent_spec - .all_fielddescrs - .iter() - .find(|spec| spec.name.ends_with(&dotted_suffix)) - }); - if let Some(spec) = matched { + let field_lookup = bh_field_lookup(cc, field); + let found_parent_field = matches!(field_lookup, BhFieldLookup::Parent(_)); + match field_lookup { + BhFieldLookup::Parent(spec) => { offset = spec.offset; field_size = spec.field_size; field_type = spec.field_type; @@ -3812,35 +3853,32 @@ fn fielddescrof( is_immutable = spec.is_immutable; is_quasi_immutable = spec.is_quasi_immutable; index_in_parent = spec.index_in_parent; - found_parent_field = true; } - } - if !found_parent_field - && let Some(layout_field) = cc - .struct_layout_for(owner) - .and_then(|layout| layout.fields.iter().find(|fl| fl.name == field.name)) - { - offset = layout_field.offset; - field_size = layout_field.size; - field_type = layout_field.field_type; - field_flag = layout_field.flag; - is_field_signed = field_flag == majit_ir::descr::ArrayFlag::Signed; - is_immutable = layout_field.is_immutable(); - is_quasi_immutable = layout_field.is_quasi_immutable(); - } else if !found_parent_field - && let Some(( - computed_offset, - computed_size, - computed_type, - computed_flag, - computed_signed, - )) = heuristic_field_layout(cc, owner, &field.name) - { - offset = computed_offset; - field_size = computed_size; - field_type = computed_type; - field_flag = computed_flag; - is_field_signed = computed_signed; + BhFieldLookup::Layout(layout_field) => { + offset = layout_field.offset; + field_size = layout_field.size; + field_type = layout_field.field_type; + field_flag = layout_field.flag; + is_field_signed = field_flag == majit_ir::descr::ArrayFlag::Signed; + is_immutable = layout_field.is_immutable(); + is_quasi_immutable = layout_field.is_quasi_immutable(); + } + BhFieldLookup::Missing => { + if let Some(( + computed_offset, + computed_size, + computed_type, + computed_flag, + computed_signed, + )) = heuristic_field_layout(cc, owner, &field.name) + { + offset = computed_offset; + field_size = computed_size; + field_type = computed_type; + field_flag = computed_flag; + is_field_signed = computed_signed; + } + } } if let Some(rank) = cc.field_immutability(Some(owner), &field_key) { diff --git a/majit/majit-translate/src/codewriter/jtransform.rs b/majit/majit-translate/src/codewriter/jtransform.rs index a94afa84c0c..b3ff55f9bad 100644 --- a/majit/majit-translate/src/codewriter/jtransform.rs +++ b/majit/majit-translate/src/codewriter/jtransform.rs @@ -268,6 +268,9 @@ pub struct Transformer<'a> { /// RPython: DependencyTracker — caches transitive analysis results. /// Shared across all getcalldescr() calls within this transform pass. analysis_cache: crate::call::AnalysisCache, + /// Results consumed as explicit arguments by a direct or indirect call in + /// the graph before call lowering rewrites those operations. + call_argument_vars: Vec, } /// RPython: jtransform.py vable_flags values @@ -625,6 +628,7 @@ impl<'a> Transformer<'a> { vable_rewrites: 0, calls_classified: 0, analysis_cache: crate::call::AnalysisCache::default(), + call_argument_vars: Vec::new(), } } @@ -687,6 +691,18 @@ impl<'a> Transformer<'a> { // `SpecTag` out of the annotator. fold_we_are_jitted_calls(&mut rewritten); + self.call_argument_vars = rewritten + .blocks + .iter() + .flat_map(|block| &block.operations) + .filter_map(|op| match &op.kind { + OpKind::Call { args, .. } | OpKind::IndirectCall { args, .. } => Some(args), + _ => None, + }) + .flatten() + .cloned() + .collect(); + let exceptblock = rewritten.exceptblock; let graph_name = rewritten.name.clone(); for block_idx in 0..rewritten.blocks.len() { @@ -2512,6 +2528,38 @@ impl<'a> Transformer<'a> { ty: &ValueType, graph_name: &str, ) -> RewriteResult { + let inline_substruct_offset = self + .callcontrol + .as_deref() + .and_then(|cc| crate::assembler::inline_substruct_field_offset(cc, field)); + if let Some(0) = inline_substruct_offset { + let OpKind::FieldRead { base, .. } = &op.kind else { + unreachable!("rewrite_op_getfield called on non-FieldRead op") + }; + return RewriteResult::Identity(base.clone()); + } + if inline_substruct_offset.is_some() + && op + .result + .as_ref() + .is_some_and(|result| self.call_argument_vars.contains(result)) + { + // `jtransform.py:945-946 rewrite_op_getsubstruct` refuses GC + // substructures during translation. Pyre must still resolve the + // portal graph, so defer the same refusal to a result-less runtime + // abort; the enclosing call then remains residual. + return RewriteResult::Replace(vec![ + SpaceOperation { + result: None, + kind: OpKind::Abort { + kind: crate::model::UnknownKind::UnsupportedExpr { + variant: crate::model::UnsupportedExprKind::RawAddr, + }, + }, + }, + op.clone(), + ]); + } let typed_ty = op .result .as_ref() diff --git a/pyre/bench/synth/type_immutable_reject.py b/pyre/bench/synth/type_immutable_reject.py index 62f8db8b7ce..b38380a3d92 100644 --- a/pyre/bench/synth/type_immutable_reject.py +++ b/pyre/bench/synth/type_immutable_reject.py @@ -8,9 +8,13 @@ # type.__setattr__ / type.__delattr__ reject mutation of a non-heap # (immutable) builtin type with TypeError before touching the type dict # (typeobject.py setdictvalue/deldictvalue heaptype guard). The raising -# STORE_ATTR / DELETE_ATTR runs every iteration, so the JIT records a -# GuardNoException after the residual store and deopts into the blackhole, -# which must resume at the loop's handler. +# STORE_ATTR / DELETE_ATTR runs every iteration, and +# `try_walker_trace_immutable_type_attr_raise` folds it: the receiver is +# pinned with a GuardValue and the TypeError is emitted as an inline +# NewWithVtable + SetfieldGc construction routed through SubRaise, so both +# the raise and its catch are paid inside the compiled loop. The recorded +# baselines beside this file read loops_compiled=1, guard_failures=1, +# bridges_compiled=0 — one bailout for the whole run, not one per iteration. def main(): acc = 0 i = 0 diff --git a/pyre/extra_tests/parity_tests/surrogate_method_cache_bases.py b/pyre/extra_tests/parity_tests/surrogate_method_cache_bases.py new file mode 100644 index 00000000000..e3023e8a66e --- /dev/null +++ b/pyre/extra_tests/parity_tests/surrogate_method_cache_bases.py @@ -0,0 +1,27 @@ +"""A hot surrogate-name lookup follows a reassigned ``__bases__`` MRO.""" + +N = 3000 +NAME = "\udc81cache_bases" + + +class Base: + pass + + +class Other: + pass + + +class Sub(Base): + pass + + +setattr(Base, NAME, "base") +setattr(Other, NAME, "other") +for _ in range(N): + assert getattr(Sub, NAME) == "base" + +Sub.__bases__ = (Other,) +assert getattr(Sub, NAME) == "other" + +print("OK") diff --git a/pyre/extra_tests/parity_tests/surrogate_method_cache_invalidation.py b/pyre/extra_tests/parity_tests/surrogate_method_cache_invalidation.py new file mode 100644 index 00000000000..6385f0d6cdf --- /dev/null +++ b/pyre/extra_tests/parity_tests/surrogate_method_cache_invalidation.py @@ -0,0 +1,30 @@ +"""A hot surrogate-name type lookup observes store and delete invalidation.""" + +N = 3000 +NAME = "\udc80cache_invalidation" + + +class Base: + pass + + +class Sub(Base): + pass + + +setattr(Base, NAME, 1) +for _ in range(N): + assert getattr(Sub, NAME) == 1 + +setattr(Base, NAME, 2) +assert getattr(Sub, NAME) == 2 + +delattr(Base, NAME) +try: + getattr(Sub, NAME) +except AttributeError: + pass +else: + raise AssertionError("deleted surrogate attribute remained cached") + +print("OK") diff --git a/pyre/extra_tests/parity_tests/surrogate_method_cache_tag_zero.py b/pyre/extra_tests/parity_tests/surrogate_method_cache_tag_zero.py new file mode 100644 index 00000000000..fa70cac1096 --- /dev/null +++ b/pyre/extra_tests/parity_tests/surrogate_method_cache_tag_zero.py @@ -0,0 +1,28 @@ +"""A class-like non-type MRO entry keeps surrogate lookup on the tag-zero path.""" + +import sys + + +if sys.implementation.name != "cpython": + NAME = "\udc82tag_zero" + + class ClassicLike: + __bases__ = () + + classic_like = ClassicLike() + + class Meta(type): + def mro(cls): + return [cls, classic_like, object] + + class Subject(metaclass=Meta): + pass + + setattr(Subject, NAME, 41) + for _ in range(3000): + assert getattr(Subject, NAME) == 41 + + setattr(Subject, NAME, 42) + assert getattr(Subject, NAME) == 42 + +print("OK") diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 988f023cc76..0260769cf48 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -6155,7 +6155,7 @@ pub(crate) unsafe fn object_getattribute_surrogate( let w_descr = if metatype.is_null() { None } else { - lookup_in_type_wtf8(metatype, name) + lookup_in_type_where_wtf8(metatype, name) }; // typeobject.py:814-819: metatype data descriptor, bound as // `__get__(self, type(self))`. @@ -6171,7 +6171,7 @@ pub(crate) unsafe fn object_getattribute_surrogate( // Internally a null receiver distinguishes this class access from // attribute access on the actual `None` singleton; `get` converts // it back to `w_None` only for a Python-visible `__get__` call. - if let Some(w_value) = lookup_in_type_wtf8(obj, name) { + if let Some(w_value) = lookup_in_type_where_wtf8(obj, name) { if let Some(result) = get(w_value, PY_NULL, obj)? { return Ok(result); } @@ -6197,7 +6197,7 @@ pub(crate) unsafe fn object_getattribute_surrogate( let w_descr = if w_type.is_null() { None } else { - lookup_in_type_wtf8(w_type, name) + lookup_in_type_wtf8_uncached(w_type, name) }; if let Some(descr) = w_descr { if is_data_descr(descr) { @@ -6280,7 +6280,7 @@ pub(crate) unsafe fn object_setattr_surrogate( crate::typedef::r#type(obj).map_or(std::ptr::null_mut(), |p| p.as_ptr()) }; if !w_type.is_null() { - if let Some(descr) = lookup_in_type_wtf8(w_type, name) { + if let Some(descr) = lookup_in_type_wtf8_uncached(w_type, name) { if set(descr, obj, value)? { return Ok(w_none()); } @@ -6374,7 +6374,7 @@ pub(crate) unsafe fn object_delattr_surrogate( crate::typedef::r#type(obj).map_or(std::ptr::null_mut(), |p| p.as_ptr()) }; if !w_type.is_null() { - if let Some(descr) = lookup_in_type_wtf8(w_type, name) { + if let Some(descr) = lookup_in_type_wtf8_uncached(w_type, name) { if is_data_descr(descr) { delete(descr, obj)?; return Ok(w_none()); @@ -6417,6 +6417,11 @@ pub(crate) unsafe fn object_delattr_surrogate( /// the original name object in the formatted AttributeError, so build the /// exception argument as WTF-8 instead of reducing it to a Rust string. fn attr_error_wtf8(obj: PyObjectRef, name: &Wtf8) -> PyError { + // The name goes in verbatim between the quotes. 3.14 keeps the code point + // itself, so `getattr(Sub, '\udcfe')` reports `... has no attribute + // '\udcfe'` holding the lone surrogate; `descroperation.py:58` renders it + // through `%R` and reports the six-character escape text instead. Measured + // on both, 2026-08-06 — do not "restore" the repr form. let mut message = Wtf8Buf::from_string(format!( "{} has no attribute '", missing_attribute_subject(obj) @@ -9034,22 +9039,21 @@ pub(crate) unsafe fn lookup_where_pair( Some((class, value)) } -/// WTF-8 keyed MRO attribute lookup — surrogate-safe sibling of -/// `lookup_in_type` / `lookup_in_type_where`. A lone-surrogate name -/// can only live in a class namespace's `DictStorage` (never as a -/// descriptor or special slot), so this walks the MRO comparing raw -/// WTF-8 bytes via `DictStorage::get_wtf8`. +/// WTF-8 keyed `_lookup_where_all_typeobjects` MRO walk, returning the +/// defining class and descriptor in one pass (typeobject.py:491-501). A +/// lone-surrogate name can only live in a class namespace's `DictStorage` +/// (never as a descriptor or special slot), so this compares raw WTF-8 bytes +/// via `DictStorage::get_wtf8`. /// /// # Safety /// `w_type` must point at a valid `W_TypeObject` (null tolerated). -pub(crate) unsafe fn lookup_in_type_wtf8(w_type: PyObjectRef, name: &Wtf8) -> Option { +pub(crate) unsafe fn lookup_where_wtf8( + w_type: PyObjectRef, + name: &Wtf8, +) -> Option<(PyObjectRef, PyObjectRef)> { if w_type.is_null() || !is_type(w_type) { return None; } - if let Ok(name) = name.as_str() { - return lookup_in_type_where(w_type, name); - } - // MethodCache is keyed on `&str`, so a non-UTF-8 name cannot use it. let cached = w_type_get_mro(w_type); let mro_owned; let mro: &[PyObjectRef] = if !cached.is_null() { @@ -9063,12 +9067,44 @@ pub(crate) unsafe fn lookup_in_type_wtf8(w_type: PyObjectRef, name: &Wtf8) -> Op continue; } if let Some(value) = crate::type_dict_lookup_wtf8(*cls, name) { - return Some(value); + return Some((*cls, value)); } } None } +/// One-pass WTF-8 pair walk behind a single residual boundary. As in the +/// scalar `lookup_where` residuals above, `lookup_where_wtf8` phi-merges its +/// cached MRO slice with the freshly computed opaque `Vec` borrow +/// (` ∪ _ptr`); keeping the whole pair residual contains that merge +/// without reconstructing it with a second MRO walk. Marker +/// `_jit_look_inside_ = False` (rlib/jit.py:139). +#[majit_macros::dont_look_inside] +pub(crate) unsafe fn lookup_where_pair_wtf8_uncached( + w_type: PyObjectRef, + name: &Wtf8, +) -> Option<(PyObjectRef, PyObjectRef)> { + lookup_where_wtf8(w_type, name) +} + +#[inline] +pub(crate) unsafe fn lookup_in_type_wtf8_uncached( + w_type: PyObjectRef, + name: &Wtf8, +) -> Option { + lookup_where_pair_wtf8_uncached(w_type, name).map(|(_src, value)| value) +} + +unsafe fn lookup_where_pair_wtf8( + w_type: PyObjectRef, + name: &Wtf8, +) -> Option<(PyObjectRef, PyObjectRef)> { + match name.as_str() { + Ok(s) => lookup_where_pair(w_type, s), + Err(_) => lookup_where_pair_wtf8_uncached(w_type, name), + } +} + /// `typeobject.py:76-101 MethodCache` — the per-space method-lookup /// cache. `space.fromcache(MethodCache)` returns one instance per object /// space; pyre's single space therefore owns one process-shared instance. @@ -9079,13 +9115,13 @@ pub(crate) unsafe fn lookup_in_type_wtf8(w_type: PyObjectRef, name: &Wtf8) -> Op /// negative result (`_lookup_where_all_typeobjects` returned /// `(None, None)`). /// -/// `names[h]` is the untranslated string stored by upstream +/// `names[h]` is the untranslated WTF-8 byte view stored by upstream /// (`typeobject.py:541` `cache.names[method_hash] == name`). A fill owns one -/// copy; a hit compares the incoming borrowed string without allocating or +/// copy; a hit compares the incoming borrowed name without allocating or /// entering the Python-string intern table. `None` is an empty slot. struct MethodCache { versions: Vec, - names: Vec>, + names: Vec>, lookup_where: Vec<(PyObjectRef, PyObjectRef)>, } @@ -9112,9 +9148,9 @@ static METHOD_CACHE: std::sync::LazyLock> = /// version_tag)`; the u64 is its own address-stable surrogate). /// `name_hash` only needs to be deterministic — the slot's validity is the /// exact `(version, name)` match, not the hash. Upstream's `compute_hash(name)` -/// is a content hash; use the same FNV-1a content digest as pyre's other -/// untranslated-name caches. -fn method_hash(version_tag: u64, name: &str) -> usize { +/// is a content hash; use the same FNV-1a content digest over the WTF-8 byte +/// view. +fn method_hash(version_tag: u64, name: &Wtf8) -> usize { let mut name_hash: u64 = 0xcbf2_9ce4_8422_2325; for &b in name.as_bytes() { name_hash ^= b as u64; @@ -9173,13 +9209,13 @@ pub(crate) unsafe fn w_type_version_tag(w_type: PyObjectRef) -> u64 { /// /// `name` arrives on this JIT-only projection as `w_name`, an interned /// immortal str object (`box_str_constant`), because the elidable call ABI -/// cannot pass a `&str`; the body reads the untranslated string back via -/// `w_str_get_value`. The ordinary interpreter passes its `&str` directly to -/// [`_cached_lookup_where_name`], matching upstream. The result here is a raw -/// pointer — null is the cached negative result (`None`), since the call -/// ABI cannot carry `Option`. The `is_type` / `version_tag == 0` guards -/// live in the front door, so this is only ever entered with a valid -/// promoted `version_tag`. +/// cannot pass a `&Wtf8`; the body reads the untranslated WTF-8 view back via +/// `w_str_get_wtf8`. The ordinary interpreter passes its borrowed name directly +/// to [`_cached_lookup_where_name`], matching upstream. The result here is a +/// raw pointer — null is the cached negative result (`None`), since the call +/// ABI cannot carry `Option`. The `is_type` / `version_tag == 0` guards live in +/// the front door, so this is only ever entered with a valid promoted +/// `version_tag`. #[majit_macros::elidable] pub unsafe fn _pure_lookup_where_with_method_cache( w_type: PyObjectRef, @@ -9225,7 +9261,7 @@ pub unsafe fn _pure_lookup_class_with_method_cache( /// _lookup_where_all_typeobjects`) and fills the slot with the /// `(w_class, w_value)` pair (`typeobject.py:545-549`). /// -/// `w_name` must be an exact string accepted by `w_str_get_value`; the shared +/// `w_name` must be an exact string accepted by `w_str_get_wtf8`; the shared /// cache itself is content-keyed, exactly like upstream's untranslated name /// array. unsafe fn _cached_lookup_where( @@ -9233,17 +9269,17 @@ unsafe fn _cached_lookup_where( w_name: PyObjectRef, version_tag: u64, ) -> (PyObjectRef, PyObjectRef) { - let name = pyre_object::unicodeobject::w_str_get_value(w_name); + let name = pyre_object::unicodeobject::w_str_get_wtf8(w_name); _cached_lookup_where_name(w_type, name, version_tag) } -/// Untranslated-string MethodCache probe/fill used by the ordinary +/// Untranslated-WTF-8 MethodCache probe/fill used by the ordinary /// interpreter. This is the direct Rust shape of /// `typeobject.py:_pure_lookup_where_with_method_cache(name, version_tag)`; /// the `w_name` wrapper above exists only for the JIT residual-call ABI. unsafe fn _cached_lookup_where_name( w_type: PyObjectRef, - name: &str, + name: &Wtf8, version_tag: u64, ) -> (PyObjectRef, PyObjectRef) { let h = method_hash(version_tag, name); @@ -9260,8 +9296,8 @@ unsafe fn _cached_lookup_where_name( if let Some(tup) = hit { return tup; } - let tup = - lookup_where_pair(w_type, name).unwrap_or((std::ptr::null_mut(), std::ptr::null_mut())); + let tup = lookup_where_pair_wtf8(w_type, name) + .unwrap_or((std::ptr::null_mut(), std::ptr::null_mut())); // Prebuilt-family store: the cache slot is reached only by // `walk_method_cache_gc`, skipped on clean minor collections. pyre_object::gc_roots::mark_prebuilt_roots_dirty(); @@ -9325,7 +9361,7 @@ pub(crate) unsafe fn lookup_where_with_method_cache( return lookup_where_pair(w_type, name); } if !majit_metainterp::jit::we_are_jitted() { - let (w_class, w_value) = _cached_lookup_where_name(w_type, name, version_tag); + let (w_class, w_value) = _cached_lookup_where_name(w_type, Wtf8::new(name), version_tag); return if w_value.is_null() { None } else { @@ -9357,14 +9393,16 @@ pub(crate) unsafe fn lookup_where_with_method_cache( } /// `lookup` value projection of [`lookup_where_with_method_cache`] -/// (typeobject.py:476 `lookup` = `self.lookup_where(name)[1]`). Under -/// the JIT this routes through the `@elidable` -/// `_pure_lookup_where_with_method_cache` so the trace records a -/// `CALL_PURE_R` and folds the `(version_tag, name)`-keyed lookup to a -/// constant. -pub(crate) unsafe fn lookup_in_type_where(w_type: PyObjectRef, name: &str) -> Option { +/// (typeobject.py:476 `lookup` = `self.lookup_where(name)[1]`). Under the JIT +/// this routes through the `@elidable` `_pure_lookup_where_with_method_cache` +/// as a `CALL_PURE_R`; the surrogate path's measured residual cost is recorded +/// at the call site below. +pub(crate) unsafe fn lookup_in_type_where_wtf8( + w_type: PyObjectRef, + name: &Wtf8, +) -> Option { if w_type.is_null() || !is_type(w_type) { - return lookup_in_type_where_uncached(w_type, name); + return lookup_in_type_wtf8_uncached(w_type, name); } // typeobject.py:505 — `promote(self)`. let _ = majit_metainterp::jit::promote(w_type); @@ -9372,7 +9410,7 @@ pub(crate) unsafe fn lookup_in_type_where(w_type: PyObjectRef, name: &str) -> Op let version_tag = w_type_version_tag(w_type); if version_tag == 0 { // typeobject.py:507-509 — no version tag: uncacheable. - return lookup_in_type_where_uncached(w_type, name); + return lookup_in_type_wtf8_uncached(w_type, name); } if !majit_metainterp::jit::we_are_jitted() { let v = _cached_lookup_where_name(w_type, name, version_tag).1; @@ -9380,14 +9418,23 @@ pub(crate) unsafe fn lookup_in_type_where(w_type: PyObjectRef, name: &str) -> Op } // The JIT elidable projection takes an interned, immortal str object // (`box_str_constant`: content-keyed, never freed) because its residual - // call ABI cannot pass a `&str`. The ordinary interpreter returned through + // call ABI cannot pass a `&Wtf8`. The ordinary interpreter returned through // `_cached_lookup_where_name` above without materialising this wrapper. - let w_name = pyre_object::unicodeobject::box_str_constant(rustpython_wtf8::Wtf8::new(name)); + // This does not fold away after tracing: each lookup calls + // `box_str_constant` (the process-global `STRING_INTERN_TABLE` mutex) and + // `_pure_lookup_where_with_method_cache` (the process-global + // `METHOD_CACHE` mutex) once per lookup per iteration. + let w_name = pyre_object::unicodeobject::box_str_constant(name); // typeobject.py:510 — `_pure_lookup_where_with_method_cache(name, version_tag)`. let v = _pure_lookup_where_with_method_cache(w_type, w_name, version_tag); if v.is_null() { None } else { Some(v) } } +#[inline] +pub(crate) unsafe fn lookup_in_type_where(w_type: PyObjectRef, name: &str) -> Option { + lookup_in_type_where_wtf8(w_type, Wtf8::new(name)) +} + /// `objspace.py:817 getfulltypename` — the type name used by the default /// object repr. A heaptype renders as `.` when it /// carries a string `__module__`; a builtin type is just its `name`. @@ -9711,10 +9758,10 @@ pub unsafe fn type_attr_value_fast_path( } // typeobject.py:814-823: a metatype data descriptor preempts the class's // own MRO, while a non-data metatype entry loses to the class value. - if lookup_in_type_wtf8(metatype, name).is_some_and(|descr| is_data_descr(descr)) { + if lookup_in_type_where_wtf8(metatype, name).is_some_and(|descr| is_data_descr(descr)) { return None; } - let w_value = lookup_in_type_wtf8(w_type, name)?; + let w_value = lookup_in_type_where_wtf8(w_type, name)?; // typeobject.py:822 calls `space.get(w_value, w_None, self)`. Only a // value with no descriptor protocol is returned unchanged. let value_type = crate::typedef::r#type(w_value)?.as_ptr(); @@ -10036,7 +10083,7 @@ pub unsafe fn super_lookup_binding( continue; } if is_type(t) { - if let Some(raw) = lookup_in_type_wtf8(t, name) { + if let Some(raw) = lookup_in_type_wtf8_uncached(t, name) { if is_staticmethod(raw) { return PY_NULL; } diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index df5e67614be..bf0ea554efa 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -360,6 +360,37 @@ unsafe fn w_memoryview_new_plain( } } +/// Build the `BytesIOView` returned by `W_BytesIO.getbuffer_w` +/// (`interp_bytesio.py:149-152`): its `BytesIOBuffer` reads the bytearray +/// backing, while `BytesIOView.__init__` reports the `W_BytesIO` as `.obj` +/// (`interp_bytesio.py:52-62`). +pub(crate) fn w_memoryview_new_simple_with_owner( + w_backing: PyObjectRef, + w_obj: PyObjectRef, +) -> PyObjectRef { + use pyre_object::bufferview::BufferView; + unsafe { + let _roots = pyre_object::gc_roots::push_roots(); + let sp = pyre_object::gc_roots::shadow_stack_len(); + pyre_object::gc_roots::pin_root(w_backing); + pyre_object::gc_roots::pin_root(w_obj); + let mv = pyre_object::memoryview::w_memoryview_alloc_header(false, true); + let r_backing = pyre_object::gc_roots::shadow_stack_get(sp); + let r_obj = pyre_object::gc_roots::shadow_stack_get(sp + 1); + pyre_object::bytearrayobject::w_bytearray_exports_incref(r_backing); + let length = pyre_object::bytearrayobject::w_bytearray_len(r_backing) as i64; + let backing = memoryview_backing_buffer(r_backing); + let view = BufferView::Simple { + backing, + w_obj: r_obj, + length, + }; + let view_ptr = pyre_object::memoryview::bufferview_alloc(view); + pyre_object::memoryview::w_memoryview_set_view(mv, view_ptr); + mv + } +} + /// Build the `W_MMap.readbuf_w`/`writebuf_w` view: one contiguous external /// byte window whose owner remains the mmap object. #[cfg(all(unix, not(feature = "sandbox")))] diff --git a/pyre/pyre-interpreter/src/module/_io/bytesio.rs b/pyre/pyre-interpreter/src/module/_io/bytesio.rs index 940625a0549..0138fc96e45 100644 --- a/pyre/pyre-interpreter/src/module/_io/bytesio.rs +++ b/pyre/pyre-interpreter/src/module/_io/bytesio.rs @@ -373,9 +373,12 @@ impl W_BytesIO { fn getbuffer(&mut self) -> Result { // interp_bytesio.py:149-152. The bytearray exporter owns the release - // accounting for the writable view returned here. + // accounting, while the BytesIO remains the view's reported owner. self.check_closed()?; - crate::builtins::w_memoryview_new_with_flags(self.buffer, 0x0001) + Ok(crate::builtins::w_memoryview_new_simple_with_owner( + self.buffer, + self.self_obj(), + )) } fn getvalue(&self) -> Result { diff --git a/pyre/pyre-interpreter/src/typedef.rs b/pyre/pyre-interpreter/src/typedef.rs index 53c33b2c320..b5eab29cd59 100644 --- a/pyre/pyre-interpreter/src/typedef.rs +++ b/pyre/pyre-interpreter/src/typedef.rs @@ -1543,7 +1543,16 @@ pub fn init_typeobjects() { .get() .map(|v| *v as PyObjectRef) .unwrap_or(PY_NULL); - for &w_typeobject_addr in reg.values() { + // `new_builtin_typeobject` stamps every type built once this loop has + // published `type`. The ones built before that read `PY_NULL` there + // and are filled here: the registry's own entries, plus + // `getset_descriptor`, whose factory the loop uses to build the other + // typedefs' descriptors and which therefore never enters `reg`. + for w_typeobject_addr in reg + .values() + .copied() + .chain(GETSET_DESCRIPTOR_TYPE.get().copied()) + { let w_typeobj = w_typeobject_addr as PyObjectRef; unsafe { if (*w_typeobj).w_class.is_null() { @@ -2219,6 +2228,30 @@ pub(crate) unsafe fn stamp_new_descr_self(ns: PyObjectRef, type_obj: PyObjectRef } } +/// Build a builtin type object with its metatype stamped. +/// +/// `baseobjspace.py getclass()` — a type object's class is its metatype, so +/// every builtin type object carries `w_class = type`. The `TYPEOBJECT_CACHE` +/// sweep stamps the types it registers, but the lazily built ones never enter +/// that registry: `getset_descriptor` (built inside the init loop as a +/// descriptor factory for the other typedefs), every exception class, +/// `posix.DirEntry`, and the `py_class_typed!` / `#[pyre_class]` natives. +/// Stamping at the single construction point covers all of them. +/// +/// The roots built before the `type` typeobject is published read `PY_NULL` +/// here; the sweep still fills them. A type whose metatype is not `type` +/// (`_ctypes`' metaclasses) overwrites the slot after construction. +fn new_builtin_typeobject( + name: &str, + bases: PyObjectRef, + dict_ptr: *mut u8, + layout_pytype: *const PyType, +) -> PyObjectRef { + let type_obj = w_type_new_builtin(name, bases, dict_ptr, layout_pytype); + unsafe { (*type_obj).w_class = w_type() }; + type_obj +} + /// Create the root `object` type. MRO = [object]. fn new_root_typeobject(name: &str, init: fn(PyObjectRef)) -> PyObjectRef { let _roots = pyre_object::gc_roots::push_roots(); @@ -2235,7 +2268,7 @@ fn new_root_typeobject(name: &str, init: fn(PyObjectRef)) -> PyObjectRef { unsafe { stamp_method_owners(ns, owner) }; } let ns = pyre_object::gc_roots::shadow_stack_get(ns_slot); - let type_obj = w_type_new_builtin( + let type_obj = new_builtin_typeobject( name, PY_NULL, ns as *mut u8, @@ -2307,7 +2340,7 @@ fn new_typeobject_with_base_and_layout( } let bases = w_tuple_new(vec![base]); let ns = pyre_object::gc_roots::shadow_stack_get(ns_slot); - let type_obj = w_type_new_builtin(name, bases, ns as *mut u8, layout_pytype); + let type_obj = new_builtin_typeobject(name, bases, ns as *mut u8, layout_pytype); // typeobject.py:1273-1280 setup_builtin_type: // parent_layout = w_bestbase.layout @@ -2398,7 +2431,7 @@ pub fn make_builtin_type_with_bases( init(ns); let bases_tuple = w_tuple_new(bases.to_vec()); let ns = pyre_object::gc_roots::shadow_stack_get(ns_slot); - let type_obj = w_type_new_builtin(name, bases_tuple, ns as *mut u8, layout_pytype); + let type_obj = new_builtin_typeobject(name, bases_tuple, ns as *mut u8, layout_pytype); unsafe { let parent_layout = pyre_object::w_type_get_layout_ptr(base); diff --git a/pyre/pyre-jit-trace/build.rs b/pyre/pyre-jit-trace/build.rs index 6deca48ae68..f33f269205a 100644 --- a/pyre/pyre-jit-trace/build.rs +++ b/pyre/pyre-jit-trace/build.rs @@ -154,7 +154,7 @@ const LLBC_CRATES: &[&str] = &["pyre-object", "pyre-interpreter", "pyre-jit"]; /// lock (deadlock), and a build script that downloads a toolchain breaks /// hermetic / offline / CI builds. /// -/// That ban does not cover the stamp comparison in `warn_if_llbc_stale`: +/// That ban does not cover the stamp comparison in `fail_if_llbc_stale`: /// `scripts/extract-llbc.py --fingerprint` returns before `extract` runs and /// only performs a `cargo metadata` walk plus `git ls-files`, so it starts no /// nested build and takes no target-directory lock. @@ -191,7 +191,7 @@ fn preflight_llbc_or_fail() { // Present is not the same as current: the artefacts are frozen // snapshots (AGENTS.md:48) and nothing above compares them to the // sources they were extracted from. - warn_if_llbc_stale(&repo_root); + fail_if_llbc_stale(&repo_root); return; } @@ -358,12 +358,16 @@ fn llbc_source_fingerprint( /// already skips a crate whose stamp still matches, so this is the comparison /// the producer trusts, evaluated by the consumer. /// -/// Warning-only by default: a stale artefact still yields a working build for -/// everything whose layout did not move, and the remedy is a multi-minute -/// re-extraction. `PYRE_LLBC_STRICT=1` promotes the same finding to a hard -/// failure for callers that want a gate, and -/// `PYRE_LLBC_SKIP_FINGERPRINT_CHECK` opts out entirely. -fn warn_if_llbc_stale(repo_root: &std::path::Path) { +/// A stale artefact fails the build. It was warning-only, on the argument +/// that the build still works for everything whose layout did not move; the +/// measured cost of that leniency is the opposite — a binary built over a +/// stale artefact reads the *old* layout while the sources say otherwise, so +/// every measurement taken from it describes code that is not in the tree, and +/// the warning scrolls past inside `target/*/build/*/output` where nobody +/// reads it. `PYRE_LLBC_STRICT=0` demotes it back to a warning for a +/// deliberately-stale working build, and `PYRE_LLBC_SKIP_FINGERPRINT_CHECK` +/// skips the comparison entirely. +fn fail_if_llbc_stale(repo_root: &std::path::Path) { println!("cargo::rerun-if-env-changed=PYRE_LLBC_STRICT"); println!("cargo::rerun-if-env-changed=PYRE_LLBC_SKIP_FINGERPRINT_CHECK"); if std::env::var_os("PYRE_LLBC_SKIP_FINGERPRINT_CHECK").is_some() { @@ -400,7 +404,7 @@ fn warn_if_llbc_stale(repo_root: &std::path::Path) { // The directive string is the only difference between the two modes, so it // is chosen once and the same lines go through it. `cargo::warning=` and // `cargo::error=` each carry a single line with no embedded newline. - let strict = std::env::var_os("PYRE_LLBC_STRICT").as_deref() == Some(std::ffi::OsStr::new("1")); + let strict = std::env::var_os("PYRE_LLBC_STRICT").as_deref() != Some(std::ffi::OsStr::new("0")); let directive = if strict { "cargo::error" } else { diff --git a/pyre/pyre-jit-trace/src/descr.rs b/pyre/pyre-jit-trace/src/descr.rs index 2e0c82f2024..ad11b9281e3 100644 --- a/pyre/pyre-jit-trace/src/descr.rs +++ b/pyre/pyre-jit-trace/src/descr.rs @@ -4028,12 +4028,6 @@ pub fn pyframe_debugdata_descr() -> DescrRef { field_descr_from_group(&PYFRAME_DESCR_GROUP, 5) } -/// rewrite.py:665-695 handle_call_assembler scalar field read for the -/// `lastblock` slot of the virtualizable expansion (Phase D-1 prereq). -pub fn pyframe_lastblock_descr() -> DescrRef { - field_descr_from_group(&PYFRAME_DESCR_GROUP, 6) -} - /// PyFrame.execution_context FieldDescr. /// inline PyFrame 생성 시 caller 의 ec 를 새 frame 으로 SetfieldGc 하기 위해. /// 호출 사이트는 `helpers.rs::emit_new_pyframe_inline*`. diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs index 5f96ce96a12..7acf9d0e73e 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs @@ -553,6 +553,19 @@ pub fn dispatch_via_miframe( // from, and stash the exception as the FINISH payload. The // remaining Python frames unwind interpreted, exactly as they // do when the raise surfaces from a residual call. + // + // The store-back belongs to that same shape and is what makes + // the unwind readable: this frame keeps running in the + // interpreter, which reads its locals out of + // `locals_cells_stack_w`, while the walk held them in the + // virtualizable boxes. `virtualizable.py:101-138 write_boxes` + // writes them on every force with no way to decline, and + // `record_top_level_application_traceback` above only performs + // it concretely, for the recording pass — so without the + // emitted store-back the compiled bridge leaves the frame + // holding whatever its entry wrote, and a `tb_frame.f_locals` + // or `sys._getframe()` on the way out reads every + // post-entry local as unbound. if !recording_instruction_is_bare_reraise(&mut wc, position) { record_top_level_application_traceback( &mut wc, @@ -564,6 +577,7 @@ pub fn dispatch_via_miframe( ); } fbw_publish_exit_last_instr(&mut wc, position); + fbw_force_virtualizable_before_return(&mut wc); fbw_terminate_with_raise(seed.exc, seed.exc_concrete); carrier_raise_escapes = true; position diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs index e29814867ef..162a4d1e838 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs @@ -1991,6 +1991,24 @@ pub(crate) enum CalleeReplaySafety { Clean, /// Clean apart from Python-level CALL residuals, whose callee is resolved /// only at walk time. + /// + /// This variant and the deferred arm of the nested-residual abort are ONE + /// contract, not two independent gates: the admission is sound only + /// because a residual the lever could not inline aborts BEFORE executing + /// and rewinds to the enclosing CALL (see the enforcer above, which states + /// the same promise from the other side). Retiring the abort on its own + /// leaves the admission standing on a promise nothing enforces. + /// + /// The axis is the EXECUTED-EFFECT delta, not raising — the rewind leg is + /// gated on `fbw_executed_effect_count() == entry_executed_effects`. A + /// narrowing keyed on "can this body raise" would admit a `list.append` + /// residual, which raises nothing and is exactly what the arm must catch, + /// so `EffectInfo::check_can_raise` is not the predicate for this decision. + /// + /// There is no upstream counterpart to defer against: `look_inside_graph` + /// (`codewriter/policy.py:48`) and `can_inline_callable` + /// (`warmstate.py:669`) decide statically before tracing and turn a "no" + /// into a residual call. DeferredCall, /// Carries a live-heap effect a replay would double. Dirty, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/heapcache_ops.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/heapcache_ops.rs index b6966fedad9..1380381331e 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/heapcache_ops.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/heapcache_ops.rs @@ -679,6 +679,6 @@ pub(crate) fn getfield_gc_via_heapcache( } /// `virtualizable_gen.rs` pyre PyFrame static-field order -/// `[last_instr, pycode, valuestackdepth, debugdata, lastblock, w_globals]`. +/// `[last_instr, pycode, valuestackdepth, debugdata, w_globals]`. pub(crate) const VABLE_CODE_FIELD_IDX: usize = 1; -pub(crate) const VABLE_NAMESPACE_FIELD_IDX: usize = 5; +pub(crate) const VABLE_NAMESPACE_FIELD_IDX: usize = 4; diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 756dbae4867..7888dd8b434 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -3512,14 +3512,25 @@ pub(crate) fn try_walker_inline_resolved_user_call( // `positional_defaults_for_inline` derives that from `len(defs_w)` // alone — the length is the only thing that has to be re-checked. // - // Guard the tuple's identity all the same. `GuardValue` over an - // `arraylen_gc` leaves the guard's only argument dead after it, and a - // bridge compiled at that fail index segfaults on entry: reassign - // `f.__defaults__` to a different length mid-loop and it is correct up - // to ~1000 post-flip iterations and dies past ~2000, with and without - // `MAJIT_NO_BRIDGE`. Identity is stricter than needed but sound, and - // it only costs the shape that builds the callee in the caller's own - // loop AND omits an argument it has a default for. + // Guard the tuple's identity all the same. Replacing this with + // `arraylen_gc` + a length `GuardValue` was implemented and reverted: + // it answers correctly on every defaults shape, including the + // specialised two-int tuple, but `synth/pickle_terminal_raise_resume` + // then segfaults deterministically (EXC_BAD_ACCESS on a null in + // compiled code, 5/5), while keeping the added class guard and + // restoring this identity `GuardValue` is clean 3/3. The length guard + // is what is unsound here; the class guard is not. + // + // Identity is stricter than needed and costs nothing measured. All + // three compilers fold an all-constant defaults list into one code + // constant — `codegen.py:582-590 _visit_defaults` takes the + // `_tuple_of_consts` branch, and pyre's own compiler emits the same + // single `LOAD_CONST (None, 7)` — so even a `def` re-executed inside + // the caller's loop hands out the same tuple every iteration. Only a + // non-constant default expression (`def f(a=mk())`, which emits + // `BUILD_TUPLE`) rebuilds it; no fixture in `bench/` has that shape, + // and `make_function_inline`, the one loop-local `def` with a default, + // records `guard_failures=1` for its whole run. let tuple_expected = ctx.trace_ctx.const_ref(defaults.tuple as i64); ctx.trace_ctx .record_guard(OpCode::GuardValue, &[defaults_op, tuple_expected], 0); diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 786cedadca3..93050bc28d3 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -1249,7 +1249,7 @@ fn record_fresh_application_traceback( /// Compile-time-constant frame fields of an inlined callee. #[derive(Clone, Copy)] pub struct InlineCalleeConsts { - /// `frame.w_globals` object (`VABLE_NAMESPACE_FIELD_IDX` = 5): the + /// `frame.w_globals` object (`VABLE_NAMESPACE_FIELD_IDX` = 4): the /// callee function's `__globals__` as a `PyObjectRef`. w_globals: usize, /// `frame.pycode` (`VABLE_CODE_FIELD_IDX` = 1): the callee's `W_Code` @@ -3801,8 +3801,8 @@ fn write_ref_reg( /// Write a pyre scalar virtualizable Ref field without stamping operand TOS. /// /// Pyre's scalar virtualizable fields are `last_instr(0)`, `pycode(1)`, -/// `valuestackdepth(2)`, `debugdata(3)`, `lastblock(4)`, and `w_globals(5)` -/// (`virtualizable_gen.rs`, `NUM_VABLE_SCALARS = 6`). They are frame +/// `valuestackdepth(2)`, `debugdata(3)`, and `w_globals(4)` +/// (`virtualizable_gen.rs`, `NUM_VABLE_SCALARS = 5`). They are frame /// bookkeeping; the Python operand stack lives in the separate /// `locals_cells_stack_w` array (`virtualizable_gen.rs`, /// `pyre-interpreter/src/pyframe.rs`). PyPy's `interp_jit.py` diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs index ddec868c749..20990a53d14 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs @@ -1196,8 +1196,7 @@ pub(crate) fn decline_inline_caller_frame_for_catch_marker( // The CALL is inside a try-block. Inline it when its exception handler // rejoins a loop (the exc-edge-bridgeable shape): the paused caller frame // resumes at the CALL fallthrough on the no-raise path, and on a raise the - // caller's `lastblock` (a static box in its virtualizable image) unwinds to - // the catch handler in the blackhole — bit-exact — while a hot raise bridges + // catch handler runs in the blackhole — bit-exact — while a hot raise bridges // into the enclosing loop via the carrier-boundary delivery // (`drive_bridge_carrier_walk`'s `finishframe_exception`). // diff --git a/pyre/pyre-jit-trace/src/jitcode_runtime.rs b/pyre/pyre-jit-trace/src/jitcode_runtime.rs index 8c33732ddc1..346e50b168f 100644 --- a/pyre/pyre-jit-trace/src/jitcode_runtime.rs +++ b/pyre/pyre-jit-trace/src/jitcode_runtime.rs @@ -1189,7 +1189,7 @@ pub fn build_default_bh_builder_with_unwired_report() -> ( let mut builder = majit_metainterp::blackhole::BlackholeInterpBuilder::new(); // blackhole.py:58-59 order: setup_insns, then setup_descrs. builder.setup_insns(insns_opname_to_byte()); - builder.setup_descrs(all_descrs().to_vec()); + builder.setup_descrs(all_descrs()); majit_metainterp::blackhole::wire_bhimpl_handlers(&mut builder); let unwired: Vec = builder .unwired_opnames() @@ -2059,6 +2059,22 @@ mod tests { } } + #[test] + fn build_default_bh_builder_shares_descr_table() { + let (mut builder, _) = build_default_bh_builder_with_unwired_report(); + assert!(!builder.descrs.is_empty(), "shared table must not be empty"); + assert_eq!(builder.descrs.len(), all_descrs().len()); + assert!( + std::ptr::eq(builder.descrs.as_ptr(), all_descrs().as_ptr()), + "builder must ALIAS the process table, not copy it" + ); + let bh = builder.acquire_interp(); + assert!( + std::ptr::eq(bh.descrs.as_ptr(), all_descrs().as_ptr()), + "acquire_interp must alias, not copy (blackhole.py:288)" + ); + } + /// Coverage of the *production* builder, which is not the default one. /// /// `build_pyre_production_bh_builder` delegates to diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 70ae86a5171..fc3cff4c9d1 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -1397,7 +1397,7 @@ pub fn resolve_bridge_walk_entry_at(jitcode_index: i32, carried_jitcode_pc: i32) /// /// * `scalar_oprefs` — the NUM_VABLE_SCALARS static field OpRefs in /// declaration order (last_instr, pycode, valuestackdepth, debugdata, -/// lastblock, w_globals). Excludes both the frame-identity slot and +/// w_globals). Excludes both the frame-identity slot and /// any non-vable extra reds (e.g. `ec`); virtualizable_boxes only /// carries the vable static fields plus array items. /// * `array_items` — pre-resolved OpRefs for the heap-side @@ -2256,8 +2256,8 @@ use crate::descr::{ w_float_size_descr, w_int_size_descr, }; use crate::frame_layout::{ - PYFRAME_DEBUGDATA_OFFSET, PYFRAME_LASTBLOCK_OFFSET, PYFRAME_LOCALS_CELLS_STACK_OFFSET, - PYFRAME_PYCODE_OFFSET, PYFRAME_VALUESTACKDEPTH_OFFSET, PYFRAME_W_GLOBALS_OFFSET, + PYFRAME_DEBUGDATA_OFFSET, PYFRAME_LOCALS_CELLS_STACK_OFFSET, PYFRAME_PYCODE_OFFSET, + PYFRAME_VALUESTACKDEPTH_OFFSET, PYFRAME_W_GLOBALS_OFFSET, }; use crate::helpers::emit_box_float_inline; @@ -2391,7 +2391,7 @@ pub struct PyreSym { /// by color. pub(crate) bridge_registers_r: Option>, /// Bridge-specific override for symbolic_local_types. - /// virtualizable.py:44 + interp_jit.py:25-31: locals_cells_stack_w[*] + /// virtualizable.py:44 + interp_jit.py:25-30: locals_cells_stack_w[*] /// is a W_Root array → all items are Type::Ref. setup_bridge_sym /// populates this with all-Ref; downstream unboxing happens in /// opcode handlers via guard_class + getfield_gc_pure_i/_f, not at @@ -2413,8 +2413,6 @@ pub struct PyreSym { #[vable(inputarg, type = ref)] pub(crate) vable_debugdata: OpRef, #[vable(inputarg, type = ref)] - pub(crate) vable_lastblock: OpRef, - #[vable(inputarg, type = ref)] pub(crate) vable_w_globals: OpRef, #[vable(array_base)] pub(crate) vable_array_base: Option, @@ -2468,10 +2466,10 @@ pub struct PyreSym { /// Live (interpreter-owned) virtualizable `PyFrame` behind the tracing /// snapshot, or 0 when tracing runs without one (tests). /// `concrete_vable_ptr` points at the `snapshot_for_tracing` copy whose - /// `debugdata` / `lastblock` are owned clones freed when the snapshot - /// drops; vable-statics capture (`flush_to_frame`) reads those - /// pointer-valued fields from this frame so the trace's resume data - /// never carries snapshot-owned pointers. RPython has no snapshot — + /// `debugdata` is an owned clone freed when the snapshot drops; + /// vable-statics capture (`flush_to_frame`) reads that pointer-valued + /// field from this frame so the trace's resume data never carries a + /// snapshot-owned pointer. RPython has no snapshot — /// `read_boxes` (virtualizable.py:86-93) always reads the live /// virtualizable, which is what this field restores. pub(crate) live_vable_frame_addr: usize, @@ -2838,7 +2836,6 @@ pub struct TestSymState { pub vable_pycode: OpRef, pub vable_valuestackdepth: OpRef, pub vable_debugdata: OpRef, - pub vable_lastblock: OpRef, pub vable_w_globals: OpRef, } @@ -4646,7 +4643,7 @@ fn current_quasiimmut_field_value( } } -/// virtualizable.py:44 + interp_jit.py:25-31 — +/// virtualizable.py:44 + interp_jit.py:25-30 — /// `locals_cells_stack_w[*]` is declared as a W_Root array, so every /// item's JIT type is GCREF (Type::Ref). W_IntObject/W_FloatObject are /// stored as Ref pointers; unboxing happens inside trace opcode handlers @@ -4677,12 +4674,8 @@ pub(crate) fn concrete_virtualizable_slot_type(_value: PyObjectRef) -> Type { /// depth must come from the analysis, not from the frame's entry value. /// /// All-or-nothing: returns false (frame untouched) when any live slot -/// lacks a shadow entry, when the depth analysis has no entry for the -/// merge pc, or when the walked region net-changed the frame's block -/// chain (`lastblock` — the flush writes only locals/stack/vsd/ -/// last_instr, so a block push/pop inside the walked region would leave -/// the adopted frame's chain inconsistent with its pc). The caller then -/// keeps the legacy replay-from-start behavior. +/// lacks a shadow entry or when the depth analysis has no entry for the +/// merge pc. The caller then keeps the legacy replay-from-start behavior. pub(crate) fn flush_walk_end_state_to_frame( ctx: &TraceCtx, frame: usize, @@ -4811,29 +4804,6 @@ fn flush_walk_end_state_to_frame_inner( let end_vsd = nlocals + depth as usize; let live = end_vsd.max(nlocals); let base = info.num_static_extra_boxes; - // Block-chain net-change check: the shadow's `lastblock` static box - // still holds the entry chain head iff the walk pushed/popped no - // blocks (a balanced push+pop allocates a fresh head and also - // declines — conservative). - let lastblock_static = info - .static_fields - .iter() - .position(|f| f.name == "lastblock"); - let Some(lastblock_idx) = lastblock_static else { - return decline("no lastblock static field"); - }; - let Some((_opref, shadow_lastblock)) = ctx.virtualizable_entry_at(lastblock_idx) else { - return decline("no shadow lastblock entry"); - }; - let frame_lastblock = unsafe { *(frame_ptr.add(PYFRAME_LASTBLOCK_OFFSET) as *const usize) }; - match shadow_lastblock { - Value::Ref(r) => { - if r.0 != frame_lastblock { - return decline("lastblock changed during walk"); - } - } - _ => return decline("shadow lastblock not a Ref"), - } // Validation pass first: it allocates nothing, so entry presence // cannot change under it. Commit only when every live slot resolves. let stack_override_at = |abs: usize| -> Option { @@ -5070,29 +5040,6 @@ pub(crate) fn flush_walk_end_state_at_outer_call( } let end_vsd = nlocals + call_stack.len(); let base = info.num_static_extra_boxes; - // Block-chain net-change check (identical to `flush_walk_end_state_to_frame`): - // the flush writes only locals/stack/vsd/last_instr, so a push/pop inside the - // walked region would leave the adopted frame's chain inconsistent with its - // resumed pc. - let Some(lastblock_idx) = info - .static_fields - .iter() - .position(|f| f.name == "lastblock") - else { - return false; - }; - let Some((_opref, shadow_lastblock)) = ctx.virtualizable_entry_at(lastblock_idx) else { - return false; - }; - let frame_lastblock = unsafe { *(frame_ptr.add(PYFRAME_LASTBLOCK_OFFSET) as *const usize) }; - match shadow_lastblock { - Value::Ref(r) => { - if r.0 != frame_lastblock { - return false; - } - } - _ => return false, - } // Validation pass first (allocates nothing): every LOCAL slot must resolve // in the shadow. The stack region is supplied by `call_stack`, not the // shadow, so it is not validated here. @@ -5224,21 +5171,6 @@ pub(crate) fn can_flush_walk_end_state_after_outer_call( if want_below != below.len() || below.iter().any(|slot| slot.is_null()) { return false; } - let Some(lastblock_idx) = info - .static_fields - .iter() - .position(|f| f.name == "lastblock") - else { - return false; - }; - let Some((_opref, Value::Ref(shadow_lastblock))) = ctx.virtualizable_entry_at(lastblock_idx) - else { - return false; - }; - let frame_lastblock = unsafe { *(frame_ptr.add(PYFRAME_LASTBLOCK_OFFSET) as *const usize) }; - if shadow_lastblock.0 != frame_lastblock { - return false; - } let base = info.num_static_extra_boxes; if let Some(abs) = (0..nlocals).find(|&abs| ctx.virtualizable_entry_at(base + abs).is_none()) { if crate::jitcode_dispatch::fbw_debug_abort_enabled() { @@ -5898,7 +5830,6 @@ impl PyreSym { vable_pycode: OpRef::NONE, vable_valuestackdepth: OpRef::NONE, vable_debugdata: OpRef::NONE, - vable_lastblock: OpRef::NONE, vable_w_globals: OpRef::NONE, vable_array_base: None, is_active_vable_owner: false, @@ -6021,11 +5952,11 @@ impl PyreSym { /// repopulate the shadow from resume data. `is_active_vable_owner` /// is cleared (`clear_active_vable`) because the bridge's /// inputarg layout lacks the `[frame, last_instr, pycode, - /// valuestackdepth, debugdata, lastblock, w_globals]` scalar + /// valuestackdepth, debugdata, w_globals]` scalar /// header that `init_vable_indices` assumes (see /// `virtualizable_spec.rs::PYFRAME_VABLE_FIELDS` for the - /// canonical 6-scalar layout — line-by-line PyPy parity with - /// `interp_jit.py:25-31`); the frame still owns the shadow + /// canonical 5-scalar layout from `interp_jit.py:25-30`); the + /// frame still owns the shadow /// semantically though. /// /// Callee inline frames (the retired inline-call path allocated a fresh @@ -6058,8 +5989,8 @@ impl PyreSym { /// Demote this frame from active virtualizable owner. Used at bridge /// setup (`setup_bridge_sym`) where the bridge's inputarg layout /// does not have the `[frame, last_instr, pycode, valuestackdepth, - /// debugdata, lastblock, w_globals]` scalar header that the - /// loop-portal `init_vable_indices` assumes (canonical 6-scalar + /// debugdata, w_globals]` scalar header that the loop-portal + /// `init_vable_indices` assumes (canonical 5-scalar /// layout in `virtualizable_spec.rs::PYFRAME_VABLE_FIELDS`); /// subsequent reads consult `bridge_local_oprefs` or fall through /// to the heap array via `locals_cells_stack_array_ref`. @@ -6084,7 +6015,6 @@ impl PyreSym { sym.vable_pycode = state.vable_pycode; sym.vable_valuestackdepth = state.vable_valuestackdepth; sym.vable_debugdata = state.vable_debugdata; - sym.vable_lastblock = state.vable_lastblock; sym.vable_w_globals = state.vable_w_globals; sym } @@ -6525,7 +6455,6 @@ impl PyreJitState { self.pycode_as_usize(), self.valuestackdepth(), self.debugdata_as_usize(), - self.lastblock_as_usize(), self.w_globals_as_usize(), meta.num_locals, meta.valuestackdepth, @@ -7014,16 +6943,6 @@ impl PyreJitState { let _ = self.write_frame_usize(PYFRAME_DEBUGDATA_OFFSET, value); } - /// pyframe.py:86 lastblock — read from heap frame. - pub fn lastblock_as_usize(&self) -> usize { - self.read_frame_usize(PYFRAME_LASTBLOCK_OFFSET).unwrap_or(0) - } - - /// pyframe.py:86 lastblock — write to heap frame. - pub fn set_lastblock(&mut self, value: usize) { - let _ = self.write_frame_usize(PYFRAME_LASTBLOCK_OFFSET, value); - } - /// Validate that the frame pointer is usable (fields readable, array present). fn validate_frame(&self) -> bool { self.frame_ptr().is_some() @@ -9423,7 +9342,7 @@ impl JitState for PyreJitState { sym.become_active_vable_owner(); sym.nlocals = _meta.num_locals; sym.valuestackdepth = _meta.valuestackdepth; - // virtualizable.py:44 + interp_jit.py:25-31: all locals_cells_stack_w + // virtualizable.py:44 + interp_jit.py:25-30: all locals_cells_stack_w // items are W_Root → Type::Ref. Unboxing happens inside trace opcode // handlers (guard_class + getfield_gc_pure_i/_f), not at slot setup. sym.symbolic_local_types = @@ -9577,7 +9496,7 @@ impl JitState for PyreJitState { // materialization are always invoked together. let nlocals = sym.nlocals; - // virtualizable.py:44 + interp_jit.py:25-31: locals_cells_stack_w[*] + // virtualizable.py:44 + interp_jit.py:25-30: locals_cells_stack_w[*] // items are declared Ref (W_Root array). Bridge resume slots stay // Ref at the virtualizable contract; any Int/Float unboxing must // happen inside trace opcode handlers, not at the inputarg level. @@ -9613,7 +9532,7 @@ impl JitState for PyreJitState { let mut concrete_values: Vec = Vec::with_capacity(vvals.len()); // resume.py:1264 `assert box.type == kind`: the vable payload is // NOT uniformly Ref — the static fields carry their declared - // kinds (interp_jit.py:25-31: last_instr/valuestackdepth are Int). + // kinds (interp_jit.py:25-30: last_instr/valuestackdepth are Int). // `virt_live_value_types` yields the full live layout WITH the // extra reds ([frame, , , // array...]); the vvals stream omits the extra reds, so strip them @@ -9669,7 +9588,7 @@ impl JitState for PyreJitState { .collect(); let bridge_valuestackdepth = concrete_values // virtualizable_values has no ec red: [vable, last_instr, - // pycode, valuestackdepth, debugdata, lastblock, w_globals, ...]. + // pycode, valuestackdepth, debugdata, w_globals, ...]. .get(first_vable_scalar_idx + 2) .map(value_to_usize) .unwrap_or(sym.valuestackdepth) @@ -10083,7 +10002,7 @@ impl JitState for PyreJitState { // stale parent OpRefs in registers_r after we set // bridge_local_oprefs here. // - // virtualizable.py:44 + interp_jit.py:25-31: array item types are + // virtualizable.py:44 + interp_jit.py:25-30: array item types are // all Ref; RETURN_VALUE / arithmetic paths unbox via // `trace_guarded_int_payload` (guard_class + getfield_gc_pure_i), // matching the RPython unbox-at-consumer model. The slot-level @@ -10208,7 +10127,7 @@ impl JitState for PyreJitState { // Layout mirrors virtualizable.py:86-98 read_boxes(): // boxes[0..NUM_SCALARS-1] = scalar fields 1..NUM_SCALARS // (vable_last_instr, vable_pycode, vable_valuestackdepth, - // vable_debugdata, vable_lastblock, vable_w_globals) + // vable_debugdata, vable_w_globals) // boxes[NUM_SCALARS-1..NUM_SCALARS-1+array_len] = array items // (bridge_locals followed by reserved stack slots) // boxes[-1] = vable identity (sym.frame) @@ -10252,7 +10171,6 @@ impl JitState for PyreJitState { sym.vable_pycode, sym.vable_valuestackdepth, sym.vable_debugdata, - sym.vable_lastblock, sym.vable_w_globals, ]; // virtualizable.py:139 load_list_of_boxes parity: the OpRef half of @@ -12488,7 +12406,6 @@ mod tests { sym.vable_pycode = code_ref; sym.vable_valuestackdepth = ctx.const_int(3); sym.vable_debugdata = ctx.const_ref(0); - sym.vable_lastblock = ctx.const_ref(0); sym.vable_w_globals = namespace_ref; sym.execution_context = ec_ref; sym.registers_r = vec![local0, stack0, stack1]; @@ -12517,7 +12434,6 @@ mod tests { let live_pycode = ctx.const_ref(0x4000); let live_vsd = ctx.const_int(2); let live_debugdata = ctx.const_ref(0x5000); - let live_lastblock = ctx.const_ref(0x6000); let live_globals = ctx.const_ref(0x7000); let live_local = ctx.const_ref(0x8000); let live_stack = ctx.const_ref(0x9000); @@ -12533,7 +12449,6 @@ mod tests { (live_pycode, Type::Ref), (live_vsd, Type::Int), (live_debugdata, Type::Ref), - (live_lastblock, Type::Ref), (live_globals, Type::Ref), (live_local, Type::Ref), (live_stack, Type::Ref), @@ -12551,7 +12466,6 @@ mod tests { live_pycode, live_vsd, live_debugdata, - live_lastblock, live_globals, live_local, live_stack, @@ -12586,9 +12500,8 @@ mod tests { assert_eq!(sym.vable_pycode, OpRef::input_arg_ref(3)); assert_eq!(sym.vable_valuestackdepth, OpRef::input_arg_int(4)); assert_eq!(sym.vable_debugdata, OpRef::input_arg_ref(5)); - assert_eq!(sym.vable_lastblock, OpRef::input_arg_ref(6)); - assert_eq!(sym.vable_w_globals, OpRef::input_arg_ref(7)); - assert_eq!(sym.vable_array_base, Some(8)); + assert_eq!(sym.vable_w_globals, OpRef::input_arg_ref(6)); + assert_eq!(sym.vable_array_base, Some(7)); assert_eq!(sym.symbolic_local_types.len(), 2); assert_eq!(sym.symbolic_stack_types.len(), 2); } @@ -12636,7 +12549,6 @@ mod tests { Value::Ref(GcRef(frame.pycode as usize)), // pycode Value::Int(4), // valuestackdepth Value::Ref(GcRef(0)), // debugdata - Value::Ref(GcRef(0)), // lastblock Value::Ref(GcRef(0)), // w_globals Value::Ref(GcRef(w_int_new(1) as usize)), // local a Value::Ref(GcRef(w_int_new(2) as usize)), // local b @@ -13301,7 +13213,6 @@ mod tests { Type::Ref, // pycode Type::Int, // valuestackdepth Type::Ref, // debugdata - Type::Ref, // lastblock Type::Ref, // w_globals Type::Ref, // local0 Type::Ref, // stack0 @@ -13331,7 +13242,6 @@ mod tests { code_ref as i64, 3, 0, - 0, globals, local0, stack0, @@ -13348,7 +13258,6 @@ mod tests { Type::Ref, Type::Ref, Type::Ref, - Type::Ref, ]; let resume_data = majit_metainterp::ResumeDataResult { frames: vec![RebuiltFrame { @@ -13356,9 +13265,9 @@ mod tests { pc: 0, py_pc: 0, values: vec![ + RebuiltValue::Box(7, Type::Ref), RebuiltValue::Box(8, Type::Ref), RebuiltValue::Box(9, Type::Ref), - RebuiltValue::Box(10, Type::Ref), ], }], virtualizable_values: vec![ @@ -13371,7 +13280,6 @@ mod tests { RebuiltValue::Box(7, Type::Ref), RebuiltValue::Box(8, Type::Ref), RebuiltValue::Box(9, Type::Ref), - RebuiltValue::Box(10, Type::Ref), ], virtualref_values: Vec::new(), storage: None, @@ -13397,14 +13305,14 @@ mod tests { assert_eq!( sym.registers_r, vec![ + OpRef::input_arg_ref(7), OpRef::input_arg_ref(8), - OpRef::input_arg_ref(9), - OpRef::input_arg_ref(10) + OpRef::input_arg_ref(9) ] ); assert_eq!(sym.symbolic_local_types, vec![Type::Ref]); assert_eq!(sym.symbolic_stack_types, vec![Type::Ref, Type::Ref]); - assert_eq!(sym.bridge_local_oprefs, Some(vec![OpRef::input_arg_ref(8)])); + assert_eq!(sym.bridge_local_oprefs, Some(vec![OpRef::input_arg_ref(7)])); } #[test] @@ -13417,7 +13325,6 @@ mod tests { Type::Ref, // pycode Type::Int, // valuestackdepth Type::Ref, // debugdata - Type::Ref, // lastblock Type::Ref, // w_globals Type::Ref, // local0 Type::Ref, // stack0 @@ -13427,7 +13334,7 @@ mod tests { // The vable static-field types come from `state.rs:1428-1438` // `#[vable(inputarg, type = ...)]` annotations: int/ref/int/ref/ - // ref/ref. Mint typed `OpRef::input_arg_*` variants matching + // ref. Mint typed `OpRef::input_arg_*` variants matching // those tags so variant-aware Eq (resoperation.rs:290) lines up // with what the production `init_vable_indices` produces. let mut sym = PyreSym::new_uninit(OpRef::input_arg_ref(0)); @@ -13438,15 +13345,14 @@ mod tests { sym.vable_pycode = OpRef::input_arg_ref(3); sym.vable_valuestackdepth = OpRef::input_arg_int(4); sym.vable_debugdata = OpRef::input_arg_ref(5); - sym.vable_lastblock = OpRef::input_arg_ref(6); - sym.vable_w_globals = OpRef::input_arg_ref(7); + sym.vable_w_globals = OpRef::input_arg_ref(6); // local0 / stack0 / stack1 are Ref-typed per `symbolic_local_types` // / `symbolic_stack_types` below — the macro mints the matching // `InputArgRef` variant. sym.registers_r = vec![ + OpRef::input_arg_ref(7), OpRef::input_arg_ref(8), OpRef::input_arg_ref(9), - OpRef::input_arg_ref(10), ]; sym.symbolic_local_types = vec![Type::Ref]; sym.symbolic_stack_types = vec![Type::Ref, Type::Ref]; @@ -13479,15 +13385,15 @@ mod tests { let jump_args = state.with_ctx(|this, ctx| this.close_loop_args_at(ctx, None, None)); - assert_eq!(jump_args.len(), 11); + assert_eq!(jump_args.len(), 10); assert_eq!(jump_args[0], OpRef::input_arg_ref(0)); assert_eq!(jump_args[1], OpRef::input_arg_ref(1)); assert_eq!( - &jump_args[8..], + &jump_args[7..], &[ + OpRef::input_arg_ref(7), OpRef::input_arg_ref(8), - OpRef::input_arg_ref(9), - OpRef::input_arg_ref(10) + OpRef::input_arg_ref(9) ] ); assert_eq!(state.sym().execution_context, OpRef::input_arg_ref(1)); @@ -13522,7 +13428,6 @@ mod tests { Type::Ref, // pycode Type::Int, // valuestackdepth Type::Ref, // debugdata - Type::Ref, // lastblock Type::Ref, // w_globals ]; input_types.extend(std::iter::repeat(Type::Ref).take(array_len)); @@ -13541,9 +13446,8 @@ mod tests { sym.vable_pycode = OpRef::input_arg_ref(2); sym.vable_valuestackdepth = OpRef::input_arg_int(3); sym.vable_debugdata = OpRef::input_arg_ref(4); - sym.vable_lastblock = OpRef::input_arg_ref(5); - sym.vable_w_globals = OpRef::input_arg_ref(6); - sym.registers_r = vec![OpRef::input_arg_ref(7)]; + sym.vable_w_globals = OpRef::input_arg_ref(5); + sym.registers_r = vec![OpRef::input_arg_ref(6)]; sym.symbolic_local_types = vec![Type::Ref]; sym.symbolic_stack_types = Vec::new(); sym.concrete_vable_ptr = frame_ptr as *mut u8; @@ -13567,9 +13471,8 @@ mod tests { OpRef::input_arg_int(3), OpRef::input_arg_ref(4), OpRef::input_arg_ref(5), - OpRef::input_arg_ref(6), ], - &[OpRef::input_arg_ref(7)], + &[OpRef::input_arg_ref(6)], array_len, &[], std::ptr::null(), diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index 9f8b56cdbba..10dd506abc5 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -658,8 +658,8 @@ fn try_commit_entry_carrier_call( if crate::jitcode_dispatch::fbw_debug_abort_enabled() { eprintln!( "[fbw-abort-flush] gh#467 CALL-forward declined at \ - call_py_pc={call_py_pc} (depth mismatch / unresolved local / \ - lastblock) — legacy replay kept" + call_py_pc={call_py_pc} (depth mismatch / unresolved local) — \ + legacy replay kept" ); } return None; @@ -1132,8 +1132,8 @@ pub fn trace_bytecode( } let cf_addr = &*concrete_frame as *const pyre_interpreter::pyframe::PyFrame as usize; // The snapshot stands in for concrete stepping only; vable-statics - // capture must read pointer-valued fields (`debugdata` / `lastblock`) - // from the live frame the compiled loop will run on. See the + // capture must read the pointer-valued `debugdata` field from the live + // frame the compiled loop will run on. See the // `live_vable_frame_addr` field doc (state.rs). Set before the // full-body-walk leg below so the production tracer sees it. // @@ -3847,8 +3847,8 @@ fn run_perfn_walk( // produces RPython-style reds (`jump_args = [frame, ec]`, len 2 for the // portal jitdriver), but pyre's runtime closes loops against the // EXPLICIT scalar inputarg vector - // `[frame, ec, next_instr, code, valuestackdepth, debugdata, lastblock, - // namespace, locals..., stack...]` (len >= NUM_SCALAR_INPUTARGS). + // `[frame, ec, next_instr, code, valuestackdepth, debugdata, namespace, + // locals..., stack...]` (len >= NUM_SCALAR_INPUTARGS). // `validate_close_with_jump_args` (state.rs) rejects the reds shape, so // rebuild the explicit vector via `close_loop_args_at`, matching // `reached_loop_header` (trace_opcode.rs close path). The @@ -3954,7 +3954,7 @@ fn run_perfn_walk( } else if crate::jitcode_dispatch::fbw_debug_abort_enabled() { eprintln!( "[fbw-end-flush] declined at header_pc={header_pc} (shadow slot \ - without concrete / depth / lastblock) — legacy replay kept" + without concrete / depth) — legacy replay kept" ); } } @@ -4289,7 +4289,7 @@ fn run_perfn_walk( } else if crate::jitcode_dispatch::fbw_debug_abort_enabled() { eprintln!( "[fbw-abort-flush] declined at resume_py_pc={resume_py_pc} \ - (shadow slot without concrete / depth / lastblock) — legacy replay kept" + (shadow slot without concrete / depth) — legacy replay kept" ); } } @@ -4404,7 +4404,7 @@ fn run_perfn_walk( } else if crate::jitcode_dispatch::fbw_debug_abort_enabled() { eprintln!( "[fbw-abort-flush] declined at resume_py_pc={resume_py_pc} \ - (shadow slot without concrete / depth / lastblock) — legacy replay kept" + (shadow slot without concrete / depth) — legacy replay kept" ); } } @@ -4491,7 +4491,7 @@ fn run_perfn_walk( } else if crate::jitcode_dispatch::fbw_debug_abort_enabled() { eprintln!( "[fbw-qmut-flush] declined at resume_py_pc={resume_py_pc} \ - (operand slot without concrete / depth / lastblock) — legacy replay kept" + (operand slot without concrete / depth) — legacy replay kept" ); } } else if crate::jitcode_dispatch::fbw_debug_abort_enabled() { @@ -4623,7 +4623,7 @@ fn run_perfn_walk( } else if crate::jitcode_dispatch::fbw_debug_abort_enabled() { eprintln!( "[fbw-branch-flush] declined at resume_py_pc={resume_py_pc} \ - (shadow slot without concrete / depth / lastblock) — legacy drop kept" + (shadow slot without concrete / depth) — legacy drop kept" ); } } diff --git a/pyre/pyre-jit-trace/src/trace_opcode.rs b/pyre/pyre-jit-trace/src/trace_opcode.rs index a2ffdcf3915..62329060557 100644 --- a/pyre/pyre-jit-trace/src/trace_opcode.rs +++ b/pyre/pyre-jit-trace/src/trace_opcode.rs @@ -513,9 +513,7 @@ use crate::descr::{ float_floatval_descr, int_intval_descr, list_strategy_descr, slice_w_start_descr, slice_w_step_descr, slice_w_stop_descr, w_float_size_descr, w_int_size_descr, }; -use crate::frame_layout::{ - PYFRAME_DEBUGDATA_OFFSET, PYFRAME_LASTBLOCK_OFFSET, PYFRAME_PYCODE_OFFSET, -}; +use crate::frame_layout::{PYFRAME_DEBUGDATA_OFFSET, PYFRAME_PYCODE_OFFSET}; /// pyjitpl.py:1188-1199 `_opimpl_setfield_vable` parity helper. /// @@ -544,16 +542,13 @@ use crate::frame_layout::{ /// Pyre matches that single-source model in spirit for the two /// per-opcode-advancing fields: `last_instr` / `valuestackdepth` are /// rewritten in `s.vable_*` to their pre-opcode value at `orgpc - 1` -/// before the snapshot is built. The other four scalars (`pycode`, -/// `debugdata`, `lastblock`, `w_globals`) keep whatever OpRef +/// before the snapshot is built. The other three scalars (`pycode`, +/// `debugdata`, `w_globals`) keep whatever OpRef /// `init_vable_indices` seeded at trace start because the tracer -/// never reaches their mutators under CPython 3.14 bytecode +/// never reaches their mutators under pyre's 3.14 bytecode /// (`pycode` / `w_globals`: only `pyframe.rs::frame_reinit`; -/// `debugdata`: only `getorcreate_debug_data` on debug paths; -/// `lastblock`: only `pyopcode.py:1268 -/// SETUP_FINALLY/SETUP_EXCEPT/POP_BLOCK` which CPython 3.14 no -/// longer emits — try/except/finally goes through the zero-cost -/// `co_exceptiontable` consulted only on raise). Convergence to +/// `debugdata`: only `getorcreate_debug_data` on debug paths). +/// Convergence to /// RPython's pure single-source model requires emitting /// `_opimpl_setfield_vable` for those handlers if/when they are /// re-introduced, after which the heap remains authoritative through @@ -573,8 +568,7 @@ use crate::frame_layout::{ /// /// `static_field_name` matches the canonical PyFrame virtualizable /// spec at `virtualizable_spec.rs::PYFRAME_VABLE_FIELDS` -/// (`last_instr`, `pycode`, `valuestackdepth`, `debugdata`, -/// `lastblock`, `w_globals`). +/// (`last_instr`, `pycode`, `valuestackdepth`, `debugdata`, `w_globals`). /// /// No-op when the virtualizable shadow is not seeded (non-virtualizable /// trace, or before `init_virtualizable_boxes`) and when only its OpRef @@ -1598,9 +1592,9 @@ impl MIFrame { /// Header layout matches `virtualizable_gen.rs:33-35` (frame + /// `extra_reds` + `virtualizable_spec.rs::PYFRAME_VABLE_FIELDS`): /// `[frame:Ref, ec:Ref, last_instr:Int, pycode:Ref, - /// valuestackdepth:Int, debugdata:Ref, lastblock:Ref, - /// w_globals:Ref]` — line-by-line PyPy parity with - /// `interp_jit.py:25-31` plus `interp_jit.py:67 reds = ['frame', 'ec']`. + /// valuestackdepth:Int, debugdata:Ref, w_globals:Ref]` — line-by-line + /// parity with `interp_jit.py:25-30` plus + /// `interp_jit.py:67 reds = ['frame', 'ec']`. fn build_fail_arg_types_for_active_boxes(&self, active_boxes: &[OpRef]) -> Vec { let mut types = crate::virtualizable_gen::virt_live_value_types(0); for &opref in active_boxes { @@ -1746,7 +1740,7 @@ impl MIFrame { // virtualizable.py:86-93 read_boxes reads statics from the LIVE // virtualizable. The root MIFrame's concrete frame is the // trace-stepping snapshot (`snapshot_for_tracing`), whose - // `debugdata` / `lastblock` are owned clones freed when tracing + // `debugdata` is an owned clone freed when tracing // ends — a const captured from the snapshot dangles in the // compiled trace's resume data, and the guard-failure vable // write (`write_from_resume_data_partial`) then stamps the @@ -1763,16 +1757,15 @@ impl MIFrame { frame_addr } }; - let (code_ptr, debugdata, lastblock) = if statics_addr != 0 { + let (code_ptr, debugdata) = if statics_addr != 0 { unsafe { ( *((statics_addr + PYFRAME_PYCODE_OFFSET) as *const usize), *((statics_addr + PYFRAME_DEBUGDATA_OFFSET) as *const usize), - *((statics_addr + PYFRAME_LASTBLOCK_OFFSET) as *const usize), ) } } else { - (0, 0, 0) + (0, 0) }; let ns_ptr = self.sym().concrete_namespace as i64; // Read from the concrete `PyFrame.valuestackdepth` rather than the @@ -1798,14 +1791,12 @@ impl MIFrame { let pycode_op = ctx.const_ref(code_ptr as i64); let vsd_op = ctx.const_int(vsd); let debugdata_op = self.sym().vable_debugdata; - let lastblock_op = ctx.const_ref(lastblock as i64); let w_globals_op = ctx.const_ref(ns_ptr); let owns = { let s = self.sym_mut(); s.vable_last_instr = last_instr_op; s.vable_pycode = pycode_op; s.vable_valuestackdepth = vsd_op; - s.vable_lastblock = lastblock_op; s.vable_w_globals = w_globals_op; s.owns_virtualizable_shadow() }; @@ -1829,12 +1820,6 @@ impl MIFrame { debugdata_op, Value::Ref(GcRef(debugdata)), ); - mirror_vable_static_to_boxes( - ctx, - "lastblock", - lastblock_op, - Value::Ref(GcRef(lastblock)), - ); mirror_vable_static_to_boxes( ctx, "w_globals", @@ -1867,14 +1852,13 @@ impl MIFrame { // the one at `orgpc`, so the snapshot must encode the pre-opcode // state (`last_instr = orgpc - 1`, `valuestackdepth = pre-opcode // depth via `pre_opcode_registers_r`). - // The other four scalars (`pycode`, `debugdata`, `lastblock`, - // `w_globals`) keep the inputarg OpRefs `init_vable_indices` + // The other three scalars (`pycode`, `debugdata`, `w_globals`) + // keep the inputarg OpRefs `init_vable_indices` // seeded at trace start because pyre-jit-trace never enters - // their mutators under CPython 3.14 bytecode: `pycode` / + // their mutators under pyre's 3.14 bytecode: `pycode` / // `w_globals` are set only by `pyframe.rs::frame_reinit`; - // `debugdata` only by `getorcreate_debug_data` on debug paths; - // `lastblock` only by `pyopcode.py:1268 SETUP_*/POP_BLOCK`, - // none of which CPython 3.14 emits. This matches RPython's + // `debugdata` only by `getorcreate_debug_data` on debug paths. + // This matches RPython's // "boxes carry vable inputargs" model — see // `mirror_vable_static_to_boxes` doc for the convergence path // when those handlers are re-introduced. @@ -1956,9 +1940,9 @@ impl MIFrame { /// `merge_point_shape_assert_prerequisite_2026_05_03.md`). /// /// Shape derivation matches `close_loop_args_at`: - /// `1 (frame) + extra_reds (ec) + 6 (vable scalars) + target_array_capacity` + /// `1 (frame) + extra_reds (ec) + 5 (vable scalars) + target_array_capacity` /// where the vable scalars are - /// `[next_instr, code, stack_depth, debugdata, lastblock, namespace]` + /// `[next_instr, code, stack_depth, debugdata, namespace]` /// and `target_array_capacity` is either the virtualizable array /// lengths sum (when known) or the fallback `nlocals + stack_only`. pub(crate) fn live_args_shape_at(&self, ctx: &TraceCtx) -> usize { @@ -1978,8 +1962,8 @@ impl MIFrame { .map(|lengths| lengths.iter().copied().sum::()) .filter(|&len| len >= nlocals) .unwrap_or(nlocals + stack_only); - // 1 (frame) + extra_reds + 6 (vable_scalars) + target_array_capacity - 7 + extra_reds + target_array_capacity + // 1 (frame) + extra_reds + 5 (vable_scalars) + target_array_capacity + 6 + extra_reds + target_array_capacity } /// TODO: bundles `pyjitpl.py:2957-2965` `live_arg_boxes` @@ -2074,7 +2058,7 @@ impl MIFrame { s.nlocals = concrete_nlocals; s.valuestackdepth = concrete_vsd; let stack_only = s.valuestackdepth.saturating_sub(s.nlocals); - // virtualizable.py:44 + interp_jit.py:25-31: locals_cells_stack_w[*] + // virtualizable.py:44 + interp_jit.py:25-30: locals_cells_stack_w[*] // is a W_Root array → every item is declared Ref. The loop-carried // types passed to the JUMP / merge point MUST be Ref for every // array slot; tracker-observed Int/Float types are internal to @@ -2222,7 +2206,6 @@ impl MIFrame { code, stack_depth, debugdata, - lastblock, namespace, nlocals, locals, @@ -2306,7 +2289,6 @@ impl MIFrame { ctx.virtualizable_box_at(2) .unwrap_or(s.vable_valuestackdepth), s.vable_debugdata, - s.vable_lastblock, s.vable_w_globals, nlocals, locals_vec, @@ -2318,14 +2300,7 @@ impl MIFrame { let mut args = vec![frame]; // NUM_EXTRA_REDS == 1 (crate const-assert): `reds = ['frame', 'ec']`. args.push(execution_context); - args.extend_from_slice(&[ - next_instr, - code, - stack_depth, - debugdata, - lastblock, - namespace, - ]); + args.extend_from_slice(&[next_instr, code, stack_depth, debugdata, namespace]); for (idx, value) in locals.into_iter().enumerate() { let target_type = inputarg_types .get(num_scalars + idx) @@ -2405,7 +2380,7 @@ impl MIFrame { // sharing one `duplicates` dict across both calls. In pyre's flat // layout `args = [frame, ni, code, vsd, ns, locals..., stack...]`, // that corresponds to every index 0..args.len(). Previously pyre - // skipped the 7 scalar header slots (frame + 6 static fields), + // skipped the scalar header slots (frame plus the vable statics), // which is a line-by-line divergence from RPython. // Track slots that the dedup actually mutated so we can mirror the // `put_back_list_of_boxes3` mutation below (pyjitpl.py:1578 writes @@ -2510,8 +2485,7 @@ impl MIFrame { 2 => s.vable_pycode = new_opref, 3 => s.vable_valuestackdepth = new_opref, 4 => s.vable_debugdata = new_opref, - 5 => s.vable_lastblock = new_opref, - 6 => s.vable_w_globals = new_opref, + 5 => s.vable_w_globals = new_opref, _ => {} }, } @@ -2689,8 +2663,8 @@ impl MIFrame { /// pyjitpl.py:2586 capture_resumedata: build fail_args for CURRENT /// top frame. Returns the scalar header plus active_boxes — /// `[frame, (ec)?, last_instr, pycode, valuestackdepth, debugdata, - /// lastblock, w_globals, active_boxes...]` — matching - /// `interp_jit.py:25-31 PyFrame._virtualizable_` / + /// w_globals, active_boxes...]` — matching + /// `interp_jit.py:25-30 PyFrame._virtualizable_` / /// `virtualizable_spec.rs::PYFRAME_VABLE_FIELDS` line-by-line. /// `NUM_EXTRA_REDS` controls whether the ec slot /// (interp_jit.py:67 `reds = ['frame', 'ec']`) is present between @@ -2717,7 +2691,6 @@ impl MIFrame { s.vable_pycode, s.vable_valuestackdepth, s.vable_debugdata, - s.vable_lastblock, s.vable_w_globals, ]); fa.extend_from_slice(&active_boxes); @@ -3122,15 +3095,15 @@ impl MIFrame { // opencoder.py:718-726 `_list_of_boxes_virtualizable(boxes)` // parity: read from `ctx.virtualizable_boxes` (the canonical // analog of RPython's `metainterp.virtualizable_boxes`) for - // the four invariant scalars (`pycode`, `debugdata`, - // `lastblock`, `w_globals`), and recompute the two + // the three invariant scalars (`pycode`, `debugdata`, + // `w_globals`), and recompute the two // per-opcode-advancing scalars (`last_instr`, `valuestackdepth`) // from `self.orgpc` / `pre_opcode_registers_r` so the snapshot encodes // the pre-opcode state at `resume_pc` (the PROBE-VABLE-DIV // diagnostic confirmed slot 0 / slot 2 are the // only divergence sources between the shared shadow and - // `s.vable_*` — slots 1/3/4/5 always agree because their - // mutators are unreachable under CPython 3.14 bytecode). + // `s.vable_*` — slots 1/3/4 always agree because their + // mutators are unreachable under pyre's 3.14 bytecode). // // The slot-0 inline override re-derives `resume_pc - 1` // because `flush_to_frame_for_guard` swaps `self.orgpc` to diff --git a/pyre/pyre-jit-trace/src/virtualizable_gen.rs b/pyre/pyre-jit-trace/src/virtualizable_gen.rs index 67c9762498c..0a6fe84b0b0 100644 --- a/pyre/pyre-jit-trace/src/virtualizable_gen.rs +++ b/pyre/pyre-jit-trace/src/virtualizable_gen.rs @@ -5,9 +5,9 @@ //! builder, field/array spec constants, and virtualizable hook helpers. use crate::frame_layout::{ - PYFRAME_DEBUGDATA_OFFSET, PYFRAME_LAST_INSTR_OFFSET, PYFRAME_LASTBLOCK_OFFSET, - PYFRAME_LOCALS_CELLS_STACK_OFFSET, PYFRAME_PYCODE_OFFSET, PYFRAME_VABLE_TOKEN_OFFSET, - PYFRAME_VALUESTACKDEPTH_OFFSET, PYFRAME_W_GLOBALS_OFFSET, + PYFRAME_DEBUGDATA_OFFSET, PYFRAME_LAST_INSTR_OFFSET, PYFRAME_LOCALS_CELLS_STACK_OFFSET, + PYFRAME_PYCODE_OFFSET, PYFRAME_VABLE_TOKEN_OFFSET, PYFRAME_VALUESTACKDEPTH_OFFSET, + PYFRAME_W_GLOBALS_OFFSET, }; use crate::state::PyreJitState; use pyre_object::FIXED_OBJECT_ARRAY_TOKEN; @@ -31,9 +31,9 @@ majit_macros::virtualizable! { }, // Layout: [frame:Ref, ec:Ref, last_instr:Int, pycode:Ref, - // valuestackdepth:Int, debugdata:Ref, lastblock:Ref, - // w_globals:Ref, array...] - // Mirrors `pypy/module/pypyjit/interp_jit.py:25-31`'s + // valuestackdepth:Int, debugdata:Ref, w_globals:Ref, + // array...] + // Mirrors `pypy/module/pypyjit/interp_jit.py:25-30`'s // `_virtualizable_` declaration line by line; `ec` is from // `interp_jit.py:67 reds = ['frame', 'ec']` (extra_reds above). inputargs = { @@ -41,7 +41,6 @@ majit_macros::virtualizable! { pycode: Ref, valuestackdepth: Int, debugdata: Ref, - lastblock: Ref, w_globals: Ref, }, @@ -49,13 +48,12 @@ majit_macros::virtualizable! { array_item_type = Ref, // VirtualizableInfo field layout (byte offsets). - // interp_jit.py:25-31 order — line-by-line PyPy parity. + // interp_jit.py:25-30 order — line-by-line PyPy parity. fields = { last_instr: int @ PYFRAME_LAST_INSTR_OFFSET, pycode: ref @ PYFRAME_PYCODE_OFFSET, valuestackdepth: int @ PYFRAME_VALUESTACKDEPTH_OFFSET, debugdata: ref @ PYFRAME_DEBUGDATA_OFFSET, - lastblock: ref @ PYFRAME_LASTBLOCK_OFFSET, w_globals: ref @ PYFRAME_W_GLOBALS_OFFSET, }, diff --git a/pyre/pyre-jit-trace/src/virtualizable_spec.rs b/pyre/pyre-jit-trace/src/virtualizable_spec.rs index 4bd9397889e..cbadd16ea46 100644 --- a/pyre/pyre-jit-trace/src/virtualizable_spec.rs +++ b/pyre/pyre-jit-trace/src/virtualizable_spec.rs @@ -7,33 +7,23 @@ pub const PYFRAME_VABLE_OWNER_ROOT: &str = "PyFrame"; /// Virtualizable scalar fields. /// -/// `pypy/module/pypyjit/interp_jit.py:25-30` declares -/// `['last_instr', 'pycode', 'valuestackdepth', 'locals_cells_stack_w[*]', -/// 'debugdata', 'w_globals']` — five scalars and one array. Pyre carries a -/// SIXTH scalar, `lastblock`, which upstream does not list; the ordering -/// below is otherwise upstream's, so `w_globals` sits one slot later here -/// than it would there. -/// -/// Note on `lastblock` semantics: PyPy's bytecode emits -/// `SETUP_FINALLY` / `SETUP_EXCEPT` / `POP_BLOCK` (`pyopcode.py:1268`) -/// which mutate `frame.lastblock` on the hot path and the JIT must -/// track those mutations via `_opimpl_setfield_vable`. CPython 3.14's -/// compiler emits no such opcodes — try/except/finally goes through -/// the zero-cost `co_exceptiontable` side table consulted only on -/// raise. Under pyre's 3.14 bytecode the slot is therefore JIT-scope -/// invariant, but the layout slot is preserved for line-by-line PyPy -/// parity (the legacy SETUP_*/POP_BLOCK interpreter path at -/// `pyre-interpreter/src/eval.rs:306-308` still mutates the heap -/// field, and any future port of those opcode handlers must emit -/// `setfield_vable_r` per RPython -/// `pyjitpl.py:1188 _opimpl_setfield_vable`). +/// This table is the scalar subset of +/// `pypy/module/pypyjit/interp_jit.py:25-30`'s `_virtualizable_` list, +/// in declaration order. `PyFrame.lastblock` is deliberately absent: +/// the frame model tracked by this tree has no block stack. Unwind uses +/// the `co_exceptiontable` lookup at +/// `pypy/interpreter/pyopcode.py:152 lookup_exceptiontable`, and pyre's +/// 3.14 bytecode emits no `SETUP_*` / `POP_BLOCK`, so nothing mutates +/// the field inside a trace. It remains an ordinary heap field with a +/// plain `FieldDescr` and a GC root slot. If block opcodes are ever +/// reintroduced, re-add it here and emit `_opimpl_setfield_vable` from +/// their handlers; a layout slot with no setfield is not tracking. pub const PYFRAME_VABLE_FIELDS: &[(&str, usize)] = &[ ("last_instr", 0), // interp_jit.py:25 last_instr ("pycode", 1), // interp_jit.py:25 pycode ("valuestackdepth", 2), // interp_jit.py:26 valuestackdepth ("debugdata", 3), // interp_jit.py:28 debugdata - ("lastblock", 4), // interp_jit.py:30 lastblock - ("w_globals", 5), // interp_jit.py:31 w_globals + ("w_globals", 4), // interp_jit.py:29 w_globals ]; /// Virtualizable array fields in canonical index order. diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index c239e5b31af..440cf8acd1f 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -1020,7 +1020,6 @@ pub extern "C" fn assembler_call_helper(jitframe_ptr: i64, _virtualizable_ref: i /// RPython: FieldDescr.offset is resolved at rtyper time. In pyre, Rust struct /// layout determines field offsets. This resolver maps (owner_type, field_name) /// to byte offsets for BhDescr::Field resolution in the blackhole. -/// Called by `bh.resolve_field_offsets()` after `setposition()`. fn resolve_field_offset(owner: &str, field_name: &str) -> usize { use pyre_interpreter::pyframe::PyFrame; match field_name { diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 0c34fc821c6..467ce325c56 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -11375,7 +11375,7 @@ fn build_resumed_frames( // Resolve ALL vable fields from resume data. // vable_values = [frame_ptr(0), last_instr(1), pycode(2), // valuestackdepth(3), debugdata(4), - // lastblock(5), w_globals(6), array...] + // w_globals(5), array...] // RPython reader.load_next_value_of_type reads ALL values sequentially. let resolved_vable: Vec = (0..vable_values.len()) .map(|i| { @@ -11468,13 +11468,12 @@ fn build_resumed_frames( if !vable_frame_ptr.is_null() { let f = unsafe { &*vable_frame_ptr }; eprintln!( - "[jit][resume][vable-sync] frame after write: ni={} vsd={} code={:?} ns={:?} debugdata={:?} lastblock={:?} vable_token={} array_len={}", + "[jit][resume][vable-sync] frame after write: ni={} vsd={} code={:?} ns={:?} debugdata={:?} vable_token={} array_len={}", f.next_instr(), f.valuestackdepth, f.pycode, f.w_globals, f.debugdata, - f.lastblock, f.vable_token, f.locals_w().len(), ); @@ -12937,7 +12936,6 @@ mod tests { vable_pycode: ctx.const_ref(frame.pycode as usize as i64), vable_valuestackdepth: ctx.const_int(1), vable_debugdata: ctx.const_ref(frame.debugdata as usize as i64), - vable_lastblock: ctx.const_ref(frame.lastblock as usize as i64), vable_w_globals: ctx.const_ref(frame.w_globals as usize as i64), } } @@ -13042,7 +13040,6 @@ mod tests { Value::Ref(GcRef(frame.pycode as usize)), // pycode Value::Int(4), // valuestackdepth Value::Ref(GcRef(0)), // debugdata - Value::Ref(GcRef(0)), // lastblock Value::Ref(GcRef(frame.w_globals as usize)), // w_globals ]; for reg in live_regs.iter() { @@ -13135,7 +13132,6 @@ mod tests { vable_pycode: ctx.const_ref(0xdead), vable_valuestackdepth: ctx.const_int(111), vable_debugdata: ctx.const_ref(0xbeef), - vable_lastblock: ctx.const_ref(0xcafe), vable_w_globals: ctx.const_ref(0xfeed), }); let ec_ref = ctx.const_ref(frame.execution_context as usize as i64); @@ -13165,11 +13161,8 @@ mod tests { ctx.constants_get_value(fail_args[4]), Some(majit_ir::Value::Int(4)), ); - // pycode / debugdata / lastblock / w_globals are JIT-scope - // invariant under CPython 3.14 bytecode (`lastblock` is mutated - // only by SETUP_*/POP_BLOCK paths the tracer never enters) and - // stay bound to the trace-start inputarg OpRefs the fixture - // seeded above. + // pycode / debugdata / w_globals stay bound to the trace-start + // inputarg OpRefs the fixture seeded above. assert_eq!( ctx.constants_get_value(fail_args[3]), Some(majit_ir::Value::Ref(majit_ir::GcRef(0xdead))), @@ -13180,10 +13173,6 @@ mod tests { ); assert_eq!( ctx.constants_get_value(fail_args[6]), - Some(majit_ir::Value::Ref(majit_ir::GcRef(0xcafe))), - ); - assert_eq!( - ctx.constants_get_value(fail_args[7]), Some(majit_ir::Value::Ref(majit_ir::GcRef(0xfeed))), ); } @@ -13251,7 +13240,6 @@ mod tests { vable_pycode: ctx.const_ref(0), vable_valuestackdepth: ctx.const_int(0), vable_debugdata: ctx.const_ref(0), - vable_lastblock: ctx.const_ref(0), vable_w_globals: ctx.const_ref(0), }); let ec_ref = ctx.const_ref(frame.execution_context as usize as i64); @@ -13347,7 +13335,6 @@ mod tests { vable_pycode: ctx.const_ref(0), vable_valuestackdepth: ctx.const_int(0), vable_debugdata: ctx.const_ref(0), - vable_lastblock: ctx.const_ref(0), vable_w_globals: ctx.const_ref(0), }); let mut state = MIFrame::from_sym(&mut ctx, &mut sym, frame_ptr, resume_pc, resume_pc); @@ -13413,7 +13400,6 @@ mod tests { vable_pycode: ctx.const_ref(0), vable_valuestackdepth: ctx.const_int(0), vable_debugdata: ctx.const_ref(0), - vable_lastblock: ctx.const_ref(0), vable_w_globals: ctx.const_ref(0), }); let mut state = MIFrame::from_sym(&mut ctx, &mut sym, frame_ptr, resume_pc, resume_pc); @@ -13531,7 +13517,6 @@ mod tests { vable_pycode: ctx.const_ref(frame.pycode as usize as i64), vable_valuestackdepth: ctx.const_int(7), vable_debugdata: ctx.const_ref(frame.debugdata as usize as i64), - vable_lastblock: ctx.const_ref(frame.lastblock as usize as i64), vable_w_globals: ctx.const_ref(frame.w_globals as usize as i64), }); trace_state::seed_compiled_trace_jitcode_test_state( @@ -13680,7 +13665,6 @@ mod tests { vable_pycode: ctx.const_ref(frame.pycode as usize as i64), vable_valuestackdepth: ctx.const_int(7), vable_debugdata: ctx.const_ref(frame.debugdata as usize as i64), - vable_lastblock: ctx.const_ref(frame.lastblock as usize as i64), vable_w_globals: ctx.const_ref(frame.w_globals as usize as i64), }); trace_state::seed_compiled_trace_jitcode_test_state( @@ -13797,7 +13781,6 @@ mod tests { vable_pycode: ctx.const_ref(frame.pycode as usize as i64), vable_valuestackdepth: ctx.const_int(3), vable_debugdata: ctx.const_ref(frame.debugdata as usize as i64), - vable_lastblock: ctx.const_ref(frame.lastblock as usize as i64), vable_w_globals: ctx.const_ref(frame.w_globals as usize as i64), }); let mut state = MIFrame::from_sym(&mut ctx, &mut sym, frame_ptr, target_pc, target_pc); diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index 02abede0347..439a55ad137 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -5709,14 +5709,14 @@ impl CodeWriter { // the codewriter call site. RPython looks up the index dynamically // through `VABLEINFO.static_field_descrs` since each backend may // reorder fields. Pyre's `_virtualizable_` order matches PyPy - // `interp_jit.py:25-31` line by line: - // [last_instr, pycode, valuestackdepth, debugdata, lastblock, - // w_globals], so the literals match + // `interp_jit.py:25-30` line by line: + // [last_instr, pycode, valuestackdepth, debugdata, w_globals], + // so the literals match // `virtualizable_spec.rs::PYFRAME_VABLE_FIELDS`. const VABLE_LAST_INSTR_FIELD_IDX: u16 = 0; const VABLE_CODE_FIELD_IDX: u16 = 1; const VABLE_VALUESTACKDEPTH_FIELD_IDX: u16 = 2; - const VABLE_NAMESPACE_FIELD_IDX: u16 = 5; + const VABLE_NAMESPACE_FIELD_IDX: u16 = 4; // regalloc.py: compile-time stack depth counter — tracks which // stack register (stack_base + depth) is the current TOS. diff --git a/pyre/pyre-jit/src/jit/flatten.rs b/pyre/pyre-jit/src/jit/flatten.rs index b1628da361f..64d3923d242 100644 --- a/pyre/pyre-jit/src/jit/flatten.rs +++ b/pyre/pyre-jit/src/jit/flatten.rs @@ -3177,11 +3177,14 @@ fn flatten_descr_by_ptr(descr: &super::flow::DescrByPtr) -> Operand { if std::sync::Arc::ptr_eq(descr_ref, &majit_ir::descr::vable_array_descr(0)) { return Operand::descr_vable_array(0); } - // VableStaticField: pyre's PyFrame _virtualizable_ has 6 static - // fields (interp_jit.py:25-31, idx 0..=5). Probe each idx in - // turn and Arc::ptr_eq against the per-idx singleton. Mirrors - // the `array_field_descrs[i]` enumeration above. - for idx in 0u16..6 { + // VableStaticField: probe each declared scalar index in turn and + // Arc::ptr_eq against the per-idx singleton. Mirrors the + // `array_field_descrs[i]` enumeration above. The bound is + // `NUM_VABLE_SCALARS` rather than a literal so it follows + // `virtualizable_spec.rs::PYFRAME_VABLE_FIELDS`; a literal here + // outlives the table it describes and asks + // `vable_static_field_descr` for an index it no longer reserves. + for idx in 0u16..pyre_jit_trace::virtualizable_gen::NUM_VABLE_SCALARS as u16 { if std::sync::Arc::ptr_eq(descr_ref, &majit_ir::descr::vable_static_field_descr(idx)) { return Operand::descr_vable_static_field(idx); }