Visitar URL original
instruction improvements by youknowone · Pull Request #6829 · RustPython/RustPython · GitHub
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
Merged

instruction improvements #6829

Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
impl LoadSmallInt
  • Loading branch information
@youknowone
youknowone committed Jan 22, 2026
commit b1f7e439ed19ebeb6b594d858261f9c1012f7b00
12 changes: 12 additions & 0 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7749,6 +7749,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

@coderabbitai coderabbitai Bot Jan 22, 2026

Copy link
Copy Markdown
Contributor

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
7 changes: 4 additions & 3 deletions crates/compiler-core/src/bytecode/instruction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub enum Instruction {
CheckExcMatch = 6,
CleanupThrow = 7,
DeleteSubscr = 8,
EndFor = 9, // Placeholder
EndFor = 9,
EndSend = 10,
ExitInitCheck = 11, // Placeholder
FormatSimple = 12,
Expand All @@ -52,7 +52,7 @@ pub enum Instruction {
Nop = 27,
NotTaken = 28, // Placeholder
PopExcept = 29,
PopIter = 30, // Placeholder
PopIter = 30,
PopTop = 31,
PushExcInfo = 32,
PushNull = 33,
Expand Down Expand Up @@ -190,7 +190,7 @@ pub enum Instruction {
LoadName(Arg<NameIdx>) = 93,
LoadSmallInt {
idx: Arg<u32>,
} = 94, // Placeholder
} = 94,
LoadSpecial {
method: Arg<SpecialMethod>,
} = 95,
Expand Down Expand Up @@ -844,6 +844,7 @@ impl InstructionMetadata for Instruction {
Self::LoadBuildClass => w!(LOAD_BUILD_CLASS),
Self::LoadFromDictOrDeref(i) => w!(LOAD_FROM_DICT_OR_DEREF, cell_name = i),
Self::LoadConst { idx } => fmt_const("LOAD_CONST", arg, f, idx),
Self::LoadSmallInt { idx } => w!(LOAD_SMALL_INT, idx),
Self::LoadDeref(idx) => w!(LOAD_DEREF, cell_name = idx),
Self::LoadFast(idx) => w!(LOAD_FAST, varname = idx),
Self::LoadFastAndClear(idx) => w!(LOAD_FAST_AND_CLEAR, varname = idx),
Expand Down
6 changes: 6 additions & 0 deletions crates/jit/src/instructions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,12 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
self.stack.push(val);
Ok(())
}
Instruction::LoadSmallInt { idx } => {
let small_int = idx.get(arg) as i64;
let val = self.builder.ins().iconst(types::I64, small_int);
self.stack.push(JitValue::Int(val));
Ok(())
}
Instruction::LoadFast(idx) => {
let local = self.variables[idx.get(arg) as usize]
.as_ref()
Expand Down
6 changes: 6 additions & 0 deletions crates/vm/src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,12 @@ impl ExecutingFrame<'_> {
self.push_value(self.code.constants[idx.get(arg) as usize].clone().into());
Ok(None)
}
Instruction::LoadSmallInt { idx } => {
// Push small integer (-5..=256) directly without constant table lookup
let value = vm.ctx.new_int(idx.get(arg) as i32);
self.push_value(value.into());
Ok(None)
}
Instruction::LoadDeref(i) => {
let idx = i.get(arg) as usize;
let x = self.cells_frees[idx]
Expand Down