GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — slice ops and a little bit of peephole by youknowone · Pull Request #6860 · RustPython/RustPython

Conversation

@youknowone

@youknowone youknowone commented Jan 24, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Refactor
    • Optimized slice compilation with improved constant-slice detection and specialized two-part slice handling
    • Introduced peephole optimization pass that fuses consecutive similar bytecode operations into single operations
    • Refactored global constant loading for frequently-used exceptions and built-in functions to use optimized loading path
    • Added new bytecode instructions including efficient multi-variable load/store operations and specialized slice handling

✏️ Tip: You can customize this high-level summary in your review settings.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@coderabbitai

coderabbitai Bot commented Jan 24, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The pull request introduces bytecode-level optimizations and new instruction types. Changes include a slice optimization path using a new ExprExt trait, peephole optimization for fusing consecutive load/store instructions, a CommonConstant enum for frequently-used constants, and VM execution paths for eight new or modified instructions.

Changes

Cohort / File(s) Summary
Slice optimization logic
crates/codegen/src/compile.rs
Added ExprExt trait with is_constant, is_constant_slice, and should_use_slice_optimization methods; introduced compile_slice_two_parts for two-part slice compilation; updated compile_subscript to use new optimization path and emit BinarySlice/StoreSlice instructions when appropriate.
Peephole optimization
crates/codegen/src/ir.rs
Added peephole_optimize method in CodeInfo.finalize_code that fuses consecutive LoadFast+LoadFast and StoreFast+StoreFast instructions into LoadFastLoadFast and StoreFastStoreFast super-instructions when both indices are < 16.
Bytecode instruction definitions
crates/compiler-core/src/bytecode/oparg.rs
Added new public enum CommonConstant with variants (AssertionError, NotImplementedError, BuiltinTuple, BuiltinAll, BuiltinAny) and Display implementation mapping to canonical Python names.
Instruction enum and metadata
crates/compiler-core/src/bytecode/instruction.rs
Changed LoadCommonConstant variant argument type from Arg to Arg; adjusted stack effects for LoadFastCheck (0→1) and StoreFastStoreFast (0→-2); minor formatting changes.
Public API exports
crates/compiler-core/src/bytecode.rs
Added CommonConstant to re-exported items from oparg module.
VM instruction execution
crates/vm/src/frame.rs
Added execution paths for eight instructions: BinarySlice, CopyFreeVars, LoadCommonConstant, LoadFastCheck, LoadFastLoadFast, MakeCell, StoreFastStoreFast, and StoreSlice with corresponding stack operations and error handling.
Stdlib attribute exposure
crates/stdlib/src/_opcode.rs
Changed ENABLE_SPECIALIZATION_FT from #[allow(dead_code)] to #[pyattr] to expose it as a Python-accessible module attribute.

Sequence Diagram

sequenceDiagram
    participant Compiler as Compiler<br/>(compile.rs)
    participant CodeGen as Code Generator<br/>(ir.rs)
    participant Bytecode as Bytecode<br/>(instruction.rs)
    participant Peephole as Peephole Opt.<br/>(ir.rs)
    participant VM as VM Executor<br/>(frame.rs)

    Compiler->>Compiler: Check is_constant_slice<br/>& should_use_slice_optimization
    alt Optimize Slice Path
        Compiler->>Bytecode: Emit BinarySlice/StoreSlice
    else Standard Path
        Compiler->>Bytecode: Emit standard slice instructions
    end

    Compiler->>CodeGen: Generate code with<br/>LoadFast/StoreFast pairs
    
    CodeGen->>Peephole: Finalize code with<br/>optimization enabled
    Peephole->>Peephole: Scan consecutive instructions
    alt Fusible Pattern Found
        Peephole->>Peephole: Fuse LoadFast+LoadFast<br/>→ LoadFastLoadFast
        Peephole->>Peephole: Fuse StoreFast+StoreFast<br/>→ StoreFastStoreFast
    end
    
    Peephole->>Bytecode: Output optimized<br/>instructions
    
    VM->>VM: Execute instruction
    alt BinarySlice
        VM->>VM: Pop container, start, stop
        VM->>VM: Create PySlice
        VM->>VM: Get item, push result
    else LoadFastLoadFast
        VM->>VM: Decode indices<br/>(idx1 << 4 | idx2)
        VM->>VM: Load both with<br/>unbound checks
        VM->>VM: Push both values
    else StoreFastStoreFast
        VM->>VM: Pop 2 values
        VM->>VM: Store to 2 locals
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Instruction 3.14 #6805: Introduces BinarySlice, LoadCommonConstant, and fused multi-opcodes (LoadFastLoadFast/StoreFastStoreFast) alongside corresponding VM execution paths that directly align with this PR's instruction additions.
  • compile_subscript #6008: Refactors compile_subscript and slice-compilation logic using is_two_element_slice detection; this PR's ExprExt::should_use_slice_optimization and compile_slice_two_parts build on that same code path.
  • Sort Instruction enum & related match arms #6322: Modifies the Instruction enum and match arms for handling new opcodes including LoadCommonConstant and other variant changes that overlap with this PR's instruction enum updates.

Suggested reviewers

  • ShaharNaveh

Poem

🐰 Slices optimized, constants shine bright,
Peephole fusion makes the bytecode lean and light,
LoadFast pairs now dance as one,
The rabbit hops faster—optimizations begun! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'slice ops and a little bit of peephole' accurately describes the main changes: slice operation optimizations (BinarySlice, StoreSlice, compile_slice_two_parts) and peephole optimization (LoadFastLoadFast, StoreFastStoreFast fusion).

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

✨ Finishing touches
  • 📝 Generate docstrings

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
more optimization
e62058c
@youknowone
youknowone force-pushed the peephole branch from c6eb979 to e62058c Compare January 25, 2026 00:16
@github-actions

