Make use of Unary opcodes (#6647)

* Make use of `Unary` opcodes

* Add `Reserved140` opcode

* re-add support for UnaryPositive in JIT

* JIT support for `ToBool` instruction

crates/codegen/src/compile.rs

@@ -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)?;

crates/compiler-core/src/bytecode.rs

@@ -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
@@ -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
@@ -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,
@@ -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)]
@@ -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 { .. }
@@ -1959,7 +1942,6 @@ impl Instruction {
             StoreAttr { .. } => -2,
             DeleteAttr { .. } => -1,
             LoadConst { .. } => 1,
-            UnaryOperation { .. } => 0,
             BinaryOp { .. } | CompareOperation { .. } => -1,
             BinarySubscript => -1,
             CopyItem { .. } => 1,
@@ -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,
         }
     }
 
@@ -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),
@@ -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),

crates/jit/src/instructions.rs

@@ -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;
 
@@ -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),
+                }
+            }
             Instruction::CompareOperation { op, .. } => {
                 let op = op.get(arg);
                 // the rhs is popped off first
@@ -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),
                 }
             }

crates/vm/src/frame.rs

@@ -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)
@@ -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
@@ -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 { .. }
@@ -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
@@ -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__);
@@ -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)