Align LOAD_FAST_BORROW analysis with CPython chain shape#7870
Draft
youknowone wants to merge 1 commit into
Draft
Align LOAD_FAST_BORROW analysis with CPython chain shape#7870youknowone wants to merge 1 commit into
youknowone wants to merge 1 commit into
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR updates RustPython's codegen to align with CPython's control-flow semantics for block reuse and cold-block identification. It introduces a block reuse helper, refines borrow-optimization fall-through rules, implements two-phase cold-block marking, defers four tests, and adds spell-check dictionary entries. ChangesSpell-check Dictionary Updates
CPython-aligned Codegen Optimization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
youknowone
added a commit
to youknowone/RustPython
that referenced
this pull request
May 13, 2026
When a `with` body contains a try/except/else whose handler ends with an open conditional (e.g. `except OSError: if not path.is_symlink(): raise`), the handler does not unconditionally scope-exit and the synthetic body-exit NOP introduced by `preserves_finally_entry_nop` is spurious because control may still fall through to the with cleanup. Guard against this by anding in `!statements_end_with_open_conditional_fallthrough` when checking handler scope exits. Also add the test `test_with_try_except_else_open_conditional_handler_drops_body_exit_nop` locking the pattern, and `fobj` to .cspell.dict/python-more.txt so the prek cspell hook on CI clears for PR RustPython#7870 (used verbatim in the already-committed test_nested_with_except_same_line_cleanup_threads_trampoline).
youknowone
force-pushed
the
bytecode-parity
branch
from
27b6e5f to
17a5041
Compare
youknowone
added a commit
to youknowone/RustPython
that referenced
this pull request
May 14, 2026
When a `with` body contains a try/except/else whose handler ends with an open conditional (e.g. `except OSError: if not path.is_symlink(): raise`), the handler does not unconditionally scope-exit and the synthetic body-exit NOP introduced by `preserves_finally_entry_nop` is spurious because control may still fall through to the with cleanup. Guard against this by anding in `!statements_end_with_open_conditional_fallthrough` when checking handler scope exits. Also add the test `test_with_try_except_else_open_conditional_handler_drops_body_exit_nop` locking the pattern, and `fobj` to .cspell.dict/python-more.txt so the prek cspell hook on CI clears for PR RustPython#7870 (used verbatim in the already-committed test_nested_with_except_same_line_cleanup_threads_trampoline).
Three changes that bring optimize_load_fast_borrow closer to CPython's optimize_load_fast in flowgraph.c: * ir.rs: split mark_cold into the CPython-style two passes. Phase 1 propagates "warm" from the entry block, phase 2 propagates "cold" from except_handler blocks. Blocks reached by neither phase keep cold=false and stay in their original b_next position, matching CPython's handling of empty placeholders left by remove_unreachable (e.g. the inner_end of a nested try/except whose incoming jumps were re-routed by optimize_cfg). * ir.rs: in optimize_load_fast_borrow, push the fall-through successor only when the current block has a last instruction (is_some_and). Empty blocks now terminate fall-through propagation, matching the `term != NULL` check in optimize_load_fast. * compile.rs: add switch_to_new_or_reuse_empty() helper and use it in compile_while. The helper reuses the current block when it is empty and unlinked, mirroring USE_LABEL absorption in cfg_builder_maybe_start_new_block. This stops a stray empty block from appearing between e.g. a try/except end_block and the following while loop header. Four codegen tests that depended on the previous fall-through-through- empty behavior are marked #[ignore] with TODO comments. Also includes a handful of dictionary entries in .cspell.dict picked up during the work.
youknowone
force-pushed
the
bytecode-parity
branch
from
9523b6b to
7481459
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three changes that bring optimize_load_fast_borrow closer to CPython's optimize_load_fast in flowgraph.c:
ir.rs: split mark_cold into the CPython-style two passes. Phase 1 propagates "warm" from the entry block, phase 2 propagates "cold" from except_handler blocks. Blocks reached by neither phase keep cold=false and stay in their original b_next position, matching CPython's handling of empty placeholders left by remove_unreachable (e.g. the inner_end of a nested try/except whose incoming jumps were re-routed by optimize_cfg).
ir.rs: in optimize_load_fast_borrow, push the fall-through successor only when the current block has a last instruction (is_some_and). Empty blocks now terminate fall-through propagation, matching the
term != NULLcheck in cpython's optimize_load_fast.compile.rs: add switch_to_new_or_reuse_empty() helper and use it in compile_while. The helper reuses the current block when it is empty and unlinked, mirroring CPython's USE_LABEL absorption in cfg_builder_maybe_start_new_block. This stops a stray empty block from appearing between e.g. a try/except end_block and the following while loop header.
Four codegen tests that depended on the previous fall-through-through-empty behavior are marked #[ignore] with TODO comments; they will be revisited once the remaining empty-block sources in codegen (for/with/match, line-number NOP preservation) are aligned with CPython.
Measured impact: scripts/compare_bytecode.py drops from 244 to 208 differing files across the full Lib tree (-14.8%).
Summary by CodeRabbit
Tests
Chores