Optimize tuple unpacking and specialization in JIT trace - #59
Merged
youknowone merged 24 commits intoJun 8, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds tuple ChangesTuple specialization and type identity
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
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/state.rs (1)
1259-1280:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve exact
boolidentity here too.
from_pyobjnow preserves non-exactintidentity, but this branch still collapses exactbooltoConcreteValue::Int. Becauseto_pyobjreboxes everyConcreteValue::Intwithw_int_new, any bool that round-trips through concrete tracking comes back as anint, and downstream exact-type checks lose the distinction again.Suggested fix
- if is_bool(obj) { - ConcreteValue::Int(w_bool_get_value(obj) as i64) + if is_bool(obj) { + ConcreteValue::Ref(obj) } else if is_trace_plain_int(obj) { ConcreteValue::Int(w_int_get_value(obj))🤖 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/state.rs` around lines 1259 - 1280, from_pyobj currently collapses exact booleans into ConcreteValue::Int which loses bool identity on round-trip; change the is_bool branch to preserve booleans as a distinct ConcreteValue (e.g. ConcreteValue::Bool(w_bool_get_value(obj) != 0) or similar) instead of ConcreteValue::Int, and update to_pyobj to handle ConcreteValue::Bool by creating/returning a boolean PyObject (use the boolean constructor w_bool_new or equivalent) so bools are not reboxed as ints; update any enum/type definitions if needed to add ConcreteValue::Bool and handle it alongside ConcreteValue::Int/Float/Ref in from_pyobj and to_pyobj.
🤖 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 `@pyre/pyre-jit-trace/src/trace_opcode.rs`:
- Around line 4528-4705: trace_build_tuple_value can emit specialised tuple
trace objects (Cls_ii/Cls_ff/Cls_oo) but leaves the corresponding concrete
PyObjectRef (concrete_items / concrete_seq) with the original plain tuple type,
causing later concrete_seq.ob_type checks to miss the specialised layout; fix
by, immediately after creating a specialised tuple branch (where you call
trace_set_tuple_w_class and record the SetfieldGc ops), also update the concrete
PyObjectRef's ob_type to the matching specialised tuple class so the
interpreter-side concrete_seq observes the same class as the trace (use the same
specialised_tuple_*_w_class_descr()/w_class symbol used in
trace_set_tuple_w_class to determine the class to write); apply the same change
to the other specialised-tuple-building site mentioned (the second
specialised-creation block around the later range).
In `@pyre/pyre-object/src/tupleobject.rs`:
- Around line 359-360: The test fixtures set (*lhs).w_class and (*rhs).w_class
to crate::noneobject::w_none(), which is wrong because w_class must point to a
real type object; replace these assignments with a dedicated test type instance
(e.g., create a distinct "fake tuple subclass" type via a helper like
make_test_subclass() or an instantiated TypeObject) and assign that type pointer
to (*lhs).w_class and (*rhs).w_class (also update the same change at the other
occurrence around lines 380–381); ensure you use the existing type-construction
helper or add one and handle any required reference management when attaching
the type to the fixtures.
---
Outside diff comments:
In `@pyre/pyre-jit-trace/src/state.rs`:
- Around line 1259-1280: from_pyobj currently collapses exact booleans into
ConcreteValue::Int which loses bool identity on round-trip; change the is_bool
branch to preserve booleans as a distinct ConcreteValue (e.g.
ConcreteValue::Bool(w_bool_get_value(obj) != 0) or similar) instead of
ConcreteValue::Int, and update to_pyobj to handle ConcreteValue::Bool by
creating/returning a boolean PyObject (use the boolean constructor w_bool_new or
equivalent) so bools are not reboxed as ints; update any enum/type definitions
if needed to add ConcreteValue::Bool and handle it alongside
ConcreteValue::Int/Float/Ref in from_pyobj and to_pyobj.
🪄 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: b0b9e87c-6e31-4c64-b678-8812a1e2ed63
⛔ Files ignored due to path filters (1)
pyre/pyre-jit-trace/tests/snapshots/opcode_handler_impls.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
pyre/pyre-jit-trace/src/descr.rspyre/pyre-jit-trace/src/opcode_handler_impls_pre.template.rspyre/pyre-jit-trace/src/state.rspyre/pyre-jit-trace/src/trace_opcode.rspyre/pyre-object/src/listobject.rspyre/pyre-object/src/tupleobject.rs
Comment on lines
+4528
to
+4705
| pub(crate) fn trace_build_tuple_value( | ||
| &mut self, | ||
| items: &[OpRef], | ||
| concrete_items: &[PyObjectRef], | ||
| ) -> Result<OpRef, PyError> { | ||
| if concrete_items.iter().any(|item| item.is_null()) { | ||
| // STRUCTURAL ADAPTATION: PyPy's `space.newtuple()` always | ||
| // reaches `wraptuple()` with concrete W_Root instances, so | ||
| // `makespecialisedtuple2()` can choose `Cls_ii` / `Cls_ff` / | ||
| // `Cls_oo` for arity 2. Pyre's trace-side FrontendOp may | ||
| // lack that concrete side channel after earlier trace-only | ||
| // operations. In that case, keep the older helper path rather | ||
| // than guessing a canonical W_TupleObject and regressing | ||
| // specialised tuple parity on escaped two-tuples. | ||
| return self.trace_build_tuple(items); | ||
| } | ||
|
|
||
| self.with_ctx(|this, ctx| unsafe { | ||
| if items.len() == 2 { | ||
| let lhs = concrete_items[0]; | ||
| let rhs = concrete_items[1]; | ||
| if pyre_object::is_plain_int1(lhs) && pyre_object::is_plain_int1(rhs) { | ||
| trace_guard_exact_python_class(this, ctx, items[0], &INT_TYPE); | ||
| trace_guard_exact_python_class(this, ctx, items[1], &INT_TYPE); | ||
| let raw0 = trace_plain_int_payload(this, ctx, items[0], lhs); | ||
| let raw1 = trace_plain_int_payload(this, ctx, items[1], rhs); | ||
| let tuple = ctx.record_op_with_descr( | ||
| OpCode::NewWithVtable, | ||
| &[], | ||
| crate::descr::specialised_tuple_ii_size_descr(), | ||
| ); | ||
| ctx.heap_cache_mut().new_object(tuple); | ||
| trace_set_tuple_w_class( | ||
| ctx, | ||
| tuple, | ||
| crate::descr::specialised_tuple_ii_w_class_descr(), | ||
| ); | ||
| ctx.record_op_with_descr( | ||
| OpCode::SetfieldGc, | ||
| &[tuple, raw0], | ||
| crate::descr::specialised_tuple_ii_value0_descr(), | ||
| ); | ||
| ctx.heapcache_setfield_cached( | ||
| tuple, | ||
| crate::descr::specialised_tuple_ii_value0_descr().index(), | ||
| raw0, | ||
| ); | ||
| ctx.record_op_with_descr( | ||
| OpCode::SetfieldGc, | ||
| &[tuple, raw1], | ||
| crate::descr::specialised_tuple_ii_value1_descr(), | ||
| ); | ||
| ctx.heapcache_setfield_cached( | ||
| tuple, | ||
| crate::descr::specialised_tuple_ii_value1_descr().index(), | ||
| raw1, | ||
| ); | ||
| return Ok(tuple); | ||
| } | ||
|
|
||
| if pyre_object::is_plain_float_strict(lhs) | ||
| && pyre_object::is_plain_float_strict(rhs) | ||
| { | ||
| trace_guard_exact_python_class(this, ctx, items[0], &FLOAT_TYPE); | ||
| trace_guard_exact_python_class(this, ctx, items[1], &FLOAT_TYPE); | ||
| let raw0 = if this.value_type(items[0]) == Type::Float { | ||
| items[0] | ||
| } else { | ||
| crate::state::trace_unbox_float_with_resume( | ||
| this, | ||
| ctx, | ||
| items[0], | ||
| &FLOAT_TYPE as *const _ as i64, | ||
| ) | ||
| }; | ||
| let raw1 = if this.value_type(items[1]) == Type::Float { | ||
| items[1] | ||
| } else { | ||
| crate::state::trace_unbox_float_with_resume( | ||
| this, | ||
| ctx, | ||
| items[1], | ||
| &FLOAT_TYPE as *const _ as i64, | ||
| ) | ||
| }; | ||
| let tuple = ctx.record_op_with_descr( | ||
| OpCode::NewWithVtable, | ||
| &[], | ||
| crate::descr::specialised_tuple_ff_size_descr(), | ||
| ); | ||
| ctx.heap_cache_mut().new_object(tuple); | ||
| trace_set_tuple_w_class( | ||
| ctx, | ||
| tuple, | ||
| crate::descr::specialised_tuple_ff_w_class_descr(), | ||
| ); | ||
| ctx.record_op_with_descr( | ||
| OpCode::SetfieldGc, | ||
| &[tuple, raw0], | ||
| crate::descr::specialised_tuple_ff_value0_descr(), | ||
| ); | ||
| ctx.heapcache_setfield_cached( | ||
| tuple, | ||
| crate::descr::specialised_tuple_ff_value0_descr().index(), | ||
| raw0, | ||
| ); | ||
| ctx.record_op_with_descr( | ||
| OpCode::SetfieldGc, | ||
| &[tuple, raw1], | ||
| crate::descr::specialised_tuple_ff_value1_descr(), | ||
| ); | ||
| ctx.heapcache_setfield_cached( | ||
| tuple, | ||
| crate::descr::specialised_tuple_ff_value1_descr().index(), | ||
| raw1, | ||
| ); | ||
| return Ok(tuple); | ||
| } | ||
|
|
||
| let tuple = ctx.record_op_with_descr( | ||
| OpCode::NewWithVtable, | ||
| &[], | ||
| crate::descr::specialised_tuple_oo_size_descr(), | ||
| ); | ||
| ctx.heap_cache_mut().new_object(tuple); | ||
| trace_set_tuple_w_class( | ||
| ctx, | ||
| tuple, | ||
| crate::descr::specialised_tuple_oo_w_class_descr(), | ||
| ); | ||
| ctx.record_op_with_descr( | ||
| OpCode::SetfieldGc, | ||
| &[tuple, items[0]], | ||
| crate::descr::specialised_tuple_oo_value0_descr(), | ||
| ); | ||
| ctx.heapcache_setfield_cached( | ||
| tuple, | ||
| crate::descr::specialised_tuple_oo_value0_descr().index(), | ||
| items[0], | ||
| ); | ||
| ctx.record_op_with_descr( | ||
| OpCode::SetfieldGc, | ||
| &[tuple, items[1]], | ||
| crate::descr::specialised_tuple_oo_value1_descr(), | ||
| ); | ||
| ctx.heapcache_setfield_cached( | ||
| tuple, | ||
| crate::descr::specialised_tuple_oo_value1_descr().index(), | ||
| items[1], | ||
| ); | ||
| return Ok(tuple); | ||
| } | ||
|
|
||
| let len = ctx.const_int(items.len() as i64); | ||
| let array_descr = crate::state::pyobject_gcarray_descr(); | ||
| let items_block = ctx.record_op_with_descr(OpCode::NewArrayClear, &[len], array_descr); | ||
| ctx.heap_cache_mut().new_array(items_block, len, true); | ||
| for (idx, &item) in items.iter().enumerate() { | ||
| let idx = ctx.const_int(idx as i64); | ||
| crate::state::trace_items_block_setitem_value(ctx, items_block, idx, item); | ||
| } | ||
|
|
||
| let tuple = ctx.record_op_with_descr( | ||
| OpCode::NewWithVtable, | ||
| &[], | ||
| crate::descr::w_tuple_size_descr(), | ||
| ); | ||
| ctx.heap_cache_mut().new_object(tuple); | ||
| trace_set_tuple_w_class(ctx, tuple, crate::descr::tuple_w_class_descr()); | ||
| let wrappeditems_descr = crate::descr::tuple_wrappeditems_descr(); | ||
| ctx.record_op_with_descr( | ||
| OpCode::SetfieldGc, | ||
| &[tuple, items_block], | ||
| wrappeditems_descr.clone(), | ||
| ); | ||
| ctx.heapcache_setfield_cached(tuple, wrappeditems_descr.index(), items_block); | ||
| Ok(tuple) | ||
| }) |
There was a problem hiding this comment.
Align specialised tuple unpacking with the traced tuple layout.
MIFrame::trace_build_tuple_value can emit _ii/_ff/_oo trace objects while the same execution’s concrete_seq is still a plain interpreter W_TupleObject. These concrete_seq.ob_type checks therefore miss the specialised tuple you just built in-trace, and the fallback then records guard_class(..., &TUPLE_TYPE) plus tuple_wrappeditems_descr() accesses against a specialised tuple OpRef. That makes the build→unpack path bail out instead of using the new specialised layout.
Also applies to: 4717-4792
🤖 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/trace_opcode.rs` around lines 4528 - 4705,
trace_build_tuple_value can emit specialised tuple trace objects
(Cls_ii/Cls_ff/Cls_oo) but leaves the corresponding concrete PyObjectRef
(concrete_items / concrete_seq) with the original plain tuple type, causing
later concrete_seq.ob_type checks to miss the specialised layout; fix by,
immediately after creating a specialised tuple branch (where you call
trace_set_tuple_w_class and record the SetfieldGc ops), also update the concrete
PyObjectRef's ob_type to the matching specialised tuple class so the
interpreter-side concrete_seq observes the same class as the trace (use the same
specialised_tuple_*_w_class_descr()/w_class symbol used in
trace_set_tuple_w_class to determine the class to write); apply the same change
to the other specialised-tuple-building site mentioned (the second
specialised-creation block around the later range).
![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: 14c8392364
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+4549
to
+4551
| if pyre_object::is_plain_int1(lhs) && pyre_object::is_plain_int1(rhs) { | ||
| trace_guard_exact_python_class(this, ctx, items[0], &INT_TYPE); | ||
| trace_guard_exact_python_class(this, ctx, items[1], &INT_TYPE); |
There was a problem hiding this comment.
trace_build_tuple_value chooses the Cls_ii path from concrete_items, but those values are synthesized via i.concrete.to_pyobj() (BUILD_TUPLE handler) rather than the original runtime refs. Because bools are stored as ConcreteValue::Int, a (True, False) pair is reconstructed as plain ints, so is_plain_int1(lhs/rhs) passes and this branch emits trace_guard_exact_python_class(..., INT_TYPE) on the original bool refs. That guard cannot hold for bool objects, so traces recorded on boolean tuple construction immediately side-exit on replay, causing a repeatable hot-path deoptimization for tuple unpack/build patterns involving bools.
Useful? React with 👍 / 👎.
lifthrasiir
force-pushed
the
trace-visible-tuple-unpacking
branch
from
May 16, 2026 13:35
14c8392 to
b5b5546
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/b5b5546ac72157d2b6d84874adabc0fe9332e639/pyre-jit-trace/src/trace_opcode.rs#L4586-L4590
trace_build_tuple_value picks the Cls_ff specialization using is_plain_float_strict (which requires exact float w_class), but the emitted unbox guard uses trace_unbox_float_with_resume(..., FLOAT_TYPE) and only checks ob_type. In this codebase, float subclasses can share ob_type and differ only by w_class (see the new tupleobject test that mutates w_class), so a trace recorded on exact floats can incorrectly accept float-subclass inputs on replay, build a specialised ff tuple, and then unpack/getitem them as plain floats, dropping subclass identity.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pyre/pyre-jit-trace/src/state.rs`:
- Around line 1265-1270: ConcreteValue::from_pyobj is using a plain-int check
(is_trace_plain_int) but elsewhere constants are folded via is_int ->
try_trace_const_boxed_int, which loses int-subclass identity; update the code to
use the new exact-int predicate consistently: replace the plain/int-type checks
(the is_trace_plain_int usage in ConcreteValue::from_pyobj and the is_int check
that gates try_trace_const_boxed_int) with the new exact-int predicate, and
apply the same change to the other occurrence block around the 1360-1371 region
so that try_trace_const_boxed_int only runs for exact-int objects.
In `@pyre/pyre-jit-trace/src/trace_opcode.rs`:
- Around line 4509-4525: The fast-path for arity-2 tuple handling uses
concrete_items[0] and [1] without ensuring concrete_items has length >= 2, which
can panic if concrete_items is shorter than items; update the guard before the
arity-2 branch (inside with_ctx closure in trace_opcode.rs) to check that
concrete_items.len() >= 2 (in addition to no nulls) and fall back to
self.trace_build_tuple(items) when that condition fails; ensure the check
surrounds the block that calls pyre_object::is_plain_int1 on lhs/rhs so the
existing generic fallback remains used when concrete slice drift occurs.
🪄 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: 92c54cd9-1934-4110-9e48-a3eccd0e826c
📒 Files selected for processing (4)
pyre/pyre-jit-trace/src/state.rspyre/pyre-jit-trace/src/trace_opcode.rspyre/pyre-object/src/listobject.rspyre/pyre-object/src/tupleobject.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d688b4456b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+4539
to
+4540
| let raw0 = trace_plain_int_payload(this, ctx, items[0], lhs); | ||
| let raw1 = trace_plain_int_payload(this, ctx, items[1], rhs); |
There was a problem hiding this comment.
The Cls_ii fast path is selected using concrete is_plain_int1 values at trace time, but replay only unboxes via trace_plain_int_payload, which ultimately guards on ob_type (INT_TYPE/LONG_TYPE) and never checks w_class. That allows later executions with app-level int subclasses (same payload layout, different w_class) to stay on this trace and get re-materialized as plain ints, losing subclass identity and diverging from the runtime makespecialisedtuple2 predicate.
Useful? React with 👍 / 👎.
Comment on lines
+4586
to
+4590
| crate::state::trace_unbox_float_with_resume( | ||
| this, | ||
| ctx, | ||
| items[0], | ||
| &FLOAT_TYPE as *const _ as i64, |
There was a problem hiding this comment.
The Cls_ff branch is trace-selected from concrete is_plain_float_strict, but the emitted replay checks unbox with trace_unbox_float_with_resume, which only guards ob_type == FLOAT_TYPE and does not validate w_class. A later float-subclass instance can therefore hit this trace and be converted into plain float payloads, breaking subtype-preserving behavior expected when runtime specialization rejects non-exact floats.
Useful? React with 👍 / 👎.
youknowone
force-pushed
the
trace-visible-tuple-unpacking
branch
from
May 28, 2026 03:55
d688b44 to
55a4a37
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55a4a37a40
ℹ️ 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
+285
to
+286
| if expected_typeobj.is_null() || frame.value_type(obj) != Type::Ref { | ||
| return; |
There was a problem hiding this comment.
When the tuple elements are freshly boxed values produced in the same trace, e.g. (a + b, c + d) or the float equivalent, wrapint/wrapfloat allocate NewWithVtable and set only the payload field (helpers.rs emits NewWithVtable + SetfieldGc for intval/floatval), leaving PyObject.w_class as zero. This new guard runs for any Type::Ref item and compares that zero field with the canonical int/float type object, so those traces side-exit every time before reaching the specialized tuple path. Either initialize w_class for the inline boxed values or avoid this guard for boxes whose exact class is already implied by the just-created vtable.
Useful? React with 👍 / 👎.
Owner
|
Static parity/perf follow-up related to this PR scope: This PR is the right place to verify tuple-unpacking performance parity because it changes trace-visible tuple construction/unpacking specialization. The specific thing to investigate is whether the strict PyPy-parity guards for exact tuple/int/float class identity still allow the specialized tuple fast paths to be selected in the benchmark cases that PyPy optimizes well. If the strict guards are correct but the trace falls back to the generic tuple path too often, tuple unpacking can remain semantically correct while still missing PyPy-like performance. Suggested checks for this PR:
If all of those hold and the benchmark is still slow, the remaining cause is probably outside tuple specialization itself, likely in surrounding trace selection, guard placement, or downstream optimization/regalloc. |
youknowone
force-pushed
the
trace-visible-tuple-unpacking
branch
6 times, most recently
from
June 4, 2026 11:39
7bc25c7 to
61b01a7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61b01a7c62
ℹ️ 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".
| pub extern "C" fn bh_unpack_sequence_fn(count: i64, seq: i64) -> i64 { | ||
| let seq = seq as pyre_object::PyObjectRef; | ||
| match pyre_interpreter::runtime_ops::unpack_sequence_exact(seq, count as usize) { | ||
| Ok(items) => pyre_interpreter::runtime_ops::build_tuple_from_refs(&items) as i64, |
There was a problem hiding this comment.
For compiled UNPACK_SEQUENCE on a non-tuple sequence with two exact floats/ints, this temporary tuple is built through build_tuple_from_refs, which calls w_tuple_new and specializes arity-2 tuples. Reading the items back with bh_unpack_item_fn then goes through w_tuple_getitem and re-wraps the unboxed payload, so code like x = 1.5; a, b = [x, x]; a is x can become false on the JIT path even though the interpreter pushes the original element refs. The validation container needs to avoid arity-2 specialization or otherwise retain the original refs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 994e93b0d5
ℹ️ 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
+4854
to
+4856
| let int_typeobj = get_instantiate(&INT_TYPE); | ||
| trace_guard_exact_w_class(this, ctx, items[0], int_typeobj); | ||
| trace_guard_exact_w_class(this, ctx, items[1], int_typeobj); |
There was a problem hiding this comment.
When tuple elements are cached small ints, this guard rejects the trace even though is_plain_int1 selected the Cls_ii path: the small-int cache initializes PyObject.w_class to null (pyre/pyre-object/src/intobject.rs:65-68), and is_plain_int1 treats that null as an exact int, but the new replay guard compares it to the canonical int type object. As a result hot traces for common tuples like (1, 2) side-exit on the first replay instead of using the specialized tuple path; either skip/relax the guard for null exact-int w_class or initialize the cached ints consistently.
Useful? React with 👍 / 👎.
youknowone
force-pushed
the
trace-visible-tuple-unpacking
branch
2 times, most recently
from
June 6, 2026 06:14
5b0b40a to
c9a2a4f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9a2a4ffaf
ℹ️ 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".
| ref_reg: u16, | ||
| dst_reg: u16, | ||
| ) -> Insn { | ||
| let effect_info = effect_info_for_call_flavor(CallFlavor::Plain); |
There was a problem hiding this comment.
When this helper is used for unpack_sequence_fn (see the Instruction::UnpackSequence call site in codewriter.rs), CallFlavor::Plain is too weak for non-list/tuple operands: unpack_sequence_exact can call user code through baseobjspace::len, getitem, iter, and next for instances/iterables. In a hot loop unpacking a custom sequence, the residual call can therefore re-enter Python without the may-force virtualizable/vref preparation that CallFlavor::MayForce provides, leaving the compiled frame state inconsistent across callbacks or exceptions.
Useful? React with 👍 / 👎.
youknowone
force-pushed
the
trace-visible-tuple-unpacking
branch
2 times, most recently
from
June 7, 2026 12:20
c039ac0 to
dc65ddb
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/dc65ddb74226c992005d95c9329209228756225d/pyre-jit/src/call_jit.rs#L3323
When the compiled UNPACK_SEQUENCE path reaches this helper for an iterable that has no sequence_len fast path, runtime_ops::unpack_sequence_exact consumes exactly count items and returns without probing for an additional item (runtime_ops.rs:835-849). That makes hot code such as a, b = iter([1, 2, 3]) continue successfully on the JIT path instead of raising the required "too many values to unpack" error; the previous codewriter path aborted rather than executing this helper. Please either add the final exhaustion check before returning the validation tuple or keep this residual path out of iterator fallback cases.
ℹ️ 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".
youknowone
force-pushed
the
trace-visible-tuple-unpacking
branch
2 times, most recently
from
June 8, 2026 13:50
58844a0 to
019b327
Compare
The tuple_unpacking synthetic benchmark was still slower than CPython after making UNPACK_SEQUENCE aware of specialised tuple instances. The remaining cost was that BUILD_TUPLE still emitted opaque jit_build_tuple_N helper calls, so the optimizer could not see or virtualize tuples that are immediately unpacked. Mirror the PyPy/RPython object shapes instead of introducing side tables: arity-2 int, float, and object tuples now trace as W_SpecialisedTupleObject_ii/ff/oo with value0/value1 fields, while other arities trace as W_TupleObject with a wrappeditems GC array. UNPACK_SEQUENCE now guards and reads the corresponding specialised tuple fields directly before falling back to the canonical W_TupleObject wrappeditems path. This keeps the trace aligned with pypy/objspace/std/specialisedtupleobject.py and tupleobject.py, and exposes the allocations and field stores to OptVirtualize. Verification: cargo fmt; cargo check --no-default-features --features dynasm; cargo test --no-default-features --features dynasm -p pyre-jit-trace; cargo test --no-default-features --features dynasm; python3 pyre/check.py --synthetic-only --synthetic-pattern tuple_unpacking.py. The focused tuple benchmark now reports dynasm about 0.01-0.02s and cranelift about 0.02s versus CPython about 0.16-0.33s and PyPy about 0.01-0.03s. Full synthetic was also run; tuple_unpacking passed, while pre-existing unrelated failures remain in class_attrs_methods, context_manager, inheritance_dispatch, and cranelift iteration_protocol.
Booleans were collapsed into ConcreteValue::Int, causing (True, False) pairs to be reconstructed as plain ints during tuple specialisation. This made trace_build_tuple_value wrongly take the Cls_ii path and emit GuardClass(INT_TYPE) guards that immediately fail on bool objects, creating a repeatable hot-path deoptimization loop. Add ConcreteValue::Bool(bool) so the round-trip preserves type identity. Also fix test fixtures that used w_none() as a fake w_class (None is not a type object) — use &INSTANCE_TYPE instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add is_trace_plain_float predicate (mirrors is_trace_plain_int) so from_pyobj only unboxes exact floats; float subclasses stay as ConcreteValue::Ref, preventing trace_build_tuple_value from wrongly taking the Cls_ff path on reconstructed subclass items. - Fix try_trace_const_boxed_int: replace is_int (which accepts subclasses and bools) with is_trace_plain_int, and reorder so is_bool is checked first. Int subclasses are no longer folded to plain int constants. - Guard concrete_items.len() >= 2 before accessing [0]/[1] in trace_build_tuple_value to prevent panic on mismatched slice lengths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
write_ref_reg and the vable struct-pointer reader sanitize Bool into the same Null path as Int/Float; write_int_reg coerces it to the i64 shadow value via 'as i64'.
trace_build_tuple_value selected the int-int and float-float specialised tuple branches on is_plain_int1 / is_plain_float_strict but emitted only ob_type guards via the unbox helpers, leaving a later int/float subclass with the same payload layout free to replay the trace and lose subclass identity. Emit GetfieldGcR(w_class) + PtrEq + GuardTrue against get_instantiate(&INT_TYPE) / get_instantiate(&FLOAT_TYPE) before the unbox so non-exact instances side-exit.
replaced_with_const was a Vec<Option<OpRef>> keyed by OpRef::raw() (a position index), so IntOp(n) and RefOp(n) aliased to the same slot. Store (old, new) OpRef pairs and match on the full OpRef in get_box_replacement / replace_box. walk_const_ptr_refs forwards only the value slot of each pair. Assisted-by: Claude
as_operation (optimizer.py:372) returns None for a Const, since a Const is not an AbstractResOp. find_producing_op called OpRef::raw() before checking, which panics on inline-Const OpRef variants when propagate_bounds_backward reaches a constant-folded operand. Assisted-by: Claude
trace_guard_exact_w_class emitted GetfieldGcR(w_class) + PtrEq + GuardTrue for every Cls_ii / Cls_ff tuple item. Items that are unboxed payloads (Type::Int / Type::Float) or boxes freshly allocated in the trace by wrapint / wrapfloat carry no divergent class: emit_box_int_inline writes only the payload field and leaves PyObject.w_class zero, so the guard compared 0 against the int/float type object and side-exited on every replay. Skip the guard for non-Ref values and for unescaped (heapcache-tracked in-trace) allocations, whose exact class is known. Keep BuildTuple / UnpackSequence on the trait-dispatch path; the walker jitcode dispatch aborts with InlineCallArityMismatch on the specialised arity-2 tuple layout. Add a test_miframe helper and a regression test for the primitive-value guard skip. Assisted-by: Claude
Two cascading JIT failures hit any tuple build/unpack that compiles: - StoreFastStoreFast's codewriter arm lowers to residual_call ops whose funcptr constants_i entries are unresolved placeholders (not rewritten by patch_constants_i_fnaddrs, which only maps build->runtime fnaddr pairs). The walker reads such a placeholder as the call target and branches to it, faulting on an unmapped address (SIGBUS). Drop StoreFastStoreFast from production_walker_handles so it dispatches through the trait handler opcode_store_fast_store_fast. - The trace-visible specialised-tuple build virtualizes the inline value0/value1 Int fields; OptVirtualize::propagate_forward then forwards the re-boxed Ref result of the unpack onto an Int field, tripping make_equal_to's Box.type invariant (Ref vs Int) and aborting the optimizer. trace_build_tuple_value now builds through the opaque jit_build_tuple_N helper, keeping the tuple a real heap object the optimizer never virtualizes. All tuple programs run crash-free and correct; tuple_unpacking is no longer a SIGBUS. The trace-level specialised-tuple optimization is disabled pending fixes to the two walker/optimizer bugs above. Assisted-by: Claude
…c-tuple build trace_guard_exact_w_class reads PyObject.w_class (offset 8) through a shared descr whose index_in_parent is 0. OptVirtualize folded that GetfieldGcR against a virtual W_IntObject (field 0 is intval, an Int), returned the Int payload, and forwarded Ref <- Int, tripping the make_equal_to Box.type invariant. This crashed the peeled loop of any spec-tuple build whose int/float items become loop-carried virtuals. - Add FieldDescr::is_w_class and SizeDescr::w_class_obj. - Name the w_class guard descr "w_class" so is_w_class recognises it. - PyreSizeDescr::w_class_obj returns get_instantiate(vtable_type). - optimize_getfield_gc resolves is_w_class reads on virtuals from the stored w_class field (specialised tuples) or the size descr's canonical class constant (int/float), instead of indexing value fields. - Re-enable the trace-visible specialised-tuple build in trace_build_tuple_value. tuple_unpacking no longer crashes; the peeled loop is pure integer arithmetic with the build->unpack tuple elided. Assisted-by: Claude
BUILD_TUPLE emitted abort_permanent, so a blackhole resume whose continuation builds a tuple (e.g. a tuple return on loop exit) hit OpKind::Abort and invalidated the loop. - bh_build_tuple_fn: (argc, item0..2) -> tuple via build_tuple_from_refs, CallFlavor::Plain like bh_build_list_fn. - build_tuple_fn added to the Cpu helper table and FnPtrIndices, bound after the existing fn_ptrs to preserve their indices. - emit_frontend_newtuple records the newtuple graph op. - BuildTuple handler emits newtuple plus a residual_call_ir_r to build_tuple_fn for argc <= 3 via the fn-index-agnostic build_build_list_fn_residual_call_ir_r_insn; argc > 3 stays abort. Assisted-by: Claude
UNPACK_SEQUENCE emitted abort_permanent, so a blackhole resume whose continuation unpacks a sequence (e.g. a variable-tuple target after a loop) hit OpKind::Abort and invalidated the loop. - bh_unpack_sequence_fn(count, seq): validates the length and returns a tuple of the items via unpack_sequence_exact, raising ValueError / TypeError on mismatch; bh_unpack_item_fn(index, seq): sequence_getitem on that validated tuple. Both publish raises through BH_LAST_EXC_VALUE and are classified CallFlavor::Plain. - unpack_sequence_fn / unpack_item_fn added to the Cpu helper table and FnPtrIndices, bound after the existing fn_ptrs to preserve indices. - build_one_int_one_ref_fn_residual_call_ir_r_insn: (Int, Ref) -> Ref residual_call_ir_r shape (Int arg first, then Ref). - UnpackSequence handler pops the sequence into a scratch reg, calls unpack_sequence_fn, then pushes each item read by unpack_item_fn in reverse so the stack top is item[0]. Assisted-by: Claude
…ct register-file length The closing GuardFutureCondition in close_loop_args_at lazy-inits every register the jitcode reports live at target_pc. With BUILD_TUPLE now a real residual-call op, its destination color is reported live one slot past the virtualizable window [nlocals..nlocals+stack_only], so registers_r extends to 4 with a NONE tail that production fills via materialize_fail_arg_slot. The same footprint occurs for BUILD_LIST and BINARY_OP; the prior exact-length expectation held only while BUILD_TUPLE was an abort stub that truncated liveness. Relax the length assertion to a lower bound covering the window and scope the non-NONE check to the virtualizable window the JUMP carries. Assisted-by: Claude
…tualize ensure_box was deleted in the BoxRef refactor; the w_class header-field branch in optimize_getfield_gc still called it. Use BoxRef::from_bound_op + get_box_replacement, matching the sibling array-field branch.
… blackhole _setup_return_value_* reads the result register as code[position-1]. The portal codewriter emits a per-push valuestackdepth setfield_vable_i immediately after a call result push, landing a 5-byte sync between the call's result-register byte and the next opcode's -live- resume anchor that position points at. Add call_result_reg() to step back over that sync (identified by BC_SETFIELD_VABLE_I + valuestackdepth VableField descr) to the result register; non-portal jitcode keeps position-1.
cmp against a Reg or Frame location truncated the immediate to i32, so a pointer-sized constant (e.g. a w_class type-object address) was compared against its sign-extended low 32 bits and never matched. PtrEq guards on such constants spuriously failed, deopting every loop iteration. Route the Reg case through emit_cmp_imm64 and add the i32::try_from / scratch-register fallback to the Frame case. Also remove the fail_arg debug eprintln from the dynasm guard handler.
youknowone
force-pushed
the
trace-visible-tuple-unpacking
branch
from
June 8, 2026 13:53
019b327 to
ef9bdc9
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/ef9bdc9c4d6b27546ab0691e27f47304cc439130/pyre-jit-trace/src/trace_opcode.rs#L4968-L4971
When an arity-2 build falls through to the Cls_oo path (for example (int_value, float_value) or (int_value, object_value)), the corresponding items[...] can still be a Type::Int/Type::Float op because exact Python ints/floats are kept unboxed in FrontendOp. This SetfieldGc stores those payload bits into a ref field, so later tuple reads can treat 1 or raw float bits as a PyObjectRef; the generic NewArrayClear path below has the same issue via trace_items_block_setitem_value. Box primitive stack values before writing them to OO/ref tuple storage.
ℹ️ 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".
youknowone
added a commit
that referenced
this pull request
Jun 8, 2026
The Bool(bool) variant (state.rs:1424) added in #59 was not covered at two walker-only match sites, leaving them non-exhaustive: - arraylen_vable_via_metainterp: a Bool concrete is not a vable struct pointer, so it joins Null/Int/Float in mapping to 0 (unseeded abort). - diagnose_inline_recognition: maps it to 'b' in the shape string. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 11, 2026
The Bool(bool) variant (state.rs:1424) added in #59 was not covered at two walker-only match sites, leaving them non-exhaustive: - arraylen_vable_via_metainterp: a Bool concrete is not a vable struct pointer, so it joins Null/Int/Float in mapping to 0 (unseeded abort). - diagnose_inline_recognition: maps it to 'b' in the shape string. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 12, 2026
The Bool(bool) variant (state.rs:1424) added in #59 was not covered at two walker-only match sites, leaving them non-exhaustive: - arraylen_vable_via_metainterp: a Bool concrete is not a vable struct pointer, so it joins Null/Int/Float in mapping to 0 (unseeded abort). - diagnose_inline_recognition: maps it to 'b' in the shape string. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 13, 2026
The Bool(bool) variant (state.rs:1424) added in #59 was not covered at two walker-only match sites, leaving them non-exhaustive: - arraylen_vable_via_metainterp: a Bool concrete is not a vable struct pointer, so it joins Null/Int/Float in mapping to 0 (unseeded abort). - diagnose_inline_recognition: maps it to 'b' in the shape string. Assisted-by: Claude
youknowone
added a commit
that referenced
this pull request
Jun 13, 2026
* refresh_virtualizable_shadow_from_heap: reserve trailing identity slot
`virtualizable_values`'s last entry stores the standard-vable identity
(`virtualizable_boxes[-1]` in RPython terms, see comment at
`virtualizable_box_at:1589`). `synchronize_virtualizable` already stops at
`static_count + sum(lengths)`; the read path now mirrors that bound so a
short or misaligned shadow (e.g. only the identity slot present, or
fewer data slots than the schema implies) cannot overwrite the
identity from heap fields.
Use `values.len().saturating_sub(1)` as the upper bound for both the
static-fields loop and the array-items cursor.
* pyframe free-function helpers: also bind bare-path aliases
`pyframe_get_pycode`, `ncells`, and `npure_cellvars` carry
`#[elidable_cannot_raise]` and are called both qualified (`pyframe::name`
from other modules) and unqualified (`name(...)` inside `pyframe.rs`
itself, e.g. pyframe.rs:783/1136/1143/1717).
`target_to_path` for a `FunctionPath` returns the segments verbatim, so
the bare call resolves to the 1-segment CallPath `["<name>"]` while a
cross-module qualified call resolves to `["pyframe", "<name>"]`. The
prior `push_fnaddr` registered only the 2-segment shape, so
`has_cannot_raise_assertion` missed the bare-path call sites and the
descrs stayed `EF_ELIDABLE_CAN_RAISE`.
Switch to `push_alias_pair` with both the 3-segment input
`pyre_interpreter::pyframe::<name>` and the 2-segment input
`pyre_interpreter::<name>`; `register_macro_helper_trace_fnaddr` strips
the leading segment of each, yielding the two CallPath shapes the
codewriter actually asks for.
* full-body walk: speculative int/float specialization for BINARY_OP/COMPARE_OP
Recognition vehicle (cross-crate: pyre-jit-trace does not depend on
pyre-jit, so fnaddr matching is unavailable):
- effectinfo: add OopSpecIndex::BinaryOp=200 / CompareOp=201, out of the
upstream OS_* range.
- flatten: build_residual_call_ir_r_insn_from_operands tags the BINARY_OP
/ COMPARE_OP helper calldescr EffectInfo with the oopspec; pin test
updated.
Full-body walk (PYRE_FULL_BODY_WALK, default-off):
- trace: run_perfn_walk / full_body_walk_trace drive dispatch_via_miframe
over the per-CodeObject JitCode with per-fn descr pool, loop-input
register seeding, and CloseLoop→CloseLoopWithArgs mapping.
- executor: execute_may_force_call runs a may-force helper concretely and
reports (boxed_result_i64, exc_value); tests for non-raising, raising,
and stale-exception-clear paths.
- jitcode_dispatch: dispatch_residual_call_iIRd_kind recognizes the
oopspec and re-emits the specialization instead of an opaque
CALL_MAY_FORCE. Int binary (Add/Sub/Mul ovf, And/Or/Xor, FloorDiv/Mod
via call_typed_with_effect_pure ll_int_py_div/mod with rhs==0 and
INT_MIN/-1 guards, Lshift ovfcheck, Rshift large-shift const-fold), int
compare (6 ops, non-fused bool box), float binary (Add/Sub/Mul/TrueDiv +
int operand CastIntToFloat), float compare (6 ops). Shared gates
walker_execute_may_force_boxed / walker_concrete_ref_object and unbox
helpers walker_unbox_int / walker_unbox_float / walker_coerce_operand_to_float.
- jitcode_runtime: overlay the full wellknown_bh_insns() universe onto the
byte→opname decode table so runtime JitCodes decode every reserved BC_*
byte.
- shadow_walker: pass authoritative=false (shadow validation must not
re-execute may-force calls).
check.py dynasm 39/39; pyre-jit-trace 232/0. Walker remains default-off;
flag-on resume (snapshot jitcode_pc coordinate) is the remaining blocker.
* full-body walk: per-opcode snapshot coordinate + branch-guard resume target
Add FULL_BODY_SNAPSHOT_SYM thread-local + FullBodySnapshotSymGuard so
walker_capture_snapshot_for_last_guard, when run from dispatch_via_miframe,
derives the snapshot's Python pc from the guard op's jitcode pc via
python_pc_for_jitcode_pc (inverse pc_map) and reads liveness from the live
walk register banks (ctx.registers_i/r/f) at that pc.
Parameterize collect_outer_active_boxes with regs_i/regs_r/regs_f slices so
the full-body walk reads its fresh top_regs banks instead of sym.registers_*.
In the goto_if_not/iL handler, pass other_target (the not-taken-in-trace
branch destination) to walker_capture_snapshot_for_last_guard instead of
op.pc, matching trace_opcode.rs resume_pc = other_target: GuardTrue resumes
at the jump target, GuardFalse at the fall-through.
PYRE_FULL_BODY_WALK=1 int_loop produces 19999999900000000 (was
20000000100000000); check.py dynasm 39/39.
* full-body walk: resolve branch-guard target through register-renaming trampoline
goto_if_not's other_target can land on a synthetic ref_copy*; goto
renaming block (emit_trampoline_for_multi_pred_link) that carries no
Python pc. resolve_branch_target_through_trampoline follows such a chain
to the py-boundary destination before stamping the guard snapshot, so
python_pc_for_jitcode_pc no longer maps the trampoline offset to a wrong
Python opcode. Consumed only on the full-body-walk snapshot path
(FULL_BODY_SNAPSHOT_SYM set); default-off control flow uses the
unresolved target unchanged.
* full-body walk: forward-skip Python trivia at branch-guard resume coordinate
The inverse-pc_map can map a resolved branch target to a Python trivia
instruction's jitcode region (e.g. a NOT_TAKEN block). A resume
coordinate must be a real opcode boundary, matching the trait path which
resumes branches at semantic_fallthrough_pc / jump_target_forward (both
forward-skip trivia). skip_python_trivia_forward advances the stamped
py_pc past Cache/ExtendedArg/Resume/Nop/NotTaken to the next opcode, so
the resume reader's backward trivia backtrack is a no-op instead of
walking a NOT_TAKEN coordinate back to the preceding branch opcode (whose
block-entry liveness differs and desynced the snapshot box-count against
the resume liveness, reading past section.values and crashing).
bool_compare full-body-walk now matches default (5809236); check.py
dynasm 39/39; int/float/nested unaffected.
* full-body walk: fold LoadConst residual_call to constant ref
Add OopSpecIndex::LoadConst (202) and tag the load_const_from_code helper
calldescr with it in build_residual_call_ir_r_single_ref_plain_insn_from_operands.
In dispatch_residual_call_iIRd_kind, when the call carries the LoadConst
oopspec and both the const index and code-pointer operands are concrete,
materialize co_consts[idx] (mirroring bh_load_const_fn) into a const_ref and
suppress the residual.
flag-on int_loop/float_loop/nested_loop now complete in 2.7-5.5s (were
10-36s / check.py timeout). check.py dynasm 39/39.
* full-body walk: bridge resume uses section pc, not vable last_instr
The full-body walk sets the concrete frame's last_instr once at the loop
header and never advances it per opcode, so decode_and_restore_guard_failure's
next_instr() (derived from the vable last_instr field) carries the header pc
for a mid-body guard. resume_in_blackhole already resumes at the per-frame
section pc (ResumedFrame.py_pc), which is correct; the bridge-compile path
read next_instr() instead and started tracing from the loop header, double-
executing the loop body before the guarded branch.
Prefer the innermost resumed-frame section pc when it disagrees with
next_instr(); for the trait tracer they always match (no-op there).
float_arithmetic flag-on now matches (was 883662 vs 998193). check.py
dynasm 39/39.
* full-body walk: skip concrete execution of void-result residual calls
try_execute_residual_call_via_walker ran every Call* concretely, including
void-result CallN/CallMayForceN (STORE_SUBSCR, list.append, dict setitem).
Those carry no value the walk specializes on; executing one only commits
its heap side effect. Since the loop body the walk traces is re-executed by
the loop itself, the committed mutation lands twice on the live heap — the
traced iteration is counted twice (list_index_update +192, dict_lookup_update
+3905). Return early when call_descr.result_type() is Void, leaving the side
effect to the loop, mirroring the trait tracer which records these
symbolically and never touches the real heap.
PYRE_FULL_BODY_WALK=1: list_index_update / dict_lookup_update / list_append_pop
now match the default backend; float_arithmetic unchanged. dynasm 39/39.
* full-body walk: abort trace on may-force call with concrete-NULL Ref arg
A user-function call whose callee is loaded from a local (or is a closure)
folds to a specialized direct-entry call: funcptr = the callee entry, with
the PUSH_NULL self-slot baked as ptr(0x0). try_execute_residual_call_via_walker
skips such a call (its NULL-receiver SEGV guard), leaving the result
unresolved, and the baked NULL arg makes the compiled call pass NULL where
the entry needs the callee's globals/closure → NULL result at runtime
(closures / locals-bound callees called in a loop returned NULL). The trait
tracer declines this shape and aborts; mirror it.
walker_abort_if_mayforce_null_ref_arg returns DispatchError::
MayForceNullRefArgUnsupported when a CallMayForce{R,I,F} carries a Ref arg
folded to GcRef(0); the driver maps the error to TraceAction::Abort so the
loop falls back to the interpreter instead of compiling a wrong trace.
Wired into the iRd / iIRd / iIRFd residual-call dispatchers.
PYRE_FULL_BODY_WALK=1: closures / function_calls / recursion /
class_attrs_methods / inheritance_dispatch now match the default backend;
24/25 synth benches match (exceptions pre-existing). dynasm 39/39,
pyre-jit-trace 232/0.
* full-body walk: abort cross-loop cut when inner merge point shape differs from jump args
The walker's jit_merge_point handler registers the inner merge point from
the JitCode merge-point reds ([frame, ec], len 2 for the portal
jitdriver), while the loop close rebuilds jump_args via close_loop_args_at
with the full virtualizable expansion (locals + stack). When the trace
starts before the loop header (loop_header_pc != start_pc) a cross-loop
cut replaces the trace inputargs with the reds-only merge-point boxes,
but next_iteration_args keeps the expanded length; for loops that
virtualize a heap object (fannkuch list, nbody body tuples) the unroll
peel then reads an inputarg slot past the cut list end and panics in
inputarg_type_at_strict.
Detect the shape mismatch at close and abort the full-body walk so the
portal falls back to the trait tracer. Loops without a heap virtual have
matching shapes and still close under the walk.
dynasm + cranelift check.py 39/39 each; pyre-jit-trace 232/0. fannkuch /
nbody flag-on now produce correct output instead of panicking.
* full-body walk: abort may-force trace in functions with exception handlers; fix walker_unbox class_now_known arg type
A may-force residual call that can raise emits GUARD_NO_EXCEPTION. When
that guard fails at runtime the deopt bridge must resume into the
function's exception handler, but the walker's guard resume snapshot does
not seed the standing exception value, so the bridge dereferences a NULL
exception object (GuardClass ldr [x0], x0=0) and segfaults
(exceptions.py, N>=~1000). Abort the walk when the jitcode body contains
any catch_exception op so the portal falls back to the trait tracer,
which resumes the handler correctly. Bodies without catch_exception
(nbody / fannkuch / loop benches) are unaffected and still compile under
the walk.
Also fix walker_unbox_int / walker_unbox_float passing GcRef to
HeapCache::class_now_known, whose parameter is i64; the type error was
latent because incremental compilation had cached the stale typecheck.
dynasm + cranelift check.py 39/39 each; pyre-jit-trace 232/0. exceptions.py
flag-on now matches flag-off (1125000) instead of SIGSEGV; nbody / loop
benches unchanged under flag-on.
* full-body-walk: bail InvalidLoop on next_iteration_args longer than inputargs
Replace the trace.rs CloseLoop cross-loop-cut shape-mismatch abort with an
optimizer-level InvalidLoop bail.
The CloseLoop guard returned TraceAction::Abort after the authoritative walk
had already executed the loop body concretely, so loops with observable
side-effects (nested_loop's print) ran twice. The unroll peel's typed_inputargs
loop now panics InvalidLoop when next_iteration_args has an entry past the cut
inputarg list end, routing through compile_loop_body's catch (interpreter
fallback) without re-executing the body, instead of inputarg_type_at_strict
hard-panicking the worker thread.
* full-body-walk: don't concretely execute LoadConst residual calls
try_execute_residual_call_via_walker skips calls whose descr carries the
LoadConst oopspec. The LOAD_CONST helper has a dedicated fold in the
residual_call dispatchers that materializes co_consts[idx] when both the
const index and the code pointer (frame.pycode) are concrete; when that fold
declines, the residual is recorded so the loop computes it at runtime from
the live frame's pycode. An inlined-callee sub-walk does not seed
frame.pycode concrete, so the fold declines, and concretely executing the
recorded residual passed the unseeded code pointer to bh_load_const_fn, which
dereferenced it via w_code_get_ptr and faulted (SIGSEGV when a float-returning
helper with a co_const is inlined into a hot loop).
* optimizeopt: fold standard-virtualizable getarrayitem read-after-write
optimize_getarrayitem_gc folds a constant-index read on the standard
virtualizable array field to the box tracked in vstate.arrays (seeded
from the inputarg layout, updated by mirror_setarrayitem), mirroring the
static-field fold in optimize_getfield_gc. The matching setarrayitem is
left emitted, so no heap write is dropped; only the redundant read is
removed. Adds VirtualizableTracker::tracked_array_element.
* jitcode_dispatch: precise may-force exception-handler abort gate
walker_abort_if_protected_may_force aborted the full-body walk for any
may-force-can-raise call whenever the body contained any catch_exception.
Resolve the call's Python PC via python_pc_for_jitcode_pc (the inverse
pc_map already used by walker_capture_snapshot_for_last_guard) reached
through FULL_BODY_SNAPSHOT_SYM, then lookup_exceptiontable on the
CodeObject's exceptiontable: abort only when that Python opcode is
actually covered by a handler. Calls with no covering handler raise out
of the frame and are safe to walk. Falls back to the whole-body scan
when no full-body sym pointer is set.
flag-on exceptions.py=1125000 and raise_catch_loop=1142858 (both match
cpython); check.py 39/39 dynasm; pyre-jit-trace 232/0.
* WIP: FBW #67/#62 experiments (candidate-b shape fix, boxing/subscr oopspec, graceful abort)
effectinfo: add OopSpecIndex::BoxInt (203) and StoreSubscr (204) tags.
trace_ctx: graceful-abort try_set_opref_concrete (returns false when no
frontend op/inputarg is recorded; routes through recorder.set_concrete_at
per the #123 canonical-op-identity model, no BoxPool side-table).
jitdriver: TraceContinuationSuspendGuard around may-force concrete execution.
optimizer: InvalidLoop bail on next_iteration_args longer than inputargs.
jitcode_dispatch: append_virtualizable_boxes merge-point shape fix +
entry_py_pc snapshot substitution + BoxInt/StoreSubscr walker dispatch.
trace: deferred-void-store event log + no-loop entry seeding.
flatten: box_int/store_subscr calldescr tagging.
Experimental; proven worse than the clean InvalidLoop-bail baseline.
FBW-flag-gated (PYRE_FULL_BODY_WALK, default off); inert at default env.
Assisted-by: Claude
* jitcode_dispatch: advance resume coordinate past may-force call for after_residual_call guards
The full-body-walk guard-snapshot helper mapped a guard's jitcode op_pc back
to the Python opcode CONTAINING it and resumed there. For GUARD_NOT_FORCED /
GUARD_NO_EXCEPTION emitted after a may-force residual call, that opcode is the
call's own opcode (e.g. STORE_SUBSCR); the call already ran in compiled code
and consumed its Python stack operands, so resuming at it re-executes the call
from a coordinate whose stack no longer holds them.
Split walker_capture_snapshot_for_last_guard into an _impl taking
after_residual_call, add walker_capture_snapshot_after_residual_call, and route
the four may-force guard sites (release-gil + iRd dispatcher GUARD_NOT_FORCED /
GUARD_NO_EXCEPTION) through it. When set, the resume coordinate advances via
semantic_fallthrough_pc to the next executable opcode, matching
capture_resumedata(after_residual_call=True) (pyjitpl.py:2599-2603).
FBW-gated (PYRE_FULL_BODY_WALK, default off). check.py 39/39 flag-off; flag-on
int/nested/list-read loops unaffected.
Assisted-by: Claude
* compile_loop_body: convert SpeculativeError to InvalidLoop bail
The optimize catch in compile_loop_body handled only InvalidLoop; a
SpeculativeError (constant_fold of an ill-typed speculative heap access,
raised via panic_any at optimizeopt/mod.rs:534) fell through to
resume_unwind, propagating the panic across the JIT FFI boundary and
crashing the interpreter. unroll.py:119-123 maps SpeculativeError to
InvalidLoop; treat both the same here so the optimized trace is
abandoned and tracing continues, rather than re-raising.
Assisted-by: Claude
* eval: full-body-walk guard failures resume via blackhole, skip bridge
Under PYRE_FULL_BODY_WALK, handle_fail now returns ResumeInBlackhole for
every guard failure instead of compiling and attaching a bridge. The
full-body walk sets the concrete frame's last_instr once at the loop
header and never advances it per opcode, so a guard's per-PC resume
coordinate and partial operand stack are reconstructed precisely only by
the blackhole's rd_numb consume_one_section decode. A bridge compiled
and attached from such a resume produces compiled code that mishandles
the resumed state — a hot in-place list-swap loop dropped one iteration
at the round an inner-loop guard's failure counter reached
trace_eagerness. Forcing the blackhole path matches the behavior proven
correct under the diagnostic no-bridge mode.
The flag is cached in a OnceLock because handle_fail is hot on
recursion-heavy guard chains; a per-call env read regressed
fib_recursive past its timeout. The trait tracer (flag-off) is
unaffected.
Assisted-by: Claude
* jitcode_dispatch tests: add raw_descrs/is_authoritative_executor to two WalkContext literals
The two vable-box-not-seeded test constructions predate the
raw_descrs/is_authoritative_executor WalkContext fields; set them to the
production defaults (RawDescrPool::Global, false).
Assisted-by: Claude
* full-body walk: abort may-force trace in any exception-handler body
walker_abort_if_protected_may_force returned Ok(()) unconditionally under
the full-body walk, relying on bridge handler-resume seeding to supply the
standing exception value. That seeding does not cover the multi-frame
shape where an inlined callee raises and the caller's handler reads the
exception (synth/exceptions: may_fail raises, main catches and reads
e.args[0]): the exception-path bridge bakes a NULL exception object and the
compiled GuardClass dereferences x0 == 0 (SIGSEGV).
Restore the conservative whole-body gate for the full-body walk too: abort
to the trait tracer whenever the walked body contains any catch_exception.
Bodies without a handler still compile under the walk.
flag-off check.py 41/41; flag-on synth/exceptions no longer crashes
(1125000, matches flag-off) and the flag-on suite is 34/41 (was 33/41 with
the crash), no other bench regressed.
Assisted-by: Claude
* walker int/float specialization: exclude bool operands
walker_int_specialization_operands and walker_float_specialization_operands
accepted bool operands because is_int returns true for W_BoolObject. The
specialization then unboxed them via the int path (GuardClass INT_TYPE +
GetfieldGcPureI intval@16), but W_BoolObject has its own &BOOL_TYPE vtable
and a 1-byte boolval field, not W_IntObject's 8-byte intval at offset 16.
A boolean compare result used as an arithmetic operand (e.g. `x = a < b;
s = s + x`) was therefore miscompiled under PYRE_FULL_BODY_WALK: the
GuardClass guarded the wrong class and the getfield read 7 bytes past
boolval.
Reject bool in both specializations so such operands route through the
generic residual call, which forces the correct value.
Assisted-by: Claude
* walker list subscr/store specialization: exclude bool index and value
try_walker_specialize_subscr and try_walker_specialize_store_subscr gated
the index operand (and the int-storage store value) on is_int, which is
true for W_BoolObject. A bool index (lst[a < b]) or bool value stored into
an int-storage list was unboxed via the int path (GuardClass INT_TYPE +
getfield intval@16), reading the 1-byte boolval as an 8-byte intval and
guarding the wrong class.
Reject bool in both gates so these operands route through the generic
residual call.
Assisted-by: Claude
* full-body walk: abort branch guard whose resume target keeps an operand-stack temp
A goto_if_not branch guard resuming at a target with operand-stack depth
> 0 — the short-circuit `and`/`or`, conditional expression, and chained
comparison shapes, where CPython keeps the tested value on the value
stack across the branch — corrupts a loop-carried slot on guard-failure
deopt: the single-frame snapshot does not model the kept temp on the
not-taken arm. Detect this via the resume-target stack depth
(branch_resume_target_stack_depth, full-body-walk only) and surface
DispatchError::BranchGuardKeptStackUnsupported, mapped to AbortPermanent
→ interpreter fallback. Plain while/if branches resume at depth 0 and
still compile.
flag-off check.py 41/41; flag-on 34/41 (unchanged, remaining 7 are perf
timeouts); pyre-jit-trace 234/0.
Assisted-by: Claude
* full-body walk: elide dead box_bool when compare result is consumed only by the branch
A COMPARE specialization (try_walker_specialize_compare_op_int/_float)
emits the raw truth plus a box_bool CallR that lands a W_Bool in the Ref
dst. When the only consumer is the immediately-following is_true
(POP_JUMP_IF_*), that op folds to the raw truth (bool_box_truth), leaving
the box dead — but it is a non-pure CallR the optimizer cannot DCE, so it
runs every loop iteration.
compare_box_provably_dead does a forward JitCode lookahead proving the
boxed Ref dst is read by exactly one op (an is_true residual), never
overwritten before it, the scan ends at a goto_if_not, and the dst color
is dead at both branch arms (so the guard snapshot cannot capture the
marker). When proven, the box is elided: write the truth into the Ref dst
as a marker and record bool_box_truth(truth, truth) so the is_true fold
resolves it. Any other shape (escape to a local, arithmetic use, second
reader, register reuse, kept-on-stack short-circuit) falls back to
emitting the real box. Full-body-walk only.
flag-off check.py 41/41; flag-on 34/41 → 36/41 (int_loop now passes,
nested_loop improved); pyre-jit-trace 234/0. Escape cases (x=i<n; s=s+x),
bool family, and plain while/if verified flag-on == cpython.
Assisted-by: Claude
* full-body walk: add env-gated call-inlining recognition probe
dispatch_residual_call_iRd_kind gains a PYRE_DIAG_INLINE_RECOG-gated
diagnostic (authoritative-executor path only) that reads each Ref arg's
concrete value, detects a pure-Python function (FUNCTION_TYPE, via
is_function + ob_type), resolves its CodeObject, and reports whether a
per-fn JitCode is present in the setup-time and codewriter stores.
On inline_helper the probe identifies the callable at arg#1 of every
call_fn and reports both stores MISS: pyre builds per-fn JitCode lazily,
so the callee JitCode is absent at caller-trace time. The on-demand build
path jitcode_for (state.rs:593) was measured to build+install these
callees successfully.
No behavior change without the env var (single bool guard per call_fn).
Assisted-by: Claude
* full-body walk: extend inline recognition probe with call_fn arg layout
The PYRE_DIAG_INLINE_RECOG probe now reports nargs, a per-arg concrete
kind shape, and the callable index. On inline_helper the layout is
[ctx, callable, positional_args..]: callable at index 1, positional args
from index 2 (1-arg call → nargs 3, 2-arg call → nargs 4).
Assisted-by: Claude
* full-body walk: add sub_jitcode_body_for_code callee-body obtain helper
sub_jitcode_body_for_code(code) builds (via jitcode_for) and views a
callee per-fn JitCode as a SubJitCodeBody. Unlike the per-function
sub_jitcode_lookup (indexed by the current function's descr pool), this
resolves an arbitrary runtime callable's code, which call inlining needs.
The Arc<PyJitCode> is program-lived in the append-only jitcodes store, so
the 'static slice extension matches the per-fn arm-entry borrow extension
at trace.rs:363.
The PYRE_DIAG_INLINE_RECOG probe now exercises it, reporting each callee's
register-bank shape (inline_helper: add/mul body 27 bytes regs_r=17,i=1;
compute 96 bytes regs_r=19). Gated; flag-off 41/41 unchanged.
Assisted-by: Claude
* full-body walk: dev-gated user-function call inline sub-walk (PYRE_FBW_INLINE)
try_walker_inline_user_call sub-walks a recognized user-function call_fn in
place of the residual call: exact-positional, closure-free callees only;
binds r_args[2..] to callee locals [0..nparams]; runs walk() over the callee
per-fn JitCode (sub_jitcode_body_for_code); writes SubReturn to the call dst;
routes SubRaise through try_catch_exception_at. Sub-walk DispatchErrors
propagate as a trace abort (sound; never mixes inlined + residual emission).
Guards inside resume to the caller CALL boundary via the inherited
single-frame snapshot.
Gated by PYRE_FBW_INLINE (default off); production flag-on path unchanged,
flag-off 41/41. Validated correct on a 200k-iter inline_helper variant
(matches CPython/flag-off/flag-on = 647933338). Currently perf-negative
(0.53s vs 0.35s gate-off) — the single-frame-resume inline yields a correct
but slower trace; trace-quality / multi-frame-resume work is the next step.
Assisted-by: Claude
* full-body walk: diagnose inline sub-walk trace aborts (PYRE_FBW_INLINE_DIAG)
Add a gated log of the callee sub-walk DispatchError. On inline_helper the
dev-gated inline produces correct output but aborts 25 traces; the errors
pinpoint two callee frame-setup gaps the bare sub-WalkContext skips:
VableBoxNotSeeded / VableArrayDescrMalformed (callee frame vable not seeded)
and ResidualCallDescrNotCallDescr (a nested call indexes the parent descr
pool, not the callee's own pool). These are the next steps for the inline to
compile rather than fall back to the interpreter.
Assisted-by: Claude
* full-body walk: decode-dump callee body under PYRE_FBW_INLINE_DIAG
Dump the callee per-fn JitCode opcodes. Confirms the inline-abort root: the
add callee reads its params via getfield_vable_r at pc 3 (a virtualizable
frame access), not from registers. So the register-binding sub-walk does not
match how a per-fn JitCode reads locals; the callee virtualizable frame must
be seeded with the args (build_pending_inline_frame-style) rather than
binding callee registers.
Assisted-by: Claude
* full-body walk: build real callee frame + per-fn descr pool for inline sub-walk
The callee per-fn JitCode is portal-shaped: LOAD_FAST lowers to
getfield_vable_r(r0=frame, localsplus[i]), so the inline sub-walk needs a
genuine heap frame at r0 (read via the _nonstandard_virtualizable ->
GETFIELD_GC fallback), not register-bound args. try_walker_inline_user_call
now builds the concrete callee frame (PyFrame::try_new_for_call_with_closure_
and_globals_obj over the concrete positional args + the caller frame's
execution context) and emits the symbolic callee-frame OpRef via the
one_arg_callee_frame_helper / callee_frame_helper(n) helpers, then seeds
callee_regs_r[0] / callee_concrete_r[0] with it.
The callee body resolves its d/j descr operands through its OWN per-fn pool,
not the caller's. sub_jitcode_descr_pool_for_code (memoized per code ptr,
mirror of trace.rs:363-400) builds the callee's adapted descr_refs +
RawDescrPool::PerFn + sub_jitcode_lookup; the sub_wc now uses them.
Moves the PYRE_FBW_INLINE sub-walk abort past VableBoxNotSeeded (no frame at
r0) and ResidualCallDescrNotCallDescr (wrong descr pool). Remaining blocker:
VableArrayDescrMalformed at the LOAD_FAST sites — the callee jitcode lacks the
(VableArray, Array) descr pair. Dev-gated PYRE_FBW_INLINE (default off);
flag-off check.py 41/41.
Assisted-by: Claude
* full-body walk: diagnose VableArrayDescrMalformed in inline sub-walk
Add a PYRE_FBW_INLINE_DIAG-gated dump at the (VableArray, Array) pair
mismatch arm of vable_array_descrs_from_jitcode + a RawDescrPool::len()
helper. On inline_helper the callee sub-walk reports pool_len=22,
field_idx=21 resolving to the body's BinaryOp Call descr and array_idx=22
out of range — the decoded descr indices point at the tail of the callee
pool, not a vable-array pair, indicating a descr-operand-offset/routing
mismatch in the callee body decode rather than an absent descr.
Dev-gated; no production path change.
Assisted-by: Claude
* full-body walk: seed callee entry registers per setup_call convention
Straight-line callee entry (pc=0, no governing loop header) places the
jitdriver args [pycode, frame, ec] at r0/r1/r2 (trace.rs:341-345), and the
body reads its vable from r1 — not r0. The inline sub-walk previously seeded
only r0=frame, so the body's getfield_vable_r read r1 (NONE) and aborted
VableBoxNotSeeded. Seed all three (pycode=const_ref(w_code), frame=callee
frame OpRef, ec=const_ref(exec_ctx)) with concretes, and require >=3 Ref
register slots.
With this the PYRE_FBW_INLINE sub-walk runs the whole callee body with zero
decode / descr / vable aborts (VableBoxNotSeeded + VableArrayDescrMalformed
both gone). The next blocker is multi-frame guard resume: a callee-emitted
GuardValue has resume_pos=-1 (store_final_boxes_in_guard panic) because the
inline guards lack a snapshot modelling the caller+callee frame stack
(#124/#65 family). Dev-gated PYRE_FBW_INLINE; default off.
Assisted-by: Claude
* full-body walk: emit callable guard_value + no-exception guard for inline
pyjitpl recursive_call specializes the inlined body on the exact callable via
guard_value(callable) before building the callee frame, and emits
GUARD_NO_EXCEPTION right after the frame-build helper
(build_pending_inline_frame:5986 + 6153). The inline sub-walk now emits both
(with walker_capture_snapshot_for_last_guard), and pre-checks frame-helper
availability before any IR so the unsupported-arity path bails cleanly.
Advances the PYRE_FBW_INLINE trace build further (the callable-specialization
GuardValue now carries a snapshot). One orphan GuardValue (resume_pos=-1)
remains at store_final_boxes_in_guard time, emitted by a non-walker-record
path (may-force promotion / multi-frame resume integration, the (4) frontier
documented at jitcode_dispatch.rs:4756). Dev-gated PYRE_FBW_INLINE; default
off.
Assisted-by: Claude
* full-body-walk inline: capture resume snapshot for helper-internal promote guards
The `_nonstandard_virtualizable` check in `TraceCtx`'s vable_* helpers
records a PTR_EQ + `promote_int` GuardValue internally for a frame that is
not the standard virtualizable, then `emit_force_virtualizable` records
GETFIELD_GC / PTR_NE / COND_CALL after it. Only an inlined callee's heap
frame is non-standard (the production main frame is the standard
virtualizable, so the check short-circuits and emits nothing), so the
walker never attached a resume snapshot to that guard and
`store_final_boxes_in_guard` reached it with `rd_resume_position == -1`.
Add Recorder::set_last_guard_op_resume_position (stamps the most-recent
guard op, skipping the non-guard ops recorded after the promote) and the
matching History::capture_snapshot_for_last_guard_op_with_vable_vref;
`set_last_op_resume_position` could not be reused because the guard is not
the last op once emit_force_virtualizable runs.
In the walker, gate a guard-count delta check (TraceCtx::num_guards)
around each vable_* call via INLINE_SUBWALK_CAPTURE_BOUNDARY (set only
during a `try_walker_inline_user_call` sub-walk). When a guard was emitted,
capture a single-frame snapshot at the caller's CALL boundary
(outer_active_boxes / entry_py_pc) — re-executing the call on deopt — and
stamp it on the guard op. The same flag routes the existing
walker_capture_snapshot_for_last_guard away from the callee-pc-via-outer-
pc_map mapping for inline sub-walks.
Dev-gated by PYRE_FBW_INLINE; no-op when the flag is unset (one num_guards
read). check.py flag-off 41/41, majit-metainterp 1277, pyre-jit-trace 234.
Assisted-by: Claude
* full-body-walk inline: seed callee registers with args (fast-path convention)
The inlined callee body is entered at pc=0 and reads its parameters
straight from registers_r[0..nparams], not via getfield_vable against a
heap frame. Decode of add/mul/square/compute confirms the body is
`ref_copy r0->r0 / ref_copy r1->r1 / residual_call(r0,r1) / ref_return r0`
— the can_skip_traced_callee_frame convention (build_pending_inline_frame:
`sym.registers_r = args.to_vec()`; same seeding dispatch_inline_call_dr_kind
uses). The previous code built a heap callee frame and seeded
[pycode@r0, frame@r1, ec@r2] (the portal-entry convention), so the callee
computed `pycode + frame` and failed at runtime with "TypeError: call
failed".
Replace the heap-frame build + frame-build helper + GuardNoException with
direct arg seeding: callee_regs_r[i] = r_args[2+i], concretes likewise.
Keep the guard_value(callable) specialization. A callee that materializes
a frame (extra locals → getfield_vable on an unseeded register) aborts
cleanly with VableBoxNotSeeded -> trait fallback, never a miscompile.
ih_tiny (single-level), ih_small (nested compute->add->square), and
bench/inline_helper.py now all produce correct output under
PYRE_FBW_INLINE. Trace quality / compile (the traces still abort or
guard-thrash, so no perf win yet) is the remaining #62 work. check.py
flag-off 41/41.
Assisted-by: Claude
* optimizeopt: migrate tracked_array_element to BoxRef array source
resolve_array_source takes &BoxRef after the box-identity migration, but
its read-path caller tracked_array_element still took OpRef and the
optimize_getarrayitem_gc call site passed an out-of-scope `array_ref`.
Under --no-default-features --features dynasm this failed to compile
(E0308 expected &BoxRef found OpRef; E0425 array_ref not in scope).
Thread the BoxRef through tracked_array_element symmetrically with the
already-migrated mirror_setarrayitem write path.
Assisted-by: Claude
* FBW: dev-gated LOAD_GLOBAL cell-cache fold (PYRE_FBW_LOADGLOBAL_FOLD)
Add OopSpecIndex::LoadGlobal (205) and tag the load_global_fn calldescr in
build_load_global_fn_insn_from_operands. The full-body walker's
dispatch_residual_call_iIRd_kind recognizes the tag and, when namei /
namespace / promoted pycode are concrete, emits the cell-cache fast path
(quasiimmut_field + record_known_result + elidable jit_namespace_cell_lookup,
plus getfield on an ObjectMutableCell) mirroring the trait load_global_value,
folding the per-iteration module-global read to a loop-invariant cell pointer.
The reader is gated three ways: is_authoritative_executor, no catch_exception
in the body (load_global_fn is CallFlavor::Plain/can-raise; a handler-bearing
body must keep falling back via walker_abort_if_protected_may_force), and the
PYRE_FBW_LOADGLOBAL_FOLD env flag (default off). When enabled, a folded
function callee that is then CALLed mis-resolves through the in-progress FBW
call-inlining path and yields wrong output for global-function-call loops, so
the fold stays default-off until that path lands.
Adds pyframe::load_name_from_code (co_names[idx] accessor).
Default behavior unchanged: check.py dynasm 41/41 flag-off, 35/41 flag-on;
majit-ir 303+5, pyre-jit-trace lib 234.
Assisted-by: Claude
* FBW: route bridge traces through full_body_walk_trace; skip merge-point for bridges
Revert the trait-bridge hybrid: trace_bytecode no longer gates the walker on
`!ctx.is_bridge_trace`, so under PYRE_FULL_BODY_WALK every trace, main and
bridge, goes through full_body_walk_trace (single tracer).
full_body_walk_trace skips add_merge_point when ctx.is_bridge_trace: a bridge
resumes at a mid-loop pc, not a loop header, so it registers no portal entry
signature there.
flag-off check.py 41/41; flag-on default 35-36/41 (both unchanged — the edits
are inert unless PYRE_FBW_BRIDGE enables guard-fail bridges, default-off). The
bridge float-accumulator resume bug (#67) is unchanged: setup_bridge_sym leaves
an unboxed loop-carried scalar's localsplus slot at its stale trace-time box.
Assisted-by: Claude
* FBW: publish last_instr=py_pc-1 to vable shadow at walker guard snapshot
walker_capture_snapshot_for_last_guard_impl resolves the guard's resume
py_pc but did not update the `last_instr` static vable scalar before
building the snapshot boxes. The walker walks JitCode and never crosses
`set_orgpc`, so `virtualizable_boxes[last_instr]` kept the value the trace
seed / previous `close_loop_args_at` override wrote (the loop-header pc).
The blackhole / vable-sync resume reads that scalar into `frame.last_instr`,
so a mid-body guard resumed at the loop header instead of its own opcode.
Mirror `MIFrame::publish_last_instr_to_vable`: publish `py_pc-1` into the
shadow (gated on `owns_virtualizable_shadow`) before
`build_snapshot_vable_vref_boxes`, and defer that build until after the
publish. Make `mirror_vable_static_to_boxes` pub(crate).
Verified: flag-on+PYRE_FBW_BRIDGE spectral_norm bridge resume now writes
frame ni=94 (was 13), matching the trait. check.py dynasm 41/41,
pyre-jit-trace lib 234/0.
Assisted-by: Claude
* FBW: emit intermediate merge-point vable→heap writeback at walker register branch
The walker `jit_merge_point` register branch returned `Continue` without
the vable→heap writeback that the trait's `close_loop_args_at`
(trace_opcode.rs:2875-2950) runs at every merge point: override the vable
`last_instr` scalar to `merge_pc - 1`, mirror it into the
`virtualizable_boxes` shadow, and on a reduced (reds-only) target LABEL
call `gen_writeback_vable_to_heap`. The walker's loop-close path already
routes through `close_loop_args_at` (run_perfn_walk) so it emitted the
final writeback, but an intermediate loop header reached mid-trace — e.g.
a bridge re-entering the inner loop — skipped it, so the compiled body
left the heap PyFrame `last_instr`/array at the trace-seed state.
With the writeback emitted, the flag-on+PYRE_FBW_BRIDGE walker bridge for a
nested while loop now carries the intermediate GcStore(_, 32, merge_pc-1)
and its op count matches the trait (pre-opt 75→105, compiled 36→46).
check.py 41/41 dynasm + 41/41 cranelift.
Assisted-by: Claude
* FBW: restore live frame to guard resume state after bridge trace
`trace_and_compile_from_bridge` set the live frame's `last_instr` to the
guard `resume_pc` before tracing so that a `BridgeCompiled` outcome
re-enters `eval_loop_jit` at the guard point — the
`ContinueRunningNormally` arm runs the interpreter forward from the live
frame's `last_instr` / value stack without reconstructing it.
The trait tracer interprets a private `snapshot_for_tracing` copy and
never touches the live frame. The full-body walker executes may-force
residual calls concretely through the shared execution context during
the walk, which advanced the live frame's `last_instr` to a walked
opcode (a jitcode-region pc) and stepped its locals (a loop counter) to
the body's concrete state. The post-bridge interpreter then resumed
mid-body (value-stack underflow) or past a dropped loop iteration
(off-by-one-iteration result) under PYRE_FULL_BODY_WALK=1 +
PYRE_FBW_BRIDGE=1.
Snapshot the resume state (last_instr, valuestackdepth,
locals_cells_stack_w) before the walk and restore it afterward via the
new `PyFrame::restore_resume_state_from`. The trait path is a no-op
(it never mutates the live frame).
Verified: nested-loop bridges f(1200,3)=2158200, f(2000,10)=89955000,
spectralnorm(50)=1.2741938369830932 all correct and deterministic under
flag-on+bridge; check.py 41/41 dynasm + 41/41 cranelift ×2.
Assisted-by: Claude
* FBW: recover guard-resume section pc from build_resumed_frames return
decode_and_restore_guard_failure now binds build_resumed_frames's return
and reads the innermost resumed frame's py_pc to derive the resume
coordinate, replacing a read of the removed LAST_GUARD_FRAMES thread-local.
Assisted-by: Claude
* compile_bridge: give up when terminal JUMP arg count differs from target LABEL
A bridge whose terminal JUMP targets an already-compiled loop must supply
as many args as that loop's LABEL; the backend regalloc asserts
arglocs.len() == target_arglocs.len(). When a full-body walk closes a bridge
against an outer-loop LABEL grown by an unroll short preamble, the counts can
disagree. Return false (giveup) before the backend stage so blackhole resume
handles it, instead of letting the backend panic. The check is skipped when
target_arglocs is empty (not-yet-compiled target).
Assisted-by: Claude
* optimizeopt: wrap virtualize test op args in BoxRef::from_opref
The tracked_array_element BoxRef-array-source migration left one
virtualize.rs unit test passing `OpRef` where `Op::with_descr`,
`setarg`, and `get_box_replacement` now take/return `BoxRef`, so the
lib-test build did not compile. Wrap the args in `BoxRef::from_opref`,
matching the already-migrated sibling test in the same file.
Assisted-by: Claude
* pyjitpl: give up cross-loop-CUT self loop with NULL-fed class-guarded slot
A cross-loop CUT synthesizes a self-loop LABEL whose inputargs include
valuestack temporaries promoted from the inner merge point via the
escaped-ref BFS. When the optimized body class-guards such a slot
(GuardClass / GuardNonnullClass / GuardNonnull) while the closing Jump
feeds back a Const NULL for that slot, the LABEL/JUMP contract is
self-inconsistent: the loop dereferences NULL on its back edge. The
no-unroll retry path (pyjitpl.py:3044-3054) carries no virtual state to
reject it, the way virtualstate.py:595-606 _generate_guards_knownclass
would. Detect the inconsistency in compile_loop_body and return
CompileOutcome::Aborted (give up to the blackhole, as the retry-failure
path does) instead of installing the loop. Gated on
cut_inner_green_key.is_some() so the full-body-walk-off path is unchanged.
This removes the fannkuch SIGSEGV under
PYRE_FULL_BODY_WALK=1 PYRE_FBW_BRIDGE=1; the giveup fires on slot 24 and
the output matches the trait-dispatch reference.
Assisted-by: Claude
* flatten: LoadGlobal residual_call tests expect oopspecindex=LoadGlobal
build_load_global_fn_insn_from_operands tags the calldescr EffectInfo
with OopSpecIndex::LoadGlobal. Three tests still constructed their
expected EffectInfo via effect_info_for_call_flavor(CallFlavor::Plain),
which leaves oopspecindex None, so they failed once the workspace test
build could run. Update the expected EffectInfo in
build_load_global_fn_residual_call_ir_r_insn_emits_residual_call_ir_r,
build_load_global_fn_with_const_pycode_uses_const_ref_for_code_only, and
build_load_global_fn_residual_call_ir_r_insn_matches_flatten_of_residual_call_op
to set oopspecindex = LoadGlobal, mirroring the LoadConst sibling test.
Assisted-by: Claude
* FBW LOAD_GLOBAL fold: index co_names by namei >> 1
try_walker_load_global_cell_fold passed the raw LOAD_GLOBAL oparg to
load_name_from_code as the co_names index. The oparg is
(name_idx << 1) | push_null_flag, so the fold resolved the wrong name for
any global whose load carries the push-NULL flag (e.g. a global at
co_names[0] read co_names[1]), folding the in-loop callee to a different
global's cell. bh_load_global_fn already derives the index as namei >> 1;
mirror that in the fold.
Reword the dev-gate comment: the fold is correct and reaches production
parity for global-function-call loops with PYRE_FBW_INLINE; the gate stays
pending Phase-5 full-FBW-bench validation.
Assisted-by: Claude
* FBW inline: bound recursive inlining at max_unroll_recursion (=7)
try_walker_inline_user_call had no recursion bound, so a self-recursive
callee (fib) unrolled its exponentially branching call tree at trace
time, never terminating. Add a thread-local inline stack of callee
w_code pointers with an RAII guard around the sub-walk; bail to a
residual call (Ok(None)) once a callee is already FBW_MAX_INLINE_RECURSION
deep, mirroring the trait tracer's recursive_depth >= max_unroll_recursion
gate. fib_recursive under PYRE_FBW_INLINE now completes with correct
output instead of exploding; loop benches stay at trait parity.
Assisted-by: Claude
* FBW inline: decline frame-materializing callees instead of aborting trace
The fast-path inline seeds only r0..nparams; a callee that reads its
params from the virtualizable frame (any *_vable_* op, emitted when a
local must survive a sub-call) hit VableBoxNotSeeded mid sub-walk, and
that Err propagated up to abort the *entire* enclosing loop trace. With
PYRE_FBW_INLINE on, inline_helper's loop therefore never compiled and
ran fully interpreted.
Add callee_fast_path_inlinable: pre-scan the callee body for vable ops
before recording any IR and return Ok(None) (residual call) when found,
mirroring the orthodox should_inline=False -> do_residual_call path.
inline_helper under PYRE_FBW_INLINE now compiles with zero aborts; loop
benches stay at trait parity.
Assisted-by: Claude
* FBW inline: resolve callee frame-static reads to constants; defer iIRd arg-bound past folds
try_walker_inline_user_call's sub-walk seeds only the callee's positional
arg registers, leaving its portal frame box unseeded. A callee that reads a
scalar getfield_vable_r off that frame (pycode field 1 / w_globals field 5,
for LOAD_CONST / LOAD_GLOBAL setup) aborted the trace with VableBoxNotSeeded,
so callee_fast_path_inlinable declined every *_vable_* callee.
Resolve those reads to the callee's compile-time constants (w_code,
function_get_globals_obj) via a thread-local InlineCalleeConsts stack pushed
for the sub-walk lifetime, mirroring the codewriter non-portal branch
(codewriter.rs:6720-6732 / :7347-7369). callee_fast_path_inlinable now admits
a callee whose only vable ops are getfield_vable_r on field 1/5.
In dispatch_residual_call_iIRd_kind, move ensure_residual_call_args_bound
after the LoadConst/LoadGlobal folds: a folded call records nothing, so an
inlined callee's load_global passing its own unseeded portal_frame_reg no
longer trips the bound check.
With PYRE_FBW_INLINE + PYRE_FBW_LOADGLOBAL_FOLD, inline_helper inlines
compute/square/add/mul fully (0 aborts) and matches trait timing (was >235x
slower). check.py dynasm+cranelift 41/41; pyre-jit-trace tests pass.
Assisted-by: Claude
* FBW: execute iIRFd may-force residual via walker, matching iRd/iIRd
dispatch_residual_call_iIRFd_kind recorded the may-force residual and ran
try_fold_pure_call_via_executor but omitted the
try_execute_residual_call_via_walker call that the iRd and iIRd
dispatchers make after the pure-fold. The boxes3 (i++r++f) arglist shares
the same do_residual_call body as the r and ir shapes, so the float-arg /
float-return shape now executes concretely and stamps the result the same
way. Void-result stores are deferred inside the helper's
result_type()==Void arm, so a compiled loop re-run does not double-apply
the store.
The irf residual shape is not emitted by reachable pyre paths today: the
execute path records 0 hits across float_loop/int_loop/fib_loop/
nested_loop/nbody_200 under PYRE_FULL_BODY_WALK=1, so this is
behavior-neutral on the current bench suite. Gate dynasm 41/41 +
cranelift 41/41, pyre-jit-trace 242 tests.
Assisted-by: Claude
* pyrex: suppress SpeculativeError panic message like InvalidLoop
The panic hook silenced InvalidLoop (a caught control-flow panic) but not
SpeculativeError, which the optimizer raises to decline a speculative heap
fold. compile_loop / compile_bridge catch SpeculativeError and abandon the
trace (blackhole fallback = correct result, rc=0), but the default hook
still printed "panicked at optimizeopt/mod.rs" to stderr during the unwind.
check.py treats any "panicked" in stderr as a failure, so a fannkuch loop
compile that gracefully declined a fold under FBW+PYRE_FBW_BRIDGE was
reported as a crash though it ran correctly (8629/30, internal_compile_
panics=0).
Suppress SpeculativeError in the hook symmetrically with InvalidLoop; both
are silent exceptions per unroll.py:119-123. A genuinely uncaught one still
aborts with a nonzero exit, so only the false crash message is removed.
Gate: default check.py dynasm 41/41 + cranelift 41/41.
Assisted-by: Claude
* FBW inline: decline callees with internal branches (non-leaf)
callee_fast_path_inlinable admitted any callee whose body had no
unsupported vable op, but the inline convention only resumes a guard
inside the callee at the caller's CALL boundary via the inherited
single-frame snapshot — sound only for straight-line leaves where
re-executing the whole call on deopt reproduces the state.
A callee with an internal conditional branch (goto_if_not / switch)
emits a branch guard whose fail snapshot must resume into the callee
mid-body. Under PYRE_FBW_INLINE+PYRE_FBW_LOADGLOBAL_FOLD the single-frame
model then serialised a resume section whose liveness shape disagreed
with the encoded stream: a folded branch operand numbered TAGINT in a
slot the outer liveness reported as a ref, crashing blackhole resume
with "resume.rs decode_ref: unexpected tag 1" (synth/function_calls,
mix's `if a & 1`). Decline such callees pre-flight so the call lowers to
an ordinary residual call (correct) until the multi-frame resume
coordinate is ported (#68).
Branchless leaves (inline_helper's add/mul/square/compute) stay inlined,
so inline_helper keeps parity (0.22s, 985375007). function_calls now
completes correctly (320103199572). Full flip-candidate sweep
(FULL_BODY_WALK+BRIDGE+INLINE+LOADGLOBAL_FOLD) dynasm 39/41, remaining 2
fails are perf-only (fib_recursive timeout, nbody 5.1x). Default gate
dynasm 41/41 + cranelift 41/41.
Assisted-by: Claude
* FBW: compile loop-free portal exits via TraceAction::Finish (gated)
full_body_walk_trace mapped every walk outcome except CloseLoop to
TraceAction::Abort, so a top-level *_return that reaches
done_with_this_frame (DispatchOutcome::Terminate, no back-edge) never
compiled: fib_recursive aborted and ran interpreted.
Under PYRE_FBW_CALL_ASSEMBLER the three top-level return arms route
through fbw_terminate_with_finish instead of ctx.finish(): re-box the
result to Type::Ref, record the vable store-back + GUARD_NOT_FORCED_2,
and stash the finish value in a thread-local. full_body_walk_trace
resets it at walk start and, on Terminate, builds TraceAction::Finish
from it, letting the compile pipeline record the FINISH -- the same
split the trait StepResult::Return path uses. Gate off leaves the
ctx.finish() + Terminate->Abort path byte-identical.
fib_recursive then compiles under PYRE_FULL_BODY_WALK (loops_compiled
0->1, loops_aborted 5->0) and matches trait output under MAJIT_STRICT.
Add the missing raw_descrs / is_authoritative_executor fields to the
drive_int_between test's WalkContext literal.
Assisted-by: Claude
* FBW: abort non-standard-virtualizable callee Finish portals to trait
A callee compiled as its own Finish portal (reached via
call_user_function_with_eval) can access its frame through a vable_* op
that finds it to be a non-standard virtualizable, emitting an internal
promote GuardValue + force store-back. Those internal ops carry a resume
snapshot / FieldDescr only on the inline sub-walk path; on the own-portal
compile path the optimizer's store_final_boxes_in_guard /
optimize_setfield_gc invariants trip.
walker_capture_inline_nonstandard_vable_guard now distinguishes the two:
the inline sub-walk still captures the snapshot, while an own-portal walk
that emitted such a guard returns NonStandardVableFinishPortalUnsupported,
aborting the trace to the trait interpreter (AbortPermanent). The method
runs interpreted until the own-portal callee frame is registered as the
standard virtualizable (a perf follow-up).
Exposed by the Finish-portal compile path (PYRE_FBW_CALL_ASSEMBLER):
synth/inheritance_dispatch's polymorphic value() methods previously
panicked in the optimizer; they now produce correct output. The full
flip-candidate sweep (FBW + all levers + CALL_ASSEMBLER) is 45/45
stdout-match, 0 panic; default check.py dynasm + cranelift 41/41.
Assisted-by: Claude
* FBW walker: guard unseeded vable array register; gate inline on call_fn oopspec
getarrayitem_vable / setarrayitem_vable / arraylen_vable read the vable Ref
register without the is_none() check the scalar getfield/setfield handlers
carry, so an unseeded OpRef::None (raw() == u32::MAX) flowed into the
metainterp vable path and resized the heapcache flag vector to ~16 GiB
instead of aborting. Add the VableBoxNotSeeded guard to all three plus a
unit test.
try_walker_inline_user_call inlined any _r_* residual whose 2nd ref arg is a
function, including store_subscr_fn (StoreSubscr oopspec): `d[f] = v` with a
1-arg function key f was mis-inlined as f(v), skipping the store. Gate the
inline on ei.oopspecindex == None, the only iRd-routed helper shape without
an oopspec (the genuine call_fn / call_fn_N).
Assisted-by: Claude
* FBW sub-descr pool: key cache by payload identity and retain the Arc
SUB_DESCR_POOL_CACHE keyed by code ptr only and cached 'static views of the
payload's exec.descrs without retaining the Arc<PyJitCode>. A merge-point
refinement that replaces the payload in place for the same code ptr left the
cache returning descrs derived from the superseded payload (or dangling if
the old Arc had no other holder). Key on (code, payload) identity and retain
Arc::clone(&pjc) in the entry so a refined payload yields a fresh, correct
pool and the borrowed slices stay valid.
Assisted-by: Claude
* optimizeopt: invalidate tracked vable array on variable-index setarrayitem
optimize_setarrayitem_gc only mirrored constant-index writes into
vstate.arrays; a non-constant-index SETARRAYITEM_GC left the tracked slots
unchanged, so a later constant-index GETARRAYITEM_GC could fold to a value
the variable-index write may have overwritten. Add
VirtualizableTracker::invalidate_array and call it on the variable-index
path, mirroring force_lazy_setarrayitem(can_cache=False). Add a regression
test alongside the read-after-write fold test.
Assisted-by: Claude
* review nits: doc link, must_use guard, probe void-defer reset
- history.rs: fix the intra-doc link to recorder::Trace (no Recorder type).
- jitdriver.rs: mark TraceContinuationSuspendGuard #[must_use] so a bare
enter() statement cannot construct-then-drop the suspend guard.
- trace.rs: reset the void-defer log and Finish payload on the diagnostic
probe_walk_perfn_jitcode path, which discards its trace via cut_trace but
never runs through the production tracer's entry reset.
Assisted-by: Claude
* EffectInfo: recognize FBW helper calls via pyre_helper, not oopspecindex
The full-body walker recognized the binary_op / compare / load_const /
box_int / store_subscr / load_global helper residual_calls by tagging their
EffectInfo with OopSpecIndex variants 200-205. has_oopspec() returns true
when oopspecindex != None, so production trait-dispatch traces carrying these
descrs were misclassified: the dynasm reghint _consider_real_call pass
skipped their real-call register hints, and the CanRaise members (load_const,
box_int, load_global) sat outside the _OS_CANRAISE set.
Add EffectInfo.pyre_helper: PyreHelperKind and move recognition onto it;
oopspecindex stays None for these helpers, so has_oopspec() and the OS_*
universe match origin/main. The CallDescrStub cache keys on the whole
EffectInfo and pyre_helper joins PartialEq/Hash, so distinct helper descrs
stay interned distinctly. Migrate the flatten.rs producers, the
jitcode_dispatch.rs walker consumers, and the try_walker_inline_user_call
inline gate; initialize pyre_helper: None in the front-end and metainterp EI
builders. Remove variants 200-205 from OopSpecIndex.
Also document BOOL_BOX_TRUTH as a pyre-only runtime reconstruction of the
jtransform optimize_goto_if_not fusion and note its reset boundary.
Assisted-by: Claude
* pyframe: document bridge-resume restore scope
restore_resume_state_from restores last_instr, valuestackdepth, and
locals_cells_stack_w — the only virtualizable fields the full-body walk
mutates on the live frame. Document that pycode / w_globals / debugdata are
intentionally not restored: frame-invariant or debug-only, never written on
the live frame during bridge tracing.
Assisted-by: Claude
* FBW: log walker abort reason under PYRE_FBW_DEBUG_ABORT
full_body_walk_trace mapped every DispatchError and non-loop-closing
DispatchOutcome to TraceAction::Abort/AbortPermanent without surfacing
which one; the metainterp abort log (pyjitpl/mod.rs:6348) reports only
the green key and permanence. Add a cached PYRE_FBW_DEBUG_ABORT gate that
prints the DispatchError variant, the non-loop-closing outcome, and the
ungated-portal-exit / run_perfn_walk-None cases at each Abort arm.
Default off, no production effect.
Assisted-by: Claude
* optimizeopt tests: adapt virtualizable regression tests to rebased BoxRef API
virtualize.rs regression tests (read-after-write fold, variable-index
invalidate): append `.to_opref()` to `get_box_replacement` (now returns
BoxRef) and pass the OpRc argument to `propagate_forward`.
Assisted-by: Claude
* jitcode_dispatch: handle ConcreteValue::Bool at two pc-map match sites
The Bool(bool) variant (state.rs:1424) added in #59 was not covered at
two walker-only match sites, leaving them non-exhaustive:
- arraylen_vable_via_metainterp: a Bool concrete is not a vable struct
pointer, so it joins Null/Int/Float in mapping to 0 (unseeded abort).
- diagnose_inline_recognition: maps it to 'b' in the shape string.
Assisted-by: Claude
* FBW: direct CALL_ASSEMBLER for self-recursive call (dev-gated PYRE_FBW_REC_CA)
The full-body-walk residual_call dispatcher emitted a generic may-force
CallMayForceR for a self-recursive call, which re-enters the callee
through the func-entry residency door: one frame build + entry-bridge per
recursive call. fib_recursive ran ~30x slower than the trait tracer,
which instead records CallAssemblerR (direct assembler->assembler jump to
the callee's own loop/pending token).
Add try_walker_call_assembler_self_recursive, invoked from
dispatch_residual_call_iRd_kind after the inline path declines. It mirrors
the trait CALL_ASSEMBLER sequence: build the callee PyFrame inline via
emit_new_pyframe_inline_self_recursive (Branch A: single positional int
local, ncells==0, non-global-storing), then FORCE_TOKEN/vable setup,
call_assembler_red_only_ref to the resolved loop or pending token,
KEEPALIVE, escaped-frame heapcache invalidation, the result writeback
(Ref dst as-is, Int dst unboxed) BEFORE the guards — mirroring
make_result_of_lastop (pyjitpl.py:2077) and the in-tree sibling residual
path (write at jitcode_dispatch.rs:6856-6857, contract at 6844-6849) so a
raising/forcing deopt's after-call resume snapshot reads the recorded dst
OpRef instead of a stale box or NONE — then GUARD_NOT_FORCED and
GUARD_NO_EXCEPTION with after-call resume snapshots.
Every precondition (single int arg, exact self-recursion via
callee-code==portal-code, resolvable token, available portal sym) bails
with Ok(None) before recording any IR, so an unmet shape falls back to the
proven residual path. Gated by PYRE_FBW_REC_CA; default-off, production
trait path unaffected.
fib(30) FBW+REC_CA user-CPU at parity with trait (~0.12s); resolves the
sole fib_recursive flip blocker (was ~30x via the func-entry door).
Assisted-by: Claude
* FBW: reconcile walker with #158 base APIs after rebase
The rebased full-body-walk commits predate two APIs the #158/#390 base
added to pyre-jit-trace:
- `WalkContext.store_subscr_fn_addr`: the inline-call sub-walk
constructor in `try_walker_inline_user_call` propagates it from the
parent context, and the six `#[cfg(test)]` WalkContext fixtures that
the release build does not compile set it to `None`.
- `walker_capture_snapshot_for_last_guard` now returns
`Result<(), DispatchError>`; the base's `WalkerFrameOps::generate_guard`
for `WalkContext` calls it in a `()` context with no abort channel, so
the snapshot is best-effort there (the STORE_SUBSCR specialization that
reaches it is env-gated and dead in production).
Assisted-by: Claude
* FBW: record void residual calls symbolically; drop VOID_DEFER_LOG
try_execute_residual_call_via_executor declined Type::Void residual calls
by pushing them onto VOID_DEFER_LOG and committing enclosing-scope stores
at the primary loop close. The may-force-safe gate (from #158) declines
the active-vable CallMayForce* subset (STORE_SUBSCR) before reaching this
branch, so the void branch no longer fires across the bench suite.
Replace the void branch with a bare `return None` so the residual op is
recorded symbolically and the compiled loop applies the side effect once
per iteration -- matching the may-force-decline path and the trait path's
net effect. do_residual_call (pyjitpl.py:2040/2104/2123) executes void
calls eagerly during tracing, but the full-body walk is symbolic and does
not advance the interpreter, so the compiled loop re-runs the traced
iteration; eager execution there would double-apply the heap mutation.
Remove the dead VOID_DEFER_LOG, VoidDeferEvent,
void_defer_push_store/push_register/commit_at_close, and the
jit_merge_point register push and loop-close commit call sites. Rename
void_defer_reset to bool_box_truth_reset, its only remaining job.
Verified: FBW-all-levers output byte-identical to the trait path on all
44 benches (incl. nbody enclosing-scope store, fannkuch, list ops);
check.py dynasm 44/44 + cranelift 44/44; pyre-jit-trace 250 +
majit-metainterp 1365 unit tests pass.
Assisted-by: Claude
* FBW: object-storage list getitem arm in try_walker_specialize_subscr
try_walker_specialize_subscr handled only int/float list storage (sid 1/2),
declining object-storage reads to the generic CALL_MAY_FORCE residual. Add
the sid=0 arm, mirroring generated_list_getitem_by_strategy: guard_class LIST
+ guard_value(strategy==0) + unbox index + IntLt bounds guard (length via
list_length_descr) + getfield(list_items_descr) + getarrayitem_gc_r against
the Ptr(GcArray(OBJECTPTR)) items block, reading the element Ref directly.
The bounds-length descriptor is selected per strategy.
Under PYRE_FULL_BODY_WALK, nbody object reads (bodies[i]) now specialize,
which makes the loaded element concrete so its float-storage reads (b[k])
also specialize; the BINARY_SUBSCR CALL_MAY_FORCE residuals and the
associated GuardNoException / FORCE_TOKEN drop to zero.
Assisted-by: Claude
* FBW: source merge-point reds from sym.frame/execution_context
The walker jit_merge_point handler registered reds read from the walk
register file (ri/rr/rf), while the loop-close path rebuilds jump args
via close_loop_args_at, which reads sym.frame / sym.execution_context.
When a register slot held a const-folded alias (e.g. a ConstPtr for the
execution context), the registered green_boxes and the close-side reds
diverged in box identity. A cross-loop cut (cut_trace_from_with_consts)
then saw the close-side box as an escaped inputarg and appended it as an
extra inputarg, producing a loop entry layout that
extend_compiled_live_values could never satisfy — every interpreter
entry aborted with reason=extend-live-values.
Override the frame/ec slots of the registration reds with sym.frame and
sym.execution_context (the same source the close path uses) when the
red signature matches the portal shape (rr.len()==2, no ints/floats).
Also add a MAJIT_LOG-gated [jit][cut-escape] diagnostic in
cut_trace_from_with_consts when an original inputarg has no pool
constant and is appended as an extra inputarg.
nbody 50k under PYRE_FULL_BODY_WALK + all levers: guard failures
51181 -> 1427, loop entry aborts 263 -> 14, runtime 1.38s -> 0.59s,
output byte-identical to the trait tracer across all 18 benches.
Assisted-by: Claude
* FBW: emit loop_header at backward jumps; gate jit_merge_point on it
The per-CodeObject codewriter emitted only jit_merge_point at loop-header
PCs. Upstream lowers can_enter_jit at each backward-JUMP site to a
loop_header op (jtransform.py:1714-1723) and opimpl_jit_merge_point
(pyjitpl.py:1547-1562) fires reached_loop_header only when a preceding
loop_header stamped seen_loop_header_for_jdindex; a fall-through arrival
returns early unless the green key already has compiled targets.
Without the gate, the FBW walker treated every jit_merge_point execution
as a loop crossing. On nbody the first trace at the inner physics header
(pc=38) closed a degenerate "inner exhausted, outer increment, fresh
iterator" loop at its fall-through re-arrival, occupying the inner
loop's green key. The trait tracer (backward-jump-triggered
close_loop_args) instead cuts at the outer header, leaving the inner key
free to re-heat into a specialized retrace.
Changes:
- codewriter.rs: JumpBackward / JumpBackwardNoInterrupt arms emit a
loop_header graph op (Constant(jdindex)) before the goto when a portal
jitdriver is registered; new emit_loop_header helper records the graph
op and serializes …
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
2 participants
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.
Summary
This pull request enhances support for Python object subclass identity in the JIT trace, particularly for tuples and integer subclasses. The main changes introduce descriptors and helpers to accurately track and guard the dynamic Python-level class (
w_class) of objects, ensuring correct behavior when handling subclassed types and specialized tuple variants.Key changes include:
Tuple and Specialized Tuple Object Model Improvements
w_classfield descriptors forW_TupleObjectand all specialized tuple variants (SPECIALISED_TUPLE_II,FF,OO), allowing the JIT to access and set the Python-level class of these objects.descr.rs.Concrete Value and Subclass Handling
ConcreteValue::from_pyobjto distinguish between exactint,bool, and integer subclasses, preserving subclass identity by returning a reference instead of unboxing for non-exact ints.is_trace_plain_inthelper to check for exact (non-subclassed) integer objects.JIT Tracing and Opcode Handling Enhancements
trace_opcode.rsto:w_classfield for tuple objects during trace construction, maintaining Python-level type information.These changes ensure that the JIT trace can accurately model Python's dynamic type system, especially for tuples and integer subclasses, improving correctness and compatibility with Python semantics.
Benchmark run:
Self-review
Prompt & Model
Model: gpt-5.5
Prompt:
Answer
(My Codex usage limits ran out right after this final assessment, so it might require some more fixes.)
Summary by CodeRabbit
Bug Fixes
Performance
New Features
Tests