diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index 24e6865aa65..ec97787fcad 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -1151,11 +1151,16 @@ impl BlackholeInterpreter { /// Locate the `catch_exception` op that belongs to the just-executed /// opcode whose post-call guard resumed at `resume_live_pos` (the next /// opcode's `-live-`). Scans op boundaries (the jitcode's `startpoints`) - /// strictly before `resume_live_pos`, newest first, and stops at the - /// first `-live-` — that is the call's own post-call `-live-`, so the only - /// `catch_exception` that can match sits inside this opcode's expansion. - /// Returns `None` (propagate) when the opcode raised outside any - /// try-block (no `catch_exception` was emitted for it). + /// strictly before `resume_live_pos`, newest first. The caught can-raise + /// op's expansion is `[op, -live-, catch_exception, -live-(block entry), + /// vable stores…]` (the guessexception split closes the block at the op's + /// trailing `-live-`, so the successor block opens with its own `-live-` + /// before the moved stores). The scan therefore hops over exactly one + /// `-live-` — the successor's block-entry marker — and accepts the + /// `catch_exception` only when it sits immediately before it; any other + /// op there is the raising op itself (no catch emitted), and a second + /// `-live-` means the raising op sits outside any in-frame try. + /// Returns `None` (propagate) in those cases. fn find_catch_before_resume_live(&self, resume_live_pos: usize) -> Option { let code = &self.jitcode.code; let startpoints = self.jitcode.startpoints.as_ref()?; @@ -1165,14 +1170,23 @@ impl BlackholeInterpreter { .filter(|&q| q < resume_live_pos) .collect(); points.sort_unstable_by(|a, b| b.cmp(a)); + let mut crossed_block_entry_live = false; for q in points { let op = code[q]; if op == self.op_catch_exception { return Some(q); } if op == self.op_live { - // The call's own post-call `-live-`: bound the scan here so a - // preceding opcode's catch can never be mis-selected. + if crossed_block_entry_live { + // Second `-live-`: the raising op has no catch. + return None; + } + crossed_block_entry_live = true; + continue; + } + if crossed_block_entry_live { + // The op immediately before the block-entry `-live-` is not a + // `catch_exception` — it is the raising op itself (uncaught). return None; } } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 02ea25a416d..b378169bec2 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -2638,10 +2638,15 @@ pub(crate) fn find_catch_for_exc_resume(code: &[u8], resume_live_pos: usize) -> /// for the walker. An after-residual-call exception guard resumes at the /// no-exception fallthrough `-live-` (the next opcode after the call); the /// `catch_exception/L` that belongs to the just-executed raising op sits BEHIND -/// that resume `-live-` (between the call's own post-call `-live-` and the next -/// op), so there is no forward catch to route to. Scan op boundaries backward -/// from `resume_live_pos`, newest first, bounded by the first `live/` (the -/// call's own post-call `-live-`), so only THIS opcode's catch can match. +/// that resume `-live-`. The caught can-raise op's expansion is `[op, -live-, +/// catch_exception, -live-(block entry), vable stores…]` (the guessexception +/// split closes the block at the op's trailing `-live-`, so the successor block +/// opens with its own `-live-` before the moved stores). Scan op boundaries +/// backward from `resume_live_pos`, newest first, hopping over exactly one +/// `-live-` (the successor's block-entry marker); accept the `catch_exception` +/// only when it sits immediately before it — any other op there is the raising +/// op itself (uncaught), and a second `-live-` means the raising op sits +/// outside any in-frame try. /// Returns the handler target (2-byte LE label after `catch_exception/L`), or /// `None` when the raising op sits outside any in-frame try (propagate). pub(crate) fn find_catch_before_resume_live(code: &[u8], resume_live_pos: usize) -> Option { @@ -2650,6 +2655,7 @@ pub(crate) fn find_catch_before_resume_live(code: &[u8], resume_live_pos: usize) .filter(|&pc| pc < resume_live_pos) .collect(); pcs.sort_unstable_by(|a, b| b.cmp(a)); + let mut crossed_block_entry_live = false; for pc in pcs { let op = decode_op_at(code, pc)?; if op.key == "catch_exception/L" { @@ -2658,8 +2664,16 @@ pub(crate) fn find_catch_before_resume_live(code: &[u8], resume_live_pos: usize) return Some(lo | (hi << 8)); } if op.key == "live/" { - // The call's own post-call `-live-`: bound the scan so a preceding - // opcode's catch can never be mis-selected. + if crossed_block_entry_live { + // Second `-live-`: the raising op has no catch. + return None; + } + crossed_block_entry_live = true; + continue; + } + if crossed_block_entry_live { + // The op immediately before the block-entry `-live-` is not a + // `catch_exception` — it is the raising op itself (uncaught). return None; } } 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 bb54441632e..ac31a6344c9 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs @@ -1447,17 +1447,23 @@ pub(crate) fn compute_inline_caller_frame( // `result_color_at_pc` (top-of-stack color at the return pc), not the flat // `stack_slot_color_map` — the result is not a live Variable here, so it // carries no pcdep entry. - let result_color = match resume_marker_jit_pc { - Some(marker) => unsafe { &(*caller_sym.jitcode()).payload } - .result_color_trivia_for_jitcode_pc(marker) - .filter(|&color| color != u16::MAX) - .map(|color| color as usize), - // Marker-miss: the after-residual result-color twin keys the same - // fallthrough coordinate by JitCode byte offset, retiring the py read. - None => unsafe { &(*caller_sym.jitcode()).payload } - .result_color_after_residual_for_jitcode_pc(call_jit_pc) + let result_color = { + let payload = unsafe { &(*caller_sym.jitcode()).payload }; + // The trivia twin resolves the return marker only while that marker + // doubles as the fallthrough PC's own resume marker (a pc_map + // block-head). A CALL whose fallthrough carries its own per-PC + // marker leaves the trailing marker un-keyed there — the + // after-residual twin keys the same fallthrough coordinate by the + // CALL's byte offset and stays exact in both shapes. + resume_marker_jit_pc + .and_then(|marker| payload.result_color_trivia_for_jitcode_pc(marker)) .filter(|&color| color != u16::MAX) - .map(|color| color as usize), + .or_else(|| { + payload + .result_color_after_residual_for_jitcode_pc(call_jit_pc) + .filter(|&color| color != u16::MAX) + }) + .map(|color| color as usize) } .ok_or(InlineCallerFrameDecline::Unavailable)?; // Null the not-yet-produced result slot, build the box list, then restore diff --git a/pyre/pyre-jit-trace/src/liveness.rs b/pyre/pyre-jit-trace/src/liveness.rs index 6320af5b2ae..b2a98fd8b19 100644 --- a/pyre/pyre-jit-trace/src/liveness.rs +++ b/pyre/pyre-jit-trace/src/liveness.rs @@ -43,6 +43,13 @@ pub struct LiveVars { /// RPython JitCode treats stack as registers with liveness. /// For Python bytecodes, stack depth determines which slots are live. stack_depth_at: Vec, + /// Memoized per-Python-PC `u16` depth table (`stack_depth_at` + /// truncated to `n`, `usize::MAX` sentinel → 0, saturated to + /// `u16::MAX`). Built once on first `depth_at_py_pc` call: the + /// resume/handback paths query it repeatedly per compiled code + /// object, and `LiveVars` is immutable after construction and + /// cached per code pointer by `liveness_for`. + depth_u16: std::sync::OnceLock>, } impl LiveVars { @@ -57,6 +64,7 @@ impl LiveVars { defined_bits: Vec::new(), words_per_pc: 0, stack_depth_at: Vec::new(), + depth_u16: std::sync::OnceLock::new(), }; } let nlocals = code.varnames.len().max(1); @@ -422,6 +430,7 @@ impl LiveVars { defined_bits, words_per_pc, stack_depth_at, + depth_u16: std::sync::OnceLock::new(), } } @@ -486,21 +495,26 @@ impl LiveVars { /// guards or be the resume target. `usize` values exceeding /// `u16::MAX` saturate; CPython's max stack depth fits comfortably /// inside `u16` for realistic bytecode. - pub fn depth_at_py_pc(&self) -> Vec { - // `stack_depth_at` has length `n + 1` (entry + each instr's - // post-state); the metadata table indexes per Python PC, so - // truncate to `n`. - let n = self.stack_depth_at.len().saturating_sub(1); - self.stack_depth_at[..n] - .iter() - .map(|&d| { - if d == usize::MAX { - 0 - } else { - u16::try_from(d).unwrap_or(u16::MAX) - } - }) - .collect() + pub fn depth_at_py_pc(&self) -> &[u16] { + // Built once and memoized: `liveness_for` caches this `LiveVars` + // per code pointer, and the resume/handback callers query the + // table repeatedly per code object. + self.depth_u16.get_or_init(|| { + // `stack_depth_at` has length `n + 1` (entry + each instr's + // post-state); the metadata table indexes per Python PC, so + // truncate to `n`. + let n = self.stack_depth_at.len().saturating_sub(1); + self.stack_depth_at[..n] + .iter() + .map(|&d| { + if d == usize::MAX { + 0 + } else { + u16::try_from(d).unwrap_or(u16::MAX) + } + }) + .collect() + }) } } diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index fa2a0244e65..cf761e57155 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -1907,6 +1907,20 @@ fn record_graph_op( ) -> super::flow::SpaceOperation { let op = super::flow::SpaceOperation::new(opname, args, result, offset); super::flow::push_op(block, op.clone()); + // `jtransform.py:311-313` / `handle_residual_call`: an operation + // whose lowering can raise is immediately followed by a `-live-` + // SpaceOperation in the graph, so `flatten.py:206-217` recognises an + // actually-raising block by its structural `[raising_op, -live-]` + // tail. Centralized at the recording funnel: every can-raise op + // (residual_call with a can-raise calldescr, pre-rtype HLOp whose + // lowering flavor can raise) gets the marker, matching the graph + // shape jtransform produces after rewriting. + if super::flatten::graph_op_can_raise(&op) { + super::flow::push_op( + block, + super::flow::SpaceOperation::new(super::flatten::OPNAME_LIVE, Vec::new(), None, offset), + ); + } op } @@ -2062,12 +2076,17 @@ fn record_residual_call_graph_op( // `jitcode_dispatch.rs`); mirrors the tag the dedicated walker-emit // builders (`flatten.rs build_*_insn`) attach to the same helpers. effect_info.pyre_helper = pyre_helper; - let can_raise = effect_info.check_can_raise(false); op_args.push( super::flatten::intern_call_descr_stub(effect_info, arg_kinds, reskind.to_kind()).into(), ); - let result_var = match reskind.to_kind() { + // `jtransform.py:311-313` / `handle_residual_call`: a residual_call + // whose calldescr can raise is immediately followed by a trailing + // `-live-` so the liveness pass records the registers alive at the + // implicit GUARD_NO_EXCEPTION. `record_graph_op` appends the marker + // via `graph_op_can_raise` (which reads the calldescr just interned + // above). + match reskind.to_kind() { Some(result_kind) => { let result = graph.fresh_variable(result_kind); record_graph_op(block, opname, op_args, Some(result.into()), offset); @@ -2077,16 +2096,7 @@ fn record_residual_call_graph_op( record_graph_op(block, opname, op_args, None, offset); None } - }; - // `jtransform.py:311-313` / `handle_residual_call`: a residual_call - // whose calldescr can raise is immediately followed by a trailing - // `-live-` so the liveness pass records the registers alive at the - // implicit GUARD_NO_EXCEPTION. `flatten.py:206-217` recognises an - // actually-raising block by scanning this trailing `-live-`. - if can_raise { - record_graph_op(block, super::flatten::OPNAME_LIVE, Vec::new(), None, offset); } - result_var } /// Emit a void-result `SpaceOperation` into `block` and return it. @@ -3225,29 +3235,6 @@ fn attach_catch_exception_edge( link } -fn carry_explicit_raise_value_on_catch_stack( - link: &super::flow::LinkRef, - target: &SpamBlockRef, - raised_value: super::flow::FlowValue, -) { - let target_state = target - .framestate() - .expect("explicit raise catch landing must carry a FrameState"); - let stack_value = target_state - .stack - .last() - .and_then(super::flow::FlowValue::as_variable) - .expect("catch landing must end its stack with the raised value"); - let input_index = target - .block() - .borrow() - .inputargs - .iter() - .position(|value| value.as_variable().is_some_and(|v| v.id == stack_value.id)) - .expect("catch stack value must appear in landing inputargs"); - link.borrow_mut().args[input_index] = Some(raised_value); -} - fn restore_canraise_exit_order(block: &super::flow::BlockRef) { let mut block_mut = block.borrow_mut(); if block_mut.exits.len() < 2 { @@ -4778,215 +4765,6 @@ fn uf_find(parent: &std::collections::HashMap, mut x: u32) -> u32 { x } -/// Build the value-equivalence union-find parent map from the splice -/// coalesce pairs (same-slot + CFG, cross-slot filtered — the exact set -/// the splice regalloc merges). Two Variables in one group are copy- -/// coalesced (same value / same color), so they must NOT be separated by -/// the co-live interference below. -fn build_value_parent( - pairs: &[(super::flow::VariableId, super::flow::VariableId)], -) -> std::collections::HashMap { - let mut value_parent: std::collections::HashMap = std::collections::HashMap::new(); - for (a, b) in pairs { - let (a, b) = (a.0, b.0); - value_parent.entry(a).or_insert(a); - value_parent.entry(b).or_insert(b); - let ra = uf_find(&value_parent, a); - let rb = uf_find(&value_parent, b); - if ra != rb { - value_parent.insert(ra, rb); - } - } - value_parent -} - -/// Liveness-correct CPython-co-live interference for the splice regalloc. -/// For each resume PC, every pair of simultaneously-CPython-live (locals -/// via `is_local_live`, stack via `depth_at_pc`), colored, NON-value- -/// equivalent Variables gets an interference edge so the chordal coloring -/// keeps them on distinct colors — the precondition for a color-indexed -/// per-PC resume map. The liveness-correct successor to the retired -/// blanket `collect_distinct_slot_interference_pairs` clique: constrains -/// ONLY slots co-live at a guard, not every distinct slot. -/// -/// Edges are gathered from BOTH the post-dispatch snapshot -/// (`pcdep_slot_var`, the after-opcode `-live-` markers) AND the -/// pre-dispatch resume-depth snapshot (`pcdep_slot_var_resume`, the snapshot -/// the shipped per-PC map is built from in `build_pcdep_color_slots`). A -/// branch guard at orgpc resumes with the deeper pre-dispatch operand stack -/// carrying the mid-opcode kept temps, so two Variables simultaneously live -/// at that depth must also separate; without the resume-depth edges the -/// coloring is free to collapse them onto one color and the color-indexed -/// resume inversion is ambiguous (the kept-operand-stack `#424` family). -fn build_colive_interference( - pcdep_slot_var: &[Vec<(u16, u32)>], - pcdep_slot_var_resume: &[Vec<(u16, u32)>], - value_parent: &std::collections::HashMap, - ref_coloring: &std::collections::HashMap, - depth_at_pc: &[u16], - code: &CodeObject, -) -> Vec<(super::flow::VariableId, super::flow::VariableId)> { - let lv = pyre_jit_trace::state::liveness_for(code as *const _); - let nloc = code.varnames.len(); - let mut interference_set: std::collections::HashSet<(u32, u32)> = - std::collections::HashSet::new(); - let mut live_here: Vec = Vec::new(); - for snap_table in [pcdep_slot_var, pcdep_slot_var_resume] { - for (py_pc, snap) in snap_table.iter().enumerate() { - if snap.is_empty() || !lv.is_reachable(py_pc) { - continue; - } - let depth = depth_at_pc.get(py_pc).copied().unwrap_or(0) as usize; - live_here.clear(); - for &(slot, var_id) in snap { - let slot = slot as usize; - if slot < nloc { - if !lv.is_local_live(py_pc, slot) { - continue; - } - } else if slot - nloc >= depth { - continue; - } - if ref_coloring.contains_key(&super::flow::VariableId(var_id)) { - live_here.push(var_id); - } - } - for i in 0..live_here.len() { - for j in (i + 1)..live_here.len() { - let (a, b) = (live_here[i], live_here[j]); - if a == b || uf_find(value_parent, a) == uf_find(value_parent, b) { - continue; - } - interference_set.insert(if a < b { (a, b) } else { (b, a) }); - } - } - } - } - interference_set - .into_iter() - .map(|(a, b)| (super::flow::VariableId(a), super::flow::VariableId(b))) - .collect() -} - -/// Derive each Variable's canonical CPython slot from the pcdep snapshots: -/// the slot it occupies at the earliest resume PC it appears in (POST before -/// RESUME within a PC). The inlined-callee frames record their own slots at -/// the callee PCs (the callee's operand-stack temp sits at its frame-stack -/// slot), so this spans all inline frames — unlike the outer-frame-only -/// co-live snapshot pairs. Each Variable takes the first slot seen for it in -/// resume-PC order. -fn pcdep_canonical_slot( - pcdep_slot_var: &[Vec<(u16, u32)>], - pcdep_slot_var_resume: &[Vec<(u16, u32)>], -) -> Vec> { - // Slot-indexed by `VariableId.0`. `None` = the Variable is absent from - // every pcdep snapshot (never live at a resume PC). - let mut slot_of: Vec> = Vec::new(); - let n = pcdep_slot_var.len().max(pcdep_slot_var_resume.len()); - for py_pc in 0..n { - for tbl in [pcdep_slot_var, pcdep_slot_var_resume] { - if let Some(snap) = tbl.get(py_pc) { - for &(slot, var_id) in snap { - let idx = var_id as usize; - if idx >= slot_of.len() { - slot_of.resize(idx + 1, None); - } - if slot_of[idx].is_none() { - slot_of[idx] = Some(slot); - } - } - } - } - } - slot_of -} - -/// Slot-identity interference for the splice coalesce filter: a coalesce -/// candidate whose two endpoints hold DISTINCT canonical CPython slots must -/// not merge, even when their SSA / CPython-slot live ranges are disjoint -/// (never co-live at a single resume PC). Merging two distinct-slot -/// Variables onto one color extends that color's liveness across a region -/// where no box is live in it — the liveness side-table then marks the color -/// active at a resume PC where `regs_r[color]` is `OpRef::NONE` -/// (`collect_outer_active_boxes` panics). This is broader than co-liveness: -/// the inline-callee operand-stack temp and the outer merge inputarg live at -/// disjoint PCs yet occupy distinct frame-stack slots, so `build_colive_ -/// interference` (which only edges simultaneously-live pairs) never separates -/// them. Feeding these edges into the coalesce `has_edge` oracle rejects the -/// cross-slot merge directly — the pcdep-sourced replacement for the retired -/// walker-slot cross-slot coalesce filter. Same-slot pairs (the walker's -/// COPY/SWAP value lineage) are untouched, so no extra color separation is -/// forced beyond the merges that were already dropped. -/// -/// The slot claim is propagated through a union-find over the pairs, so a -/// TRANSITIVE cross-slot chain is caught even when the bridging Variable has -/// no canonical slot of its own. A pass-through link/inputarg temp that never -/// appears at a resume PC is absent from the pcdep snapshots, so the pairs -/// `(slot0_var, temp)` and `(temp, slot1_var)` name no directly-distinct -/// slots; but merging both aliases slot0 and slot1 onto one color. The group -/// carries slot0's claim through the first merge, so the second pair's slot1 -/// conflicts and is rejected. A slot number is unique across locals and stack -/// (stack slots are `>= nlocals`), so one claim per group suffices — a group -/// holds at most one distinct slot, and any second distinct slot rejects. The -/// rejecting edge is emitted between the pair's direct endpoints; -/// `DependencyGraph::coalesce` in `filter_coalesce_pairs_by_interference` moves -/// the edge onto the surviving rep as earlier pairs merge, so the `has_edge` -/// replay sees it. -fn build_slot_disjoint_interference( - pairs: &[(super::flow::VariableId, super::flow::VariableId)], - canonical_slot: &[Option], -) -> Vec<(super::flow::VariableId, super::flow::VariableId)> { - use super::flow::VariableId; - let slot_of = - |id: VariableId| -> Option { canonical_slot.get(id.0 as usize).copied().flatten() }; - fn find(parent: &mut HashMap, x: VariableId) -> VariableId { - let mut root = x; - while let Some(&p) = parent.get(&root) { - if p == root { - break; - } - root = p; - } - let mut cur = x; - while let Some(&p) = parent.get(&cur) { - if p == root { - break; - } - parent.insert(cur, root); - cur = p; - } - root - } - let mut parent: HashMap = HashMap::new(); - // Canonical slot claimed by each union-find root (absent = the group - // touches no slotted Variable yet). - let mut group_slot: HashMap = HashMap::new(); - let mut edges = Vec::new(); - for &(a, b) in pairs { - parent.entry(a).or_insert(a); - parent.entry(b).or_insert(b); - let ra = find(&mut parent, a); - let rb = find(&mut parent, b); - if ra == rb { - continue; - } - let sa = group_slot.get(&ra).copied().or_else(|| slot_of(a)); - let sb = group_slot.get(&rb).copied().or_else(|| slot_of(b)); - if let (Some(x), Some(y)) = (sa, sb) { - if x != y { - // Merging would alias two distinct slots onto one color. - edges.push((a, b)); - continue; - } - } - parent.insert(rb, ra); - if let Some(s) = sa.or(sb) { - group_slot.insert(ra, s); - } - } - edges -} - /// #348 Part (2): build the per-PC `(color, semantic_slot)` map shipped in /// `PyJitCodeMetadata::pcdep_color_slots`. For each reachable PC, every slot /// live and restorable there contributes `(true SSA color, slot)`, sorted by @@ -5097,9 +4875,27 @@ fn validate_pcdep_color_map( rename: &[Vec; 3], code: &CodeObject, depth_at_pc: &[u16], - value_parent: &std::collections::HashMap, + coalesce_pairs: &[(super::flow::VariableId, super::flow::VariableId)], label: &str, ) { + // Value-equivalence partition of the accepted coalesce pairs: two + // Variables in one group are copy-coalesced (same value / same color), + // so their sharing a color is not an injectivity violation. + let value_parent: std::collections::HashMap = { + let mut parent: std::collections::HashMap = std::collections::HashMap::new(); + for (a, b) in coalesce_pairs { + let (a, b) = (a.0, b.0); + parent.entry(a).or_insert(a); + parent.entry(b).or_insert(b); + let ra = uf_find(&parent, a); + let rb = uf_find(&parent, b); + if ra != rb { + parent.insert(ra, rb); + } + } + parent + }; + let value_parent = &value_parent; let live_vars = pyre_jit_trace::state::liveness_for(code as *const _); let nlocals = code.varnames.len(); let mut checked = 0usize; @@ -6331,16 +6127,15 @@ impl CodeWriter { // constant / sentinel rather than a Variable. let mut top_of_stack_var_at_pc: Vec> = vec![None; num_instrs]; - // Stage 1a (#348, ADDITIVE / gated by `PYRE_PCDEP_VALIDATE`): - // per-PC snapshot of `slot -> SSA Variable.id` taken from the - // post-opcode FrameState. The splice regalloc derives the - // liveness-correct CPython-co-live interference from it (each - // resume PC's simultaneously-live, non-value-equivalent locals - // must land on distinct colors); the `PYRE_PCDEP_VALIDATE` gate - // additionally validates the resulting per-PC color map. Each - // entry is `(slot, Variable.id)` where `slot` is the local index - // in `[0..nlocals)` or `nlocals + stack_depth` for an operand- - // stack value. + // CPython liveness oracle for the per-PC `-live-` force-alive args + // below (locals gated by `is_local_live`, matching + // `filter_liveness_in_place` / `build_pcdep_color_slots`). + let frame_liveness = pyre_jit_trace::state::liveness_for(code as *const _); + // #348: per-PC snapshot of `slot -> SSA Variable.id` taken from the + // post-opcode FrameState. Feeds the gated `PYRE_PCDEP_VALIDATE` + // injectivity check. Each entry is `(slot, Variable.id)` where + // `slot` is the local index in `[0..nlocals)` or + // `nlocals + stack_depth` for an operand-stack value. let pcdep_validate = std::env::var_os("PYRE_PCDEP_VALIDATE").is_some(); let mut pcdep_slot_var: Vec> = vec![Vec::new(); num_instrs]; // #355 B2: the PRE-dispatch resume-depth `slot -> Variable.id` snapshot, @@ -6733,30 +6528,24 @@ impl CodeWriter { None }; if let Some(catch_label) = catch_label_opt { - // Raise inside a try/except range: RPython - // canraise-arm shape. The block's exception edge - // goes to the catch landing, not `graph.exceptblock`. - // `emit_catch_exception!` both pushes the - // `catch_exception/L` insn - // AND calls `attach_catch_exception_edge` (the - // exception Link onto `current_block.exits`). + // Raise inside a try/except range: the `raise` op + // closes the block as its last operation and the sole + // exit is the exception edge to the catch landing. + // The standard flattener serializes `raise ` + // from the block body and emits the byte-adjacent + // `catch_exception` dispatch from the graph shape + // alone (single-exit canraise arm); the raised value + // reaches the handler through the runtime exception + // state (`route_to_catch` → `last_exc_value`), the + // same delivery every canraise catch uses. + record_graph_op( + ¤t_block.block(), + "raise", + vec![evalue_fv.into()], + None, + offset, + ); emit_catch_exception!(catch_label); - // Carry the normalized raised value on the - // just-attached exception edge. Unlike the - // `graph.exceptblock` arm below (whose - // `explicit_raise_state` puts the raised value in - // `link.args[1]`), `attach_catch_exception_edge` - // links directly to the catch landing and seeds - // `extravars` with a FRESH (type, value) read-back - // pair — so the canonical flatten cannot recover the - // `raise` operand from the link. Record it here so - // `insert_exits`' single-exit explicit-raise arm - // emits `raise `. - if let Some(raised) = evalue_fv.as_variable() { - if let Some(exc_link) = current_block.block().borrow().exits.last() { - exc_link.borrow_mut().explicit_raise_value = Some(raised); - } - } } else { // `flowcontext.py:1246-1261 Raise.nomoreblocks` shape: // link = Link([w_exc.w_type, w_exc.w_value], @@ -6960,13 +6749,14 @@ impl CodeWriter { // merge PC, so `mergeblock` appends a spurious normal exit onto // the raise-terminated block (making it multi-exit); then // `insert_exits` lowers the raise edge as a plain catch and - // drops the `raise` op, so the handler never runs. + // drops the `raise` op, so the handler never runs. A + // raise-terminated block is recognized structurally: its + // raising op (last non-`-live-` operation) is the `raise`. let has_explicit_raise = current_block .block() .borrow() - .exits - .iter() - .any(|e| e.borrow().explicit_raise_value.is_some()); + .raising_op() + .is_some_and(|op| op.opname == "raise"); let canraise_pending = !has_explicit_raise && matches!( current_block.block().borrow().exitswitch, @@ -7218,12 +7008,61 @@ impl CodeWriter { // every PC would create a `-live-` cluster the upstream graph // never holds. macro_rules! emit_live_placeholder { - () => {{ - // Per-PC `-live-` is produced by the canonical splice from - // the graph; the portal red args (`pypy/module/pypyjit/ - // interp_jit.py:67 reds = ['frame', 'ec']`) are kept alive by - // the splice's force-alive mechanism (`liveness.py:11-12`). - // The walker no longer emits a per-block copy. + ($py_pc:expr) => {{ + let py_pc: usize = $py_pc; + // Only real instruction starts are resume points; a CACHE + // unit inside a multi-unit instruction (or an EXTENDED_ARG + // prefix) carries a mid-instruction FrameState that must not + // be forced alive. + let at_instruction_start = !matches!( + pyre_interpreter::decode_instruction_at(code, py_pc), + None | Some(( + Instruction::Cache | Instruction::ExtendedArg | Instruction::NotTaken, + _ + )) + ); + // Per-PC `-live-` graph op with force-alive args + // (`liveness.py:8-12`: "You can also force extra variables + // to be alive by putting them as args of the '-live-' + // operation in the first place"). Every CPython-frame-live + // Ref Variable at this resume point — locals gated by the + // liveness oracle, the whole operand stack — is an explicit + // arg, so `RegAllocator.make_dependencies` sees each frame + // slot's value live across the marker: co-live frame slots + // interfere structurally and the ordinary + // dependency/coalesce/color pass keeps them on distinct + // colors without a side-table interference oracle. The + // portal reds (`interp_jit.py:67 reds = ['frame', 'ec']`) + // are excluded — they keep their dedicated colors above the + // allocator range via the post-color pin. + // A closed block accepts no further operations; the next + // boundary (mergeblock / catch landing) opens the block the + // resume point belongs to. + let block_open = current_block.block().borrow().exits.is_empty(); + let mut live_args: Vec = Vec::new(); + for (i, lv) in current_state.locals_w.iter().enumerate() { + if let Some(super::flow::FlowValue::Variable(v)) = lv { + if v.kind == Some(Kind::Ref) && frame_liveness.is_local_live(py_pc, i) { + live_args.push((*v).into()); + } + } + } + for sv in ¤t_state.stack { + if let super::flow::FlowValue::Variable(v) = sv { + if v.kind == Some(Kind::Ref) { + live_args.push((*v).into()); + } + } + } + if block_open && at_instruction_start { + record_graph_op( + ¤t_block.block(), + super::flatten::OPNAME_LIVE, + live_args, + None, + py_pc as i64, + ); + } }}; } @@ -7811,10 +7650,8 @@ impl CodeWriter { let block_closed_by_terminator = { let block_rc = current_block.block(); let block = block_rc.borrow(); - let has_explicit_raise = block - .exits - .iter() - .any(|e| e.borrow().explicit_raise_value.is_some()); + let has_explicit_raise = + block.raising_op().is_some_and(|op| op.opname == "raise"); !block.exits.is_empty() && (has_explicit_raise || !matches!( @@ -7830,7 +7667,7 @@ impl CodeWriter { if loop_header_pcs.contains(&py_pc) && !block_closed_by_terminator { // jtransform.py:1710-1711 op3: -live- before // jit_merge_point, "for inlined short preambles". - emit_live_placeholder!(); + emit_live_placeholder!(py_pc); if is_true_portal { let jdindex = portal_jd_index .expect("portal jit_merge_point requires a registered jitdriver"); @@ -7885,7 +7722,9 @@ impl CodeWriter { } } - emit_live_placeholder!(); + if !block_closed_by_terminator { + emit_live_placeholder!(py_pc); + } // Dead-code dispatch gate: `current_block` has already // been closed by a previous terminator emit (`emit_goto!`, @@ -12103,25 +11942,7 @@ impl CodeWriter { py_pc as i64, ); let exc_flow: super::flow::FlowValue = exc_value.into(); - emit_raise!(0u16, exc_flow.clone(), py_pc as i64, true); - if let Some(catch_label) = catch_for_pc.get(py_pc).copied().flatten() { - let site = catch_sites - .iter() - .find(|site| site.landing_label == catch_label) - .expect("catch site for DELETE_FAST raise"); - let link = current_block - .block() - .borrow() - .exits - .last() - .cloned() - .expect("DELETE_FAST raise catch edge"); - carry_explicit_raise_value_on_catch_stack( - &link, - &site.landing, - exc_flow, - ); - } + emit_raise!(0u16, exc_flow, py_pc as i64, true); // The bound arm is the continuing block. The // clear is one PY_NULL write, matching @@ -12430,19 +12251,134 @@ impl CodeWriter { // stray catch link to a block whose exits the // just-emitted opcode already closed. let block_already_closed = !current_block.block().borrow().exits.is_empty(); - if !block_already_closed { - // `flatten.py:206-217` + `jtransform.py:311-313`: - // a `catch_exception` must be immediately - // preceded by a `-live-` so the blackhole's - // after-residual-call resume - // (`pyjitpl.py:2610-2624 capture_resumedata`, - // `resumepc=-1`) lands on a marker it can - // decode (`blackhole.py:396-410 - // handle_exception_in_frame` skips one - // `-live-` then reads the catch). A repeated - // `-live-` here folds in `remove_repeated_live`. - emit_live_placeholder!(); + // `guessexception` closes the block AT the caught + // can-raise operation, so the block tail is + // structurally `[raising_op, -live-]` and any + // later operation belongs to the normal-flow + // successor. Find the last can-raise op this + // opcode recorded (`record_graph_op` appended its + // `-live-`); a covered PC whose opcode recorded no + // can-raise op attaches no exception edge — same + // set of catch edges the flatten early-return used + // to drop post-hoc. + let split_at = if block_already_closed { + None + } else { + current_block + .block() + .borrow() + .operations + .iter() + .rposition(|op| { + op.offset == py_pc as i64 + && super::flatten::graph_op_can_raise(op) + }) + .map(|raising_pos| raising_pos + 2) + }; + if let Some(split_at) = split_at { + debug_assert!( + current_block + .block() + .borrow() + .operations + .get(split_at - 1) + .is_some_and(|op| op.opname == super::flatten::OPNAME_LIVE), + "can-raise graph op must carry its trailing -live- \ + (record_graph_op invariant) at py_pc {py_pc}", + ); + // Move the opcode's post-raise operations (vable + // mirror stores of the pushed result, vsd syncs) + // into the normal-flow successor — the + // `unsimplify.py:44 split_block` shape. The + // walker keeps emitting into the successor, so + // subsequent PCs land there. + let moved: Vec = current_block + .block() + .borrow_mut() + .operations + .split_off(split_at); emit_catch_exception!(catch_label); + // Successor continuation: identity split — same + // Variables, no freshening (the UNPACK_SEQUENCE / + // FOR_ITER split precedent). The block's own + // framestate stays at `py_pc` (mid-opcode + // continuation, like those precedents) so the + // next PC's boundary machinery — branch-target + // force, joinpoint merge — still observes a + // block that did not start there. The walker's + // `current_state` keeps its already-advanced + // `next_offset = py_pc + 1`. + let mut next_state = current_state.clone(); + next_state.next_offset = py_pc; + next_state.blocklist = frame_blocks_for_offset(code, py_pc); + let next_block = SpamBlockRef::new( + graph.new_block(Vec::new()), + Some(next_state.clone()), + ); + all_walker_blocks.push(next_block.clone()); + // `unsimplify.py:59-76 split_block` varmap rules: + // * a Variable PRODUCED by a moved op + // (`vars_produced_in_new_block`) is defined + // inside the successor — it must NOT be + // passed on the link even when the + // FrameState already lists it (e.g. a + // follow-up op's result the dispatch pushed + // onto the symbolic stack); + // * a Variable a moved op CONSUMES that the + // FrameState no longer lists (a receiver + // popped before the raising op) must be + // threaded through the link so the + // successor's inputargs cover every use. + let moved_results: Vec = moved + .iter() + .filter_map(|op| match &op.result { + Some(super::flow::FlowValue::Variable(r)) => Some(r.id.0), + _ => None, + }) + .collect(); + let mut inputargs: Vec = next_state + .getvariables() + .into_iter() + .filter(|value| { + value + .as_variable() + .is_none_or(|v| !moved_results.contains(&v.id.0)) + }) + .collect(); + // Identity split: the link passes each surviving + // Variable through unchanged, so `link_args` + // mirrors `inputargs`. + let mut link_args = inputargs.clone(); + { + let mut known: Vec = inputargs + .iter() + .filter_map(super::flow::FlowValue::as_variable) + .map(|v| v.id.0) + .collect(); + known.extend(moved_results.iter().copied()); + for op in &moved { + for arg in &op.args { + for v in arg.variables() { + if !known.contains(&v.id.0) { + known.push(v.id.0); + inputargs.push(v.into()); + link_args.push(v.into()); + } + } + } + } + } + next_block.block().borrow_mut().inputargs = inputargs; + append_exit( + ¤t_block.block(), + super::flow::Link::new(link_args, Some(next_block.block()), None) + .into_ref(), + ); + restore_canraise_exit_order(¤t_block.block()); + for op in moved { + super::flow::push_op(&next_block.block(), op); + } + current_block = next_block; } } } @@ -12787,16 +12723,15 @@ impl CodeWriter { // short-circuit `(i and C)` PHI ↔ loop-var merge that collapses the // kept operand-stack slot's color onto the loop var (#124 float). let cfg_variable_pairs = collect_cfg_coalesce_pairs(&graph); - // `&[]`: honour SSA-liveness interference only, seeding the gate-off - // `graph_regallocs` coloring. The CPython-slot co-live / cross-slot - // merges this SSA-only pass misses are rejected on the SPLICE pairs - // below, where the co-live + slot-identity edges feed this same filter's - // `has_edge` oracle. + // The per-PC `-live-` graph ops force every frame-live Ref Variable + // alive, so `make_dependencies` inside the filter models CPython + // frame-slot liveness alongside SSA liveness and the `has_edge` + // guard rejects both the co-live and the frame-lifetime-overlap + // merges in one pass. let cfg_variable_pairs = super::regalloc::filter_coalesce_pairs_by_interference( &graph, Kind::Ref, &cfg_variable_pairs, - &[], ); let mut graph_regallocs = super::regalloc::perform_register_allocation_all_kinds_with_pairs( &graph, @@ -12817,98 +12752,19 @@ impl CodeWriter { // color space — the spliced body carries graph-lifetime colors, // not walker stack-slot register numbers. // - // Splice coalesce pairs (same-slot + CFG, cross-slot filtered). - // Built once here — outside the IIFE — so the same set feeds the - // splice regalloc, the co-live interference, the value-equivalence - // partition, and the gated validation below. - // - // Order `same_slot` BEFORE `cfg` so each walker slot's Variables - // first cohere into one union-find group; the cfg pairs then fold - // those whole groups into the frame-local groups consistently. With - // the reverse order, a cfg chain can split one slot's Variables - // across two different frame-local groups and the later same_slot - // pair that would reunite them is dropped by the filter — leaving - // that slot with two colors. When the filter drops nothing (graphs - // with no cross-slot merge) the union-find partition is order- - // independent, so this reorder is a no-op there. The cross-slot - // filter drops coalesce pairs whose union would transitively merge - // two distinct frame-local slots into one regalloc group — - // otherwise the slots share a union-find rep and the co-live - // interference between them is a self-edge no-op. - // Same-slot coalescing retired (#267): RPython's flatten has no - // walker-slot coalescing. Body locals are colored freely by the chordal - // coloring; a guard resume reconstructs each live local/stack value via - // the per-PC color→slot map plus the virtualizable-frame overlay - // (`overlay_local` in `setup_bridge_sym`), so a frame slot no longer - // needs one canonical color across its re-read Variables. Only the CFG - // value-equivalence pairs (the walker's COPY/SWAP lineage) remain, to - // merge provably-equal Variables. - // - // Reject the coalesce merges that would break the color-indexed per-PC - // resume via the interference `has_edge` oracle (the RPython-faithful - // `regalloc.py:105` guard), replacing the walker-slot post-filter. The - // filter was purely slot-based, so its pcdep-sourced successor is too: - // `build_slot_disjoint_interference` edges any coalesce candidate whose - // endpoints hold distinct canonical CPython slots. This covers the - // disjoint-live case (never co-live at a single PC) that dominates - // inlined callees — a callee operand-stack temp and the outer merge - // inputarg occupy distinct frame-stack slots but live at disjoint PCs, - // so merging them extends a color's liveness across a box-less region - // and the resume reads `OpRef::NONE`. Canonical slots come from the - // pcdep snapshots (which record each inline frame's slots at that - // frame's PCs), so no walker slot map is consulted here. Co-liveness is - // NOT needed to gate coalescing — the co-live separations the coloring - // needs are applied to the interference graph below (`splice_ - // interference`), not to the coalesce filter. - let canonical_slot = pcdep_canonical_slot(&pcdep_slot_var, &pcdep_slot_var_resume); - let splice_coalesce_oracle = - build_slot_disjoint_interference(&cfg_variable_pairs, &canonical_slot); - let splice_pairs = super::regalloc::filter_coalesce_pairs_by_interference( - &graph, - Kind::Ref, - &cfg_variable_pairs, - &splice_coalesce_oracle, - ); - // Liveness-correct CPython-co-live interference: each resume PC's - // simultaneously-CPython-live, non-value-equivalent locals/stack - // Variables interfere, so the chordal coloring keeps them on - // distinct colors. Without it the coloring is free to give two - // frame-live locals one color (their SSA live ranges are disjoint - // between `LOAD_FAST` re-reads, but CPython slot liveness keeps the - // dead one live across its SSA death), which a color-indexed per-PC - // resume map cannot disambiguate. The liveness-correct successor to - // the retired blanket `collect_distinct_slot_interference_pairs` - // clique: it constrains only slots co-live at a guard. - let splice_value_parent = build_value_parent(&splice_pairs); - let splice_interference = build_colive_interference( - &pcdep_slot_var, - &pcdep_slot_var_resume, - &splice_value_parent, - &graph_regallocs[Kind::Ref.index()].coloring, - &depth_at_pc, - code, - ); + // The per-PC `-live-` graph ops carry every frame-live Ref Variable + // as a force-alive arg (`liveness.py:8-12`), so + // `RegAllocator.make_dependencies` sees each frame slot's value live + // through every resume point it covers: co-live frame slots + // interfere structurally and a coalesce whose endpoints' frame + // lifetimes overlap is rejected by the ordinary `regalloc.py:105 + // has_edge` guard inside the filter above. One + // dependency/coalesce/color pass therefore suffices — the + // pcdep-derived slot-identity + co-live interference side tables and + // the second Ref allocation are retired (#371). + let splice_pairs = cfg_variable_pairs; let (canonical, splice_regallocs) = (|| { - // Re-run regalloc with the merged pairs + co-live interference - // so the chordal coloring re-optimizes the surrounding - // Variables around the forced merges/separations — a naive - // post-hoc color rewrite would not. Kept separate from - // production `graph_regallocs` (the gate-off path) so gate-off - // stays byte-identical. - // - // Body locals are colored freely by the chordal coloring; - // `same_slot_pairs` merges each slot's re-read Variables onto - // one color, the co-live interference separates distinct - // frame-live locals, and the per-PC resume map - // (`pcdep_color_slots` → `semantic_ref_slot_for_reg_color`) - // records each local's color so the decode never assumes - // `color == slot`. - let mut splice_regallocs = - super::regalloc::perform_register_allocation_all_kinds_with_pairs_and_interference( - &graph, - &splice_pairs, - &splice_interference, - ); + let mut splice_regallocs = graph_regallocs.clone(); // `interp_jit.py:67 reds = ['frame', 'ec']`: both portal inputs // are live in every MIFrame at every guard. Give them dedicated // Ref colors above the ordinary allocator range so neither can be @@ -13234,17 +13090,13 @@ impl CodeWriter { &depth_at_pc, ); // #348 (gated, no runtime effect): self-check that the production - // splice coloring — now built with the co-live interference — gives - // an injective per-PC color map. `splice_value_parent` is the same - // value-equivalence partition the interference excluded, so the - // check only flags a clash between two DIFFERENT-value (different - // union-find rep) live Variables sharing one color. Expectation: - // `inj_violations=0`. Only runs under `PYRE_PCDEP_VALIDATE`. + // coloring gives an injective per-PC color map. The value-equivalence + // partition (built inside the validator from the accepted coalesce + // pairs) excludes copy-coalesced Variables, so the check only flags a + // clash between two DIFFERENT-value (different union-find rep) live + // Variables sharing one color. Expectation: `inj_violations=0`. Only + // runs under `PYRE_PCDEP_VALIDATE`. if pcdep_validate { - eprintln!( - "PCDEP[production] PAIRS: {} co-live interference edges", - splice_interference.len(), - ); validate_pcdep_color_map( &pcdep_slot_var, [ @@ -13260,7 +13112,7 @@ impl CodeWriter { &alloc_result.rename, code, &depth_at_pc, - &splice_value_parent, + &splice_pairs, "production", ); // The SHIPPED per-PC map is built from `pcdep_slot_var_resume` @@ -13268,10 +13120,8 @@ impl CodeWriter { // (post-dispatch): a branch guard at orgpc resumes with the deeper // operand stack carrying the mid-opcode kept temps. Those temps // live only in the resume snapshot, so the "production" check above - // does not cover them. Validate the actual shipped source — its - // injectivity is what the resume-depth co-live interference in - // `build_colive_interference` now guarantees (expectation: - // `inj_violations=0`). + // does not cover them. Validate the actual shipped source + // (expectation: `inj_violations=0`). validate_pcdep_color_map( &pcdep_slot_var_resume, [ @@ -13287,7 +13137,7 @@ impl CodeWriter { &alloc_result.rename, code, &depth_at_pc, - &splice_value_parent, + &splice_pairs, "production-resume", ); } @@ -13649,6 +13499,20 @@ impl CodeWriter { let merge_entry_by_green: Vec<(u32, u32)> = trace_entry_pcs .into_iter() .filter_map(|py_pc| { + // A truncated body (an untranslatable opcode bakes + // `abort_permanent` mid-function) emits no ops at-or-after a + // later trace entry, yet the dense pc_map still hands that + // py_pc the LAST marker by carry-forward. Walking from such + // a bogus entry re-reaches the abort marker, and the abort + // flush rewinds the live frame to the marker's own py_pc — + // re-executing bytecode the frame already ran (duplicate + // side effects). Cover only entries the body reaches. + let covered = first_jit_pc_by_py_pc + .get(py_pc..) + .is_some_and(|tail| tail.iter().any(|&v| v != usize::MAX)); + if !covered { + return None; + } let off = resolve_marker(py_pc); off.map(|off| (py_pc as u32, off as u32)) }) @@ -14655,6 +14519,22 @@ pub fn find_branch_target_pcs(code: &pyre_interpreter::CodeObject) -> VecSet super::super::flow::SpaceOperation { + block + .borrow() + .operations + .iter() + .rev() + .find(|op| op.opname != super::super::flatten::OPNAME_LIVE) + .cloned() + .expect("a non--live- op should be recorded") + } use super::{ FrameState, SpamBlockRef, attach_catch_exception_edge, entry_arg_slots, entry_frame_state, entry_inputargs, mergeblock, new_shadow_graph, @@ -14669,51 +14549,6 @@ mod tests { use pyre_interpreter::compile_exec; use std::sync::Arc; - /// A coalesce candidate whose two endpoints hold distinct canonical - /// CPython slots — including the disjoint-live inline-callee case that is - /// never co-live at a single resume PC — yields a slot-identity - /// interference edge, while a within-slot pair yields none. - #[test] - fn build_slot_disjoint_interference_edges_cross_slot_only() { - // v0 → slot 0, v1 → slot 3, v2 → slot 3, v3 → (no snapshot). - let canonical = vec![Some(0u16), Some(3), Some(3)]; - let pairs = vec![ - (VariableId(0), VariableId(1)), // slot 0 ≠ 3 → edge - (VariableId(1), VariableId(2)), // slot 3 == 3 → no edge (COPY lineage) - (VariableId(1), VariableId(3)), // v3 absent → no edge (never live at a resume) - ]; - let edges = build_slot_disjoint_interference(&pairs, &canonical); - assert_eq!(edges, vec![(VariableId(0), VariableId(1))]); - } - - /// A transitive cross-slot chain through a Variable with no canonical slot - /// is still rejected: `(slot0_var, temp)` then `(temp, slot1_var)` where - /// `temp` is absent from the pcdep snapshots (never live at a resume PC). - /// The union-find carries slot0's claim through the first merge, so the - /// second pair's slot1 conflicts and is edged. - #[test] - fn build_slot_disjoint_interference_rejects_transitive_cross_slot_chain() { - // v0 → slot 0, v2 → slot 1, v1 (temp) → no snapshot. - let canonical = vec![Some(0u16), None, Some(1)]; - let pairs = vec![ - (VariableId(0), VariableId(1)), // slot0_var → temp: merge, group claims slot 0 - (VariableId(1), VariableId(2)), // temp → slot1_var: group slot 0 ≠ 1 → edge - ]; - let edges = build_slot_disjoint_interference(&pairs, &canonical); - assert_eq!(edges, vec![(VariableId(1), VariableId(2))]); - } - - /// The canonical slot is the slot a Variable occupies at the earliest PC - /// it appears in across the pcdep snapshots (POST before RESUME). - #[test] - fn pcdep_canonical_slot_takes_earliest_pc() { - // v5 first appears at py_pc 1 slot 2, later at slot 0 → canonical 2. - let post = vec![vec![], vec![(2u16, 5u32)], vec![(0u16, 5u32)]]; - let resume: Vec> = vec![vec![], vec![], vec![(0u16, 5u32)]]; - let slots = pcdep_canonical_slot(&post, &resume); - assert_eq!(slots.get(5).copied().flatten(), Some(2)); - } - #[test] fn frame_layout_slot_places_operand_stack_after_non_argument_deref_slots() { // A free var or pure cell occupies one physical slot between the three @@ -14897,8 +14732,7 @@ mod tests { let result = emit_frontend_neg(&mut graph, &start, operand.into(), 33); - let block = start.borrow(); - let op = block.operations.last().expect("neg op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "neg"); assert_eq!(op.offset, 33); assert_eq!(op.args, vec![operand.into()]); @@ -14922,11 +14756,7 @@ mod tests { 46, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("newslice op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "newslice"); assert_eq!(op.offset, 46); assert_eq!(op.args, vec![w_start.into(), w_stop.into(), w_step.into()]); @@ -14949,11 +14779,7 @@ mod tests { 44, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("newlist op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "newlist"); assert_eq!(op.offset, 44); assert_eq!(op.args, vec![item0.into(), item1.into(), item2.into()]); @@ -14976,11 +14802,7 @@ mod tests { 45, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("newtuple op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "newtuple"); assert_eq!(op.offset, 45); assert_eq!(op.args, vec![item0.into(), item1.into(), item2.into()]); @@ -15001,9 +14823,20 @@ mod tests { lower_frontend_collection_ops(&graph); let block = start.borrow(); - let ops = &block.operations; - // new_array_clear(Const(2)) + 2× setarrayitem_gc_r + newlist_from_array. + // new_array_clear(Const(2)) + 2× setarrayitem_gc_r + newlist_from_array; + // the can-raise `newlist` HLOp's structural trailing `-live-` + // survives the lowering after the `*_from_array` residual. + let ops: Vec<_> = block + .operations + .iter() + .filter(|op| op.opname != super::super::flatten::OPNAME_LIVE) + .cloned() + .collect(); assert_eq!(ops.len(), 4); + assert_eq!( + block.operations.last().map(|op| op.opname.as_str()), + Some(super::super::flatten::OPNAME_LIVE), + ); assert_eq!(ops[0].opname, "new_array_clear"); assert_eq!( ops[0].args, @@ -15042,8 +14875,14 @@ mod tests { lower_frontend_collection_ops(&graph); let block = start.borrow(); - let ops = &block.operations; - // new_array_clear(Const(1)) + 1× setarrayitem_gc_r + newtuple_from_array. + // new_array_clear(Const(1)) + 1× setarrayitem_gc_r + newtuple_from_array, + // followed by the can-raise HLOp's structural trailing `-live-`. + let ops: Vec<_> = block + .operations + .iter() + .filter(|op| op.opname != super::super::flatten::OPNAME_LIVE) + .cloned() + .collect(); assert_eq!(ops.len(), 3); assert_eq!(ops[0].opname, "new_array_clear"); assert_eq!(ops[1].opname, "setarrayitem_gc_r"); @@ -15069,11 +14908,7 @@ mod tests { 47, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("BUILD_SLICE argc=2 should record newslice"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "newslice"); assert_eq!(op.offset, 47); assert_eq!(op.args[0], w_start.into()); @@ -15109,11 +14944,7 @@ mod tests { 48, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("BUILD_SLICE argc=3 should record newslice"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "newslice"); assert_eq!(op.offset, 48); assert_eq!(op.args, vec![w_start.into(), w_stop.into(), w_step.into()]); @@ -15138,11 +14969,7 @@ mod tests { 55, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("setitem op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "setitem"); assert_eq!(op.offset, 55); assert_eq!(op.args, vec![obj.into(), key.into(), value.into()]); @@ -15201,11 +15028,7 @@ mod tests { 57, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("getattr op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "getattr"); assert_eq!(op.offset, 57); assert_eq!( @@ -15236,11 +15059,7 @@ mod tests { 66, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("binary op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "inplace_add"); assert_eq!(op.offset, 66); assert_eq!(op.args, vec![lhs.into(), rhs.into()]); @@ -15263,11 +15082,7 @@ mod tests { 77, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("compare op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "le"); assert_eq!(op.offset, 77); assert_eq!(op.args, vec![lhs.into(), rhs.into()]); @@ -15283,8 +15098,7 @@ mod tests { let result = emit_frontend_bool(&mut graph, &start, operand.into(), 78); assert_eq!(result.kind, Some(Kind::Int)); - let block = start.borrow(); - let op = block.operations.last().expect("bool op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "bool"); assert_eq!(op.offset, 78); assert_eq!(op.args, vec![operand.into()]); @@ -15394,11 +15208,7 @@ mod tests { 88, ); - let block = start.borrow(); - let op = block - .operations - .last() - .expect("simple_call op should be recorded"); + let op = last_recorded_op(&start); assert_eq!(op.opname, "simple_call"); assert_eq!(op.offset, 88); assert_eq!( @@ -16640,47 +16450,6 @@ def f(i): assert!(catch_state.last_exception.is_some()); } - #[test] - fn explicit_raise_stack_value_preserves_exception_link_kinds() { - let code = first_nested_function_code("def f(a):\n return a\n"); - let mut graph = new_shadow_graph(&code); - let catch_block = graph.new_block(Vec::new()); - let catch_ref = SpamBlockRef::new(catch_block, None); - let source_state = FrameState::new(Vec::new(), Vec::new(), None, Vec::new(), 0); - let startblock_ref = graph.startblock.clone(); - let site = synthetic_catch_site(&catch_ref); - let link = attach_catch_exception_edge( - &code, - &mut graph, - &startblock_ref, - &catch_ref, - &source_state, - &site, - ); - let (last_exception, last_exc_value) = { - let link = link.borrow(); - (link.last_exception, link.last_exc_value) - }; - - carry_explicit_raise_value_on_catch_stack( - &link, - &catch_ref, - Variable::new(VariableId(100), Kind::Ref).into(), - ); - - let link = link.borrow(); - assert_eq!(link.last_exception, last_exception); - assert_eq!(link.last_exc_value, last_exc_value); - assert_eq!( - link.last_exception.and_then(|variable| variable.kind), - Some(Kind::Int) - ); - assert_eq!( - link.last_exc_value.and_then(|variable| variable.kind), - Some(Kind::Ref) - ); - } - #[test] fn attach_catch_exception_edge_populates_target_inputargs() { let code = first_nested_function_code("def f(a):\n return a\n"); diff --git a/pyre/pyre-jit/src/jit/flatten.rs b/pyre/pyre-jit/src/jit/flatten.rs index a88a87b39e5..a186922640f 100644 --- a/pyre/pyre-jit/src/jit/flatten.rs +++ b/pyre/pyre-jit/src/jit/flatten.rs @@ -1314,6 +1314,20 @@ impl<'a> GraphFlattener<'a> { if self.lowering_ctx.is_some() && is_pyre_canonical_elidable_hlop(&op.opname) { return; } + // Forced-alive `-live-` args (`liveness.py:8-12`) are graph-side + // information: `make_dependencies` reads them off the graph op to + // keep co-live frame slots interfering (and so on distinct colors). + // The serialized marker stays argless — `compute_liveness` derives + // the runtime resume window from actual uses, so a mid-body entry + // (bridge sub-walk reconstruction) is never asked to source a + // register whose value only lives in the virtualizable. + let stripped_live; + let op = if self.lowering_ctx.is_some() && op.opname == OPNAME_LIVE && !op.args.is_empty() { + stripped_live = SpaceOperation::new(OPNAME_LIVE, Vec::new(), None, op.offset); + &stripped_live + } else { + op + }; // Record FIRST insn position per // non-negative `op.offset` (Python PC) into // `ssarepr.pc_first_insn_pos`. Drives `pc_map` construction at @@ -1321,7 +1335,12 @@ impl<'a> GraphFlattener<'a> { // `offset = -1` (insert_renamings ref_copy / overflow // trampolines / catch-landing entries) are skipped — they have // no Python PC counterpart. Sparse `Vec<(py_pc, first_insn_pos)>`. - if op.offset >= 0 { + // `-live-` markers are also skipped: a PC whose only insn is its + // own resume marker must stay "stack-only" for the resolver + // (`derive_pc_live_indices_from_sparse`'s can-raise fallthrough + // re-key fires only for PCs with no real op), matching the pc + // ownership the stream had before per-PC marker emission. + if op.offset >= 0 && op.opname != OPNAME_LIVE { let py_pc = op.offset; let already_seen = self .ssarepr @@ -1698,7 +1717,7 @@ impl<'a> GraphFlattener<'a> { } } - fn insert_exits(&mut self, block: &BlockRef, handling_ovf: bool, block_emit_start: usize) { + fn insert_exits(&mut self, block: &BlockRef, handling_ovf: bool) { let exits = block.borrow().exits.clone(); if exits.len() == 1 { // `flatten.py:181 assert link.exitcase in (None, False, True)` @@ -1724,29 +1743,27 @@ impl<'a> GraphFlattener<'a> { must be None / False / True, got {other:?}" ), } - // Explicit `raise X` inside a try block. pyre's walker - // (`emit_raise!` → `emit_catch_exception!`) wires the raise's - // exception edge directly to its catch landing — a single - // exit (exitcase=None, `block.canraise()`) — instead of to - // `graph.exceptblock` (where `make_link`→`make_exception_link` - // would emit `raise link.args[1]`). The `raise` op itself is - // walker-inline-only (no graph SpaceOp), so without this arm - // the canonical flatten lowers the block as a plain - // `make_link` goto into the landing: the spliced block then - // falls into the landing's `last_exception` with no pending - // exception, and blackhole `handler_last_exception` reads a - // null `exception_last_value`. Reproduce the inline shape — - // `raise ` then a byte-adjacent `catch_exception ` - // dispatch — using the raised value recorded on the link - // (`explicit_raise_value`). `L` is the handler entry placed + // Explicit `raise X` inside a try block. The walker records + // the `raise` op as the block's LAST operation (serialized by + // the body loop above) and closes the block with a single + // exception exit to the catch landing (`exitswitch == + // c_last_exception`, extravars seeded, the raised value + // threaded through the link args at the landing's exception + // stack slot). Emit the byte-adjacent `catch_exception ` + // dispatch and lower the handler entry through + // `make_exception_link` — `L` is the handler entry placed // right before `make_exception_link`'s `last_exception` / // `last_exc_value` emission, mirroring the canraise arm's // `catch_exception TLabel(normal) … Label(normal) … // make_exception_link` layout (flatten.py:139-180). - let explicit_raise_value = link.borrow().explicit_raise_value; - if let Some(raised) = explicit_raise_value { - let raise_operand = self.getcolor(&raised.into()); - self.emitline(Insn::op("raise", vec![raise_operand])); + if block.borrow().canraise() { + debug_assert!( + block + .borrow() + .raising_op() + .is_some_and(|op| op.opname == "raise"), + "single-exit canraise block must be raise-terminated" + ); let catch_label = self.tlabel_for_link(link); self.emitline(Insn::op("catch_exception", vec![catch_label])); let handler_label = self.label_for_link(link); @@ -1832,119 +1849,26 @@ impl<'a> GraphFlattener<'a> { // that survived metainterp policy but lack a real // raising-op-with-trailing-`-live-` pattern. // - // Vable ops are baked into the graph here: upstream - // blocks end at `[raising_op, -live-]` because flowspace's - // `guessexception` (flowcontext.py:130-156) closes the block at - // each can-raise op. Pyre's walker does NOT split there, so the - // raising op's vable-mirror stores (setfield_vable_i / - // setarrayitem_vable_r for the post-op frame state) follow the - // `-live-` in the SAME block — the block's last op is a vable - // store, never `-live-`. The graph-tail `last_op_is_live` scan - // therefore reports `index == -1` for EVERY pyre canraise block - // (verified raise_catch_loop: 50/50 blocks), dropping all - // `catch_exception` emission and stranding the exception edge. - // On the canonical (lowering_ctx) path detect the raising op off - // the EMITTED stream instead: `serialize_op` lowers HLOps - // (mod/eq/add/simple_call) and residual calls to - // `residual_call_*` and appends a trailing `-live-` exactly when - // `insn_needs_trailing_live` (calldescr_canraise), so the same - // predicate over the block's emitted insns recognises a real - // raising op regardless of the trailing vable stores. Spurious - // canraise blocks (empty joinpoints, vable-only, non-raising - // residual_call_r_r/r_v) carry no such insn and keep the - // early-return. The block has at most one raising op (verified - // raise_catch_loop), so one `catch_exception` at block end — - // after the raising op's vable stores, matching the production - // walker's per-PC `emit_catch_exception!` placement — is - // correct. Graph CFG is untouched, so regalloc / gate-off - // bytes are unchanged. - let block_can_raise = if self.lowering_ctx.is_some() { - self.ssarepr.insns[block_emit_start..] - .iter() - .any(insn_needs_trailing_live) - } else { - block - .borrow() - .operations - .last() - .map_or(false, |op| op.opname == OPNAME_LIVE) - }; + // The walker closes a caught can-raise op's block at the + // op's trailing `-live-` (`flowcontext.py:130-156 + // guessexception` shape); the post-op vable-mirror stores + // land in the normal-flow successor, so the graph tail is + // structurally `[raising_op, -live-]` and `catch_exception` + // lands directly after the `-live-` — the adjacency + // `handle_exception_in_frame` (`blackhole.py:396`) and + // `derive_after_call_indices_from_sparse` (the + // after-residual-call resume anchor) both require. + let block_can_raise = block + .borrow() + .operations + .last() + .map_or(false, |op| op.opname == OPNAME_LIVE); if !self.include_all_exc_links && !block_can_raise { self.make_link(&normal_link, false); return; } - // RPython flowspace (`flowcontext.py:130-156 guessexception`) - // closes a canraise block at the raising op, so its post-call - // `-live-` is the block's last insn and `catch_exception` lands - // directly after it — the adjacency `handle_exception_in_frame` - // (`blackhole.py:396`) and `derive_after_call_indices_from_sparse` - // (the after-residual-call resume anchor) both require. Pyre - // does not split there, so the raising op's post-op vable-mirror - // stores (`setfield_vable_i` / `setarrayitem_vable_r` for the - // post-op frame state) were serialized into THIS block after the - // `-live-`. Hoist `catch_exception` to directly follow that - // `-live-` and move the trailing stores after it; `catch_exception` - // is a no-op on the normal fall-through (`bhimpl_catch_exception`), - // so the stores still execute there, while the exception path now - // finds the catch one op past the resume `-live-`. - let hoisted_tail: Vec = { - let raising_rel = self.ssarepr.insns[block_emit_start..] - .iter() - .rposition(insn_needs_trailing_live); - match raising_rel { - Some(rel) => { - let live_idx = block_emit_start + rel + 1; - let has_trailing_stores = live_idx + 1 < self.ssarepr.insns.len(); - if self.ssarepr.insns.get(live_idx).is_some_and(Insn::is_live) - && has_trailing_stores - { - // Inserting one `catch_exception` before the moved - // stores shifts their positions by one; keep the - // sparse `pc_first_insn_pos` anchors aligned. - for (_pc, pos) in self.ssarepr.pc_first_insn_pos.iter_mut() { - if *pos > live_idx { - *pos += 1; - } - } - self.ssarepr.insns.split_off(live_idx + 1) - } else { - Vec::new() - } - } - None => Vec::new(), - } - }; let catch_label = self.tlabel_for_link(&normal_link); self.emitline(Insn::op("catch_exception", vec![catch_label])); - // flatten.py:206-220 statically guarantees `catch_exception` - // directly follows the raising op's trailing `-live-` (the block is - // closed at the canraise op, then the trailing-`-live-` walk-back - // places the catch right after it). Pyre rebuilds that adjacency - // via the hoist above rather than by block-splitting, so the - // invariant is not structural — enforce it here. Both - // `handle_exception_in_frame` (blackhole.py:396, skip one `-live-` - // then expect the catch) and `derive_after_call_indices_from_sparse` - // (resume anchor keyed on `insns[catch-1].is_live()`) silently - // mis-resume if a vable-mirror store serialized between the two. - // Pyre blocks may hold several can-raise ops (the walker does not - // block-split like flowspace); only the caught op — the last raising op, - // which the hoist above targets — needs this adjacency, so the invariant - // enforced here is adjacency, not a per-block can-raise-op count. - assert!( - !block_can_raise - || self - .ssarepr - .insns - .iter() - .rev() - .nth(1) - .is_some_and(Insn::is_live), - "catch_exception must directly follow the raising op's -live- \ - (block_emit_start={block_emit_start})", - ); - for insn in hoisted_tail { - self.emitline(insn); - } self.make_link(&normal_link, false); let normal_label = self.label_for_link(&normal_link); self.emitline(normal_label); @@ -2330,12 +2254,6 @@ impl<'a> GraphFlattener<'a> { let operations = block.borrow().operations.clone(); let exits_len = block.borrow().exits.len(); let exitswitch_is_last_exception = block.borrow().canraise(); - // First emitted-insn index of this block's serialized ops, so - // `insert_exits` can detect a real raising op off the lowered - // stream (a `residual_call_*` whose calldescr can raise) rather - // than the graph tail, which pyre's vable-mirror stores push past - // the `-live-`. See the raising-op detection in `insert_exits`. - let block_emit_start = self.ssarepr.insns.len(); for op in &operations { // `flatten.py:120-125` `_ovf` validity check: an overflow- // checked op must live in a canraise block with 2 or 3 @@ -2356,7 +2274,7 @@ impl<'a> GraphFlattener<'a> { } self.serialize_op(op); } - self.insert_exits(&block, handling_ovf, block_emit_start); + self.insert_exits(&block, handling_ovf); } fn flatten_space_operation(&mut self, op: &SpaceOperation) -> Insn { @@ -2630,6 +2548,99 @@ fn insn_needs_trailing_live(insn: &Insn) -> bool { false } +/// `flowcontext.py:130-156 guessexception` attaches an exception edge +/// only to an operation that can raise; the walker's per-PC catch-attach +/// site consults this classifier on the graph `SpaceOperation`s it just +/// recorded to decide whether the opcode closes its block with an +/// exception edge (and the structural `[op, -live-]` tail). +/// +/// * `residual_call_*` — `call.py:353-355 calldescr_canraise`, read off +/// the recorded `CallDescrStub` descr operand (the graph-side twin of +/// [`insn_needs_trailing_live`]'s emitted-stream rule). +/// * `inline_call_*` — always (`jtransform.py:481 handle_regular_call`). +/// * pre-rtype HLOps — the `calldescr_canraise` of the residual call the +/// lowering dispatcher produces for the opname; the arms below mirror +/// the per-family `CallFlavor` each `lower_*_hlop_to_insn` records +/// (`MayForce`/`Plain` → can raise, `PlainCannotRaise*` → cannot). +/// `newlist`/`newtuple` classify as their post- +/// `lower_frontend_collection_ops` `*_from_array` residual. +/// * everything else (vable mirrors, `ptr_*` tests, copies, markers, +/// elidable HLOps) — cannot raise. +pub fn graph_op_can_raise(op: &super::flow::SpaceOperation) -> bool { + let opname = op.opname.as_str(); + if opname.starts_with("residual_call") { + return op.args.iter().any(|arg| match arg { + super::flow::SpaceOperationArg::Descr(descr) => descr + .0 + .as_any() + .and_then(|any| any.downcast_ref::()) + .is_some_and(|stub| stub.effect_info.check_can_raise(false)), + _ => false, + }); + } + if opname.starts_with("inline_call") { + return true; + } + // BINARY_OP / COMPARE_OP families (both lower with `MayForce`). + if binary_op_tag_for_opname(opname).is_some() || compare_op_tag_for_opname(opname).is_some() { + return true; + } + // `load_method_self`, `store_deref_value`, `unbound_local_error` + // lower with `PlainCannotRaise` and are intentionally absent. + matches!( + opname, + "bool" + | "setitem" + | "store_slice" + | "load_name" + | "store_name" + | "store_global" + | "simple_call" + | "getattr" + | "load_special" + | "load_special_self" + | "load_fast_check" + | "store_attr" + | "binary_slice" + | "newslice" + | "format_simple" + | "format_with_spec" + | "convert_value" + | "load_common_constant" + | "load_deref_value" + | "make_cell_value" + | "make_function_value" + | "set_function_attribute" + | "neg" + | "invert" + | "pos" + | "list_to_tuple" + | "not_" + | "import_name" + | "import_from" + | "load_from_dict_or_globals" + | "load_super_attr" + | "super_attr_unwrap" + | "delete_subscr" + | "delete_attr" + | "list_extend" + | "set_add" + | "set_update" + | "dict_update" + | "map_add" + | "dict_merge" + | "call_kw" + | "call_function_ex" + | "newlist" + | "newtuple" + | "newlist_from_array" + | "newtuple_from_array" + | "build_map_from_array" + | "build_set_from_array" + | "build_string_from_array" + ) +} + /// `call.py:353-355 calldescr_canraise` — `calldescr.get_extra_info() /// .check_can_raise()` (default `ignore_memoryerror=False`). Reads the /// `EffectInfo` off the residual call's trailing `CallDescrStub` operand; diff --git a/pyre/pyre-jit/src/jit/flow.rs b/pyre/pyre-jit/src/jit/flow.rs index d044a2e748f..bf6430eeb49 100644 --- a/pyre/pyre-jit/src/jit/flow.rs +++ b/pyre/pyre-jit/src/jit/flow.rs @@ -794,19 +794,6 @@ pub struct Link { /// Exception-passing variables attached to the edge. pub last_exception: Option, pub last_exc_value: Option, - /// Raised-value Variable for an explicit-raise edge (pyre adaptation). - /// - /// pyre's `attach_catch_exception_edge` wires an explicit `raise X` - /// inside a try block directly to its catch landing rather than to - /// `graph.exceptblock` (where RPython's `make_exception_link` would - /// read `link.args[1]` as the raised value). The landing's - /// `last_exception` / `last_exc_value` are fresh read-back Variables, - /// not the value that was raised, so the canonical flatten cannot - /// recover the `raise` operand from the link args. This field carries - /// the normalized raised value so `insert_exits` can emit - /// `raise ` on the single-exit explicit-raise arm. - /// `None` for ordinary (op-canraise) exception edges. - pub explicit_raise_value: Option, } impl Link { @@ -834,7 +821,6 @@ impl Link { prevblock: None, last_exception: None, last_exc_value: None, - explicit_raise_value: None, } } @@ -886,7 +872,6 @@ impl Link { newlink.prevblock = self.prevblock.clone(); newlink.last_exception = self.last_exception.map(&mut rename); newlink.last_exc_value = self.last_exc_value.map(&mut rename); - newlink.explicit_raise_value = self.explicit_raise_value.map(&mut rename); newlink.llexitcase = self.llexitcase.clone(); newlink } @@ -982,9 +967,17 @@ impl Block { } /// `model.py:219-222` `Block.raising_op`. + /// + /// A caught can-raise operation carries a structural trailing + /// `-live-` marker (`jtransform.py:311-313`), so the raising op is + /// the last NON-`-live-` operation — the same back-scan + /// `flatten.py:206-217` performs. pub fn raising_op(&self) -> Option<&SpaceOperation> { if self.canraise() { - self.operations.last() + self.operations + .iter() + .rev() + .find(|op| op.opname != super::flatten::OPNAME_LIVE) } else { None } diff --git a/pyre/pyre-jit/src/jit/regalloc.rs b/pyre/pyre-jit/src/jit/regalloc.rs index 22d3eb02b81..5e0f4efa199 100644 --- a/pyre/pyre-jit/src/jit/regalloc.rs +++ b/pyre/pyre-jit/src/jit/regalloc.rs @@ -368,34 +368,6 @@ impl<'a> RegAllocator<'a> { } } - /// Record an interference edge between two frame-local slot - /// representatives so the chordal coloring assigns them DISTINCT - /// colors, even when their SSA register live ranges are disjoint. - /// - /// Complement of [`try_coalesce_pin_ids`]: that merges Variables onto - /// one color, this forces them apart. Used by the splice regalloc to - /// reproduce the walker's bijective slot→register assignment so the - /// per-slot resume reverse map is injective. Both endpoints are - /// projected through `_unionfind.find_rep`, so a slot whose Variables - /// were already coalesced into one rep (by the same-slot coalesce - /// pairs) contributes a single node; self-edges are skipped before - /// calling the shared RPython `DependencyGraph`. `add_node` registers - /// each rep in `_depgraph.all_nodes` so `find_node_coloring`'s - /// `getnodes` filter keeps it (matching `try_coalesce_pin_ids`). - fn add_interference_pin_ids( - &mut self, - v_id: super::flow::VariableId, - w_id: super::flow::VariableId, - ) { - let v0 = self._unionfind.find_rep(v_id); - let w0 = self._unionfind.find_rep(w_id); - self._depgraph.add_node(v0); - self._depgraph.add_node(w0); - if v0 != w0 { - self._depgraph.add_edge(v0, w0); - } - } - fn find_node_coloring(&mut self) { self._coloring = self ._depgraph @@ -475,33 +447,6 @@ pub fn perform_register_allocation_with_pairs( graph: &FlowGraph, kind: Kind, extra_coalesce_pairs: &[(super::flow::VariableId, super::flow::VariableId)], -) -> GraphAllocationResult { - perform_register_allocation_with_pairs_and_interference(graph, kind, extra_coalesce_pairs, &[]) -} - -/// Splice adaptation: like [`perform_register_allocation_with_pairs`] but -/// also records an interference edge between the union-find reps named in -/// each `interference_pairs` entry, forcing those reps onto DISTINCT -/// colors. -/// -/// `interference_pairs` is the complement of `extra_coalesce_pairs`: -/// where the coalesce pairs merge same-slot Variables onto one color, the -/// interference pairs separate distinct frame-local slots whose SSA -/// register live ranges happen to be disjoint (each `LOAD_FAST` re-reads -/// the local, so a local's SSA value dies between reads). Without this -/// the chordal coloring is free to give two frame-live locals one color, -/// and the splice resume reverse map (`pcdep_color_slots` → -/// `semantic_ref_slot_for_reg_color`) collapses them onto one slot. -/// The edges are added after `make_dependencies` (the base liveness graph -/// must exist) and before `coalesce_variables` (so a cross-slot coalesce -/// is blocked by the `try_coalesce` `has_edge` guard) and -/// `find_node_coloring`. Splice-only — production callers pass `&[]`, -/// leaving the coloring byte-identical. -pub fn perform_register_allocation_with_pairs_and_interference( - graph: &FlowGraph, - kind: Kind, - extra_coalesce_pairs: &[(super::flow::VariableId, super::flow::VariableId)], - interference_pairs: &[(super::flow::VariableId, super::flow::VariableId)], ) -> GraphAllocationResult { // `rpython/tool/algo/regalloc.py:11-15`: // regalloc = RegAllocator(graph, consider_var, ListOfKind) @@ -528,13 +473,6 @@ pub fn perform_register_allocation_with_pairs_and_interference( } } allocator.make_dependencies(); - // Record interference between the named slot reps so the - // chordal coloring keeps distinct frame-local slots on distinct - // colors. Added after `make_dependencies` so the base graph exists, - // before `coalesce_variables`/`find_node_coloring` so both honour it. - for &(a_id, b_id) in interference_pairs { - allocator.add_interference_pin_ids(a_id, b_id); - } allocator.coalesce_variables(); // External pins — re-apply via `try_coalesce_pin_ids` after // `make_dependencies` so the surviving rep is explicitly added to @@ -625,33 +563,18 @@ pub fn perform_register_allocation_with_pairs_and_interference( /// canonical coloring still matches the walker's emit, while the /// interfering `(i, PHI)` pair is rejected. /// -/// `extra_interference` seeds additional edges into the dependency graph -/// (via `add_interference_pin_ids`) before the coalesce replay, so a pair -/// whose endpoints are co-live under a liveness `make_dependencies` does not -/// see — the CPython-slot co-live locals whose SSA ranges are disjoint — -/// is also rejected. Callers pass `&[]` to honour SSA-liveness alone. +/// The per-PC `-live-` graph ops carry every frame-live Ref Variable as a +/// force-alive arg (`liveness.py:8-12`), so `make_dependencies` here +/// already models CPython frame-slot liveness: a pair whose endpoints' +/// frame lifetimes overlap interferes structurally and is rejected by the +/// same `has_edge` guard — no external interference seeding. pub fn filter_coalesce_pairs_by_interference( graph: &FlowGraph, kind: Kind, pairs: &[(super::flow::VariableId, super::flow::VariableId)], - extra_interference: &[(super::flow::VariableId, super::flow::VariableId)], ) -> Vec<(super::flow::VariableId, super::flow::VariableId)> { let mut allocator = RegAllocator::new(graph, kind); allocator.make_dependencies(); - // Inject caller-supplied interference edges into the dependency graph - // before replaying `_try_coalesce`, so the `has_edge` guard (regalloc.py:105) - // rejects a coalesce whose endpoints are separated by an interference the - // SSA `make_dependencies` graph does not model. The splice caller - // (codewriter.rs) supplies the slot-identity edges - // (`build_slot_disjoint_interference`): two Variables at distinct CPython - // frame slots whose SSA live ranges are disjoint between `LOAD_FAST` - // re-reads. Injecting them here is what retired the walker-slot - // `filter_cross_slot_coalesce_pairs` — slot rejection now happens through - // this `has_edge` guard rather than a separate post-filter. An empty - // slice is a no-op: the filter then honours SSA-liveness interference only. - for &(a_id, b_id) in extra_interference { - allocator.add_interference_pin_ids(a_id, b_id); - } let mut kept = Vec::with_capacity(pairs.len()); for &(v_id, w_id) in pairs { if v_id == w_id { @@ -725,29 +648,6 @@ pub fn perform_register_allocation_all_kinds_with_pairs( ] } -/// Like [`perform_register_allocation_all_kinds_with_pairs`] but also -/// records `ref_interference_pairs` as Ref-kind interference edges (the -/// liveness-correct CPython-co-live separation that keeps two frame -/// locals simultaneously live at a guard on distinct colors). Int and -/// Float take the empty-pair path — `walker_slot_for_variable` tracks -/// only Ref slots. -pub fn perform_register_allocation_all_kinds_with_pairs_and_interference( - graph: &FlowGraph, - ref_coalesce_pairs: &[(super::flow::VariableId, super::flow::VariableId)], - ref_interference_pairs: &[(super::flow::VariableId, super::flow::VariableId)], -) -> [GraphAllocationResult; 3] { - [ - perform_register_allocation(graph, Kind::Int), - perform_register_allocation_with_pairs_and_interference( - graph, - Kind::Ref, - ref_coalesce_pairs, - ref_interference_pairs, - ), - perform_register_allocation(graph, Kind::Float), - ] -} - /// Mirrors `rpython/jit/codewriter/flatten.py:88-100 enforce_input_args` /// at the graph level (sibling to the SSA-side private /// `enforce_ssarepr_input_args` further down that handles per- @@ -1592,15 +1492,16 @@ mod tests { } #[test] - fn filter_coalesce_pairs_by_interference_extra_edge_rejects_ssa_disjoint_pair() { + fn filter_coalesce_pairs_by_interference_live_marker_rejects_ssa_disjoint_pair() { // v0 (inputarg) is copied into v1 and dies at the copy, so v0 and v1 // have DISJOINT SSA live ranges: `make_dependencies` records no edge - // between them and the coalesce pair (v0, v1) is accepted. A - // caller-supplied `extra_interference` edge models a CPython-slot - // co-live separation the SSA graph cannot see (two locals co-live at a - // guard across `LOAD_FAST` re-reads); the `has_edge` guard must then - // reject the pair. - let build = || { + // between them and the coalesce pair (v0, v1) is accepted. A `-live-` + // graph op carrying both as force-alive args (`liveness.py:8-12`) + // models a CPython-slot co-live separation the plain SSA graph cannot + // see (two locals co-live at a guard across `LOAD_FAST` re-reads); + // `make_dependencies` then records the edge and the `has_edge` guard + // rejects the pair. + let build = |with_live_marker: bool| { let v0 = flow_var(0, Kind::Ref); let v1 = flow_var(1, Kind::Ref); let start = Block::shared(vec![v0.into()]); @@ -1609,6 +1510,17 @@ mod tests { &start, SpaceOperation::new("ref_copy", vec![v0.into()], Some(v1.into()), 0), ); + if with_live_marker { + push_op( + &start, + SpaceOperation::new( + super::super::flatten::OPNAME_LIVE, + vec![v0.into(), v1.into()], + None, + 0, + ), + ); + } start.closeblock(vec![ Link::new(vec![v1.into()], Some(graph.returnblock.clone()), None).into_ref(), ]); @@ -1616,21 +1528,20 @@ mod tests { }; // SSA-liveness only: the disjoint pair is accepted. - let (g_ssa, a, b) = build(); - let kept_ssa = filter_coalesce_pairs_by_interference(&g_ssa, Kind::Ref, &[(a, b)], &[]); + let (g_ssa, a, b) = build(false); + let kept_ssa = filter_coalesce_pairs_by_interference(&g_ssa, Kind::Ref, &[(a, b)]); assert_eq!( kept_ssa, vec![(a, b)], - "SSA-disjoint pair must be accepted when no extra interference is supplied" + "SSA-disjoint pair must be accepted without a forcing -live- marker" ); - // With an injected co-live edge, the has_edge guard rejects it. - let (g_colive, a, b) = build(); - let kept_colive = - filter_coalesce_pairs_by_interference(&g_colive, Kind::Ref, &[(a, b)], &[(a, b)]); + // With a force-alive `-live-` marker, the has_edge guard rejects it. + let (g_colive, a, b) = build(true); + let kept_colive = filter_coalesce_pairs_by_interference(&g_colive, Kind::Ref, &[(a, b)]); assert!( kept_colive.is_empty(), - "an injected co-live interference edge must reject the otherwise-accepted pair" + "a forcing -live- marker must reject the otherwise-accepted pair" ); }