Skip to content

instruction improvements #6829

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 6 commits into from
Jan 22, 2026
+433 −253
Merged

instruction improvements #6829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
96a2111
align psuedo ops
youknowone Jan 21, 2026
7e01e55
impl EndFor
youknowone Jan 21, 2026
b1f7e43
impl LoadSmallInt
youknowone Jan 21, 2026
62aa69a
Align generator, async function
youknowone Jan 22, 2026
d7a7395
mark dis regression; requiring update to 3.14
youknowone Jan 21, 2026
5b67da5
loadcolcals
youknowone Jan 21, 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
40 changes: 21 additions & 19 deletions Lib/_opcode_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@
'BINARY_OP_EXTEND': 132,
'BINARY_OP_MULTIPLY_FLOAT': 133,
'BINARY_OP_MULTIPLY_INT': 134,
'BINARY_SUBSCR_DICT': 135,
'BINARY_SUBSCR_GETITEM': 136,
'BINARY_SUBSCR_LIST_INT': 137,
'BINARY_SUBSCR_LIST_SLICE': 138,
'BINARY_SUBSCR_STR_INT': 139,
'BINARY_SUBSCR_TUPLE_INT': 140,
'BINARY_OP_SUBSCR_DICT': 135,
'BINARY_OP_SUBSCR_GETITEM': 136,
'BINARY_OP_SUBSCR_LIST_INT': 137,
'BINARY_OP_SUBSCR_LIST_SLICE': 138,
'BINARY_OP_SUBSCR_STR_INT': 139,
'BINARY_OP_SUBSCR_TUPLE_INT': 140,
'BINARY_OP_SUBTRACT_FLOAT': 141,
'BINARY_OP_SUBTRACT_INT': 142,
'CALL_ALLOC_AND_ENTER_INIT': 143,
Expand Down Expand Up @@ -234,19 +234,21 @@
'INSTRUMENTED_JUMP_BACKWARD': 253,
'INSTRUMENTED_LINE': 254,
'ENTER_EXECUTOR': 255,
'JUMP': 256,
'JUMP_NO_INTERRUPT': 257,
'RESERVED_258': 258,
'LOAD_ATTR_METHOD': 259,
'LOAD_SUPER_METHOD': 260,
'LOAD_ZERO_SUPER_ATTR': 261,
'LOAD_ZERO_SUPER_METHOD': 262,
'POP_BLOCK': 263,
'SETUP_CLEANUP': 264,
'SETUP_FINALLY': 265,
'SETUP_WITH': 266,
'STORE_FAST_MAYBE_NULL': 267,
'LOAD_CLOSURE': 268,
'ANNOTATIONS_PLACEHOLDER': 256,
'JUMP': 257,
'JUMP_IF_FALSE': 258,
'JUMP_IF_TRUE': 259,
'JUMP_NO_INTERRUPT': 260,
'LOAD_CLOSURE': 261,
'POP_BLOCK': 262,
'SETUP_CLEANUP': 263,
'SETUP_FINALLY': 264,
'SETUP_WITH': 265,
'STORE_FAST_MAYBE_NULL': 266,
'LOAD_ATTR_METHOD': 267,
'LOAD_SUPER_METHOD': 268,
'LOAD_ZERO_SUPER_ATTR': 269,
'LOAD_ZERO_SUPER_METHOD': 270,
}

# CPython 3.13 compatible: opcodes < 44 have no argument
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test__opcode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def test_stack_effect(self):
self.assertRaises(ValueError, stack_effect, code)
self.assertRaises(ValueError, stack_effect, code, 0)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_stack_effect_jump(self):
FOR_ITER = dis.opmap['FOR_ITER']
self.assertEqual(stack_effect(FOR_ITER, 0), 1)
Expand Down
4 changes: 4 additions & 0 deletions Lib/test/test_dis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2185,24 +2185,28 @@ def test_bytecode_co_positions(self):
assert instr.positions == positions

class TestBytecodeTestCase(BytecodeTestCase):
@unittest.expectedFailure # TODO: RUSTPYTHON; RETURN_VALUE
def test_assert_not_in_with_op_not_in_bytecode(self):
code = compile("a = 1", "<string>", "exec")
self.assertInBytecode(code, "LOAD_CONST", 1)
self.assertNotInBytecode(code, "LOAD_NAME")
self.assertNotInBytecode(code, "LOAD_NAME", "a")

@unittest.expectedFailure # TODO: RUSTPYTHON; RETURN_VALUE
def test_assert_not_in_with_arg_not_in_bytecode(self):
code = compile("a = 1", "<string>", "exec")
self.assertInBytecode(code, "LOAD_CONST")
self.assertInBytecode(code, "LOAD_CONST", 1)
self.assertNotInBytecode(code, "LOAD_CONST", 2)