github-actions Bot commented Jan 25, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[+] test: cpython/Lib/test/test_exceptions.py
[+] test: cpython/Lib/test/test_baseexception.py
[+] test: cpython/Lib/test/test_except_star.py
[+] test: cpython/Lib/test/test_exception_group.py
[+] test: cpython/Lib/test/test_exception_hierarchy.py
[+] test: cpython/Lib/test/test_exception_variations.py

dependencies:

dependent tests: (no tests depend on exception)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
youknowone marked this pull request as ready for review January 25, 2026 02:53
coderabbitai[bot]
coderabbitai Bot reviewed Jan 25, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)
crates/codegen/src/compile.rs (1)

514-531: Guard the optimized slice path from Invalid contexts.
Right now Invalid can slip into the optimized branch and hit unreachable!(). It’s safer to limit the optimization to Load/Store and let the fallback path return the syntax error.

🐛 Suggested fix
-        if slice.should_use_slice_optimization() && !matches!(ctx, ast::ExprContext::Del) {
+        if slice.should_use_slice_optimization()
+            && matches!(ctx, ast::ExprContext::Load | ast::ExprContext::Store)
+        {
             match slice {
                 ast::Expr::Slice(s) => self.compile_slice_two_parts(s)?,
                 _ => unreachable!(
                     "should_use_slice_optimization should only return true for ast::Expr::Slice"
                 ),
             };
             match ctx {
                 ast::ExprContext::Load => {
                     emit!(self, Instruction::BinarySlice);
                 }
                 ast::ExprContext::Store => {
                     emit!(self, Instruction::StoreSlice);
                 }
-                _ => unreachable!(),
+                _ => unreachable!(),
             }
         } else {
♻️ Duplicate comments (1)
crates/vm/src/frame.rs (1)

1806-1820: Same packed‑index constraint as LoadFastLoadFast applies here.

🧹 Nitpick comments (2)
crates/codegen/src/ir.rs (1)

407-466: Guard fusion across differing spans/handlers.
Fusing across different except_handler or source spans can alter exception-table coverage or line events. Consider guarding (or asserting) that both instructions share the same span/handler before combining.

💡 Suggested guard
                     let curr = &block.instructions[i];
                     let next = &block.instructions[i + 1];
 
+                    if curr.except_handler != next.except_handler
+                        || curr.location != next.location
+                        || curr.end_location != next.end_location
+                    {
+                        i += 1;
+                        continue;
+                    }
+
                     // Only combine if both are real instructions (not pseudo)
                     let (Some(curr_instr), Some(next_instr)) =
                         (curr.instr.real(), next.instr.real())
                     else {
crates/compiler-core/src/bytecode/instruction.rs (1)

171-247: Add disassembly formatting for new/common-constant & fused ops.
Now that LoadCommonConstant, LoadFastLoadFast, and StoreFastStoreFast are used, fmt_dis falls back to RUSTPYTHON_PLACEHOLDER for them. Adding explicit cases improves debug output.

♻️ Suggested disassembly cases
@@
-            Self::LoadConst { idx } => fmt_const("LOAD_CONST", arg, f, idx),
+            Self::LoadConst { idx } => fmt_const("LOAD_CONST", arg, f, idx),
+            Self::LoadCommonConstant { idx } => {
+                let c = idx.get(arg);
+                write!(f, "{:pad$}({})", "LOAD_COMMON_CONSTANT", c)
+            }
@@
             Self::LoadFastAndClear(idx) => w!(LOAD_FAST_AND_CLEAR, varname = idx),
+            Self::LoadFastLoadFast { .. } => {
+                let packed = arg.0;
+                let idx1 = packed >> 4;
+                let idx2 = packed & 0xF;
+                write!(f, "LOAD_FAST_LOAD_FAST ({}, {})", varname(idx1), varname(idx2))
+            }
@@
             Self::StoreFast(idx) => w!(STORE_FAST, varname = idx),
+            Self::StoreFastStoreFast { .. } => {
+                let packed = arg.0;
+                let idx1 = packed >> 4;
+                let idx2 = packed & 0xF;
+                write!(f, "STORE_FAST_STORE_FAST ({}, {})", varname(idx1), varname(idx2))
+            }

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@youknowone
youknowone enabled auto-merge (squash) January 25, 2026 04:10
@youknowone
youknowone requested a review from ShaharNaveh January 25, 2026 05:27
ShaharNaveh
ShaharNaveh approved these changes Jan 25, 2026
View reviewed changes

@ShaharNaveh ShaharNaveh left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

That's great!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@youknowone
youknowone merged commit 7eceb14 into RustPython:main Jan 25, 2026
14 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
youknowone deleted the peephole branch January 25, 2026 08:18
@coderabbitai coderabbitai Bot mentioned this pull request Jan 29, 2026
More Load instructions #6886
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Feb 6, 2026
Fix traceback, syntax errors, and exception handling #7015
Merged
This was referenced Feb 28, 2026
instruction CACHE #7251
Merged
Bytecode enum named oparg #7294
Merged
Fold const bool with unary not #7357
Merged
This was referenced Mar 12, 2026
Enable PEP 709 inlined comprehensions #7412
Merged
Newtype var_nums oparg #7431
Merged
Align _opcode_metadata.py to 3.14.3 #7456
Merged
This was referenced Mar 20, 2026
Bytecode parity #7475
Merged
Bytecode parity #7507
Merged
This was referenced Mar 28, 2026
Compiler parity: docstring dedent, StopIteration wrapper, constant folding #7530
Merged
Bytecode parity #7541
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@ShaharNaveh ShaharNaveh ShaharNaveh approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@youknowone @ShaharNaveh
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.