Skip to content

str: JIT quasi-immut retrace + DELETE_FAST walker lowering; utf8_mode flag#533

Merged
youknowone merged 15 commits into
mainfrom
str
Jul 15, 2026
Merged

str: JIT quasi-immut retrace + DELETE_FAST walker lowering; utf8_mode flag#533
youknowone merged 15 commits into
mainfrom
str

Conversation

@youknowone

@youknowone youknowone commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Branch str over main — three independent changes, all verified 164/164 on pyre/check.py --backend dynasm.

Commits

  • pyrex/importing: resolve sys.flags.utf8_mode from -X utf8 / PYTHONUTF8 / locale (default -1 locale-selected), matching launcher parity.

  • majit: re-trace quasi-immutable-invalidated loops instead of interpreting them. When a module-dict version bump invalidates a compiled loop's baked quasi-immutable cell (e.g. reassigning a global), the loop's procedure token becomes non-jumpable; the warmstate entry points now detect has_seen_a_procedure_token() && get_procedure_token().is_none(), clean the cell chain, and re-arm the counter so the loop re-traces against the fresh value (warmstate.py:483-500) rather than falling back to the interpreter. Adds paired QuasiimmutField + GUARD_NOT_INVALIDATED recording for namespace cells. Fixes global_reassign (14× → 1.0× CPython).

  • majit: lower DELETE_FAST in the walker and add UnboundLocalError. DELETE_FAST was lowered to an abort_permanent marker, so any hot loop whose body contained a del — including the implicit del e an except E as e: handler emits — ran fully interpreted. It now has a walker-native lowering that writes PY_NULL into the local slot (mirroring delete_fast / STORE_FAST's setarrayitem_vable_r), gated by PYRE_FBW_DELETE_FAST (default on). UnboundLocalError is added as a proper NameError subclass so unbound fast-local reads raise it and except NameError still catches it (unbound freevars keep raising NameError).

Verification

  • pyre/check.py --backend dynasm: 164/164.
  • Same-binary A/B (kill-switch) for DELETE_FAST: swap_except_return_resume −97% (38×), finally_bare_raise −94% (16×), binary_slice_index −39%, build_set_hashability −37%, exception_oserror_fields −34%; the rest within ±4% noise. Zero regressions.
  • UnboundLocalError semantics match CPython 3.14 (MRO, issubclass(UnboundLocalError, NameError), except NameError catch).

Summary by CodeRabbit

  • Bug Fixes

    • Uninitialized local variables now consistently raise UnboundLocalError, including in optimized and traced execution paths.
    • Invalidated optimized code is cleaned up correctly, allowing affected code to become hot and retrace.
    • UTF-8 mode now follows explicit options, environment settings, and locale defaults more accurately.
  • Performance

    • Improved optimization of method calls, local-variable deletion, and namespace invalidation tracking.
    • Enhanced loop optimization dependency tracking for more reliable recompilation.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@youknowone, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6aaf87c9-e7bb-4844-9ac3-cae613f91aab

📥 Commits

Reviewing files that changed from the base of the PR and between 8bc1bd4 and 0a94099.

📒 Files selected for processing (23)
  • majit/majit-backend-wasm/src/lib.rs
  • majit/majit-ir/src/effectinfo.rs
  • majit/majit-metainterp/src/optimizeopt/unroll.rs
  • majit/majit-metainterp/src/pyjitpl.rs
  • majit/majit-metainterp/src/warmstate.rs
  • pyre/pyre-interpreter/src/baseobjspace.rs
  • pyre/pyre-interpreter/src/builtins.rs
  • pyre/pyre-interpreter/src/error.rs
  • pyre/pyre-interpreter/src/eval.rs
  • pyre/pyre-interpreter/src/importing.rs
  • pyre/pyre-interpreter/src/objspace/descroperation.rs
  • pyre/pyre-interpreter/src/pyframe.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs
  • pyre/pyre-jit-trace/src/opcode_handler_impls.rs
  • pyre/pyre-jit-trace/src/state.rs
  • pyre/pyre-jit-trace/src/trace_opcode.rs
  • pyre/pyre-jit/src/call_jit.rs
  • pyre/pyre-jit/src/eval.rs
  • pyre/pyre-jit/src/jit/codewriter.rs
  • pyre/pyre-jit/src/jit/flatten.rs
  • pyre/pyre-object/src/interp_exceptions.rs
  • pyre/pyre-object/src/pyobject.rs
  • pyre/pyrex/src/lib.rs

Walkthrough

This PR adds full-body-walker method handling, explicit UnboundLocalError support, UTF-8 mode resolution, quasi-immutable dependency propagation, dead-token cleanup, and gated DELETE_FAST lowering across the interpreter and JIT.

Changes

Full-body walker and JIT runtime paths

Layer / File(s) Summary
Method-form contracts and gates
majit/majit-ir/src/effectinfo.rs, pyre/pyre-jit/src/jit/flatten.rs, pyre/pyre-jit-trace/src/jitcode_dispatch.rs
Adds the LoadMethodSelf helper tag, method-form residual recognition, inline feasibility checks, and environment-controlled feature gates.
Method-form inline argument handling
pyre/pyre-jit-trace/src/jitcode_dispatch.rs
Threads shifted receiver and argument vectors through method-form inlining, bindings, boxing, shadow state, and diagnostics.
LOAD_METHOD specialization and dispatch
pyre/pyre-jit-trace/src/jitcode_dispatch.rs
Adds code-object name resolution, LOAD_ATTR specialization, LOAD_METHOD_SELF folding, and method-form CALL dispatch.
Namespace quasi-immutability recording
pyre/pyre-jit-trace/src/{jitcode_dispatch.rs,opcode_handler_impls.rs,state.rs}
Centralizes namespace dependency recording and flushes pending guard-not-invalidated state during walker tracing and store folding.
DELETE_FAST lowering
pyre/pyre-jit/src/jit/codewriter.rs, pyre/pyre-jit-trace/src/{trace_opcode.rs,jitcode_dispatch.rs}
Adds gated graph and direct-dispatch lowering that writes the unbound sentinel to local slots.

UnboundLocalError support

Layer / File(s) Summary
Exception contract
pyre/pyre-object/src/interp_exceptions.rs, pyre/pyre-interpreter/src/error.rs
Adds the exception kind, public type, mappings, constructor, and round-trip coverage.
Interpreter unbound-local raising
pyre/pyre-interpreter/src/{eval.rs,pyframe.rs,builtins.rs}
Raises UnboundLocalError for uninitialized locals and distinguishes free variables from locals or cells.
JIT unbound-local raising
pyre/pyre-jit/src/call_jit.rs, pyre/pyre-jit/src/jit/codewriter.rs, pyre/pyre-jit-trace/src/trace_opcode.rs
Updates residual and walker fast-load paths to raise UnboundLocalError for unbound local slots.

Optimizer dependency and warmstate updates

Layer / File(s) Summary
Quasi-immutable dependency propagation
majit/majit-metainterp/src/optimizeopt/unroll.rs, majit/majit-metainterp/src/pyjitpl.rs
Collects, deduplicates, propagates, and stores dependencies across optimization phases and retry paths.
Heap ownership and compiled-target checks
majit/majit-metainterp/src/optimizeopt/heap.rs, majit/majit-metainterp/src/pyjitpl.rs
Excludes constant owners from allocation tracking and requires a compiled loop before checking compiled targets.
Dead-token cleanup and rearming
majit/majit-metainterp/src/warmstate.rs
Cleans invalidated procedure-token cells, rearms hotness, and tests retracing behavior.

UTF-8 runtime flag resolution

Layer / File(s) Summary
UTF-8 flag resolution
pyre/pyrex/src/lib.rs, pyre/pyre-interpreter/src/importing.rs
Resolves UTF-8 mode from CLI, environment, or locale and installs the resulting runtime flag.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Walker
  participant MethodFolder
  participant InlineCall
  participant ResidualCall
  Walker->>MethodFolder: specialize LOAD_ATTR and fold LOAD_METHOD_SELF
  MethodFolder->>InlineCall: provide method-form receiver and arguments
  InlineCall->>ResidualCall: emit tagged LoadMethodSelf residual call
Loading

Possibly related issues

  • youknowone/pyre#205 — Covers the same LOAD_ATTR/LOAD_METHOD folding and method-cache tracing paths.
  • youknowone/pyre#495 — Overlaps the full-body-walker inline residual execution and retry behavior.

Possibly related PRs

  • youknowone/pyre#159 — Modifies the same LOAD_ATTR/LOAD_METHOD canonical splice and LOAD_METHOD_SELF residual machinery.
  • youknowone/pyre#441 — Touches the same loop compilation and retry state propagation in pyjitpl.rs.

Suggested reviewers: lifthrasiir

Poem

I hop through methods, quick and bright,
Tagging residual calls just right.
Unbound locals now speak their name,
UTF-8 flags resolve the same.
Dead tokens fade, hot paths renew—
A carrot-shaped cheer for the code review!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: UTF-8 mode resolution, quasi-immutable retrace, and DELETE_FAST walker lowering.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch str

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8ab01c5fe

ℹ️ 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 thread pyre/pyrex/src/lib.rs Outdated
if !flags.ignore_environment {
if let Ok(value) = std::env::var("PYTHONUTF8") {
if !value.is_empty() {
return if value == "0" { 0 } else { 1 };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject invalid PYTHONUTF8 values

When PYTHONUTF8 is set to any non-empty value other than 0, this branch silently enables UTF-8 mode; PyPy's launcher only accepts 0 or 1 and otherwise exits with the "invalid PYTHONUTF8 environment variable value" error (pypy/interpreter/app_main.py:759-771, also covered by lib-python/3/test/test_utf8_mode.py:95-98). In an environment like PYTHONUTF8=xxx, pyre now continues with sys.flags.utf8_mode == 1 instead of failing during startup.

Useful? React with 👍 / 👎.

Comment thread pyre/pyrex/src/lib.rs Outdated
.or_else(|| std::env::var("LANG").ok());
match locale.as_deref() {
None | Some("") | Some("C") | Some("POSIX") => true,
Some(value) => !value.contains('.'),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Limit locale auto-enable to C/POSIX

This treats every locale string without a dot as a request for UTF-8 mode, but the PyPy launcher only turns it on when the effective LC_CTYPE locale is exactly C or POSIX (pypy/interpreter/app_main.py:775-780). With a non-C locale name that lacks an encoding suffix, such as LANG=en_US, sys.flags.utf8_mode is reported as 1 even though the runtime should leave UTF-8 mode off, which changes locale-dependent stdlib behavior.

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 49b7525).

Files in the reviewed diff
majit/majit-backend-wasm/src/lib.rs
majit/majit-ir/src/effectinfo.rs
majit/majit-metainterp/src/optimizeopt/unroll.rs
majit/majit-metainterp/src/pyjitpl.rs
majit/majit-metainterp/src/warmstate.rs
pyre/pyre-interpreter/src/baseobjspace.rs
pyre/pyre-interpreter/src/builtins.rs
pyre/pyre-interpreter/src/error.rs
pyre/pyre-interpreter/src/eval.rs
pyre/pyre-interpreter/src/importing.rs
pyre/pyre-interpreter/src/objspace/descroperation.rs
pyre/pyre-interpreter/src/pyframe.rs
pyre/pyre-jit-trace/src/jitcode_dispatch.rs
pyre/pyre-jit-trace/src/opcode_handler_impls.rs
pyre/pyre-jit-trace/src/state.rs
pyre/pyre-jit-trace/src/trace_opcode.rs
pyre/pyre-jit/src/call_jit.rs
pyre/pyre-jit/src/eval.rs
pyre/pyre-jit/src/jit/codewriter.rs
pyre/pyre-jit/src/jit/flatten.rs
pyre/pyre-object/src/interp_exceptions.rs
pyre/pyre-object/src/pyobject.rs
pyre/pyrex/src/lib.rs

1. Regressions to PyPy parity introduced by this patch

None.

2. Other mismatches introduced by this patch

None.

3. Pre-existing mismatches (already present before this patch)

  • majit/majit-metainterp/src/warmstate.rs:582 ↔ rpython/jit/metainterp/warmstate.py:483: DONT_TRACE_HERE returns before the new dead-token cleanup. PyPy checks get_procedure_token() is None and calls cleanup_chain() even after a previously seen token; pyre leaves an invalidated cell resident indefinitely. The same inherited ordering also makes the newly added cleanup unreachable at warmstate.rs:545 and warmstate.rs:1396.

  • pyre/pyrex/src/lib.rs:117 ↔ pypy/interpreter/app_main.py:775: locale detection examines the first present environment variable rather than the effective LC_CTYPE locale. In particular, LC_ALL="" LC_CTYPE=en_US.UTF-8 yields UTF-8 mode 1 in pyre, but PyPy’s setlocale(LC_CTYPE, None) resolves the named locale and yields 0.

  • pyre/pyre-jit/src/jit/codewriter.rs:11460 ↔ pypy/interpreter/pyopcode.py:597: pyre permanently aborts JIT compilation for DELETE_DEREF, while PyPy checks the cell, raises the appropriate unbound error if needed, and clears it. DELETE_GLOBAL and DELETE_NAME are likewise permanently aborted at codewriter.rs:11461, whereas PyPy implements them at pyopcode.py:958 and pyopcode.py:878.

4. Structural adaptations

  • pyre/pyrex/src/lib.rs:190 ↔ pypy/interpreter/app_main.py:561: pyre treats an unknown -X utf8=<value> as a CPython-style preinitialization fatal error; this differs from the local PyPy launcher, which only recognizes utf8, utf8=1, and utf8=0. This is a permitted Python-version/CPython-compatibility adaptation.

  • majit/majit-backend-wasm/src/lib.rs:1046 ↔ rpython/jit/metainterp/compile.py:320: the wasm backend declines a re-traced loop whose closing JUMP targets a non-final LABEL, rather than compiling a native PyPy-style target-token graph. It preserves interpreter semantics through fallback; the distinction follows the separate-module wasm backend architecture.

  • pyre/pyre-object/src/interp_exceptions.rs:208 ↔ pypy/module/exceptions/interp_exceptions.py:1115: PyPy represents UnboundLocalError as an exception class subclass; pyre adds an ExcKind enum variant and explicitly encodes NameError matching. This is a Rust representation adaptation, and the patch preserves the subclass behavior.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8bc1bd4cfc

ℹ️ 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 static EXC_TYPE_ERROR_TYPE: PyType = crate::pyobject::new_pytype("TypeError");
pub static EXC_VALUE_ERROR_TYPE: PyType = crate::pyobject::new_pytype("ValueError");
pub static EXC_NAME_ERROR_TYPE: PyType = crate::pyobject::new_pytype("NameError");
pub static EXC_UNBOUND_LOCAL_ERROR_TYPE: PyType = crate::pyobject::new_pytype("UnboundLocalError");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Register UnboundLocalError in the PyType hierarchy

Adding a new PyType here also needs a corresponding (EXC_UNBOUND_LOCAL_ERROR_TYPE, EXC_NAME_ERROR_TYPE) entry in pyre-object/src/pyobject.rs::all_foreign_pytypes; otherwise objects materialized for an unbound fast-local read have ob_type == EXC_UNBOUND_LOCAL_ERROR_TYPE but fail pyre_object::is_exception(). In that scenario the runtime skips exception-only paths such as traceback attachment in pytraceback::record and the special BaseException attribute handling, so the new exception class behaves unlike other W_BaseException subclasses.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
majit/majit-metainterp/src/pyjitpl.rs (1)

6546-7020: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Propagate quasi-immutable deps from compile_retrace
compile_retrace should move unroll_opt.quasi_immutable_deps into self.last_quasi_immutable_deps on success, like the other compile paths. Otherwise register_quasi_immutable_deps drains an empty list after retrace, so those loops never register their invalidation watchers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@majit/majit-metainterp/src/pyjitpl.rs` around lines 6546 - 7020, On the
successful `compile_retrace` path, move `unroll_opt.quasi_immutable_deps` into
`self.last_quasi_immutable_deps` before the optimizer is discarded, matching the
propagation performed by the other compile paths. Ensure
`register_quasi_immutable_deps` can subsequently drain and register these
retrace dependencies.
🤖 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/jitcode_dispatch.rs`:
- Around line 14786-14792: Update the callable/receiver recovery loop to detect
and preserve the GcRef::NO_CONCRETE sentinel returned by ctx.trace_ctx.box_value
before constructing ConcreteValue::Ref. Do not convert usize::MAX into a
PyObjectRef; keep the concrete value null or otherwise follow the existing
unresolved-value path so invalid arguments never reach resolve_inlinable_callee
or method dispatch.

In `@pyre/pyre-object/src/interp_exceptions.rs`:
- Around line 1327-1329: Add regression assertions to
test_exc_kind_matches_hierarchy covering UnboundLocalError: verify it matches
NameError, Exception, and BaseException through the hierarchy-matching API. Keep
the existing hierarchy cases unchanged and place the assertions alongside the
current NameError coverage.

In `@pyre/pyrex/src/lib.rs`:
- Around line 123-128: Update the PYTHONUTF8 handling in the environment-flag
logic to accept only “0” and “1”; return the corresponding UTF-8 mode for those
values, and reject any other non-empty value by propagating an initialization
error instead of defaulting to enabled. Preserve the existing behavior for unset
or empty values and the ignore_environment path.
- Around line 108-117: Update locale_implies_utf8_mode to determine UTF-8 mode
from the active locale/codeset state using the platform locale APIs, rather than
treating locale strings without a dot as UTF-8. Preserve the existing true
behavior for unset, empty, C, and POSIX locales, and ensure values such as en_US
follow the active codeset result.

---

Outside diff comments:
In `@majit/majit-metainterp/src/pyjitpl.rs`:
- Around line 6546-7020: On the successful `compile_retrace` path, move
`unroll_opt.quasi_immutable_deps` into `self.last_quasi_immutable_deps` before
the optimizer is discarded, matching the propagation performed by the other
compile paths. Ensure `register_quasi_immutable_deps` can subsequently drain and
register these retrace dependencies.
🪄 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: 20f0a50c-8e88-41f2-856c-cd047ddd2a60

📥 Commits

Reviewing files that changed from the base of the PR and between 51a2c2c and 8bc1bd4.

📒 Files selected for processing (19)
  • majit/majit-ir/src/effectinfo.rs
  • majit/majit-metainterp/src/optimizeopt/heap.rs
  • majit/majit-metainterp/src/optimizeopt/unroll.rs
  • majit/majit-metainterp/src/pyjitpl.rs
  • majit/majit-metainterp/src/warmstate.rs
  • pyre/pyre-interpreter/src/builtins.rs
  • pyre/pyre-interpreter/src/error.rs
  • pyre/pyre-interpreter/src/eval.rs
  • pyre/pyre-interpreter/src/importing.rs
  • pyre/pyre-interpreter/src/pyframe.rs
  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs
  • pyre/pyre-jit-trace/src/opcode_handler_impls.rs
  • pyre/pyre-jit-trace/src/state.rs
  • pyre/pyre-jit-trace/src/trace_opcode.rs
  • pyre/pyre-jit/src/call_jit.rs
  • pyre/pyre-jit/src/jit/codewriter.rs
  • pyre/pyre-jit/src/jit/flatten.rs
  • pyre/pyre-object/src/interp_exceptions.rs
  • pyre/pyrex/src/lib.rs

Comment thread pyre/pyre-jit-trace/src/jitcode_dispatch.rs
Comment thread pyre/pyre-object/src/interp_exceptions.rs
Comment thread pyre/pyrex/src/lib.rs
Comment thread pyre/pyrex/src/lib.rs
youknowone added a commit that referenced this pull request Jul 14, 2026
- compile_retrace moves unroll_opt.quasi_immutable_deps into
  last_quasi_immutable_deps on the compiled-true path, so retraced loops
  register their quasi-immutable invalidation watchers like the other
  compile paths (register_quasi_immutable_deps otherwise drains an empty
  list after retrace).
- the walker method-form call recovery rejects GcRef::NO_CONCRETE and null
  refs before writing ConcreteValue::Ref into the callable/receiver slots.
- test_exc_kind_matches_hierarchy asserts UnboundLocalError matches
  NameError and the Exception/BaseException hierarchy.

Assisted-by: Claude

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d18088b58

ℹ️ 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 +11310 to +11317
record_graph_op(
&current_block.block(),
"setarrayitem_vable_r",
vable_setarrayitem_ref_graph_args(
frame_var.into(),
v_idx.into(),
super::flow::Constant::none().into(),
),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve DELETE_FAST's unbound-local check

When DELETE_FAST runs on a slot that is already PY_NULL (for example def f(): del x or a second del x after the first clear), the interpreter now raises UnboundLocalError in eval.rs::delete_fast, but this JIT lowering only writes PY_NULL and continues. With PYRE_FBW_DELETE_FAST default-on, a traced/compiled path can suppress that exception and fall through or return normally, so the lowering needs the same null check and raise before clearing the local.

Useful? React with 👍 / 👎.

youknowone added a commit that referenced this pull request Jul 14, 2026
- compile_retrace moves unroll_opt.quasi_immutable_deps into
  last_quasi_immutable_deps on the compiled-true path, so retraced loops
  register their quasi-immutable invalidation watchers like the other
  compile paths (register_quasi_immutable_deps otherwise drains an empty
  list after retrace).
- the walker method-form call recovery rejects GcRef::NO_CONCRETE and null
  refs before writing ConcreteValue::Ref into the callable/receiver slots.
- test_exc_kind_matches_hierarchy asserts UnboundLocalError matches
  NameError and the Exception/BaseException hierarchy.

Assisted-by: Claude

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1597b7a778

ℹ️ 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 +590 to +594
if cleanup_dead_token_cell {
// warmstate.py:483-500 — an invalidated/dead procedure token is
// removed from the chain, resetting the hot counter so it re-arms.
self.cleanup_chain(green_key_hash);
return HotResult::NotHot;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove invalidated force-finish cells before rearming

If a compiled loop has previously been marked FORCE_FINISH (the trace-too-long segmenting path) and is later invalidated by a quasi-immutable dependency, this cleanup does not actually remove the dead cell: cleanup_chain() delegates to install_new_cell(), whose should_remove_jitcell() keeps any FORCE_FINISH cell even when get_procedure_token() is now None. Every subsequent entry sees the same has_seen_a_procedure_token && !has_procedure_token condition, resets the counter again, and returns NotHot, so that loop never re-arms or retraces after invalidation.

Useful? React with 👍 / 👎.

…8 / locale

Default utf8_mode to 0; a C/POSIX or unset locale (LC_ALL/LC_CTYPE/LANG)
coerces it to 1, and -X utf8 / PYTHONUTF8 override (PYTHONUTF8 suppressed under
-E/-I), matching CPython 3.14. utf8_mode is reported only through
sys.flags.utf8_mode; pyre's I/O stays UTF-8 regardless.

Assisted-by: Claude
…ting them

A green key whose procedure token was invalidated by a quasi-immutable
mutation (module-dict `ModuleDictStrategy.version` bump on a new global key)
was never re-traced: `loops_compiled` stayed 1 and later calls ran the loop
interpreted.

warmstate now re-arms such a cell — the loop-header (`counter_tick_checked`,
`maybe_compile`, `maybe_compile_with_key`) and function-entry
(`should_trace_function_entry`) counter paths detect a cell that has seen a
procedure token but whose `get_procedure_token()` is now `None`, remove the
dead-token cell via `cleanup_chain`, and re-count from cold
(`warmstate.py:483-500`), so the loop re-traces when hot again.

Supporting the re-trace cycle:
- namespace quasi-immutable folds route through `record_namespace_quasiimmut_field`,
  which records `QuasiimmutField` and arms the paired `GUARD_NOT_INVALIDATED`
  once per heapcache epoch (`pyjitpl.py:1074-1089`);
- the unroll optimizer merges `quasi_immutable_deps` from both phase optimizers
  and `pyjitpl` threads them into `last_quasi_immutable_deps` so a re-traced
  loop re-registers its version watcher (`compile.py:204-207`);
- `has_compiled_targets` requires a live loop so an invalidated token is not
  treated as jumpable.

global_reassign 0.58s -> 0.04s, global_reassign_obj 0.57s -> 0.04s; 163/163.

Assisted-by: Claude
DELETE_FAST lowered to an abort_permanent marker, so any hot loop whose body
contained a `del` — including the implicit `del e` an `except E as e:` handler
emits — ran fully interpreted (loops_compiled=0).

Give DELETE_FAST a walker-native lowering that writes PY_NULL into the local
slot (mirroring eval.rs delete_fast and STORE_FAST's setarrayitem_vable_r),
gated by PYRE_FBW_DELETE_FAST (default on; 0/false restores the marker). A
LOAD_FAST_CHECK / LOAD_FAST pair reading a slot proven unbound raises
UnboundLocalError.

Add UnboundLocalError as a NameError subclass (ExcKind::UnboundLocalError):
unbound fast-local reads (load_local_checked_value, deref_unbound_error for
cellvars, bh_load_fast_check_fn) raise it, and `except NameError` catches it.
Unbound freevars keep raising NameError.

Assisted-by: Claude
…he method-form CALL

try_walker_specialize_load_method_attr resolves a monomorphic instance method to
a green ConstRef W_Function behind guard_class + exact-class + type version_tag +
instance-map guards (via load_method_fast_path), and try_walker_fold_load_method_self
const-folds compute_load_method_bound once attr is concrete. The CALL inliner now
inlines the method form bh_call_fn_N(callable, self, args...) by passing the
receiver as the first positional argument, so an inlined method body matches a
plain inlined call. Gated by PYRE_FBW_LOADMETHOD_FOLD (default on).

Adds PyreHelperKind::LoadMethodSelf (enum tail, repr(u8) discriminant-stable) and
tags the load_method_self residual so the walker recognises it.

Assisted-by: Claude
…f8 coercion

PYTHONUTF8 now accepts only "0"/"1"/empty; any other value emits the
preconfig_init_utf8_mode fatal error and exits 1. -X utf8=<other> emits
the same fatal error. locale_implies_utf8_mode coerces utf8_mode to 1 only
for the C/POSIX or unset/empty locale; every named locale (en_US, C.UTF-8,
en_US.UTF-8, ...) leaves it 0.

Assisted-by: Claude
…ption GC tid

DELETE_FAST on an already-unbound local now raises UnboundLocalError instead
of silently succeeding. UnboundLocalError is listed in all_foreign_pytypes
with NameError as its base so instances carry BaseException in the MRO
(.args, isinstance against NameError/Exception).

The per-ExcKind GC pre-registration loop in pyre-jit now covers
UnboundLocalError, binding it to the shared W_BaseException tid; the
foreign-pytype loop then skips it via the pytype_to_tid guard. Without the
pre-registration that loop would assign an undersized sizeof(PyObject)
standalone tid and shift every hardcoded post-loop GC tid, corrupting the
descr-to-tid map (bigint external-size dispatch) in release builds where the
debug_assert_eq guards are compiled out.

Assisted-by: Claude
- compile_retrace moves unroll_opt.quasi_immutable_deps into
  last_quasi_immutable_deps on the compiled-true path, so retraced loops
  register their quasi-immutable invalidation watchers like the other
  compile paths (register_quasi_immutable_deps otherwise drains an empty
  list after retrace).
- the walker method-form call recovery rejects GcRef::NO_CONCRETE and null
  refs before writing ConcreteValue::Ref into the callable/receiver slots.
- test_exc_kind_matches_hierarchy asserts UnboundLocalError matches
  NameError and the Exception/BaseException hierarchy.

Assisted-by: Claude
getitem_list / getitem_tuple / getitem_str iterated `while i < stop` with
`i += step`, which overflowed i64 when step approached sys.maxsize and produced
one extra element (list('abcde')[1::sys.maxsize] yielded ['b','e'] instead of
['b']). Route the three through slice_unpack + slice_adjust_indices and iterate
the adjusted slicelength, advancing i only between elements so the final step
never overflows. memoryview_slice_view moves to the same overflow-safe
slicelength form. Fixes test.test_str StrTest.test_extended_getslice.

Assisted-by: Claude
str_repeat judged the repeat overflow at the WTF-8 byte length, so a
non-ASCII character (wider in WTF-8 than its Py_UCS storage) could push
the byte product past isize::MAX and raise OverflowError where the
code-point-count product does not overflow. Judge overflow on the
code-point count (count * char_len) like unicode_repeat and let
try_reserve_exact report the allocation failure as MemoryError.

Fixes StrTest.test_raiseMemError.

Assisted-by: Claude
The walker and codewriter DELETE_FAST lowering cleared the local slot
without the bound check that pyopcode.py:998 DELETE_FAST and the
interpreter delete_fast perform, so a JIT-compiled `del x` on an
already-unbound fast local silently succeeded instead of raising
UnboundLocalError.

The walker raises when the local is unbound at trace time — checking both
the concrete-local shadow and the traced IR box — and guards the slot
non-null otherwise so a slot that is bound at trace time but null at
runtime deopts and re-raises. The codewriter emits a load_fast_check
residual that raises on PY_NULL and branches to an abort/deopt on a null
runtime slot. Bridge and deopt assembly seed concrete_locals with
Ref(PY_NULL) for unbound slots so the concrete check fires in bridge
traces.

Assisted-by: Claude
When the codewriter models a LOAD_FAST_CHECK local as a `ConstantValue::None`
slot (proven unbound at compile time, e.g. after a preceding DELETE_FAST), the
load unconditionally raises UnboundLocalError. The previous lowering emitted the
`load_fast_check` residual + `GuardNoException` + push, leaving a normal-return
`Finish` continuation; a later guard-failure bridge resolved into that return
and dropped the raise, so `x = 1; del x; return x` in a hot loop counted 2217
raises instead of 20000. Bail to the interpreter at the known-unbound load so it
raises, matching the DeleteFast known-unbound handling.

Assisted-by: Claude
A bridge compiled from an exception guard (GUARD_NO_EXCEPTION) whose raising
op is not caught in the current frame walked from the no-exception fallthrough
resume_pc and emitted `Finish(<null raised-call result>)`, so the compiled
bridge handed a NULL up to the caller ("call failed" crash) instead of
propagating the exception. The `emit_exception_bridge_prologue` GUARD_EXCEPTION
path that would trace the propagate-out continuation requires resume-data
replay that is not wired (its synthetic guard carries no rd_resume_position),
so extend the pending-exception decline to the escaping case, matching the
existing caught-in-frame handling; the blackhole resume propagates the
exception out of the frame to the caller's handler.

Assisted-by: Claude
…ast label

The wasm loop lowering assumes the closing back-edge JUMP targets the
LAST Label in the trace: `build_function` picks the loop-back label with
`ops.rposition(Label)`, `find_label_args` returns that label's args, and
the JUMP arm maps the jump args onto it positionally with
`n = min(jump_args, last_label_args)` then `br(0)`.

A loop re-traced after a quasi-immutable module-global invalidation can
instead close with a JUMP back to its 12-arg preamble entry label
(`LoopTargetDescr(start:0)`) while a narrower 4-arg peeled header sits
last. The lowering then maps the wide jump's args onto the narrow last
label's params, leaking preamble constants (the induction step) into
loop-carried slots and branching to the wrong label, so
`bench/synth/global_reassign` and `global_reassign_obj` returned wrong
values ("TypeError: not an iterator").

`compile_bridge` already declines the analogous non-last-label
loop-closing JUMP (BRIDGE_DIAG 9/10). Add the same guard to the
`compile_loop` path in `wasm_unsupported_trace_reason`: when the closing
JUMP's descr identity differs from the last Label's, decline so the
interpreter runs the loop. The invalidated source token is filtered by
`get_procedure_token`, so the declined re-trace falls back to the
interpreter rather than dispatching the stale first compile.

Assisted-by: Claude

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a94099f2c

ℹ️ 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 +14277 to +14279
if method_form {
callee_args.push(r_args[1]);
callee_arg_concretes.push(arg_concretes[1]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard method-form self before inlining

When this method-form path is taken, the trace-time non-null null_or_self decision is used to prepend r_args[1] to the inlined callee, but no guard is emitted that the load_method_self result stays non-null. If the LOAD_METHOD fold did not install its class/map guards (for example with PYRE_FBW_LOADMETHOD_FOLD=0 or a method name the fold declines) and the instance later shadows the method with the same function object, the real bh_call_fn_impl would see null_or_self == PY_NULL and not prepend self, while the compiled inline path still passes the null sentinel as the first argument. Please either guard r_args[1] non-null before inlining the method form or decline this path unless the guarded LOAD_METHOD fold proved the binding decision.

Useful? React with 👍 / 👎.

@youknowone youknowone merged commit 7103208 into main Jul 15, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant