Align load-fast borrow barriers with CPython by youknowone · Pull Request #7930 · RustPython/RustPython · GitHub

Summary by CodeRabbit

Refactor
- Improved load/variable preservation across folded conditionals, loops, try/finally and with/context patterns to match CPython semantics and avoid weakening successor loads.
Tests
- Added extensive unit tests covering nested try/finally, folded conditionals, loop tails, and with/exception-tail scenarios.
Chores
- Added "ifexp" to the spell-check allowlist.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 69f5a9d5-9166-4cf0-81f7-4c91018ec757

📥 Commits

Reviewing files that changed from the base of the PR and between 01d82fa and 388a620.

📒 Files selected for processing (1)

crates/codegen/src/compile.rs

📝 Walkthrough

Walkthrough

Refines LOAD_FAST_BORROW optimization to match CPython empty-label reuse by adding a Block passthrough flag, CFG fallthrough/barrier analysis, stack/AST helpers, and compile-time preservation for folded ifexp, try/finally, loop tails, and with-cleanup; includes many tests.

Changes

Load-Fast Label Reuse Passthrough Refinement

Layer / File(s)	Summary
IR block field and folded-load escape detection crates/codegen/src/ir.rs	Block struct gains load_fast_label_reuse_passthrough. Adds folded_load_escapes_block and integrates per-instruction folded-escape checks into optimize_load_fast_borrow.
Fallthrough predecessor matching & empty-try barrier crates/codegen/src/ir.rs	Adds has_fallthrough_predecessor_matching and rewrites has_cpython_empty_try_end_barrier to consider finally-cleanup fallthrough and refined predicates; tightens canonical_target traversal.
Passthrough redirection predicates crates/codegen/src/ir.rs	redirect_load_fast_passthrough_targets extended with warm-fallthrough, assertion NOP/failure, try-else/orelse entry, and labeled-successor predicates; passthrough_target rewired to use these and the new flag.
Compiler helpers and AST/stack inspection crates/codegen/src/compile.rs	Adds current_block_stack_depth, mark_load_fast_label_reuse_passthrough_block, expands statements_contain_with for nested forms, and adds statements_end_with_try_finally_finalbody_with.
Constant-folded ifexp assignment handling crates/codegen/src/compile.rs	When assignment RHS is constant-foldable ifexp, detects empty end blocks using stack inspection and inserts load-fast barriers plus continuation blocks to preserve successor load strength.
Try-finally exit preservation and marking crates/codegen/src/compile.rs	Extends preservation checks to consider preserved empty labels for try-finally exits and switches marking to mark_load_fast_label_reuse_passthrough_block at relevant points.
While-loop / try-orelse / with tail preservation crates/codegen/src/compile.rs	Adds flags for preserving successor-strong loads after nested try/orelse and with/orelse ends, preserves empty test-prefix boundaries, disables load-fast borrowing when needed, and marks passthrough targets on normal/with-cleanup exits.
Flag rename, OR-merge update, tests, and cspell crates/codegen/src/compile.rs, crates/codegen/src/ir.rs, .cspell.json	Renames load_fast_passthrough → load_fast_label_reuse_passthrough, updates OR-merge logic, adds many unit tests validating LoadFast vs LoadFastBorrow across control-flow shapes, and adds ifexp to cspell allowlist.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

RustPython/RustPython#7870: Modifies optimize_load_fast_borrow / empty-label fallthrough and CFG traversal behavior in the same LOAD_FAST_BORROW parity area.
RustPython/RustPython#7773: Also changes crates/codegen/src/ir.rs optimize_load_fast* traversal and passthrough/barrier handling affecting borrow vs strong load decisions.

Poem

🐰 I hopped through labels, folds, and tries,
I nudged the stack where mystery lies,
With passthrough flags and careful art,
Strong loads keep strength and play their part,
A tiny rabbit's bytecode prize.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.21% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main objective of the pull request: aligning load-fast borrow barriers with CPython behavior. The changes across three files (compile.rs, ir.rs, and .cspell.json) all work toward this central goal of improving CPython parity in the bytecode compiler.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 🧪 Generate unit tests (beta)

