Skip to content

instruction CACHE #7251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 4 commits into from
Mar 1, 2026
Merged

instruction CACHE #7251

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd9787f
Emit CACHE code units in bytecode to match CPython 3.14
youknowone Feb 27, 2026
f68f7ef
Use relative jump offsets and fix bytecode layout
youknowone Feb 27, 2026
ccc95f6
Align CPython 3.14 LOAD_GLOBAL null-bit and RERAISE semantics
youknowone Feb 28, 2026
b28fc61
Remove redundant CPython-referencing comments
youknowone Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Remove redundant CPython-referencing comments 
Clean up comments that unnecessarily mention CPython per project
convention. Replace with concise descriptions of the behavior itself.
  • Loading branch information
@youknowone
youknowone committed Mar 1, 2026
commit b28fc61a39a0c9e22d1e8a45b66943f89251bfc0
9 changes: 3 additions & 6 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2374,16 +2374,14 @@ impl Compiler {
}
}
ast::Stmt::Break(_) => {
// Match CPython line-tracing behavior (codegen_break emits NOP).
emit!(self, Instruction::Nop);
emit!(self, Instruction::Nop); // NOP for line tracing
// Unwind fblock stack until we find a loop, emitting cleanup for each fblock
self.compile_break_continue(statement.range(), true)?;
let dead = self.new_block();
self.switch_to_block(dead);
}
ast::Stmt::Continue(_) => {
// Match CPython line-tracing behavior (codegen_continue emits NOP).
emit!(self, Instruction::Nop);
emit!(self, Instruction::Nop); // NOP for line tracing
// Unwind fblock stack until we find a loop, emitting cleanup for each fblock
self.compile_break_continue(statement.range(), false)?;
let dead = self.new_block();
Expand Down Expand Up @@ -2456,8 +2454,7 @@ impl Compiler {
}
}
ast::Stmt::Pass(_) => {
// Match CPython line-tracing behavior (Pass_kind emits NOP).
emit!(self, Instruction::Nop);
emit!(self, Instruction::Nop); // NOP for line tracing
}
ast::Stmt::TypeAlias(ast::StmtTypeAlias {
name,
Expand Down
24 changes: 11 additions & 13 deletions crates/codegen/src/ir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ impl CodeInfo {

// Convert pseudo ops and remove resulting NOPs (keep line-marker NOPs)
convert_pseudo_ops(&mut blocks, varname_cache.len() as u32);
// Remove redundant NOPs (CPython basicblock_remove_redundant_nops):
// keep line-marker NOPs only when they are needed to preserve tracing.
// Remove redundant NOPs, keeping line-marker NOPs only when
// they are needed to preserve tracing.
let mut block_order = Vec::new();
let mut current = BlockIdx(0);
while current != BlockIdx::NULL {
Expand All @@ -283,9 +283,12 @@ impl CodeInfo {
}
// Remove if the next instruction has same line or no line.
else if src < src_instructions.len() - 1 {
let next_lineno = src_instructions[src + 1]
.lineno_override
.unwrap_or_else(|| src_instructions[src + 1].location.line.get() as i32);
let next_lineno =
src_instructions[src + 1]
.lineno_override
.unwrap_or_else(|| {
src_instructions[src + 1].location.line.get() as i32
});
if next_lineno == lineno {
remove = true;
} else if next_lineno < 0 {
Expand Down Expand Up @@ -344,7 +347,6 @@ impl CodeInfo {
// This is the index into the final instructions array, including EXTENDED_ARG and CACHE
let mut block_to_index = vec![0u32; blocks.len()];
// The offset (in code units) of END_SEND from SEND in the yield-from sequence.
// Matches CPython's END_SEND_OFFSET in Python/assemble.c.
const END_SEND_OFFSET: u32 = 5;
Comment on lines +349 to +350
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 1, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check the actual code at the specified lines
head -360 crates/codegen/src/ir.rs | tail -20

Repository: RustPython/RustPython

Length of output: 1178

🏁 Script executed:

# Search for END_SEND_OFFSET usage and context
rg -n "END_SEND_OFFSET" --type rust -A 5 -B 2

Repository: RustPython/RustPython

Length of output: 1571

🏁 Script executed:

# Check SEND instruction definition
rg -n "SEND" --type rust | grep -E "(Instruction|Send|cache)" | head -20

Repository: RustPython/RustPython

Length of output: 269

🏁 Script executed:

# Look for bytecode instruction definitions to understand the structure
rg -n "enum.*Instruction" --type rust -A 30 | head -80

Repository: RustPython/RustPython

Length of output: 6327

🏁 Script executed:

# Search for SEND instruction definition with cache entries
rg -n "SEND|cache" --type rust -A 3 -B 3 | grep -A 10 -B 10 "Send"

Repository: RustPython/RustPython

Length of output: 11832

🏁 Script executed:

# Look for instruction cache entries definitions
rg -n "cache_entries" --type rust | head -20

Repository: RustPython/RustPython

Length of output: 1880

🏁 Script executed:

# Search for bytecode layout or instruction size definitions
rg -n "cache.*entry|CACHE|code.*unit" --type rust | head -30

Repository: RustPython/RustPython

Length of output: 3168

🏁 Script executed:

# Check if there's any documentation or comments about SEND's structure
rg -n "Send.*cache|Send.*offset" --type rust -A 5 -B 5

Repository: RustPython/RustPython

Length of output: 842

🏁 Script executed:

# Find the cache_entries implementation for SEND instruction
rg -n "fn cache_entries" --type rust -A 60 | grep -A 60 "cache_entries"

Repository: RustPython/RustPython

Length of output: 5666

🏁 Script executed:

# More specifically, look for SEND's cache entries in the match statement
rg -n "Self::Send" --type rust -B 5 -A 5 | head -100

Repository: RustPython/RustPython

Length of output: 4600

🏁 Script executed:

# Check the full cache_entries implementation
sed -n '517,650p' crates/compiler-core/src/bytecode/instruction.rs

Repository: RustPython/RustPython

Length of output: 4849

🏁 Script executed:

# Find any other references to END_SEND_OFFSET or documentation about the bytecode layout
rg -n "END_SEND|ForIter|CLEANUP_THROW" --type rust -B 3 -A 3 | grep -A 10 -B 10 "offset\|sequence"

Repository: RustPython/RustPython

Length of output: 9223

🏁 Script executed:

# Check what SEND and related instructions look like in compile.rs for context
sed -n '6750,6810p' crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 2192

🏁 Script executed:

# Let me check the actual bytecode layout for ForIter vs Send to understand the pattern
rg -n "ForIter.*cache|ForIter.*=>" --type rust -B 2 -A 2 | head -50

Repository: RustPython/RustPython

Length of output: 3990

🏁 Script executed:

# Check if there are any tests or documentation about END_SEND_OFFSET
rg -n "END_SEND_OFFSET|5.*code.*unit|offset.*SEND" --type rust

Repository: RustPython/RustPython

Length of output: 453

🏁 Script executed:

# Check what instructions come between SEND and END_SEND in practice
# by looking at the actual code generation
grep -n "YIELD_VALUE\|RESUME\|SETUP_FINALLY" crates/codegen/src/compile.rs | sed -n '6770,6810p'

Repository: RustPython/RustPython

Length of output: 47

🏁 Script executed:

# Verify the exact bytecode units by checking any test cases
find . -name "*.rs" -type f | xargs grep -l "END_SEND_OFFSET\|yield.*from.*sequence" | head -5

Repository: RustPython/RustPython

Length of output: 123

Verify the breakdown of END_SEND_OFFSET.

The value of 5 is correct and matches CPython's END_SEND_OFFSET. However, the breakdown needs clarification: SEND (1 unit) + its 1 cache entry (1 unit) + intermediate instructions (3 units) = 5 units to END_SEND. The original comment's accounting of "SEND (1) + cache (1) + END_SEND (1) + potential intermediate" is incomplete as it doesn't explicitly detail the 3 intermediate code units between the SEND/CACHE pair and END_SEND.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/ir.rs` around lines 346 - 348, Clarify the comment
explaining END_SEND_OFFSET (const END_SEND_OFFSET: u32 = 5) by explicitly
breaking down the 5 code units: SEND instruction (1), its cache entry (1), the
three intermediate code units between the SEND/CACHE pair and END_SEND (3),
summing to 5; update the comment near END_SEND_OFFSET to list these components
and their counts and reference SEND and END_SEND so readers can verify the
correspondence with CPython's END_SEND_OFFSET.

loop {
let mut num_instructions = 0;
Expand Down Expand Up @@ -373,9 +375,8 @@ impl CodeInfo {
let mut op = info.instr.expect_real();
let old_arg_size = info.arg.instr_size();
let old_cache_entries = info.cache_entries;
// Keep offsets fixed within this pass, like CPython's
// resolve_jump_offsets(): changes in jump arg/cache sizes only
// take effect in the next pass.
// Keep offsets fixed within this pass: changes in jump
// arg/cache sizes only take effect in the next iteration.
let offset_after = current_offset + old_arg_size as u32 + old_cache_entries;

if target != BlockIdx::NULL {
Expand Down Expand Up @@ -448,7 +449,6 @@ impl CodeInfo {
};
linetable_locations.extend(core::iter::repeat_n(lt_loc, info.arg.instr_size()));
// CACHE entries inherit parent instruction's location
// (matches CPython assemble_location_info: instr_size includes caches)
if cache_count > 0 {
linetable_locations.extend(core::iter::repeat_n(lt_loc, cache_count));
}
Expand Down Expand Up @@ -702,7 +702,6 @@ impl CodeInfo {
}
}

/// CPython flowgraph.c:
/// LOAD_GLOBAL <even> + PUSH_NULL -> LOAD_GLOBAL <odd>, NOP
fn optimize_load_global_push_null(&mut self) {
for block in &mut self.blocks {
Expand Down Expand Up @@ -1547,8 +1546,7 @@ fn normalize_jumps(blocks: &mut [Block]) {
}
}

// Resolve JUMP/JUMP_NO_INTERRUPT pseudo instructions before offset fixpoint,
// matching CPython's resolve_unconditional_jumps().
// Resolve JUMP/JUMP_NO_INTERRUPT pseudo instructions before offset fixpoint.
let mut block_order = vec![0u32; blocks.len()];
for (pos, &block_idx) in visit_order.iter().enumerate() {
block_order[block_idx.idx()] = pos as u32;
Expand Down
2 changes: 1 addition & 1 deletion crates/compiler-core/src/bytecode/instruction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,7 @@ impl InstructionMetadata for Instruction {
Self::GetYieldFromIter => w!(GET_YIELD_FROM_ITER),
Self::BuildTemplate => w!(BUILD_TEMPLATE),
Self::BuildInterpolation { oparg } => w!(BUILD_INTERPOLATION, oparg),
_ => write!(f, "{self:?}"),
_ => w!(RUSTPYTHON_PLACEHOLDER),
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions crates/vm/src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2651,7 +2651,6 @@ impl ExecutingFrame<'_> {
}
Instruction::InstrumentedJumpForward => {
let src_offset = (self.lasti() - 1) * 2;
// JumpForward: 0 caches, forward
let target_idx = self.lasti() + u32::from(arg);
let target = bytecode::Label(target_idx);
self.jump(target);
Expand All @@ -2662,7 +2661,6 @@ impl ExecutingFrame<'_> {
}
Instruction::InstrumentedJumpBackward => {
let src_offset = (self.lasti() - 1) * 2;
// JumpBackward: 1 cache, backward
let target_idx = self.lasti() + 1 - u32::from(arg);
let target = bytecode::Label(target_idx);
self.jump(target);
Expand All @@ -2673,7 +2671,6 @@ impl ExecutingFrame<'_> {
}
Instruction::InstrumentedForIter => {
let src_offset = (self.lasti() - 1) * 2;
// ForIter: 1 cache, forward relative
let target = bytecode::Label(self.lasti() + 1 + u32::from(arg));
let continued = self.execute_for_iter(vm, target)?;
if continued {
Expand Down Expand Up @@ -2720,7 +2717,6 @@ impl ExecutingFrame<'_> {
}
Instruction::InstrumentedPopJumpIfTrue => {
let src_offset = (self.lasti() - 1) * 2;
// PopJumpIfTrue: 1 cache, forward
let target_idx = self.lasti() + 1 + u32::from(arg);
let obj = self.pop_value();
let value = obj.try_to_bool(vm)?;
Expand Down
Loading