@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: AssertionError not raised
def test_assert_not_in_with_arg_in_bytecode(self):
code = compile("a = 1", "<string>", "exec")
with self.assertRaises(AssertionError):
self.assertNotInBytecode(code, "LOAD_CONST", 1)

class TestFinderMethods(unittest.TestCase):
@unittest.expectedFailure # TODO: RUSTPYTHON
def test__find_imports(self):
cases = [
("import a.b.c", ('a.b.c', 0, None)),
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_import/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1385,7 +1385,7 @@ class PycRewritingTests(unittest.TestCase):
import sys
code_filename = sys._getframe().f_code.co_filename
module_filename = __file__
constant = 1
constant = 1000
def func():
pass
func_filename = func.__code__.co_filename
Expand Down Expand Up @@ -1455,7 +1455,7 @@ def test_foreign_code(self):
code = marshal.load(f)
constants = list(code.co_consts)
foreign_code = importlib.import_module.__code__
pos = constants.index(1)
pos = constants.index(1000)
constants[pos] = foreign_code
code = code.replace(co_consts=tuple(constants))
with open(self.compiled_name, "wb") as f:
Expand Down
54 changes: 41 additions & 13 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,13 @@ impl Compiler {
// Set the source range for the RESUME instruction
// For now, just use an empty range at the beginning
self.current_source_range = TextRange::default();

// For async functions/coroutines, emit RETURN_GENERATOR + POP_TOP before RESUME
if scope_type == CompilerScope::AsyncFunction {
emit!(self, Instruction::ReturnGenerator);
emit!(self, Instruction::PopTop);
}

emit!(
self,
Instruction::Resume {
Expand Down Expand Up @@ -1371,7 +1378,7 @@ impl Compiler {
if preserve_tos {
emit!(self, Instruction::Swap { index: 2 });
}
emit!(self, Instruction::PopTop);
emit!(self, Instruction::PopIter);
}

FBlockType::TryExcept => {
Expand Down Expand Up @@ -3623,6 +3630,7 @@ impl Compiler {
self.current_code_info().flags |= bytecode::CodeFlags::HAS_DOCSTRING;
}
// If no docstring, don't add None to co_consts
// Note: RETURN_GENERATOR + POP_TOP for async functions is emitted in enter_scope()

// Compile body statements
self.compile_statements(body)?;
Expand Down Expand Up @@ -4348,10 +4356,7 @@ impl Compiler {

// PEP 649: Initialize __classdict__ cell for class annotation scope
if self.current_symbol_table().needs_classdict {
let locals_name = self.name("locals");
emit!(self, Instruction::LoadName(locals_name));
emit!(self, Instruction::PushNull);
emit!(self, Instruction::Call { nargs: 0 });
emit!(self, Instruction::LoadLocals);
let classdict_idx = self.get_cell_var_index("__classdict__")?;
emit!(self, Instruction::StoreDeref(classdict_idx));
}
Expand Down Expand Up @@ -4978,8 +4983,10 @@ impl Compiler {
if is_async {
emit!(self, Instruction::EndAsyncFor);
} else {
// Pop the iterator after loop ends
emit!(self, Instruction::PopTop);
// END_FOR + POP_ITER pattern (CPython 3.14)
// FOR_ITER jumps to END_FOR, but VM skips it (+1) to reach POP_ITER
emit!(self, Instruction::EndFor);
emit!(self, Instruction::PopIter);
}
self.compile_statements(orelse)?;

Expand Down Expand Up @@ -6527,8 +6534,11 @@ impl Compiler {
}
);

// JUMP_NO_INTERRUPT send (regular JUMP in RustPython)
emit!(self, PseudoInstruction::Jump { target: send_block });
// JUMP_BACKWARD_NO_INTERRUPT send
emit!(
self,
PseudoInstruction::JumpNoInterrupt { target: send_block }
);

// fail: CLEANUP_THROW
// Stack when exception: [receiver, yielded_value, exc]
Expand Down Expand Up @@ -7424,7 +7434,9 @@ impl Compiler {
emit!(self, Instruction::EndAsyncFor);
emit!(self, Instruction::PopTop);
} else {
emit!(self, Instruction::PopTop);
// END_FOR + POP_ITER pattern (CPython 3.14)
emit!(self, Instruction::EndFor);
emit!(self, Instruction::PopIter);
}
}

Expand Down Expand Up @@ -7621,9 +7633,13 @@ impl Compiler {
self.switch_to_block(after_block);
if is_async {
emit!(self, Instruction::EndAsyncFor);
// Pop the iterator
emit!(self, Instruction::PopTop);
} else {
// END_FOR + POP_ITER pattern (CPython 3.14)
emit!(self, Instruction::EndFor);
emit!(self, Instruction::PopIter);
}
// Pop the iterator
emit!(self, Instruction::PopTop);
}

// Step 8: Clean up - restore saved locals
Expand Down Expand Up @@ -7741,6 +7757,18 @@ impl Compiler {
}

fn emit_load_const(&mut self, constant: ConstantData) {
// Use LOAD_SMALL_INT for integers in small int cache range (-5..=256)
// Still add to co_consts for compatibility (CPython does this too)
if let ConstantData::Integer { ref value } = constant
&& let Some(small_int) = value.to_i32()
&& (-5..=256).contains(&small_int)
{
// Add to co_consts even though we use LOAD_SMALL_INT
let _idx = self.arg_constant(constant);
// Store as u32 (two's complement for negative values)
self.emit_arg(small_int as u32, |idx| Instruction::LoadSmallInt { idx });
return;
Comment on lines +7760 to +7770
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 22, 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:

rg -n "LoadSmallInt" crates -g '*.rs' -C 3

Repository: RustPython/RustPython

Length of output: 3919

LoadSmallInt VM decoder is correct; JIT decoder has a potential issue.

The VM handler correctly decodes LoadSmallInt as signed i32 (line 1188 of crates/vm/src/frame.rs):

let value = vm.ctx.new_int(idx.get(arg) as i32);

Casting u32 to i32 preserves the two's complement bit pattern, so negative values are materialized correctly. However, the JIT handler (crates/jit/src/instructions.rs:576) casts directly to i64:

let small_int = idx.get(arg) as i64;

This uses zero-extension instead of sign-extension, causing small negative integers (e.g., -1 encoded as 0xFFFFFFFF) to become large positive integers (4294967295) in the JIT path. Consider casting to i32 first, then to i64 to preserve the sign.

🤖 Prompt for AI Agents 
In `@crates/codegen/src/compile.rs` around lines 7760 - 7770, The JIT path
mis-decodes LoadSmallInt by zero-extending the stored u32 when computing
small_int (currently doing idx.get(arg) as i64); update the JIT handler that
decodes LoadSmallInt (the place that assigns small_int from idx.get(arg)) to
cast to i32 first and then to i64 (i.e., small_int = (idx.get(arg) as i32) as
i64) so negative values preserve two's-complement sign like the VM handler
(which uses idx.get(arg) as i32); ensure any comments/reference to LoadSmallInt,
emit_arg, and arg_constant remain consistent.

}
let idx = self.arg_constant(constant);
self.emit_arg(idx, |idx| Instruction::LoadConst { idx })
}
Expand Down Expand Up @@ -7924,7 +7952,7 @@ impl Compiler {

// For break in a for loop, pop the iterator
if is_break && is_for_loop {
emit!(self, Instruction::PopTop);
emit!(self, Instruction::PopIter);
}

// Jump to target
Expand Down
23 changes: 16 additions & 7 deletions crates/codegen/src/ir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,16 @@ impl CodeInfo {
info.arg = OpArg(new_idx);
info.instr = Instruction::LoadFast(Arg::marker()).into();
}
PseudoInstruction::Jump { .. } => {
// PseudoInstruction::Jump instructions are handled later
PseudoInstruction::Jump { .. } | PseudoInstruction::JumpNoInterrupt { .. } => {
// Jump pseudo instructions are handled later
}
PseudoInstruction::JumpNoInterrupt { .. }
| PseudoInstruction::Reserved258
PseudoInstruction::AnnotationsPlaceholder
| PseudoInstruction::JumpIfFalse { .. }
| PseudoInstruction::JumpIfTrue { .. }
| PseudoInstruction::SetupCleanup
| PseudoInstruction::SetupFinally
| PseudoInstruction::SetupWith
| PseudoInstruction::StoreFastMaybeNull => {
| PseudoInstruction::StoreFastMaybeNull(_) => {
unimplemented!("Got a placeholder pseudo instruction ({instr:?})")
}
}
Expand Down Expand Up @@ -335,6 +336,14 @@ impl CodeInfo {
}
}
}
AnyInstruction::Pseudo(PseudoInstruction::JumpNoInterrupt { .. })
if target != BlockIdx::NULL =>
{
// JumpNoInterrupt is always backward (used in yield-from/await loops)
Instruction::JumpBackwardNoInterrupt {
target: Arg::marker(),
}
}
other => other.expect_real(),
};

Expand Down Expand Up @@ -468,7 +477,7 @@ impl CodeInfo {
let block = &self.blocks[block_idx];
for ins in &block.instructions {
let instr = &ins.instr;
let effect = instr.stack_effect(ins.arg, false);
let effect = instr.stack_effect(ins.arg);
if DEBUG {
let display_arg = if ins.target == BlockIdx::NULL {
ins.arg
Expand All @@ -493,7 +502,7 @@ impl CodeInfo {
}
// Process target blocks for branching instructions
if ins.target != BlockIdx::NULL {
let effect = instr.stack_effect(ins.arg, true);
// Both jump and non-jump paths have the same stack effect
let target_depth = depth.checked_add_signed(effect).ok_or({
if effect < 0 {
InternalError::StackUnderflow
Expand Down
Loading
Loading