Create PR with unit tests

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 and usage tips.

Actionable comments posted: 2

🧹 Nitpick comments (1)

crates/codegen/src/compile.rs (1)
15228-15446: 💤 Low value

Consider extracting common test patterns into helper functions.

These tests share significant boilerplate for variable/instruction lookup patterns (e.g., finding loads by varname, finding attribute accesses by name, window matching). Extracting helpers like find_loads_for_var(code, var_name) or find_attr_instruction(instructions, attr_name) would reduce duplication and make future test additions easier.
🤖 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 `@crates/codegen/src/compile.rs` around lines 15228 - 15446, Many tests (e.g.,
test_while_before_folded_boolop_if_keeps_successor_load_fast_strong,
test_with_try_except_tail_keeps_successor_load_fast_strong,
test_loop_try_orelse_nested_try_before_next_try_keeps_load_fast_strong,
test_try_orelse_with_before_next_try_keeps_load_fast_strong,
test_with_try_finally_nested_with_keeps_successor_load_fast_strong) duplicate
the same varname/instruction scanning boilerplate; extract shared helpers like
find_loads_for_var(code_or_func, "varname") that returns the filtered
LoadFast/LoadFastBorrow ops and find_instrs_excluding_cache(instructions) or
find_attr_instruction(instructions, "attr") to encapsulate the unit.op matching
and OpArg conversion logic used in these tests, then replace the inline iterator
chains in each test with calls to those helpers to remove duplication and
improve readability.

🤖 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 `@crates/codegen/src/ir.rs`:
- Around line 5487-5493: The code is testing load_fast_passthrough on the
already-normalized target (after next_nonempty_block()), so the empty-label case
is never detected; change the check to inspect the raw successor chain (the
block that may be an empty passthrough) before calling next_nonempty_block() or
use the predecessor/raw successor index directly. Concretely, compute or capture
the successor block index prior to normalization (the raw successor used to
produce target), and test blocks[raw_successor_idx].load_fast_passthrough
(instead of blocks[target.idx()].load_fast_passthrough) when deciding the
finally-cleanup escape hatch in the falls_through_from_finally_cleanup /
has_fallthrough_predecessor_matching path; keep the existing calls to
next_nonempty_block()/block_has_finally_cleanup_handler otherwise.
- Around line 6919-6925: has_warm_fallthrough_predecessor currently only checks
immediate predecessors (block.next == target) and thus misses warm predecessors
that reach target via preserved empty passthrough/label-reuse blocks; change it
to perform the same backward fallthrough walk used by the try-end barrier logic:
starting from each candidate block follow fallthrough successors/links (using
block_has_fallthrough and the block.next chain) backwards/forward as that logic
does until you hit a non-empty/real block or a cold/except boundary, and return
true if any such walked chain reaches the target while remaining warm and not an
except_handler. Apply the identical traversal change to the other occurrence
around handler_resume_end (the similar function at the other location noted in
the comment).

---

Nitpick comments:
In `@crates/codegen/src/compile.rs`:
- Around line 15228-15446: Many tests (e.g.,
test_while_before_folded_boolop_if_keeps_successor_load_fast_strong,
test_with_try_except_tail_keeps_successor_load_fast_strong,
test_loop_try_orelse_nested_try_before_next_try_keeps_load_fast_strong,
test_try_orelse_with_before_next_try_keeps_load_fast_strong,
test_with_try_finally_nested_with_keeps_successor_load_fast_strong) duplicate
the same varname/instruction scanning boilerplate; extract shared helpers like
find_loads_for_var(code_or_func, "varname") that returns the filtered
LoadFast/LoadFastBorrow ops and find_instrs_excluding_cache(instructions) or
find_attr_instruction(instructions, "attr") to encapsulate the unit.op matching
and OpArg conversion logic used in these tests, then replace the inline iterator
chains in each test with calls to those helpers to remove duplication and
improve readability.

🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: fb5ff390-c16f-444b-9356-3e690bd6f79c

📥 Commits

Reviewing files that changed from the base of the PR and between 62b081b and 246479c.

📒 Files selected for processing (3)

.cspell.json
crates/codegen/src/compile.rs
crates/codegen/src/ir.rs

Actionable comments posted: 1

🤖 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 `@crates/codegen/src/ir.rs`:
- Around line 6926-6938: The DFS currently calls block_has_fallthrough() before
checking for empty passthrough labels, causing empty synthetic labels with
load_fast_passthrough or load_fast_label_reuse_passthrough to be treated as
having fallthrough and stop traversal; move the existing empty-passthrough
branch (the condition using block.instructions.is_empty() &&
(block.load_fast_passthrough || block.load_fast_label_reuse_passthrough) &&
!seen[block_idx]) to execute before the block_has_fallthrough(block) check, mark
seen[block_idx] and push the block on the stack there, and only call
block_has_fallthrough(block) afterward so passthrough predecessors are traversed
correctly (references: block_has_fallthrough, load_fast_passthrough,
load_fast_label_reuse_passthrough, seen, stack).

🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 60ad3cbf-583d-40ed-abce-52fa159968e5

📥 Commits

Reviewing files that changed from the base of the PR and between 246479c and 7388f23.

📒 Files selected for processing (1)

crates/codegen/src/ir.rs

* Align load-fast borrow barriers with CPython * Allow CPython ifexp compiler name in spellcheck * Address load-fast passthrough review feedback * Traverse empty load-fast passthrough predecessors * Extract load-fast test helpers

Align load-fast borrow barriers with CPython

dfe8904

Allow CPython ifexp compiler name in spellcheck

246479c

youknowone marked this pull request as ready for review May 20, 2026 00:21

coderabbitai Bot reviewed May 20, 2026

View reviewed changes

Comment thread crates/codegen/src/ir.rs

Comment thread crates/codegen/src/ir.rs Outdated

Address load-fast passthrough review feedback

7388f23

coderabbitai Bot reviewed May 20, 2026

View reviewed changes

Comment thread crates/codegen/src/ir.rs Outdated

youknowone added 2 commits May 20, 2026 10:11

Traverse empty load-fast passthrough predecessors

01d82fa

Extract load-fast test helpers

388a620

youknowone mentioned this pull request May 20, 2026

Subtask: align borrow and fused fast-local opcode selection with CPython youknowone/RustPython#31

Closed

youknowone merged commit d51069b into RustPython:main May 20, 2026
26 checks passed

youknowone deleted the bytecode-parity branch May 20, 2026 05:11

coderabbitai Bot mentioned this pull request May 21, 2026

Align nested code object bytecode parity #7942

Merged

Align load-fast borrow barriers with CPython by youknowone · Pull Request #7930 · RustPython/RustPython · GitHub

Repository navigation

Align load-fast borrow barriers with CPython - #7930

Align load-fast borrow barriers with CPython#7930
youknowone merged 5 commits into
RustPython:mainfrom
youknowone:bytecode-parity

youknowone commented May 19, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented May 19, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Align load-fast borrow barriers with CPython by youknowone · Pull Request #7930 · RustPython/RustPython · GitHub

Repository navigation

Align load-fast borrow barriers with CPython - #7930

Align load-fast borrow barriers with CPython#7930youknowone merged 5 commits intoRustPython:mainRustPython/RustPython:mainfrom youknowone:bytecode-parityyouknowone/RustPython:bytecode-parityCopy head branch name to clipboard

Conversation

youknowone commented May 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Align load-fast borrow barriers with CPython#7930
youknowone merged 5 commits into
RustPython:mainfrom
youknowone:bytecode-parity

youknowone commented May 19, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented May 19, 2026 •
edited
Loading