Skip to content

Make use of Unary opcodes #6647

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 10 commits into from
Jan 5, 2026
Merged

Make use of Unary opcodes #6647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
eece3bf
Make use of `Unary` opcodes
ShaharNaveh Jan 4, 2026
fd3ef1d
Fix jit
ShaharNaveh Jan 5, 2026
c54b881
Merge remote-tracking branch 'upstream/main' into bytecode-split-unary
ShaharNaveh Jan 5, 2026
15d9e0a
Add `Reserved140` opcode
ShaharNaveh Jan 5, 2026
5641bc9
re-add support for UnaryPositive in JIT
ShaharNaveh Jan 5, 2026
cf8cf21
resolve arg
ShaharNaveh Jan 5, 2026
f12d2f3
Use correct instruction
ShaharNaveh Jan 5, 2026
50763ce
JIT support for `ToBool` instruction
ShaharNaveh Jan 5, 2026
0696124
Fix unarynot
ShaharNaveh Jan 5, 2026
f86f0d5
Merge remote-tracking branch 'upstream/main' into bytecode-split-unary
ShaharNaveh Jan 5, 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
19 changes: 13 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 @@ -5656,13 +5656,20 @@ impl Compiler {
self.compile_expression(operand)?;

// Perform operation:
let op = match op {
UnaryOp::UAdd => bytecode::UnaryOperator::Plus,
UnaryOp::USub => bytecode::UnaryOperator::Minus,
UnaryOp::Not => bytecode::UnaryOperator::Not,
UnaryOp::Invert => bytecode::UnaryOperator::Invert,
match op {
UnaryOp::UAdd => emit!(
self,
Instruction::CallIntrinsic1 {
func: bytecode::IntrinsicFunction1::UnaryPositive
}
),
UnaryOp::USub => emit!(self, Instruction::UnaryNegative),
UnaryOp::Not => {
emit!(self, Instruction::ToBool);
emit!(self, Instruction::UnaryNot);
}
UnaryOp::Invert => emit!(self, Instruction::UnaryInvert),
};
emit!(self, Instruction::UnaryOperation { op });
}
Expr::Attribute(ExprAttribute { value, attr, .. }) => {
self.compile_expression(value)?;
Expand Down
50 changes: 17 additions & 33 deletions crates/compiler-core/src/bytecode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ op_arg_enum!(
ImportStar = 2,
// StopIterationError = 3,
// AsyncGenWrap = 4,
// UnaryPositive = 5,
UnaryPositive = 5,
/// Convert list to tuple
ListToTuple = 6,
/// Type parameter related
Expand Down Expand Up @@ -758,11 +758,11 @@ pub enum Instruction {
StoreSubscript,
// 40: TO_BOOL
ToBool,
// 41: UNARY_INVERT - placeholder (RustPython uses UnaryOperation)
// 41: UNARY_INVERT
UnaryInvert,
// 42: UNARY_NEGATIVE - placeholder
// 42: UNARY_NEGATIVE
UnaryNegative,
// 43: UNARY_NOT - placeholder
// 43: UNARY_NOT
UnaryNot,
// ==================== With-argument instructions (opcode >= 44) ====================
// 44: WITH_EXCEPT_START
Expand Down Expand Up @@ -1091,11 +1091,8 @@ pub enum Instruction {
SetExcInfo,
// 139: SUBSCRIPT
Subscript,
// 140: UNARY_OP (combines UNARY_*)
UnaryOperation {
op: Arg<UnaryOperator>,
},
// 141-148: Reserved (padding to keep RESUME at 149)
// 140-148: Reserved (padding to keep RESUME at 149)
Reserved140,
Reserved141,
Reserved142,
Reserved143,
Expand Down Expand Up @@ -1538,18 +1535,6 @@ impl fmt::Display for BinaryOperator {
}
}

op_arg_enum!(
/// The possible unary operators
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[repr(u8)]
pub enum UnaryOperator {
Not = 0,
Invert = 1,
Minus = 2,
Plus = 3,
}
);

op_arg_enum!(
/// Whether or not to invert the operation.
#[repr(u8)]
Expand Down Expand Up @@ -1910,23 +1895,21 @@ impl Instruction {
/// # Examples
///
/// ```
/// use rustpython_compiler_core::bytecode::{Arg, Instruction, Label, UnaryOperator};
/// use rustpython_compiler_core::bytecode::{Arg, Instruction, Label};
/// let (target, jump_arg) = Arg::new(Label(0xF));
/// let jump_instruction = Instruction::Jump { target };
/// let (op, invert_arg) = Arg::new(UnaryOperator::Invert);
/// let invert_instruction = Instruction::UnaryOperation { op };
/// assert_eq!(jump_instruction.stack_effect(jump_arg, true), 0);
/// assert_eq!(invert_instruction.stack_effect(invert_arg, false), 0);
/// ```
///
pub fn stack_effect(&self, arg: OpArg, jump: bool) -> i32 {
match self {
// Dummy/placeholder instructions (never executed)
Cache | Reserved3 | Reserved17 | Reserved141 | Reserved142 | Reserved143
| Reserved144 | Reserved145 | Reserved146 | Reserved147 | Reserved148 => 0,
Cache | Reserved3 | Reserved17 | Reserved140 | Reserved141 | Reserved142
| Reserved143 | Reserved144 | Reserved145 | Reserved146 | Reserved147 | Reserved148 => {
0
}
BinarySlice | EndFor | ExitInitCheck | GetYieldFromIter | InterpreterExit
| LoadAssertionError | LoadLocals | PushNull | ReturnGenerator | StoreSlice
| UnaryInvert | UnaryNegative | UnaryNot => 0,
| LoadAssertionError | LoadLocals | PushNull | ReturnGenerator | StoreSlice => 0,
BuildConstKeyMap { .. }
| CopyFreeVars { .. }
| DictMerge { .. }
Expand Down Expand Up @@ -1959,7 +1942,6 @@ impl Instruction {
StoreAttr { .. } => -2,
DeleteAttr { .. } => -1,
LoadConst { .. } => 1,
UnaryOperation { .. } => 0,
BinaryOp { .. } | CompareOperation { .. } => -1,
BinarySubscript => -1,
CopyItem { .. } => 1,
Expand Down Expand Up @@ -2086,6 +2068,9 @@ impl Instruction {
MatchKeys => 1, // Pop 2 (subject, keys), push 3 (subject, keys_or_none, values_or_none)
MatchClass(_) => -2,
ExtendedArg => 0,
UnaryInvert => 0,
UnaryNegative => 0,
UnaryNot => 0,
}
}

Expand Down Expand Up @@ -2158,8 +2143,8 @@ impl Instruction {
match self {
// Dummy/placeholder instructions
Cache => w!(CACHE),
Reserved3 | Reserved17 | Reserved141 | Reserved142 | Reserved143 | Reserved144
| Reserved145 | Reserved146 | Reserved147 | Reserved148 => w!(RESERVED),
Reserved3 | Reserved17 | Reserved140 | Reserved141 | Reserved142 | Reserved143
| Reserved144 | Reserved145 | Reserved146 | Reserved147 | Reserved148 => w!(RESERVED),
BinarySlice => w!(BINARY_SLICE),
EndFor => w!(END_FOR),
ExitInitCheck => w!(EXIT_INIT_CHECK),
Expand Down Expand Up @@ -2302,7 +2287,6 @@ impl Instruction {
Subscript => w!(SUBSCRIPT),
Swap { index } => w!(SWAP, index),
ToBool => w!(TO_BOOL),
UnaryOperation { op } => w!(UNARY_OP, ?op),
UnpackEx { args } => w!(UNPACK_EX, args),
UnpackSequence { size } => w!(UNPACK_SEQUENCE, size),
WithExceptStart => w!(WITH_EXCEPT_START),
Expand Down
53 changes: 35 additions & 18 deletions crates/jit/src/instructions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use cranelift::codegen::ir::FuncRef;
use cranelift::prelude::*;
use num_traits::cast::ToPrimitive;
use rustpython_compiler_core::bytecode::{
self, BinaryOperator, BorrowedConstant, CodeObject, ComparisonOperator, Instruction, Label,
OpArg, OpArgState, UnaryOperator,
self, BinaryOperator, BorrowedConstant, CodeObject, ComparisonOperator, Instruction,
IntrinsicFunction1, Label, OpArg, OpArgState,
};
use std::collections::HashMap;

Expand Down Expand Up @@ -474,6 +474,21 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
_ => Err(JitCompileError::BadBytecode),
}
}
Instruction::CallIntrinsic1 { func } => {
match func.get(arg) {
IntrinsicFunction1::UnaryPositive => {
match self.stack.pop().ok_or(JitCompileError::BadBytecode)? {
JitValue::Int(val) => {
// Nothing to do
self.stack.push(JitValue::Int(val));
Ok(())
}
_ => Err(JitCompileError::NotSupported),
}
}
_ => Err(JitCompileError::NotSupported),
}
}
Comment on lines +477 to +491
Copy link
Contributor

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

# Search for UnaryPositive references and Float handling patterns
rg -n 'UnaryPositive' crates/jit/src/ --type rust -C 2

Repository: RustPython/RustPython

Length of output: 528

🏁 Script executed:

# Check if Float is handled for other unary operations
rg -n 'JitValue::Float' crates/jit/src/instructions.rs -C 3 | head -100

Repository: RustPython/RustPython

Length of output: 5237

🏁 Script executed:

# Search for IntrinsicFunction1 definition to understand what operations exist
rg -n 'enum IntrinsicFunction1|IntrinsicFunction1::' crates/jit/src/ --type rust -B2 -A5

Repository: RustPython/RustPython

Length of output: 790

🏁 Script executed:

# Check if there are any Float support patterns in the codebase
rg -n 'Float.*=>|JitValue::Float' crates/jit/src/instructions.rs --type rust -B2 -A2 | head -50

Repository: RustPython/RustPython

Length of output: 2409

🏁 Script executed:

# Search for other unary operations to see the pattern
rg -n 'UnaryNegative|UnaryInvert|UnaryNot' crates/jit/src/instructions.rs --type rust -B2 -A8

Repository: RustPython/RustPython

Length of output: 940

🏁 Script executed:

# Get more context for UnaryNegative to see if Float is handled
rg -n 'Instruction::UnaryNegative' crates/jit/src/instructions.rs --type rust -A20

Repository: RustPython/RustPython

Length of output: 1102

Add Float support for unary operations.

The implementation correctly handles UnaryPositive for Int, but Float support is missing. This is inconsistent with binary operations throughout the file (Add, Subtract, Multiply, TrueDivide, Power all support Float). Both UnaryPositive and UnaryNegative should handle JitValue::Float with the same semantics as Int (identity for +, negate for -).

🤖 Prompt for AI Agents 
In @crates/jit/src/instructions.rs around lines 477-491, The unary intrinsic
handling in Instruction::CallIntrinsic1 { func } currently only supports
JitValue::Int for IntrinsicFunction1::UnaryPositive; update the match arms so
both IntrinsicFunction1::UnaryPositive and IntrinsicFunction1::UnaryNegative
handle JitValue::Float as well (for UnaryPositive push the same JitValue::Float
back, for UnaryNegative push a JitValue::Float with the negated value),
mirroring the existing Int semantics around the
self.stack.pop().ok_or(JitCompileError::BadBytecode)? pattern and returning the
same error branches for unsupported types.

Instruction::CompareOperation { op, .. } => {
let op = op.get(arg);
// the rhs is popped off first
Expand Down Expand Up @@ -620,28 +635,30 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
self.stack.swap(i, j);
Ok(())
}
Instruction::UnaryOperation { op, .. } => {
let op = op.get(arg);
Instruction::ToBool => {
let a = self.stack.pop().ok_or(JitCompileError::BadBytecode)?;
match (op, a) {
(UnaryOperator::Minus, JitValue::Int(val)) => {
// Compile minus as 0 - a.
let value = self.boolean_val(a)?;
self.stack.push(JitValue::Bool(value));
Ok(())
}
Instruction::UnaryNot => {
let boolean = match self.stack.pop().ok_or(JitCompileError::BadBytecode)? {
JitValue::Bool(val) => val,
_ => return Err(JitCompileError::BadBytecode),
};
let not_boolean = self.builder.ins().bxor_imm(boolean, 1);
self.stack.push(JitValue::Bool(not_boolean));
Ok(())
}
Instruction::UnaryNegative => {
match self.stack.pop().ok_or(JitCompileError::BadBytecode)? {
JitValue::Int(val) => {
// Compile minus as 0 - val.
let zero = self.builder.ins().iconst(types::I64, 0);
let out = self.compile_sub(zero, val);
self.stack.push(JitValue::Int(out));
Ok(())
}
(UnaryOperator::Plus, JitValue::Int(val)) => {
// Nothing to do
self.stack.push(JitValue::Int(val));
Ok(())
}
(UnaryOperator::Not, a) => {
let boolean = self.boolean_val(a)?;
let not_boolean = self.builder.ins().bxor_imm(boolean, 1);
self.stack.push(JitValue::Bool(not_boolean));
Ok(())
}
_ => Err(JitCompileError::NotSupported),
}
}
Comment on lines +653 to 664
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for fneg and Float handling in unary operations
rg -nP --type=rust -C3 'fneg|UnaryNegative.*Float|Float.*UnaryNegative' crates/jit/src/

Repository: RustPython/RustPython

Length of output: 609

🏁 Script executed:

#!/bin/bash
# Check the full context around UnaryNegative and look for Float handling patterns
rg -nP --type=rust -B5 -A10 'Instruction::UnaryNegative' crates/jit/src/

Repository: RustPython/RustPython

Length of output: 1435

🏁 Script executed:

#!/bin/bash
# Search for UnaryPositive to see if it has similar patterns
rg -nP --type=rust -B2 -A8 'Instruction::UnaryPositive' crates/jit/src/

Repository: RustPython/RustPython

Length of output: 47

🏁 Script executed:

#!/bin/bash
# Look for Float handling in other unary or binary operations for reference
rg -nP --type=rust 'JitValue::Float.*=>' crates/jit/src/ | head -20

Repository: RustPython/RustPython

Length of output: 556

Add Float support to UnaryNegative handler.

The implementation handles Int correctly but should also support JitValue::Float. Since the codebase uses fneg for float negation and Float is handled in other binary operations (e.g., line 529), add a case for JitValue::Float:

JitValue::Float(val) => {
    let out = self.builder.ins().fneg(val);
    self.stack.push(JitValue::Float(out));
    Ok(())
}
🤖 Prompt for AI Agents 
In @crates/jit/src/instructions.rs around lines 650-661, The UnaryNegative
handler currently only handles JitValue::Int by compiling 0 - val; add a branch
for JitValue::Float in the match inside Instruction::UnaryNegative to call
self.builder.ins().fneg on the popped float value and push JitValue::Float(out)
onto self.stack, returning Ok(()); keep the existing Int branch (using
compile_sub) and the fallback error branch unchanged.

Expand Down
45 changes: 20 additions & 25 deletions crates/vm/src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1652,7 +1652,6 @@ impl ExecutingFrame<'_> {
self.push_value(vm.ctx.new_bool(bool_val).into());
Ok(None)
}
bytecode::Instruction::UnaryOperation { op } => self.execute_unary_op(vm, op.get(arg)),
bytecode::Instruction::UnpackEx { args } => {
let args = args.get(arg);
self.execute_unpack_ex(vm, args.before, args.after)
Expand Down Expand Up @@ -1762,7 +1761,24 @@ impl ExecutingFrame<'_> {
.map_err(|_| vm.new_type_error("exception expected".to_owned()))?;
Err(exc)
}

bytecode::Instruction::UnaryInvert => {
let a = self.pop_value();
let value = vm._invert(&a)?;
self.push_value(value);
Ok(None)
}
bytecode::Instruction::UnaryNegative => {
let a = self.pop_value();
let value = vm._neg(&a)?;
self.push_value(value);
Ok(None)
}
bytecode::Instruction::UnaryNot => {
let obj = self.pop_value();
let value = obj.try_to_bool(vm)?;
self.push_value(vm.ctx.new_bool(!value).into());
Ok(None)
}
// Placeholder/dummy instructions - these should never be executed
bytecode::Instruction::Cache
| bytecode::Instruction::Reserved3
Expand All @@ -1776,9 +1792,6 @@ impl ExecutingFrame<'_> {
| bytecode::Instruction::PushNull
| bytecode::Instruction::ReturnGenerator
| bytecode::Instruction::StoreSlice
| bytecode::Instruction::UnaryInvert
| bytecode::Instruction::UnaryNegative
| bytecode::Instruction::UnaryNot
| bytecode::Instruction::BuildConstKeyMap { .. }
| bytecode::Instruction::CopyFreeVars { .. }
| bytecode::Instruction::DictMerge { .. }
Expand All @@ -1797,6 +1810,7 @@ impl ExecutingFrame<'_> {
| bytecode::Instruction::PopJumpIfNotNone { .. }
| bytecode::Instruction::SetUpdate { .. }
| bytecode::Instruction::StoreFastStoreFast { .. }
| bytecode::Instruction::Reserved140
| bytecode::Instruction::Reserved141
| bytecode::Instruction::Reserved142
| bytecode::Instruction::Reserved143
Expand Down Expand Up @@ -2440,26 +2454,6 @@ impl ExecutingFrame<'_> {
Ok(None)
}

#[cfg_attr(feature = "flame-it", flame("Frame"))]
fn execute_unary_op(
&mut self,
vm: &VirtualMachine,
op: bytecode::UnaryOperator,
) -> FrameResult {
let a = self.pop_value();
let value = match op {
bytecode::UnaryOperator::Minus => vm._neg(&a)?,
bytecode::UnaryOperator::Plus => vm._pos(&a)?,
bytecode::UnaryOperator::Invert => vm._invert(&a)?,
bytecode::UnaryOperator::Not => {
let value = a.try_to_bool(vm)?;
vm.ctx.new_bool(!value).into()
}
};
self.push_value(value);
Ok(None)
}

#[cold]
fn setup_annotations(&mut self, vm: &VirtualMachine) -> FrameResult {
let __annotations__ = identifier!(vm, __annotations__);
Expand Down Expand Up @@ -2634,6 +2628,7 @@ impl ExecutingFrame<'_> {
self.import_star(vm)?;
Ok(vm.ctx.none())
}
bytecode::IntrinsicFunction1::UnaryPositive => vm._pos(&arg),
bytecode::IntrinsicFunction1::SubscriptGeneric => {
// Used for PEP 695: Generic[*type_params]
crate::builtins::genericalias::subscript_generic(arg, vm)
Expand Down
Loading