Skip to content

Commit ead7e0c

Browse files
ShaharNavehShaharNaveh
authored
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
1 parent 85bafc0 commit ead7e0c

File tree

4 files changed

+85
-82
lines changed

4 files changed

+85
-82
lines changed

‎crates/codegen/src/compile.rs‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5656,13 +5656,20 @@ impl Compiler {
56565656
self.compile_expression(operand)?;
56575657

56585658
// Perform operation:
5659-
let op = match op {
5660-
UnaryOp::UAdd => bytecode::UnaryOperator::Plus,
5661-
UnaryOp::USub => bytecode::UnaryOperator::Minus,
5662-
UnaryOp::Not => bytecode::UnaryOperator::Not,
5663-
UnaryOp::Invert => bytecode::UnaryOperator::Invert,
5659+
match op {
5660+
UnaryOp::UAdd => emit!(
5661+
self,
5662+
Instruction::CallIntrinsic1 {
5663+
func: bytecode::IntrinsicFunction1::UnaryPositive
5664+
}
5665+
),
5666+
UnaryOp::USub => emit!(self, Instruction::UnaryNegative),
5667+
UnaryOp::Not => {
5668+
emit!(self, Instruction::ToBool);
5669+
emit!(self, Instruction::UnaryNot);
5670+
}
5671+
UnaryOp::Invert => emit!(self, Instruction::UnaryInvert),
56645672
};
5665-
emit!(self, Instruction::UnaryOperation { op });
56665673
}
56675674
Expr::Attribute(ExprAttribute { value, attr, .. }) => {
56685675
self.compile_expression(value)?;

‎crates/compiler-core/src/bytecode.rs‎

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ op_arg_enum!(
640640
ImportStar = 2,
641641
// StopIterationError = 3,
642642
// AsyncGenWrap = 4,
643-
// UnaryPositive = 5,
643+
UnaryPositive = 5,
644644
/// Convert list to tuple
645645
ListToTuple = 6,
646646
/// Type parameter related
@@ -758,11 +758,11 @@ pub enum Instruction {
758758
StoreSubscript,
759759
// 40: TO_BOOL
760760
ToBool,
761-
// 41: UNARY_INVERT - placeholder (RustPython uses UnaryOperation)
761+
// 41: UNARY_INVERT
762762
UnaryInvert,
763-
// 42: UNARY_NEGATIVE - placeholder
763+
// 42: UNARY_NEGATIVE
764764
UnaryNegative,
765-
// 43: UNARY_NOT - placeholder
765+
// 43: UNARY_NOT
766766
UnaryNot,
767767
// ==================== With-argument instructions (opcode >= 44) ====================
768768
// 44: WITH_EXCEPT_START
@@ -1091,11 +1091,8 @@ pub enum Instruction {
10911091
SetExcInfo,
10921092
// 139: SUBSCRIPT
10931093
Subscript,
1094-
// 140: UNARY_OP (combines UNARY_*)
1095-
UnaryOperation {
1096-
op: Arg<UnaryOperator>,
1097-
},
1098-
// 141-148: Reserved (padding to keep RESUME at 149)
1094+
// 140-148: Reserved (padding to keep RESUME at 149)
1095+
Reserved140,
10991096
Reserved141,
11001097
Reserved142,
11011098
Reserved143,
@@ -1538,18 +1535,6 @@ impl fmt::Display for BinaryOperator {
15381535
}
15391536
}
15401537

1541-
op_arg_enum!(
1542-
/// The possible unary operators
1543-
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1544-
#[repr(u8)]
1545-
pub enum UnaryOperator {
1546-
Not = 0,
1547-
Invert = 1,
1548-
Minus = 2,
1549-
Plus = 3,
1550-
}
1551-
);
1552-
15531538
op_arg_enum!(
15541539
/// Whether or not to invert the operation.
15551540
#[repr(u8)]
@@ -1910,23 +1895,21 @@ impl Instruction {
19101895
/// # Examples
19111896
///
19121897
/// ```
1913-
/// use rustpython_compiler_core::bytecode::{Arg, Instruction, Label, UnaryOperator};
1898+
/// use rustpython_compiler_core::bytecode::{Arg, Instruction, Label};
19141899
/// let (target, jump_arg) = Arg::new(Label(0xF));
19151900
/// let jump_instruction = Instruction::Jump { target };
1916-
/// let (op, invert_arg) = Arg::new(UnaryOperator::Invert);
1917-
/// let invert_instruction = Instruction::UnaryOperation { op };
19181901
/// assert_eq!(jump_instruction.stack_effect(jump_arg, true), 0);
1919-
/// assert_eq!(invert_instruction.stack_effect(invert_arg, false), 0);
19201902
/// ```
19211903
///
19221904
pub fn stack_effect(&self, arg: OpArg, jump: bool) -> i32 {
19231905
match self {
19241906
// Dummy/placeholder instructions (never executed)
1925-
Cache | Reserved3 | Reserved17 | Reserved141 | Reserved142 | Reserved143
1926-
| Reserved144 | Reserved145 | Reserved146 | Reserved147 | Reserved148 => 0,
1907+
Cache | Reserved3 | Reserved17 | Reserved140 | Reserved141 | Reserved142
1908+
| Reserved143 | Reserved144 | Reserved145 | Reserved146 | Reserved147 | Reserved148 => {
1909+
0
1910+
}
19271911
BinarySlice | EndFor | ExitInitCheck | GetYieldFromIter | InterpreterExit
1928-
| LoadAssertionError | LoadLocals | PushNull | ReturnGenerator | StoreSlice
1929-
| UnaryInvert | UnaryNegative | UnaryNot => 0,
1912+
| LoadAssertionError | LoadLocals | PushNull | ReturnGenerator | StoreSlice => 0,
19301913
BuildConstKeyMap { .. }
19311914
| CopyFreeVars { .. }
19321915
| DictMerge { .. }
@@ -1959,7 +1942,6 @@ impl Instruction {
19591942
StoreAttr { .. } => -2,
19601943
DeleteAttr { .. } => -1,
19611944
LoadConst { .. } => 1,
1962-
UnaryOperation { .. } => 0,
19631945
BinaryOp { .. } | CompareOperation { .. } => -1,
19641946
BinarySubscript => -1,
19651947
CopyItem { .. } => 1,
@@ -2086,6 +2068,9 @@ impl Instruction {
20862068
MatchKeys => 1, // Pop 2 (subject, keys), push 3 (subject, keys_or_none, values_or_none)
20872069
MatchClass(_) => -2,
20882070
ExtendedArg => 0,
2071+
UnaryInvert => 0,
2072+
UnaryNegative => 0,
2073+
UnaryNot => 0,
20892074
}
20902075
}
20912076

@@ -2158,8 +2143,8 @@ impl Instruction {
21582143
match self {
21592144
// Dummy/placeholder instructions
21602145
Cache => w!(CACHE),
2161-
Reserved3 | Reserved17 | Reserved141 | Reserved142 | Reserved143 | Reserved144
2162-
| Reserved145 | Reserved146 | Reserved147 | Reserved148 => w!(RESERVED),
2146+
Reserved3 | Reserved17 | Reserved140 | Reserved141 | Reserved142 | Reserved143
2147+
| Reserved144 | Reserved145 | Reserved146 | Reserved147 | Reserved148 => w!(RESERVED),
21632148
BinarySlice => w!(BINARY_SLICE),
21642149
EndFor => w!(END_FOR),
21652150
ExitInitCheck => w!(EXIT_INIT_CHECK),
@@ -2302,7 +2287,6 @@ impl Instruction {
23022287
Subscript => w!(SUBSCRIPT),
23032288
Swap { index } => w!(SWAP, index),
23042289
ToBool => w!(TO_BOOL),
2305-
UnaryOperation { op } => w!(UNARY_OP, ?op),
23062290
UnpackEx { args } => w!(UNPACK_EX, args),
23072291
UnpackSequence { size } => w!(UNPACK_SEQUENCE, size),
23082292
WithExceptStart => w!(WITH_EXCEPT_START),

‎crates/jit/src/instructions.rs‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use cranelift::codegen::ir::FuncRef;
44
use cranelift::prelude::*;
55
use num_traits::cast::ToPrimitive;
66
use rustpython_compiler_core::bytecode::{
7-
self, BinaryOperator, BorrowedConstant, CodeObject, ComparisonOperator, Instruction, Label,
8-
OpArg, OpArgState, UnaryOperator,
7+
self, BinaryOperator, BorrowedConstant, CodeObject, ComparisonOperator, Instruction,
8+
IntrinsicFunction1, Label, OpArg, OpArgState,
99
};
1010
use std::collections::HashMap;
1111

@@ -474,6 +474,21 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
474474
_ => Err(JitCompileError::BadBytecode),
475475
}
476476
}
477+
Instruction::CallIntrinsic1 { func } => {
478+
match func.get(arg) {
479+
IntrinsicFunction1::UnaryPositive => {
480+
match self.stack.pop().ok_or(JitCompileError::BadBytecode)? {
481+
JitValue::Int(val) => {
482+
// Nothing to do
483+
self.stack.push(JitValue::Int(val));
484+
Ok(())
485+
}
486+
_ => Err(JitCompileError::NotSupported),
487+
}
488+
}
489+
_ => Err(JitCompileError::NotSupported),
490+
}
491+
}
477492
Instruction::CompareOperation { op, .. } => {
478493
let op = op.get(arg);
479494
// the rhs is popped off first
@@ -620,28 +635,30 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
620635
self.stack.swap(i, j);
621636
Ok(())
622637
}
623-
Instruction::UnaryOperation { op, .. } => {
624-
let op = op.get(arg);
638+
Instruction::ToBool => {
625639
let a = self.stack.pop().ok_or(JitCompileError::BadBytecode)?;
626-
match (op, a) {
627-
(UnaryOperator::Minus, JitValue::Int(val)) => {
628-
// Compile minus as 0 - a.
640+
let value = self.boolean_val(a)?;
641+
self.stack.push(JitValue::Bool(value));
642+
Ok(())
643+
}
644+
Instruction::UnaryNot => {
645+
let boolean = match self.stack.pop().ok_or(JitCompileError::BadBytecode)? {
646+
JitValue::Bool(val) => val,
647+
_ => return Err(JitCompileError::BadBytecode),
648+
};
649+
let not_boolean = self.builder.ins().bxor_imm(boolean, 1);
650+
self.stack.push(JitValue::Bool(not_boolean));
651+
Ok(())
652+
}
653+
Instruction::UnaryNegative => {
654+
match self.stack.pop().ok_or(JitCompileError::BadBytecode)? {
655+
JitValue::Int(val) => {
656+
// Compile minus as 0 - val.
629657
let zero = self.builder.ins().iconst(types::I64, 0);
630658
let out = self.compile_sub(zero, val);
631659
self.stack.push(JitValue::Int(out));
632660
Ok(())
633661
}
634-
(UnaryOperator::Plus, JitValue::Int(val)) => {
635-
// Nothing to do
636-
self.stack.push(JitValue::Int(val));
637-
Ok(())
638-
}
639-
(UnaryOperator::Not, a) => {
640-
let boolean = self.boolean_val(a)?;
641-
let not_boolean = self.builder.ins().bxor_imm(boolean, 1);
642-
self.stack.push(JitValue::Bool(not_boolean));
643-
Ok(())
644-
}
645662
_ => Err(JitCompileError::NotSupported),
646663
}
647664
}

‎crates/vm/src/frame.rs‎

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,7 +1652,6 @@ impl ExecutingFrame<'_> {
16521652
self.push_value(vm.ctx.new_bool(bool_val).into());
16531653
Ok(None)
16541654
}
1655-
bytecode::Instruction::UnaryOperation { op } => self.execute_unary_op(vm, op.get(arg)),
16561655
bytecode::Instruction::UnpackEx { args } => {
16571656
let args = args.get(arg);
16581657
self.execute_unpack_ex(vm, args.before, args.after)
@@ -1762,7 +1761,24 @@ impl ExecutingFrame<'_> {
17621761
.map_err(|_| vm.new_type_error("exception expected".to_owned()))?;
17631762
Err(exc)
17641763
}
1765-
1764+
bytecode::Instruction::UnaryInvert => {
1765+
let a = self.pop_value();
1766+
let value = vm._invert(&a)?;
1767+
self.push_value(value);
1768+
Ok(None)
1769+
}
1770+
bytecode::Instruction::UnaryNegative => {
1771+
let a = self.pop_value();
1772+
let value = vm._neg(&a)?;
1773+
self.push_value(value);
1774+
Ok(None)
1775+
}
1776+
bytecode::Instruction::UnaryNot => {
1777+
let obj = self.pop_value();
1778+
let value = obj.try_to_bool(vm)?;
1779+
self.push_value(vm.ctx.new_bool(!value).into());
1780+
Ok(None)
1781+
}
17661782
// Placeholder/dummy instructions - these should never be executed
17671783
bytecode::Instruction::Cache
17681784
| bytecode::Instruction::Reserved3
@@ -1776,9 +1792,6 @@ impl ExecutingFrame<'_> {
17761792
| bytecode::Instruction::PushNull
17771793
| bytecode::Instruction::ReturnGenerator
17781794
| bytecode::Instruction::StoreSlice
1779-
| bytecode::Instruction::UnaryInvert
1780-
| bytecode::Instruction::UnaryNegative
1781-
| bytecode::Instruction::UnaryNot
17821795
| bytecode::Instruction::BuildConstKeyMap { .. }
17831796
| bytecode::Instruction::CopyFreeVars { .. }
17841797
| bytecode::Instruction::DictMerge { .. }
@@ -1797,6 +1810,7 @@ impl ExecutingFrame<'_> {
17971810
| bytecode::Instruction::PopJumpIfNotNone { .. }
17981811
| bytecode::Instruction::SetUpdate { .. }
17991812
| bytecode::Instruction::StoreFastStoreFast { .. }
1813+
| bytecode::Instruction::Reserved140
18001814
| bytecode::Instruction::Reserved141
18011815
| bytecode::Instruction::Reserved142
18021816
| bytecode::Instruction::Reserved143
@@ -2440,26 +2454,6 @@ impl ExecutingFrame<'_> {
24402454
Ok(None)
24412455
}
24422456

2443-
#[cfg_attr(feature = "flame-it", flame("Frame"))]
2444-
fn execute_unary_op(
2445-
&mut self,
2446-
vm: &VirtualMachine,
2447-
op: bytecode::UnaryOperator,
2448-
) -> FrameResult {
2449-
let a = self.pop_value();
2450-
let value = match op {
2451-
bytecode::UnaryOperator::Minus => vm._neg(&a)?,
2452-
bytecode::UnaryOperator::Plus => vm._pos(&a)?,
2453-
bytecode::UnaryOperator::Invert => vm._invert(&a)?,
2454-
bytecode::UnaryOperator::Not => {
2455-
let value = a.try_to_bool(vm)?;
2456-
vm.ctx.new_bool(!value).into()
2457-
}
2458-
};
2459-
self.push_value(value);
2460-
Ok(None)
2461-
}
2462-
24632457
#[cold]
24642458
fn setup_annotations(&mut self, vm: &VirtualMachine) -> FrameResult {
24652459
let __annotations__ = identifier!(vm, __annotations__);
@@ -2634,6 +2628,7 @@ impl ExecutingFrame<'_> {
26342628
self.import_star(vm)?;
26352629
Ok(vm.ctx.none())
26362630
}
2631+
bytecode::IntrinsicFunction1::UnaryPositive => vm._pos(&arg),
26372632
bytecode::IntrinsicFunction1::SubscriptGeneric => {
26382633
// Used for PEP 695: Generic[*type_params]
26392634
crate::builtins::genericalias::subscript_generic(arg, vm)

0 commit comments

Comments
 (0)