walker concrete-execute for non-elidable residual_calls + Phase 5.B body wire - #148
Merged
Conversation
WalkthroughThis PR implements concrete execution of non-elidable residual calls across the walker and JIT evaluation loop. A new executor helper performs concrete function calls with exception capture via ChangesNon-elidable residual-call concrete execution across walker and JIT
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Replace `unreachable!()` in the Phase 5.B arm of `eval_loop_jit` with
a call into `dispatch_arm_via_blackhole`. The stub returns
`unimplemented!()` because `production_blackhole_handles` is `false`
everywhere today, so the path is unreachable at runtime.
Locks in the signature
fn dispatch_arm_via_blackhole(
frame: &mut PyFrame,
instruction: &Instruction,
) -> Result<StepResult<PyObjectRef>, PyError>
so subsequent slices fill the body (arm jitcode lookup, BH builder
acquire/release, register marshalling, `interp.run()`) without
revisiting the call site.
Assisted-by: Claude
Slice 3.1 of Phase 5.B body. Replace the leading `let _ = (...)` with a real `jitcode_for_instruction(instruction)` lookup that returns the arm `Arc<JitCode>`; map `None` to a `SystemError` PyError, mirroring the walker-side fallback in `MIFrame::dispatch_via_walker_for_opcode`. Remaining body (BH builder acquire/release, register marshalling, `bh.run()`) is still `unimplemented!()`. `production_blackhole_ handles` is `false` everywhere, so the path is unreachable at runtime; behavior is byte-identical. Assisted-by: Claude
Slice 3.2 of Phase 5.B body. Add `metainterp_jitcode_for_instruction` / `_for_arm` / `_by_index` to pyre-jit-trace, parallel to the existing canonical-core lookups. Cache wraps `Arc<majit_translate::jitcode::JitCode>` (walker form) into `Arc<majit_metainterp::jitcode::JitCode>` (BH form) via `JitCode::from_canonical`, lazily per-(thread × jitcode-index). `thread_local!` mirrors `ALL_JITCODES`'s !Sync constraint. Build-time canonical arm jitcodes have no per-jitcode descrs (mod.rs:400-403 — they resolve through `ALL_DESCRS`), so each wrapper's `exec` field stays empty. Same `.code` / constants / live slots; only the descr pool field differs. `dispatch_arm_via_blackhole` now resolves jitcode through the metainterp variant, ready for slice 3.3 (acquire_interp + set `bh.jitcode`). Predicate still `false` everywhere; path unreachable; byte-identical. Assisted-by: Claude
Slice 3.3 of Phase 5.B body. Add a `BH_BUILDER_ARM` thread_local in `dispatch_arm_via_blackhole` mirroring `call_jit.rs::blackhole_resume_via_rd_numb`'s `BH_BUILDER_RD` pattern. Separate pool from the resume-data path so mutable borrows cannot collide on re-entry. Inside the `.with` closure: sync cached control opcodes, acquire a BH interp, install `jitcode = metainterp_jitcode_for_instruction(..)` and `position = 0`, then release. Register marshalling + `bh.run()` lands in slice 3.4+. Predicate `production_blackhole_handles` is still `false` everywhere; the path is unreachable; the thread_local stays uninitialised in production. Byte-identical. Assisted-by: Claude
Slice 3.4 of Phase 5.B body. Inside `BH_BUILDER_ARM.with`, after acquire_interp and jitcode install, wire: bh.virtualizable_ptr = frame as *mut PyFrame as i64 bh.virtualizable_info = get_virtualizable_info() bh.registers_r[0] = virtualizable_ptr // RPython MIFrame.setup Mirrors `call_jit.rs:1400-1423` minus the caller-chain propagation (single-frame arm dispatch has no `nextblackholeinterp`). The r0 = frame seed matches `trace_opcode.rs:6657-6660`'s walker entry. Predicate still `false` everywhere; path unreachable; byte-identical. Slice 3.5+ adds per-opcode-family register marshalling + `bh.run()`. Assisted-by: Claude
Slice 3.5 of Phase 5.B body.
Run the arm jitcode via `bh.run()` inside the BH_BUILDER_ARM closure,
then extract `mergepoint_args`, `got_exception`, `exception_last_
value`, and `return_type` before `release_interp(bh)` consumes the
interp.
Triage outside the closure:
* `got_exception=true` → todo!() (slice 3.6: BH exc → PyError)
* `mergepoint_args=Some(_)` → unreachable!() (opcode arms never issue
jit_merge_point — only portal arms do)
* otherwise → `Ok(StepResult::Continue)`
(void-return arms; per-return-type push
handling lands in slice 3.7)
Predicate still `false` everywhere; path unreachable; byte-identical.
Assisted-by: Claude
Slice 3.6 of Phase 5.B body.
Replace the `todo!()` on the `got_exception` branch with the chain-
exit pattern from `call_jit.rs:1486-1506`:
let err = if exception_value != 0 {
unsafe { PyError::from_exc_object(exception_value as PyObjectRef) }
} else {
PyError::new(RuntimeError, "...") // defensive: null exc_value
};
return Err(err);
Predicate still `false` everywhere; path unreachable; byte-identical.
Assisted-by: Claude
Slice 3.7 of Phase 5.B body. Replace the bare `Ok(StepResult::Continue)` with a match on `return_type`: * BhReturnType::Void → Continue (arm already mutated heap, no push) * Int / Ref / Float → todo!() (slice 3.8: tmpreg_* → PyFrame push) Void is the StoreSubscr-activation target and lands first; non-void result marshalling is opcode-shape-specific. Predicate still `false` everywhere; path unreachable; byte-identical. Assisted-by: Claude
Slice 3.8 of Phase 5.B body.
Replace the manual
bh.jitcode = jitcode;
bh.position = 0;
with
bh.setposition(jitcode, 0);
`setposition` (blackhole.py:312 / blackhole.rs:491) calls
`init_register_files_from_runtime_jitcode` which sizes registers_i /
registers_r / registers_f to `num_regs_X + constants_X.len()` and
copies the constants pool into the upper portion of each bank.
Without that, the first `bhimpl_*` that loads a constant would read
unwritten memory.
Predicate still `false` everywhere; path unreachable; byte-identical.
Assisted-by: Claude
…alls Add a "PyPy parity gap" section to `try_fold_pure_call_via_executor`'s doc comment recording the diagnosis behind the M4 walker SIGBUS root cause: PyPy `_opimpl_*` concrete-execute every `do_residual_call` regardless of `check_is_elidable()`, while pyre's walker executes only the elidable path. The Phase 5.B `dispatch_arm_via_blackhole` path (commits 6bc48d3a16..6b88234279) is a workaround that runs the arm jitcode through BlackholeInterpreter instead; the orthodox alignment widens this function to cover Call* / CallMayForce* with concrete- known args, mirroring `executor.execute_varargs(opnum, argboxes, descr, exc=can_raise, pure=is_elidable)`. Multi-session work tracked in task #390 — out of scope here. Assisted-by: Claude
…te gap Task #390 sub-slice 1 — document the per-EI-branch concrete-execute status table inside `select_residual_call_opcode` so sub-slice 2 (widening) has a single authoritative reference. Maps each branch's recorded opcode to PyPy's `executor.execute_varargs(opnum, argboxes, descr, exc=can_raise, pure=is_elidable)` invocation shape, names the three walker dispatch sites (`dispatch_residual_call_iRd_kind` / `dispatch_residual_call_iIRd_kind` / `dispatch_residual_call_iIRFd_kind`), and ranks the widening priority by SIGBUS blast radius (Call* default → CallLoopinvariant* → CallMayForce*). Doc-only; gate dynasm+cranelift 41/41x2 green. Assisted-by: Claude
Task #390 sub-slice 2.1. Add `execute_residual_call(descr, func_ptr, args) -> Result<i64, i64>` parallel to `execute_pure_call`, simplified for the walker layer (no `MetaInterp` to thread). Clears `BH_LAST_EXC_VALUE` before dispatch, runs the helper, returns the BH exc seam via `Result::Err` so the walker can wire it into `WalkContext.last_exc_value` + emit `GUARD_NO_EXCEPTION` — mirroring PyPy `execute_varargs`'s `metainterp.execute_raised(bh_exc, constant=False)` + `handle_possible_exception` flow without the metainterp dependency. No caller yet; sub-slice 2.2 wires `try_execute_residual_call_via_ executor` in `jitcode_dispatch.rs`. Doc-only behavior change today. Assisted-by: Claude
Add the walker-friendly counterpart of try_fold_pure_call_via_executor for non-elidable Call*/CallLoopinvariant* shapes. Routes through the new majit_metainterp::executor::execute_residual_call entry, returns Result<i64, i64> so the caller can wire BH_LAST_EXC_VALUE into WalkContext.last_exc_value without a MetaInterp seam. Function is dead-code marked: dispatch-site wire lands in a follow-up slice. CallMayForce*/CallReleaseGil*/CallAssembler* intentionally excluded pending a force-virtual / GIL / jitdriver-re-entry audit. Task #390 sub-slice 2.2. Assisted-by: Claude
…ise=false only) Invoke the non-elidable concrete-execute helper from all three residual_call dispatchers (iRd / iIRd / iIRFd) alongside try_fold_pure_call_via_executor. Gated on !can_raise: BH exception propagation to WalkContext.last_exc_value lands in sub-slice 3, which unblocks the M4 SIGBUS paths (store_subscr_fn / set_current_exception class — these sit on can_raise=true). Drop the function's #[allow(dead_code)] and refresh its wire-status doc-comment now that the dispatchers reach it. Task #390 sub-slice 2.3. Assisted-by: Claude
…nfra Task #390 sub-slice 3 scaffolding for the can_raise=true unblock. Lands the Err-arm infrastructure with activation deferred to sub-slice 4 (helper-side liveness audit pending). try_execute_residual_call_via_executor: * Err(bh_exc) seeds ctx.last_exc_value (const_ref) and ctx.last_exc_value_concrete (ConcreteValue::Ref) so subsequent reraise / last_exc_value / handle_possible_exception walker chain reads non-null state. * Restores BH_LAST_EXC_VALUE = bh_exc after execute_residual_call's read-clear, so the eval-loop walker-skip path can detect the pending exception and route into the bytecode interpreter via PyError::from_exc_object. walker_record_guard_exception: * Walker-side port of pyjitpl.py:2156-2168 handle_possible_exception exception branch. Reads exc obj from ctx.last_exc_value_concrete, extracts ob_header.ob_type, emits GuardException(exc_type_const) + snapshot. Falls back to GuardNoException when concrete is missing. dispatch_residual_call_{iRd,iIRd,iIRFd}_kind: * resid_raised tracks whether the recording-time helper raised; the Err-arm path emits GuardException via the new helper and skips write_residual_call_result_to_dst + loopinvariant_now_known (no valid result to stamp / cache). * Activation still gated on !can_raise empirically — lifting the gate SIGBUSes bytes_ops + closures on both backends (suspected stale-Ref shadow on can_raise=true object helpers); the gate stays until sub-slice 4 audits walker-shadow-vs-live-frame liveness. eval.rs walker-skip path: * After walker dispatch returns, reads + clears BH_LAST_EXC_VALUE and surfaces a non-zero value as PyError::from_exc_object so the bytecode interpreter's exception handler runs. Assisted-by: Claude
…ot-cause Sub-slice 4 audit (probe with PYRE_PROBE_RESIDUAL_CALL=1) found the can_raise=true SIGBUS on synth/bytes_ops + synth/closures is NOT walker-shadow staleness as originally suspected. The funcptr operand itself is GARBAGE (e.g. 0xacc1d48d7993df7d for bytes_ops's CallR) — an UNPATCHED build-time fnaddr. runtime_fnaddr_patch covers the helpers whose path appears in pyre_interpreter::jit_trace_fnaddrs(); helpers outside that registry retain the stale build-process address and dereferencing as a code pointer SIGBUSes. Refresh the gate comment with the precise root cause. The !can_raise gate stays as the safe activation boundary; sub-slice 4 will either extend the patcher registry to cover the missing helpers or add a fnaddr-sanity probe (e.g. caller-side `is_valid_code_addr`) before widening. Assisted-by: Claude
…anity gate Task #390 sub-slice 4. Lift the !can_raise gate at the 3 dispatch sites and move the safety check into try_execute_residual_call_via_executor itself as a fnaddr-sanity guard: reject any funcptr whose bits >= 47 are set. Root-cause: pyre's codewriter mints `symbolic_fnaddr_for_path(path) = stable_symbolic_fnaddr(path)` (a DefaultHasher 64-bit hash) when the build-time `pyre_interpreter::jit_trace_fnaddrs()` snapshot has no entry for the helper (majit-translate/src/jit_codewriter/call.rs:4926). The runtime_fnaddr_patch rebinds build → runtime addresses only for helpers whose path appears in both registries; unregistered helpers retain the hash. Walker concrete-execute would then call into the hash and SIGBUS. The 47-bit gate filters those out cleanly: valid macOS/Linux user-space code addresses fit in 47 bits (canonical low half); hash values typically have bits >= 47 set. PyPy-orthodox shape now matches `_opimpl_residual_call*` — every residual_call concrete-executes regardless of EI; only the post-call guard shape (`GUARD_EXCEPTION` vs `GUARD_NO_EXCEPTION`) varies. Sub- slice 3's Err-arm infrastructure (walker_record_guard_exception, ctx.last_exc_value seeding, eval.rs BH_LAST_EXC_VALUE propagation) is now live for the activated set. Gate check.py: dynasm + cranelift 41/41 each (sub-slice 3 broke bytes_ops + closures; the sanity gate restores them). Out of scope for this slice: extending `jit_trace_fnaddrs()` to register the helpers currently rejected by the sanity gate — that would widen activation further but requires per-helper audit. The M4 SIGBUS unblock (StoreSubscr arm's store_subscr_fn / set_current_exception) lands once those specific helpers are registered or the gate widens. Assisted-by: Claude
youknowone
marked this pull request as ready for review
June 6, 2026 23:25
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 963c2d807e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+4627
to
+4628
| if resid_raised { | ||
| walker_record_guard_exception(ctx, op.pc); |
There was a problem hiding this comment.
When a walker-handled opcode's can_raise residual call actually raises, this branch only emits GuardException and then falls through with DispatchOutcome::Continue; the walker will keep executing the remaining arm bytecodes (often recording normal stack effects or a void FINISH) before eval_loop_jit later converts BH_LAST_EXC_VALUE into a PyError. RPython's handle_possible_exception immediately routes to finishframe_exception on this path, so raising helpers in walker-enabled opcodes such as attribute/import/container helpers can leave an inconsistent trace for an exceptional execution. The raising branch needs to terminate/reroute the walker instead of continuing after walker_record_guard_exception.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyre/pyre-jit-trace/src/jitcode_dispatch.rs (1)
3480-5004: 🧹 Nitpick | 🔵 TrivialPlease attach the dynasm test run and full JIT benchmark results for this path.
These changes alter walker-side residual-call execution, guard emission, and exception propagation, but the supplied PR context includes no
cargo check,cargo test --features dynasm, or 8-benchmark results. This is exactly the class of*jit*.rschange the repo asks to validate before merge.As per coding guidelines,
**/*.rsmust “Always run cargo check and cargo test with --features dynasm before committing,” and**/*jit*.rsmust “Run full benchmark suite (all 8 benchmarks) after JIT changes. Do not commit if any regress.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-jit-trace/src/jitcode_dispatch.rs` around lines 3480 - 5004, The reviewer requests you run the required build/test/benchmark validation for the residual-call/JIT changes: run `cargo check` and `cargo test --features dynasm` and attach the full dynasm test logs, then run the full 8-benchmark JIT suite and attach the benchmark results (before/after if available). Specifically exercise the modified code paths (try_execute_residual_call_via_executor, dispatch_residual_call_iRd_kind, dispatch_residual_call_iIRd_kind, dispatch_residual_call_iIRFd_kind, direct_call_release_gil, and walker_record_guard_exception) and include the command outputs, failing tests, and benchmark numbers in the PR so we can verify no regressions before merge.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@majit/majit-metainterp/src/executor.rs`:
- Around line 851-878: Add seam-focused tests for execute_residual_call that
exercise the BH_LAST_EXC_VALUE clear/read/clear contract: in the test module
(next to existing execute_pure_call tests) add a small matrix that (1) sets
BH_LAST_EXC_VALUE to a nonzero sentinel before calling execute_residual_call to
verify the function clears it pre-call, (2) invokes a helper callee that leaves
BH_LAST_EXC_VALUE==0 and returns normally and assert execute_residual_call
returns Ok and BH_LAST_EXC_VALUE remains cleared, and (3) invokes a helper
callee that sets BH_LAST_EXC_VALUE to a nonzero value to simulate a walker
exception and assert execute_residual_call returns Err(with that sentinel) and
that BH_LAST_EXC_VALUE is cleared after the call; reference the
execute_residual_call function and the BH_LAST_EXC_VALUE TLS accessor to locate
the code under test and mirror the execute_pure_call test patterns for
setup/teardown.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch.rs`:
- Around line 3565-3616: In the Ok(result_i64) success path, clear the walker
exception slot before stamping results: set ctx.last_exc_value to None, set
ctx.last_exc_value_concrete to the concrete "no-exception" sentinel (the
ConcreteValue variant used for no-ref), and reset
majit_metainterp::blackhole::BH_LAST_EXC_VALUE to the null/zero value; do this
at the start of the Ok branch in jitcode_dispatch.rs (before using
ctx.trace_ctx.set_opref_concrete) so stale exception state cannot leak into
last_exc_value/>r, reraise/, or catch_exception/L handling.
In `@pyre/pyre-jit-trace/src/jitcode_runtime.rs`:
- Around line 334-337: metainterp_jitcode_for_instruction currently calls
arm_id_for_instruction(instruction) which formats the Instruction and does an
O(n) scan via get_arm()/ALL_OPCODE_ARMS on every dispatch; change this by
avoiding Debug formatting and linear scans: compute and store the arm id (or
direct jitcode index) once when the Instruction/variant is created or when the
switch is first compiled, then thread that arm/jitcode index through the
eval-loop/switch so metainterp_jitcode_for_instruction can do a direct table
lookup (or index into a precomputed by-arm-id array) instead of calling
arm_id_for_instruction and metainterp_jitcode_for_arm on every hot-path
dispatch.
In `@pyre/pyre-jit/src/eval.rs`:
- Around line 3055-3108: The dispatch_arm_via_blackhole path currently only
checks bh.got_exception / bh.exception_last_value but must also drain the
thread-local BH_LAST_EXC_VALUE set by residual-call executor; after bh.run()
(and before returning/continuing) read BH_LAST_EXC_VALUE, clear it (set to 0)
and treat a non-zero value as a raised Python exception (construct a
pyre_interpreter::PyError from that exc object, similar to the existing
exception_value branch), returning Err(err) if present; reference
BH_LAST_EXC_VALUE, bh.run(), got_exception/exception_value and the
dispatch_arm_via_blackhole return path to locate where to add the drain and
clear.
---
Outside diff comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch.rs`:
- Around line 3480-5004: The reviewer requests you run the required
build/test/benchmark validation for the residual-call/JIT changes: run `cargo
check` and `cargo test --features dynasm` and attach the full dynasm test logs,
then run the full 8-benchmark JIT suite and attach the benchmark results
(before/after if available). Specifically exercise the modified code paths
(try_execute_residual_call_via_executor, dispatch_residual_call_iRd_kind,
dispatch_residual_call_iIRd_kind, dispatch_residual_call_iIRFd_kind,
direct_call_release_gil, and walker_record_guard_exception) and include the
command outputs, failing tests, and benchmark numbers in the PR so we can verify
no regressions before merge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 425d125f-f7af-42b6-a697-66ee8786f50a
📒 Files selected for processing (4)
majit/majit-metainterp/src/executor.rspyre/pyre-jit-trace/src/jitcode_dispatch.rspyre/pyre-jit-trace/src/jitcode_runtime.rspyre/pyre-jit/src/eval.rs
Comment on lines
+851
to
+878
| pub fn execute_residual_call( | ||
| descr: &dyn majit_ir::descr::CallDescr, | ||
| func_ptr: i64, | ||
| args: &[i64], | ||
| ) -> Result<i64, i64> { | ||
| crate::blackhole::BH_LAST_EXC_VALUE.with(|c| c.set(0)); | ||
| let func_ptr = func_ptr as *const (); | ||
| let result = match descr.result_type() { | ||
| majit_ir::Type::Int | majit_ir::Type::Ref => { | ||
| crate::pyjitpl::call_int_function(func_ptr, args) | ||
| } | ||
| majit_ir::Type::Void => { | ||
| crate::pyjitpl::call_void_function(func_ptr, args); | ||
| 0 | ||
| } | ||
| // See `execute_varargs`'s Float arm for the i64-bits ABI | ||
| // rationale: `#[jit_module]` Float helpers expose | ||
| // `concrete_ptr` as `extern "C" fn(...) -> i64` with the f64 | ||
| // pre-packed via `f64::to_bits`. | ||
| majit_ir::Type::Float => crate::pyjitpl::call_int_function(func_ptr, args), | ||
| }; | ||
| let bh_exc = crate::blackhole::BH_LAST_EXC_VALUE.with(|c| { | ||
| let v = c.get(); | ||
| c.set(0); | ||
| v | ||
| }); | ||
| if bh_exc != 0 { Err(bh_exc) } else { Ok(result) } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add seam-focused tests for execute_residual_call.
This helper now owns the BH_LAST_EXC_VALUE clear/read/clear contract, but the test module only exercises execute_pure_call. Please add a small matrix for Line 856 pre-clear, Line 872 post-call capture, and the Ok/Err split so the new walker exception path does not regress silently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@majit/majit-metainterp/src/executor.rs` around lines 851 - 878, Add
seam-focused tests for execute_residual_call that exercise the BH_LAST_EXC_VALUE
clear/read/clear contract: in the test module (next to existing
execute_pure_call tests) add a small matrix that (1) sets BH_LAST_EXC_VALUE to a
nonzero sentinel before calling execute_residual_call to verify the function
clears it pre-call, (2) invokes a helper callee that leaves BH_LAST_EXC_VALUE==0
and returns normally and assert execute_residual_call returns Ok and
BH_LAST_EXC_VALUE remains cleared, and (3) invokes a helper callee that sets
BH_LAST_EXC_VALUE to a nonzero value to simulate a walker exception and assert
execute_residual_call returns Err(with that sentinel) and that BH_LAST_EXC_VALUE
is cleared after the call; reference the execute_residual_call function and the
BH_LAST_EXC_VALUE TLS accessor to locate the code under test and mirror the
execute_pure_call test patterns for setup/teardown.
Comment on lines
+3565
to
+3616
| let exec_result = | ||
| majit_metainterp::executor::execute_residual_call(call_descr, func_ptr, &args); | ||
| match exec_result { | ||
| Ok(result_i64) => { | ||
| // pyjitpl.py:1392 `result_box.value = result` analogue — stamp | ||
| // the recorded OpRef with the executed concrete so downstream | ||
| // `concrete_of_opref` / `box_value` consumers see the folded | ||
| // value. Void callees do not stamp (no result to record); the | ||
| // heap mutation has already happened via `call_void_function`. | ||
| match call_descr.result_type() { | ||
| majit_ir::Type::Int => { | ||
| ctx.trace_ctx | ||
| .set_opref_concrete(recorded, majit_ir::Value::Int(result_i64)); | ||
| } | ||
| majit_ir::Type::Ref => { | ||
| ctx.trace_ctx.set_opref_concrete( | ||
| recorded, | ||
| majit_ir::Value::Ref(majit_ir::GcRef(result_i64 as usize)), | ||
| ); | ||
| } | ||
| majit_ir::Type::Float => { | ||
| ctx.trace_ctx.set_opref_concrete( | ||
| recorded, | ||
| majit_ir::Value::Float(f64::from_bits(result_i64 as u64)), | ||
| ); | ||
| } | ||
| majit_ir::Type::Void => {} | ||
| } | ||
| } | ||
| Err(bh_exc) => { | ||
| // pyjitpl.py:1690-1696 `metainterp.execute_raised(exception, | ||
| // constant=False)` analogue — seed the standing exception | ||
| // state so downstream walker chain (`reraise/`, | ||
| // `last_exc_value/>r`, `handle_possible_exception` guard | ||
| // emission) sees a non-null `last_exc_value` and routes | ||
| // through the GuardException path. | ||
| // | ||
| // `execute_residual_call` cleared `BH_LAST_EXC_VALUE` on read; | ||
| // restore it so the eval-loop walker-skip path | ||
| // (`eval.rs:3285-3308`) can detect the pending exception and | ||
| // route into the bytecode-interpreter's exception handler | ||
| // via `PyError::from_exc_object` — matching RPython's | ||
| // metainterp framestack scan after a raising residual call | ||
| // (`pyjitpl.py:2156-2168 handle_possible_exception` + | ||
| // `pyjitpl.py:3380 finishframe_exception`). | ||
| ctx.last_exc_value = Some(ctx.trace_ctx.const_ref(bh_exc)); | ||
| ctx.last_exc_value_concrete = | ||
| ConcreteValue::Ref(bh_exc as usize as pyre_object::PyObjectRef); | ||
| majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.set(bh_exc)); | ||
| } | ||
| } | ||
| Some(exec_result) |
There was a problem hiding this comment.
Clear the walker exception slot before a successful residual call.
This helper mirrors upstream clear_exception() semantics, but the success path never resets ctx.last_exc_value / ctx.last_exc_value_concrete. If the same walk previously handled an exception, that stale state survives a later successful concrete residual call and can misdrive last_exc_value/>r, reraise/, or the normal-path catch_exception/L assertion.
Suggested fix
- let exec_result =
- majit_metainterp::executor::execute_residual_call(call_descr, func_ptr, &args);
+ // Mirror pyjitpl.py `metainterp.clear_exception()` before executing
+ // the helper so a prior handled exception does not leak past a
+ // successful residual call in the same walk.
+ ctx.last_exc_value = None;
+ ctx.last_exc_value_concrete = ConcreteValue::Null;
+ let exec_result =
+ majit_metainterp::executor::execute_residual_call(call_descr, func_ptr, &args);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch.rs` around lines 3565 - 3616, In the
Ok(result_i64) success path, clear the walker exception slot before stamping
results: set ctx.last_exc_value to None, set ctx.last_exc_value_concrete to the
concrete "no-exception" sentinel (the ConcreteValue variant used for no-ref),
and reset majit_metainterp::blackhole::BH_LAST_EXC_VALUE to the null/zero value;
do this at the start of the Ok branch in jitcode_dispatch.rs (before using
ctx.trace_ctx.set_opref_concrete) so stale exception state cannot leak into
last_exc_value/>r, reraise/, or catch_exception/L handling.
Comment on lines
+334
to
+337
| pub fn metainterp_jitcode_for_instruction( | ||
| instruction: &Instruction, | ||
| ) -> Option<Arc<majit_metainterp::jitcode::JitCode>> { | ||
| metainterp_jitcode_for_arm(arm_id_for_instruction(instruction)?) |
There was a problem hiding this comment.
Avoid Debug formatting and linear arm scans on every production dispatch.
Line 337 routes the new blackhole path through arm_id_for_instruction(...) and then metainterp_jitcode_for_arm(...), which means each dispatch pays for format!("{instruction:?}")/String creation plus an O(n) get_arm() scan over ALL_OPCODE_ARMS before execution even starts. In the eval-loop hot path, that lookup cost is likely to dominate the Phase 5.B wiring. Thread the resolved arm/jitcode index through the switch, or precompute a direct by-arm-id/by-variant table once.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/jitcode_runtime.rs` around lines 334 - 337,
metainterp_jitcode_for_instruction currently calls
arm_id_for_instruction(instruction) which formats the Instruction and does an
O(n) scan via get_arm()/ALL_OPCODE_ARMS on every dispatch; change this by
avoiding Debug formatting and linear scans: compute and store the arm id (or
direct jitcode index) once when the Instruction/variant is created or when the
switch is first compiled, then thread that arm/jitcode index through the
eval-loop/switch so metainterp_jitcode_for_instruction can do a direct table
lookup (or index into a precomputed by-arm-id array) instead of calling
arm_id_for_instruction and metainterp_jitcode_for_arm on every hot-path
dispatch.
Comment on lines
+3055
to
+3108
| let (mergepoint_args, got_exception, exception_value, return_type) = | ||
| BH_BUILDER_ARM.with(|cell| unsafe { | ||
| let builder = &mut *cell.get(); | ||
| sync_control_opcodes(builder); | ||
| let mut bh = builder.acquire_interp(); | ||
| // `setposition` (blackhole.py:312) initialises register | ||
| // banks + copies the constants pool into the upper portion | ||
| // of each bank. Setting `jitcode` / `position` directly | ||
| // would leave the banks sized to the previous interp's | ||
| // jitcode (or empty on a fresh acquire), with no constants | ||
| // copied — wild reads at the first `bhimpl_*` that loads a | ||
| // constant. | ||
| bh.setposition(jitcode, 0); | ||
| // Virtualizable wiring — mirrors `call_jit.rs:1400-1423`. | ||
| // Single-arm dispatch has no caller chain so the | ||
| // `nextblackholeinterp` propagation loop reduces to no-op. | ||
| bh.virtualizable_ptr = frame as *mut pyre_interpreter::pyframe::PyFrame as i64; | ||
| bh.virtualizable_info = get_virtualizable_info(); | ||
| // RPython MIFrame.setup parity: r0 = frame (per | ||
| // `trace_opcode.rs:6657-6660` walker-side seed). Remaining | ||
| // working-bank slots stay zero/null; per-arm stack peeks | ||
| // are issued inside the arm body via `bhimpl_*` operand | ||
| // decoding (vable_get/setarrayitem_r against PyFrame's | ||
| // `locals_cells_stack_w`). | ||
| if !bh.registers_r.is_empty() { | ||
| bh.registers_r[0] = bh.virtualizable_ptr; | ||
| } | ||
| let mergepoint_args = bh.run(); | ||
| let got_exception = bh.got_exception; | ||
| let exception_value = bh.exception_last_value; | ||
| let return_type = bh.return_type; | ||
| builder.release_interp(bh); | ||
| (mergepoint_args, got_exception, exception_value, return_type) | ||
| }); | ||
|
|
||
| if got_exception { | ||
| // Mirrors `call_jit.rs:1486-1506` (BH chain-exit propagation): | ||
| // null `exception_value` means the arm raised without a | ||
| // payload (defensive RuntimeError); otherwise the value is a | ||
| // GC ref to a `W_BaseException` instance to re-wrap. | ||
| let err = if exception_value != 0 { | ||
| unsafe { | ||
| pyre_interpreter::PyError::from_exc_object( | ||
| exception_value as pyre_object::PyObjectRef, | ||
| ) | ||
| } | ||
| } else { | ||
| pyre_interpreter::PyError::new( | ||
| pyre_interpreter::PyErrorKind::RuntimeError, | ||
| "dispatch_arm_via_blackhole: arm raised exception with null exc_value", | ||
| ) | ||
| }; | ||
| return Err(err); | ||
| } |
There was a problem hiding this comment.
Drain BH_LAST_EXC_VALUE on the arm-blackhole path.
This helper only consults bh.got_exception / bh.exception_last_value, but this feature’s residual-call executor reports raised Python exceptions through BH_LAST_EXC_VALUE. On this path, a raised residual call can currently fall through as StepResult::Continue, advance the frame PC, and leave the TLS exception latched for a later opcode.
💡 Suggested fix
- let (mergepoint_args, got_exception, exception_value, return_type) =
+ let (mergepoint_args, got_exception, exception_value, tls_exception_value, return_type) =
BH_BUILDER_ARM.with(|cell| unsafe {
+ majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.set(0));
let builder = &mut *cell.get();
sync_control_opcodes(builder);
let mut bh = builder.acquire_interp();
...
let mergepoint_args = bh.run();
let got_exception = bh.got_exception;
let exception_value = bh.exception_last_value;
+ let tls_exception_value =
+ majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| {
+ let v = c.get();
+ c.set(0);
+ v
+ });
let return_type = bh.return_type;
builder.release_interp(bh);
- (mergepoint_args, got_exception, exception_value, return_type)
+ (
+ mergepoint_args,
+ got_exception,
+ exception_value,
+ tls_exception_value,
+ return_type,
+ )
});
- if got_exception {
+ let exception_value = if exception_value != 0 {
+ exception_value
+ } else {
+ tls_exception_value
+ };
+ if got_exception || exception_value != 0 {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit/src/eval.rs` around lines 3055 - 3108, The
dispatch_arm_via_blackhole path currently only checks bh.got_exception /
bh.exception_last_value but must also drain the thread-local BH_LAST_EXC_VALUE
set by residual-call executor; after bh.run() (and before returning/continuing)
read BH_LAST_EXC_VALUE, clear it (set to 0) and treat a non-zero value as a
raised Python exception (construct a pyre_interpreter::PyError from that exc
object, similar to the existing exception_value branch), returning Err(err) if
present; reference BH_LAST_EXC_VALUE, bh.run(), got_exception/exception_value
and the dispatch_arm_via_blackhole return path to locate where to add the drain
and clear.
Owner
Author
|
Addressed the actionable items from the Codex + CodeRabbit reviews on this round in commit Fixed in the live residual-call path (
Fixed in the inventory comment:
Documented as latent (Phase 5.B body wire is unreachable — predicate
All five are now explicitly listed in Section 3 (pre-existing mismatches) all sit in the CodeRabbit nitpick (executor.rs:878 — seam tests for Gate dynasm + cranelift 41/41 × 2 GREEN at — commented by Claude |
youknowone
added a commit
that referenced
this pull request
Jun 9, 2026
…se (#158) * opcode_ops + jit_fnaddr: register execute_store_subscr fnaddr via bh_execute_store_subscr wrapper Add `bh_execute_store_subscr` to `pyre-interpreter::opcode_ops` as a C-ABI bridge over `crate::pyopcode::execute_store_subscr::<PyFrame>` (whose `Result<StepResult<_>, PyError>` does not fit a residual_call's single- register Ref result slot). Errors propagate via `majit_metainterp::blackhole::BH_LAST_EXC_VALUE`, success returns non-zero. Register the bare `"execute_store_subscr"` path in `pyre_interpreter::jit_trace_fnaddrs()`. Without this entry the build-time codewriter falls back to `symbolic_fnaddr_for_path` for the `Instruction::StoreSubscr#28` arm's residual_call_r_r, mints a DefaultHasher 64-bit value, and `runtime_fnaddr_patch` leaves the hash in `JitCode.constants_i` because no runtime entry matches the bare path. The registration makes the patcher swap the build address for the wrapper's runtime address, satisfying sub-slice 4's 47-bit fnaddr-sanity gate. Assisted-by: Claude * jitcode_dispatch + eval: address PR #148 reviewer findings - try_execute_residual_call_via_executor (jitcode_dispatch.rs:3565): clear ctx.last_exc_value / last_exc_value_concrete on the success path so a prior raising helper in the same walk does not leak past a later non-raising residual call. Mirrors pyjitpl.py:1685-1690 implicit clear_exception() in _opimpl_residual_call*'s no-raise tail. - dispatch_residual_call_{iRd,iIRd,iIRFd}_kind: on the resid_raised branch, after walker_record_guard_exception, return DispatchOutcome::SubRaise { exc, exc_concrete } instead of falling through to Continue. pyjitpl.py:2156-2168 handle_possible_exception routes the raising arm through finishframe_exception() immediately; walker continuing past this point recorded dead arm IR (e.g. the arm's tail *_return) onto the exception path. - select_residual_call_opcode doc comment (jitcode_dispatch.rs:3004): the previous "PyPy do_residual_call always calls execute_varargs regardless of EI branch" was an overstatement. OS_NOT_IN_TRACE short-circuits via do_not_in_trace_call (pyjitpl.py:2003-2006) and the release-gil branch runs through do_call_release_gil (pyjitpl.py:3671-3681) instead. Updated the comment to enumerate both narrow branches explicitly. - dispatch_arm_via_blackhole doc (eval.rs:3013): added a "Latent gaps before this path can be activated" section enumerating five reviewer-flagged items (non-void return-type push not implemented, bh.aborted ignored, jitdrivers_sd not seeded, BH_LAST_EXC_VALUE not drained, hot-path lookup cost). Predicate is `false` everywhere today so the function is unreachable; the doc surfaces what activation requires. Assisted-by: Claude * heapcache+trace_opcode: add PYRE_PROBE_SUBSCR-gated probes + 5c marker - trace_ctx::heapcache_invalidate_caches_varargs: env-gated eprintln dumping opnum, argboxes.len, EI extraeffect/forces_vorv/can_raise/plain_call/oopspec on every invalidation call. - state::opimpl_getfield_gc_i: env-gated eprintln dumping obj/field_index/ struct_ptr/descr_pure/cached/loaded before the cache-hit sanity assert when the values diverge. - trace_opcode::production_walker_handles + apply_walker_stack_effect: commented-out Instruction::StoreSubscr entries flag the sub-slice 5c activation site (strategy-aware specialization port pending). Probes default off (PYRE_PROBE_SUBSCR unset = zero impact). Assisted-by: Claude * jitcode_dispatch: env-gated probe on dispatch_residual_call_iRd_kind for 5c Sub-slice 5c step 2 instrumentation: log funcptr addr, dst_bank, r_args.len, and per-arg raw addrs whenever PYRE_PROBE_SUBSCR is set. Probe sits at the entry of dispatch_residual_call_iRd_kind so step 5's STORE_SUBSCR activation can capture the fnaddr/arg shape immediately as walker handles the opcode. PyObjectRef construction from the GcRef payload is deliberately deferred to step 4 (FrameOps lift); the probe stays at the raw-usize layer so the GcRef→PyObjectRef conversion has a single defined seam. Probe defaults off (PYRE_PROBE_SUBSCR unset = zero impact). Assisted-by: Claude * walker_frame_ops: add WalkerFrameOps trait + MIFrame impl Sub-slice 5c step 3. Defines the 6-method surface that the strategy-aware STORE_SUBSCR specialization helpers (`generated_list_setitem_by_strategy`, `generated_list_setslice_same_len_by_strategy`, the `store_subscr_value` gate sequence) emit through: - value_type (Box.type query) - generate_guard (multi-frame resume snapshot + guard record) - implement_guard_value, guard_class, guard_int_object_value, guard_list_strategy — all default impls composed over the two above. `generate_guard` is the lone load-bearing method. MIFrame walks its own parent_frames/orgpc/fallthrough_pc state; the walker reaches the same semantic responsibility through walker_capture_snapshot_for_last_guard + the dispatch-time WalkContext register banks. Two distinct implementations are unavoidable; the trait makes the call sites interchangeable. MIFrame impl delegates value_type/generate_guard to existing methods; the four default impls are direct ports of the corresponding MIFrame bodies and produce byte-identical recorded IR for the trait path. The const-arm `flush_guard_not_invalidated` in `MIFrame::guard_class` (trace_opcode.rs:4681) is documented as a default-impl deviation: it fires only when a quasi-immut field read left a pending guard, and the `store_subscr_value` precondition (concrete obj/key/value are direct stack reads, not quasi-immut loads) excludes that path. Walker impl in step 4 will assert the precondition before delegating. WalkContext impl follows in step 4; `generated_*` helpers in majit-translate/src/codegen.rs become generic over WalkerFrameOps in step 5. Gate dynasm+cranelift 41/41. Assisted-by: Claude * walker_frame_ops: WalkContext impl + redesign trait to self-only signatures Sub-slice 5c step 4. Trait redesign: the earlier signature (`fn generate_guard(&mut self, ctx: &mut TraceCtx, ...)`) was unsound for `WalkContext` because the caller must split-borrow `self.trace_ctx` from `self`, and `MIFrame`/`WalkContext` both reach the same `TraceCtx` transitively through `self` and the explicit arg — the resulting double mut borrow can't be expressed in safe Rust without the caller juggling raw pointers. New shape: trait methods take `&mut self` only; `ctx` is reached through `self.ctx_mut()` / `self.ctx()` accessors that each impl provides. `MIFrame::ctx_mut` materialises the borrow via `unsafe { &mut *self.ctx }` (the `*mut TraceCtx` raw pointer field); `WalkContext::ctx_mut` returns its `self.trace_ctx` field. The accessor is scoped to a single statement at each callsite so the borrow ends before the next `self`-mut-call. WalkContext impl: - `generate_guard` records the guard via `self.trace_ctx.record_guard` then delegates snapshot capture to `walker_capture_snapshot_for_last_guard`. `flush_guard_not_invalidated` is intentionally omitted — the walker STORE_SUBSCR path has no quasi-immut field reads, so no pending GUARD_NOT_INVALIDATED can exist when the trait helpers run. - `value_type` / `guard_class` / `guard_int_object_value` / `guard_list_strategy` / `implement_guard_value` use the default impls, which compose `ctx_mut` + `generate_guard` into byte-identical emit sequences to the trait path. MIFrame impl: - `generate_guard` delegates to the existing `MIFrame::generate_guard` method, preserving the full `flush_guard_not_invalidated` / `parent_frames` / `build_framestack_snapshot` plumbing for the trait dispatch leg. - The raw pointer re-borrow inside the delegate is sound because `self.ctx`'s lifetime invariant guarantees it points at the live `TraceCtx` owned by the enclosing dispatch frame. `walker_capture_snapshot_for_last_guard` lifted from `fn` to `pub(crate) fn` so the walker impl can call it from another module. Default impls of the 4 composed methods skip the const-arm `flush_guard_not_invalidated` that `MIFrame::guard_class` performs at trace_opcode.rs:4681; that flush fires only for quasi-immut-field sequences, which `store_subscr_value` excludes by construction. Gate dynasm+cranelift 41/41. Assisted-by: Claude * state: trace_unbox_*_with_resume family generic over WalkerFrameOps Convert `trace_unbox_int_with_resume`, `trace_unbox_int_with_resume_descr`, `trace_unbox_long_with_resume`, `trace_unbox_float_with_resume` from `(frame: &mut MIFrame, ctx: &mut TraceCtx, ...)` to `(frame: &mut F, ...)` where `F: WalkerFrameOps`. Frame impl reaches ctx via `frame.ctx_mut()` / `frame.ctx()`; guard emit via `frame.generate_guard(opcode, args)` without explicit ctx arg. Update 22 callsites in codegen.rs + 1 internal in state.rs + 1 test. Sub-slice 5c step 5.1. Assisted-by: Claude * codegen: generated_* STORE_SUBSCR helpers generic over WalkerFrameOps Convert `opimpl_check_resizable_neg_index`, `unbox_int_or_long_for_int_strategy`, `generated_list_setitem_by_strategy`, `generated_list_setslice_same_len_by_strategy`, `generated_store_subscr_value` from `(frame: &mut MIFrame, ctx: &mut TraceCtx, ...)` to `(frame: &mut F, ...)` where `F: WalkerFrameOps`. Ctx access goes through `frame.ctx_mut()` per statement; trait methods (`guard_class`, `guard_list_strategy`, `implement_guard_value`, `generate_guard`) drop the explicit ctx arg. Add `extern crate self as pyre_jit_trace` in lib.rs so generic bounds written for majit-translate's crate name (`pyre_jit_trace::walker_frame_ops`) resolve when codegen.rs is `include!`d into pyre-jit-trace's `generated*` modules. Update trait-leg `store_subscr_value` call sites in trace_opcode.rs (drop ctx arg; `with_ctx` closure ignores ctx). Sub-slice 5c steps 5.2 + 5.3 + 5.4. Assisted-by: Claude * jitcode_dispatch: env-gated STORE_SUBSCR walker specialization hook Add `try_walker_store_subscr_specialization` invoked from `dispatch_residual_call_iRd_kind` between the probe and the EI-decision prologue. Gate conditions: - `dst_bank == 'v'` (STORE_SUBSCR returns void) - `r_args.len() == 3` (3-arg `[obj, key, value]` shape from `codewriter.rs:7042 build_store_subscr_fn_residual_call_r_v_insn`) - `PYRE_WALKER_STORE_SUBSCR_FNADDR=<hex|decimal>` env var matches runtime funcptr (cross-crate plumbing of `bh_store_subscr_fn` address from pyre-jit is deferred to 5.5b) - All 3 `concrete_registers_r` shadow slots are `ConcreteValue::Ref(_)` On all gates pass + `generated_store_subscr_value` returns true, calls the helper concretely via raw fn-ptr cast to mutate the heap, then returns `DispatchOutcome::Continue` skipping the blackbox CallN path. Helper-raise (return non-zero) declines specialization → fallthrough to existing dispatcher path emits `CallMayForce*` + `GuardException`/ `SubRaise`. Sub-slice 5c step 5.5a. Gate 41/41×2 GREEN (hook inert without env var). Assisted-by: Claude * jitcode_dispatch: fix bh_store_subscr_fn return-code polarity `bh_store_subscr_fn` (call_jit.rs:3295) returns 1 on success and 0 on raise (exception stashed in `BH_LAST_EXC_VALUE`). The step 5.5a hook inverted this — declining on success and continuing on raise. Fix: invert the gate so `success == 0` declines (lets the dispatcher fall through to the standard `try_execute_residual_call_via_executor` + `GuardException` + `SubRaise` path) and `success != 0` proceeds with specialization. Note: this diverges from `jit_setitem` (opcode_ops.rs:225) which returns 0 on success and panics on error — the trait path's `emit_trace_call_int_typed(jit_setitem, ...)` assumes infallible trace-time recording. Walker uses the fail-soft `bh_store_subscr_fn` because residual_call execution is not a panic site. Assisted-by: Claude * WalkContext: add store_subscr_fn_addr Option<usize> field Field carries the runtime address of `bh_store_subscr_fn` (`pyre-jit::cpu.store_subscr_fn` binding) so `try_walker_store_subscr_specialization` (step 5.5a) can recognise 3-arg `residual_call_r_v(store_subscr_fn, obj, key, value)` without the `PYRE_WALKER_STORE_SUBSCR_FNADDR` env var. Top-level entry points (`dispatch_via_miframe`, `dispatch_via_miframe_at_opcode_entry`) default to `None`; production plumbing of the address from `cpu.store_subscr_fn` lands in 5.5c. Sub-walk WalkContext constructions inherit the parent's `ctx.store_subscr_fn_addr` (3 sites). 102 test-fixture constructions default to `None` (env var still works as the fallback). Hook now reads field first, falls back to env var when `None`. Sub-slice 5c step 5.5b. Assisted-by: Claude * relocate bh_store_subscr_fn pyre-jit → pyre-interpreter Move `bh_store_subscr_fn(obj, key, value) -> i64` from `pyre-jit/src/call_jit.rs:3295` to `pyre-interpreter/src/opcode_ops.rs` and register the address in `pyre_interpreter::jit_trace_fnaddrs()` under paths `pyre_interpreter::opcode_ops::bh_store_subscr_fn` + `pyre_interpreter::bh_store_subscr_fn`. Body unchanged. Update the `cpu.store_subscr_fn` binding at `pyre-jit/src/jit/cpu.rs:151` to the new path. Motivation: walker specialization (`try_walker_store_subscr_specialization` in pyre-jit-trace) needs to recognise the runtime address of the helper. Registering in `jit_trace_fnaddrs()` lets pyre-jit-trace look up the address via path key without taking a `pyre-jit-trace → pyre-jit` dep edge. Task #391. Assisted-by: Claude * jitcode_dispatch: plumb store_subscr_fn_addr from jit_trace_fnaddrs Add `bh_store_subscr_fn_addr_cached()` helper that resolves the runtime address of `pyre_interpreter::opcode_ops::bh_store_subscr_fn` via `pyre_interpreter::jit_trace_fnaddrs()` linear scan with `OnceLock` caching. Production entry sites in `dispatch_via_miframe` and `dispatch_via_miframe_at_opcode_entry` now seed `WalkContext.store_subscr_fn_addr` from this lookup instead of `None`. With the field populated, `try_walker_store_subscr_specialization` fires without the `PYRE_WALKER_STORE_SUBSCR_FNADDR` env var. Hook remains inert for STORE_SUBSCR specifically (not in `production_walker_handles` yet — step 5.6), but ready for any non-StoreSubscr arm that embeds a 3-arg `residual_call_r_v( store_subscr_fn, ...)`. Gate 41/41×2 GREEN. Sub-slice 5c step 5.5c-d. Assisted-by: Claude * trace_opcode: revert StoreSubscr walker activation (nbody/fannkuch regress) Sub-slice 5c step 5.6 attempted to enable `Instruction::StoreSubscr` in `production_walker_handles` + `apply_walker_stack_effect`. With the hook actively dispatching the specialized SETARRAYITEM_GC shape, gate runs to completion but nbody (~31× slowdown, 0.21s → 6.5s) and fannkuch (timeout >30s) regress — these are the two STORE_SUBSCR-hot benchmarks the specialization was meant to accelerate. Hook itself (`try_walker_store_subscr_specialization` + `bh_store_subscr_fn_addr_cached` plumbing) preserved in place; the remaining gap is in the walker leg of `generated_store_subscr_value` — likely the `WalkerFrameOps::generate_guard` snapshot delta vs the trait's `with_ctx + MIFrame::generate_guard` flush + parent-frame chain. Marker re-commented with the investigation note; activation deferred to sub-step 5.6b (snapshot / heapcache divergence root-cause). Gate 41/41×2 GREEN with marker off. Assisted-by: Claude * trace_opcode: 5.6b root-cause walker StoreSubscr SubRaise (marker note only) 5.6 activation surfaced SubRaise every trace iteration: walker dispatch of `Instruction::StoreSubscr` walks the auto-generated arm jitcode `int_copy, residual_call_r_r(bh_execute_store_subscr, frame), live, ref_return`. `try_execute_residual_call_via_executor` matches the `CallR` shape and concrete-executes `bh_execute_store_subscr(frame)`, which casts `frame` to `*mut PyFrame` and calls `pyopcode::execute_store_subscr(frame)`. That helper pops 3 values from `PyFrame.locals_cells_stack_w` and feeds them to `setitem(obj, key, value)`. Walker LOAD_FAST / LOAD_CONST / etc. populate only MIFrame's symbolic stack + concrete-shadow stack — not the concrete `PyFrame`'s `locals_cells_stack_w`. `setfield_vable_i(vsd)` advances the PyFrame's depth counter without writing the slots. When `bh_execute_store_subscr` runs, the popped slots are uninitialized → `bh_store_subscr_fn(null, null, null)` raises "store subscript on null operand" → arm returns 0 → walker dispatch yields SubRaise → trace aborts. Repro (PYRE_PROBE_SUBSCR=1 MAJIT_LOG=1 on a tiny `a[0]=i` loop): [PYRE_PROBE_SUBSCR] dispatch_residual_call_iRd_kind pc=3 dst_bank=r r_args.len=1 funcptr_addr=Some("0x...e6c8") arg_addrs=[Some("<concrete_frame>")] [jit][probe] instr=StoreSubscr trace=[ "int_copy", "residual_call_r_r", "live", "ref_return"] [jit][abort-reason] StoreSubscr SubRaise `...e6c8` resolves to `bh_execute_store_subscr` (`pyre-interpreter/src/opcode_ops.rs:265`); the hash matches the `nm` offset at `__ZN16pyre_interpreter10opcode_ops23bh_execute_store_subscr`. Marker stays commented; comment expanded with the root cause. Existing `try_walker_store_subscr_specialization` hook keyed on `dst_bank == 'v' && r_args.len() == 3` (the codewriter trace IR shape `build_store_subscr_fn_residual_call_r_v_insn`) never fires against the arm jitcode's `_r_r` shape — they are two distinct emission paths. 5.6b fix candidates left for future sub-slice: (A) Populate the concrete `PyFrame` stack from walker LOAD/CONST handlers (orthodox PyPy-parity but wide-scope). (B) Intercept `StoreSubscr` at `dispatch_via_walker_for_opcode` entry (before the arm walk) and route through `MIFrame::store_subscr_value` using the symbolic stack — bypasses the arm jitcode entirely, mirrors the trait path which also avoids concrete heap mutation at trace time. Gate dynasm 41/41 + cranelift 41/41 with marker off. Assisted-by: Claude * trace_opcode: 5.6b activate StoreSubscr walker via trait-path delegation Re-enable `Instruction::StoreSubscr` in `production_walker_handles` + intercept at `dispatch_via_walker_for_opcode` entry before the arm walk. Pops 3 values via `SharedOpcodeHandler::pop_value` (which updates symbolic + concrete-shadow stacks + vsd shadow through `MIFrame::pop_value`), delegates to the existing `MIFrame::store_subscr_value` which records the same specialized `guard_class + SETARRAYITEM_GC`-family IR shape via `generated_store_subscr_value` (or the `Call(jit_setitem, ...)` fallback via `trace_store_subscr`) that the trait dispatch already emits, then returns `StepResult::Continue` without entering the arm walk. `apply_walker_stack_effect` is not reached because the hook returns before the arm-walk match. The arm jitcode's `residual_call_r_r(bh_execute_store_subscr, frame)` shape — which would otherwise raise on the unpopulated `PyFrame.locals_cells_stack_w` slots — is bypassed entirely. Verified on dynasm release build: * `a[0]=i` smoke loop, n=2000 — correct output (1999) * `pyre/bench/nbody_50k.py` 1.10s → 0.73s (~33% improvement) * `pyre/bench/nbody.py` (n=500000) 4.51s → 4.04s * `pyre/bench/fannkuch.py` (n=9) timeout → 0.81s, output 8629/30 ×7 Gate dynasm 41/41 + cranelift 41/41 GREEN. The pre-existing 5.5a hook `try_walker_store_subscr_specialization` in `dispatch_residual_call_iRd_kind` remains dead (the codewriter-trace-IR `_r_v` shape it gates on never fires inside the arm jitcode); leaving it in place for now — the 141 cutover plan moves trace-IR emission to the walker and that hook will gain a reachable path then. Assisted-by: Claude * eval/trace_opcode: retire Phase 5.B dispatch_arm_via_blackhole body wire Deletions: - `dispatch_arm_via_blackhole` in `pyre-jit/src/eval.rs` - `production_blackhole_handles` in `pyre-jit-trace/src/trace_opcode.rs` and its re-export in `pyre-jit-trace/src/lib.rs` - `metainterp_jitcode_by_index` / `metainterp_jitcode_for_arm` / `metainterp_jitcode_for_instruction` and `METAINTERP_JITCODE_CACHE` thread-local in `pyre-jit-trace/src/jitcode_runtime.rs` `eval_loop_jit` walker-dispatched branch collapses to the BH_LAST_EXC_VALUE drain that surfaces a non-elidable residual_call's raised exception (kept because `try_execute_residual_call_via_executor` from Task #390 sub-slice 3 seeds the TLS on Err). Updates the stale Phase 5.B-replacement doc block on `try_fold_pure_call_via_executor` to point at `try_execute_residual_call_via_executor` as the orthodox covering function for non-elidable arms. Assisted-by: Claude * jitcode_dispatch: admit CallMayForce* in try_execute_residual_call_via_executor when no active virtualizable Extends the executor match arm of `try_execute_residual_call_via_executor` from `Call*` / `CallLoopinvariant*` to also include `CallMayForce*` when `ctx.trace_ctx.standard_virtualizable_box()` returns None. With no active vinfo box, `vable_after_residual_call` (`pyjitpl.py:3349-3366` parity at `trace_opcode.rs:2646`) early-returns, so executing the helper here matches PyPy's `do_residual_call`'s forces-virtual branch (`pyjitpl.py:2017-2082`) without requiring a walker-side `walker_vable_after_residual_call`. Active-vable `CallMayForce*` continues to decline. Doc updated to spell out the gate. Assisted-by: Claude * trace_opcode: activate Reraise on production walker via MIFrame::reraise delegation Thread `op_arg` into `dispatch_via_walker_for_opcode` (single call site at `trace_code_step`) and add a `Instruction::Reraise { depth }` entry hook that delegates to the existing `MIFrame::reraise` impl through `OpcodeStepExecutor::reraise(self, depth.get(op_arg))`. The trait impl reads `reraise_lasti` from `concrete_stack[stack_idx]` and seeds `err.reraise_lasti` (`pyopcode.py:1357-1376` parity), so the walker hook's propagated `Err(PyError)` carries the lasti through `step_result.err().map(|e| e.reraise_lasti)` the same way the trait dispatch leg does. Add `Instruction::Reraise { .. }` to `production_walker_handles` and drop the exclusion comment. `StoreSubscr` hook acquires `let _ = op_arg` since `StoreSubscr` is a unit variant with no encoded arg. Assisted-by: Claude * trace_opcode: extract try_walker_direct_opcode_dispatch surface for PyPy _opimpl_* entries Refactors the two direct-record opcode hooks (StoreSubscr 5.6b delegation, Reraise lasti propagation) out of the head of `dispatch_via_walker_for_opcode` into a dedicated `try_walker_direct_opcode_dispatch` helper. No functional change. The new function is the structural home for opcodes that bypass the auto-gen arm-jitcode walk and emit IR / produce concrete effects directly from the tracer, mirroring PyPy `MetaInterp.interpret`'s `pyjitpl.py:1346+ _opimpl_*` dispatch pattern where the tracer invokes `history.record*` and concrete-executes helpers inline rather than walking an intermediate jitcode representation. Future opcode ports land here as additional `if let Instruction::X = ...` arms, making the "walker emits IR directly like PyPy _opimpl_*" surface area a single named location. Assisted-by: Claude * remove duplicated fmt_time * opcode_ops: null-check value in bh_store_subscr_fn Match the obj/key null branch by also rejecting a null value before forwarding to baseobjspace::setitem. Assisted-by: Claude * pyre-jit-trace: cache PYRE_PROBE_SUBSCR env lookup behind OnceLock state.rs and jitcode_dispatch.rs probes sit on hot trace paths and called std::env::var_os on every cache hit / residual_call dispatch. Move the read into a OnceLock-backed probe_subscr_enabled() and gate the state.rs branch on (loaded != cached_int) first so the cached case bypasses the probe entirely. Assisted-by: Claude * jitcode_dispatch: disable STORE_SUBSCR specialization on dispatch_via_miframe entry The dispatch_via_miframe path (test / shadow_walker fixture) hard-codes outer_jitcode_index = 0 and an empty outer_active_boxes; production dispatch via dispatch_via_miframe_at_opcode_entry seeds both from sym.jitcode and collect_outer_active_boxes. Leaving store_subscr_fn_addr populated here would let a specialized generated_store_subscr_value guard capture resume data pointing at the wrong outer frame, so set it to None on this entry. Assisted-by: Claude * jitcode_dispatch: route try_walker_store_subscr_specialization raise through SubRaise On a helper raise, drain BH_LAST_EXC_VALUE into ctx.last_exc_value / ctx.last_exc_value_concrete, record GuardException via walker_record_guard_exception, and return DispatchOutcome::SubRaise. Previously the function returned None on raise so the caller fell through to the generic residual-call dispatcher, which would record a second call op against the same opcode position even though the specialized IR was already emitted. Assisted-by: Claude * eval: mirror eval_loop_jit's walker-dispatched bypass in eval_loop_jit_bridge eval_loop_jit consults walker_dispatched_this_opcode after jit_merge_point_hook so production_walker_handles allow-listed opcodes skip execute_opcode_step (preventing double-mutation of the live frame) and drain BH_LAST_EXC_VALUE into PyError if a raise is pending. The bridge loop now does the same. Assisted-by: Claude * jit_fnaddr: add regression test for STORE_SUBSCR helper bindings Assert jit_trace_fnaddrs() registers the bare execute_store_subscr key and both bh_store_subscr_fn module-path aliases. A typo in the registration would otherwise surface only at trace time. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jul 3, 2026
Port the ordered-dict lookup read vertical (rordereddict.py): ll_malloc_indexes_and_choose_lookup (:520-541, size-threshold cascade), ll_dict_create_initial_index (:934-953; rehash_after_translation arm documented unreachable and omitted), ll_ensure_indexes, ll_call_lookup_function (:46-65; FUNC_* while-loop collapsed to ensure+single lookup since all DICTINDEX widths alias Unsigned locally, noted for #148), ll_dict_getitem (:655-663, KeyError raise via exceptblock per the rlist IndexError idiom) and ll_dict_contains (:1462-1468), each as a cached helper graph. OrderedDictRepr::rtype_getitem follows rordereddict.py:441-467: implicit-KeyError registration, lookup-chain direct_calls, recast_value + convertvar on the result. contains dispatches via a new pairtype arm mirroring the tuple-contains shape. Fail-closed eq-gate: build_ll_dict_lookup_helper_graph's direct_compare_op hardcodes ptr_eq for Ptr keys, which is wrong for key reprs defining a real get_ll_eq_function (str: ll_streq). getitem and contains now require a direct-comparable key first; str/instance keys raise a TyperError classified in is_known_unported under a new DICT-KEY-EQ census bucket. The key hash function is threaded from key_repr.get_ll_hash_function into the helper graphs. 9 new unit tests (builder CFG shapes, eq-gate fail-closed on str keys, contains wiring). Census neutral: phaseA 599 / phaseB 55 bucket-identical vs clean HEAD on the fresh post-#322 corpus; no dicttable/odictentry owner reaches assembly; check.py 169/169 x2; dunder_repr_str_errors + exception_oserror_fields probes 6x each green. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jul 3, 2026
Port the rordereddict.py setitem chain onto OrderedDictRepr: - OrderedDictRepr::rtype_setitem (rordereddict.py:448-455) with the custom_eq_hash exception_is_here / exception_cannot_occur split, plus the (OrderedDictRepr, _, "setitem") pairtype arm. - ll_dict_setitem_helper mints the helper chain, reusing the Slice-2 lookup_chain_helpers path (and thus the require_direct_compare_key eq-gate: str/instance keys stay fail-closed). - 9 new helper-graph builders replacing the deferred stubs: store_clean, insert_clean, entries_arraycopy, grow, reindex, resize_to, resize, setitem_lookup_done (overwrite-vs-insert branch, rordereddict.py:518-560), setitem. Documented mint-time deviations (in-code comments cite upstream lines): compaction/shrink/entry-reuse branches collapsed while the closure is delete-free (ll_dict_remove_deleted_items not yet ported; restored with delitem), DICTINDEX width collapse to the single Unsigned width (#148), no MemoryError rescue (_ll_dict_rescue). Gates: cargo test -p majit-translate 2845 passed (rordereddict 29/29, 7 new); both release backends build; check.py 169/169 x2; #323 probes 6x2 byte-identical dynasm; census unchanged at phaseA 599 / phaseB 55 with zero dicttable/odictentry/ll_dict_ occurrences in the prepass log. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jul 3, 2026
Port the rordereddict.py deletion family onto OrderedDictRepr: - rtype_delitem (rordereddict.py:456-463: has_implicit_exception KeyError when not custom_eq_hash, exception_is_here) + Repr trait default + the (OrderedDictRepr, _, "delitem") pairtype arm. - Deletion chain builders: ll_dict_delitem -> _ll_dict_del (with the 87.5%-dead shrink rule) -> _ll_dict_del_entry (f_valid=False, num_live_items -= 1, conditional GC ref clears) + ll_call_delete_by_entry_index -> ll_dict_delete_by_entry_index. - ll_dict_remove_deleted_items (rordereddict.py:802-851) and restore the branches Slice 3 collapsed as delete-free-unreachable: ll_dict_grow's compaction branch and _ll_dict_resize_to's shrink handling now follow upstream shape. _ll_dict_entries_size_too_big stays collapsed (inert while DICTINDEX_* alias Unsigned, #148), documented in-code. - rtype_method get/setdefault (rordereddict.py:285-301) via the existing Repr::rtype_method hook, with ll_dict_get/ll_dict_setdefault helpers; 2-arg d.get(k) synthesizes the None default locally. - File brought to rustfmt conformance (prior slices had fmt drift). All lookup-dependent helpers thread through lookup_chain_helpers, so the require_direct_compare_key eq-gate still fails closed for str/instance keys. Gates: cargo test -p majit-translate green (rordereddict 42/42, 13 new); both release backends build; check.py 169/169 x2; #323 probes 3x2 byte-identical dynasm; census identical to the post-rebase baseline phaseA 488 / phaseB 154 (bucket-exact, 357 [CodeWriter] graphs, zero dicttable/odictentry/ll_dict_ occurrences in the log). Assisted-by: Codex Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jul 3, 2026
* rtyper: stamp OBJECTPTR for dont_look_inside PyObject returns dont_look_inside_return_token stamped the generic "ref" token for every reference return, including *mut PyObject. That token lowered to GCREF in cutover and, because it is not None, also blocked the merge_hints_from_llbcs fallback that would have applied OBJECTPTR_RETURN_TYPE. An opaque callee returning a PyObjectRef (e.g. lookup_exc_class_for_kind) whose result is used as a Python object was then rtyped against the generic opaque pointer instead of the typed OBJECTPTR. Special-case output_type_is_objectptr before the generic ref arm so a *mut PyObject result stamps OBJECTPTR_RETURN_TYPE, which cutover maps to the typed OBJECTPTR. Other reference returns keep the GCREF ref token. Assisted-by: Claude * rtyper: fail-closed unfused malloc_typed in translate_op An unfused `lltype::malloc_typed` FunctionPath reaches `flowspace_adapter::translate_op` with no ported `jtransform.rewrite_op_malloc` general lowering. `fuse_boxing_alloc` rewrites only the numeric boxing structs (W_Float/W_Int/W_Complex/W_Long, per `model.rs payload_fields`) to `NewWithVtable` before the rtyper runs; any other mallocable GC struct survives. Layer-1 misses it (cutover skips registering `malloc_typed`), so it resolved to the HOST_ENV builtin at Layer-3b and matched a residual `simple_call` carrying a symbolic fnaddr the executor cannot run. Reject such a path with a `TyperError` classified in `is_known_unported` so the graph census-Skips to the legacy walker. Add an UNFUSED-MALLOC disposition bucket to `classify_unported_reason` and correct the stale three-struct fusion notes to include W_LongObject. Census-neutral: prepass phaseA 598 / phaseB 55 unchanged; 2 already-failing graphs re-bucket from FUNCPATH-OTHER/THREADLOCAL into UNFUSED-MALLOC. check.py 169/169 x2 (dynasm + cranelift). Assisted-by: Claude * rtyper: port ll_newdict vertical (newdict op wiring) Add the newdict translate arm (rtyper.py:531-532) dispatching to rdict::rtype_newdict, now real per rdict.py:60-65: no-arg inputargs, downcast hop.r_result to OrderedDictRepr, gendirectcall ll_newdict. OrderedDictRepr::ll_newdict mints a cached helper graph via lowlevel_helper_function_with_builder; build_ll_dict_newdict_helper_graph synthesizes the single-block body of rordereddict.py:1160-1169 + ll_no_initial_index :509-518 — malloc(DICT), zero-length entries array (malloc_varsize; _ll_empty_array memo-sharing :1155-1158 deferred, fresh allocation per call), num_live_items=0, num_ever_used_items=0, lookup_function_no=FUNC_MUST_REINDEX, indexes=null GCREF. The Void cDICT class argument is baked into the helper closure instead of threaded through gendirectcall, matching the rlist newlist helper shape. Remove the ll_newdict dead checklist stub; keep the sibling _ll_empty_array/_ll_malloc_* markers per file precedent. Add a structural unit test for the builder (op sequence, field consts, FUNC_MUST_REINDEX seed, null indexes, return type). No census subject currently reaches newdict (prepass phaseA 598 / phaseB 55 unchanged, zero newdict occurrences in the closure log; no dicttable/odictentry owner reaches assembly). check.py 169/169 x2 (dynasm + cranelift); dunder_repr_str_errors + exception_oserror_fields probes 6x each byte-identical on dynasm. Assisted-by: Claude * rtyper: port dict getitem/contains read path Port the ordered-dict lookup read vertical (rordereddict.py): ll_malloc_indexes_and_choose_lookup (:520-541, size-threshold cascade), ll_dict_create_initial_index (:934-953; rehash_after_translation arm documented unreachable and omitted), ll_ensure_indexes, ll_call_lookup_function (:46-65; FUNC_* while-loop collapsed to ensure+single lookup since all DICTINDEX widths alias Unsigned locally, noted for #148), ll_dict_getitem (:655-663, KeyError raise via exceptblock per the rlist IndexError idiom) and ll_dict_contains (:1462-1468), each as a cached helper graph. OrderedDictRepr::rtype_getitem follows rordereddict.py:441-467: implicit-KeyError registration, lookup-chain direct_calls, recast_value + convertvar on the result. contains dispatches via a new pairtype arm mirroring the tuple-contains shape. Fail-closed eq-gate: build_ll_dict_lookup_helper_graph's direct_compare_op hardcodes ptr_eq for Ptr keys, which is wrong for key reprs defining a real get_ll_eq_function (str: ll_streq). getitem and contains now require a direct-comparable key first; str/instance keys raise a TyperError classified in is_known_unported under a new DICT-KEY-EQ census bucket. The key hash function is threaded from key_repr.get_ll_hash_function into the helper graphs. 9 new unit tests (builder CFG shapes, eq-gate fail-closed on str keys, contains wiring). Census neutral: phaseA 599 / phaseB 55 bucket-identical vs clean HEAD on the fresh post-#322 corpus; no dicttable/odictentry owner reaches assembly; check.py 169/169 x2; dunder_repr_str_errors + exception_oserror_fields probes 6x each green. Assisted-by: Claude * rtyper: port ordered dict setitem write path Port the rordereddict.py setitem chain onto OrderedDictRepr: - OrderedDictRepr::rtype_setitem (rordereddict.py:448-455) with the custom_eq_hash exception_is_here / exception_cannot_occur split, plus the (OrderedDictRepr, _, "setitem") pairtype arm. - ll_dict_setitem_helper mints the helper chain, reusing the Slice-2 lookup_chain_helpers path (and thus the require_direct_compare_key eq-gate: str/instance keys stay fail-closed). - 9 new helper-graph builders replacing the deferred stubs: store_clean, insert_clean, entries_arraycopy, grow, reindex, resize_to, resize, setitem_lookup_done (overwrite-vs-insert branch, rordereddict.py:518-560), setitem. Documented mint-time deviations (in-code comments cite upstream lines): compaction/shrink/entry-reuse branches collapsed while the closure is delete-free (ll_dict_remove_deleted_items not yet ported; restored with delitem), DICTINDEX width collapse to the single Unsigned width (#148), no MemoryError rescue (_ll_dict_rescue). Gates: cargo test -p majit-translate 2845 passed (rordereddict 29/29, 7 new); both release backends build; check.py 169/169 x2; #323 probes 6x2 byte-identical dynasm; census unchanged at phaseA 599 / phaseB 55 with zero dicttable/odictentry/ll_dict_ occurrences in the prepass log. Assisted-by: Claude * rtyper: port dict delitem, get/setdefault, deleted-item compaction Port the rordereddict.py deletion family onto OrderedDictRepr: - rtype_delitem (rordereddict.py:456-463: has_implicit_exception KeyError when not custom_eq_hash, exception_is_here) + Repr trait default + the (OrderedDictRepr, _, "delitem") pairtype arm. - Deletion chain builders: ll_dict_delitem -> _ll_dict_del (with the 87.5%-dead shrink rule) -> _ll_dict_del_entry (f_valid=False, num_live_items -= 1, conditional GC ref clears) + ll_call_delete_by_entry_index -> ll_dict_delete_by_entry_index. - ll_dict_remove_deleted_items (rordereddict.py:802-851) and restore the branches Slice 3 collapsed as delete-free-unreachable: ll_dict_grow's compaction branch and _ll_dict_resize_to's shrink handling now follow upstream shape. _ll_dict_entries_size_too_big stays collapsed (inert while DICTINDEX_* alias Unsigned, #148), documented in-code. - rtype_method get/setdefault (rordereddict.py:285-301) via the existing Repr::rtype_method hook, with ll_dict_get/ll_dict_setdefault helpers; 2-arg d.get(k) synthesizes the None default locally. - File brought to rustfmt conformance (prior slices had fmt drift). All lookup-dependent helpers thread through lookup_chain_helpers, so the require_direct_compare_key eq-gate still fails closed for str/instance keys. Gates: cargo test -p majit-translate green (rordereddict 42/42, 13 new); both release backends build; check.py 169/169 x2; #323 probes 3x2 byte-identical dynasm; census identical to the post-rebase baseline phaseA 488 / phaseB 154 (bucket-exact, 357 [CodeWriter] graphs, zero dicttable/odictentry/ll_dict_ occurrences in the log). Assisted-by: Codex Assisted-by: Claude
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #122
Summary
Two structurally-linked epic foundations that together prepare for retiring the M4 SIGBUS workaround in [[project_m4_walker_remaining_unactivated_taxonomy_2026_06_06]]:
Task #390 — widen walker concrete-execute to non-elidable residual_calls. PyPy
_opimpl_residual_call{1,2,3}(pyjitpl.py:1346/1349/1354) concrete-executes everydo_residual_callregardless ofcheck_is_elidable()— the EI flag picks the record opcode (CALL_PURE_*vsCALL_*), not whether execution happens. The pyre walker currently concrete-executes only the elidable path viatry_fold_pure_call_via_executor. Non-elidable helpers (store_subscr_fn/set_current_exception/ …) are recorded but never executed;eval_loop_jit:3111's walker-dispatched skip then leaves the heap unchanged → SIGBUS in 5 STORE_SUBSCR-hot benches. This PR lands sub-slices 1-5 of the orthodox fix:baf59d132f,109bccbd6d)majit_metainterp::executor::execute_residual_callwalker-friendly entry +try_execute_residual_call_via_executorforCall*/CallLoopinvariant*shapes, wired to 3 dispatch sites with!can_raisegate (f7b12284e5,e042d274b1,ca780d7cb3)walker_record_guard_exceptionhelper,WalkContext.last_exc_valueseeding,BH_LAST_EXC_VALUErestore,eval.rswalker-skip path surfaces non-zero exc asErr(PyError)(7dd4238bb8)(func_ptr as u64) >> 47 != 0rejects unpatchedsymbolic_fnaddr_for_pathDefaultHasher placeholders; lifts the!can_raisegate at all 3 dispatch sites (963c2d807e)bh_execute_store_subscrC-ABI wrapper inpyre-interpreter::opcode_ops+jit_trace_fnaddrs()entry for path"execute_store_subscr"; build-time codewriter now bakes the real address intoconstants_iandruntime_fnaddr_patchmaps build→runtime correctly (0a4cfca1bc)StoreSubscr walker activation itself is not in this PR — it surfaces a heapcache EffectInfo gap (setitem's list/dict strategy-field writes aren't visible across the trait-dispatch boundary the codewriter analyses). Sub-slice 5b will widen EffectInfo via per-type wrappers (
bh_list_setitem/bh_dict_setitem/ …) following the PyPybhimpl_setarrayitem_gc_*precedent.Phase 5.B
dispatch_arm_via_blackholebody wire (workaround scaffolding). The original workaround path for the same M4 class: run the arm jitcode throughBlackholeInterpreterto concrete-execute the missing helpers. Foundation work committed here (dca58bbdbd→08bc3d210a) — JitCode wrapper cache, thread-local BH builder, acquire/release wiring,bh.run()+ outcome triage, virtualizable wiring, exception → PyError propagation, return-type dispatch,setposition()BH register-bank init.production_blackhole_handlespredicate returnsfalsefor every opcode today; the wire is unreachable until #390 sub-slice 6 retires it once StoreSubscr activation lands on the orthodox path.pyre/check.py41/41 green on dynasm + cranelift at HEAD0a4cfca1bc.Self-review
This patch is not AI-generated. (Assisted by Claude across the session captured in the linked memory note; commits authored by Jeong, YunWon.)