From 8c7674cabbc05a9414c58570a663f4ba6a4caba9 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Fri, 28 Nov 2025 16:33:24 +0200 Subject: [PATCH 1/4] Remove `Instruction::Duplicate2?` --- crates/codegen/src/compile.rs | 27 +++++++++++++------------- crates/compiler-core/src/bytecode.rs | 6 ------ crates/vm/src/frame.rs | 29 +++++++++------------------- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 5f8caebf272..43af774683f 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -1063,7 +1063,7 @@ impl Compiler { if let Stmt::Expr(StmtExpr { value, .. }) = &last { self.compile_expression(value)?; - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); emit!(self, Instruction::PrintExpr); } else { self.compile_statement(last)?; @@ -1094,7 +1094,7 @@ impl Compiler { Stmt::FunctionDef(_) | Stmt::ClassDef(_) => { let pop_instructions = self.current_block().instructions.pop(); let store_inst = compiler_unwrap_option(self, pop_instructions); // pop Instruction::Store - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); self.current_block().instructions.push(store_inst); } _ => self.emit_load_const(ConstantData::None), @@ -1656,7 +1656,7 @@ impl Compiler { for (i, target) in targets.iter().enumerate() { if i + 1 != targets.len() { - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); } self.compile_store(target)?; } @@ -1917,7 +1917,7 @@ impl Compiler { ); } - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); self.store_name(name.as_ref())?; } TypeParam::ParamSpec(TypeParamParamSpec { name, default, .. }) => { @@ -1943,7 +1943,7 @@ impl Compiler { ); } - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); self.store_name(name.as_ref())?; } TypeParam::TypeVarTuple(TypeParamTypeVarTuple { name, default, .. }) => { @@ -1970,7 +1970,7 @@ impl Compiler { ); } - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); self.store_name(name.as_ref())?; } }; @@ -2030,7 +2030,7 @@ impl Compiler { // check if this handler can handle the exception: if let Some(exc_type) = type_ { // Duplicate exception for test: - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); // Check exception type: self.compile_expression(exc_type)?; @@ -2251,7 +2251,7 @@ impl Compiler { // Handle docstring if present if let Some(doc) = doc_str { - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); self.emit_load_const(ConstantData::Str { value: doc.to_string().into(), }); @@ -2726,7 +2726,7 @@ impl Compiler { if let Some(classcell_idx) = classcell_idx { emit!(self, Instruction::LoadClosure(classcell_idx.to_u32())); - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); let classcell = self.name("__classcell__"); emit!(self, Instruction::StoreLocal(classcell)); } else { @@ -4121,7 +4121,7 @@ impl Compiler { for (op, val) in mid_ops.iter().zip(mid_exprs) { self.compile_expression(val)?; // store rhs for the next comparison in chain - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); emit!(self, Instruction::Rotate3); compile_cmpop(self, op); @@ -4322,7 +4322,8 @@ impl Compiler { // But we can't use compile_subscript directly because we need DUP_TOP2 self.compile_expression(value)?; self.compile_expression(slice)?; - emit!(self, Instruction::Duplicate2); + emit!(self, Instruction::CopyItem { index: 2_u32 }); + emit!(self, Instruction::CopyItem { index: 2_u32 }); emit!(self, Instruction::Subscript); AugAssignKind::Subscript } @@ -4330,7 +4331,7 @@ impl Compiler { let attr = attr.as_str(); self.check_forbidden_name(attr, NameUsage::Store)?; self.compile_expression(value)?; - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); let idx = self.name(attr); emit!(self, Instruction::LoadAttr { idx }); AugAssignKind::Attr { idx } @@ -4896,7 +4897,7 @@ impl Compiler { range: _, }) => { self.compile_expression(value)?; - emit!(self, Instruction::Duplicate); + emit!(self, Instruction::CopyItem { index: 1_u32 }); self.compile_store(target)?; } Expr::FString(fstring) => { diff --git a/crates/compiler-core/src/bytecode.rs b/crates/compiler-core/src/bytecode.rs index 2cfb70e2782..7e377ea9c2a 100644 --- a/crates/compiler-core/src/bytecode.rs +++ b/crates/compiler-core/src/bytecode.rs @@ -616,8 +616,6 @@ pub enum Instruction { ToBool, Rotate2, Rotate3, - Duplicate, - Duplicate2, GetIter, GetLen, CallIntrinsic1 { @@ -1476,8 +1474,6 @@ impl Instruction { Swap { .. } => 0, ToBool => 0, Rotate2 | Rotate3 => 0, - Duplicate => 1, - Duplicate2 => 2, GetIter => 0, GetLen => 1, CallIntrinsic1 { .. } => 0, // Takes 1, pushes 1 @@ -1680,8 +1676,6 @@ impl Instruction { ToBool => w!(ToBool), Rotate2 => w!(Rotate2), Rotate3 => w!(Rotate3), - Duplicate => w!(Duplicate), - Duplicate2 => w!(Duplicate2), GetIter => w!(GetIter), // GET_LEN GetLen => w!(GetLen), diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index ab90f52b2df..b42a2b28165 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -719,27 +719,16 @@ impl ExecutingFrame<'_> { self.state.stack.swap(i, j); Ok(None) } - // bytecode::Instruction::ToBool => { - // dbg!("Shouldn't be called outside of match statements for now") - // let value = self.pop_value(); - // // call __bool__ - // let result = value.try_to_bool(vm)?; - // self.push_value(vm.ctx.new_bool(result).into()); - // Ok(None) - // } - bytecode::Instruction::Duplicate => { - // Duplicate top of stack - let value = self.top_value(); - self.push_value(value.to_owned()); - Ok(None) - } - bytecode::Instruction::Duplicate2 => { - // Duplicate top 2 of stack - let len = self.state.stack.len(); - self.push_value(self.state.stack[len - 2].clone()); - self.push_value(self.state.stack[len - 1].clone()); - Ok(None) + /* + bytecode::Instruction::ToBool => { + dbg!("Shouldn't be called outside of match statements for now") + let value = self.pop_value(); + // call __bool__ + let result = value.try_to_bool(vm)?; + self.push_value(vm.ctx.new_bool(result).into()); + Ok(None) } + */ // splitting the instructions like this offloads the cost of "dynamic" dispatch (on the // amount to rotate) to the opcode dispatcher, and generates optimized code for the // concrete cases we actually have From 0f4f62fee020065d14f913e436f74c7d01bda35a Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Fri, 28 Nov 2025 16:40:08 +0200 Subject: [PATCH 2/4] Remove `Instruction::Rotate*` instructions --- crates/codegen/src/compile.rs | 13 +++++++------ crates/compiler-core/src/bytecode.rs | 5 ----- crates/vm/src/frame.rs | 12 ------------ 3 files changed, 7 insertions(+), 23 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 43af774683f..b2dc10cda6c 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -2255,7 +2255,7 @@ impl Compiler { self.emit_load_const(ConstantData::Str { value: doc.to_string().into(), }); - emit!(self, Instruction::Rotate2); + emit!(self, Instruction::Swap { index: 2 }); let doc_attr = self.name("__doc__"); emit!(self, Instruction::StoreAttr { idx: doc_attr }); } @@ -4121,8 +4121,8 @@ impl Compiler { for (op, val) in mid_ops.iter().zip(mid_exprs) { self.compile_expression(val)?; // store rhs for the next comparison in chain - emit!(self, Instruction::CopyItem { index: 1_u32 }); - emit!(self, Instruction::Rotate3); + emit!(self, Instruction::Swap { index: 2 }); + emit!(self, Instruction::CopyItem { index: 2_u32 }); compile_cmpop(self, op); @@ -4151,7 +4151,7 @@ impl Compiler { // early exit left us with stack: `rhs, comparison_result`. We need to clean up rhs. self.switch_to_block(break_block); - emit!(self, Instruction::Rotate2); + emit!(self, Instruction::Swap { index: 2 }); emit!(self, Instruction::Pop); self.switch_to_block(after_block); @@ -4351,12 +4351,13 @@ impl Compiler { } AugAssignKind::Subscript => { // stack: CONTAINER SLICE RESULT - emit!(self, Instruction::Rotate3); + emit!(self, Instruction::Swap { index: 3 }); + emit!(self, Instruction::Swap { index: 2 }); emit!(self, Instruction::StoreSubscript); } AugAssignKind::Attr { idx } => { // stack: CONTAINER RESULT - emit!(self, Instruction::Rotate2); + emit!(self, Instruction::Swap { index: 2 }); emit!(self, Instruction::StoreAttr { idx }); } } diff --git a/crates/compiler-core/src/bytecode.rs b/crates/compiler-core/src/bytecode.rs index 7e377ea9c2a..fe28517cda8 100644 --- a/crates/compiler-core/src/bytecode.rs +++ b/crates/compiler-core/src/bytecode.rs @@ -614,8 +614,6 @@ pub enum Instruction { index: Arg, }, ToBool, - Rotate2, - Rotate3, GetIter, GetLen, CallIntrinsic1 { @@ -1473,7 +1471,6 @@ impl Instruction { Pop => -1, Swap { .. } => 0, ToBool => 0, - Rotate2 | Rotate3 => 0, GetIter => 0, GetLen => 1, CallIntrinsic1 { .. } => 0, // Takes 1, pushes 1 @@ -1674,8 +1671,6 @@ impl Instruction { Pop => w!(Pop), Swap { index } => w!(Swap, index), ToBool => w!(ToBool), - Rotate2 => w!(Rotate2), - Rotate3 => w!(Rotate3), GetIter => w!(GetIter), // GET_LEN GetLen => w!(GetLen), diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index b42a2b28165..8deaada0827 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -729,11 +729,6 @@ impl ExecutingFrame<'_> { Ok(None) } */ - // splitting the instructions like this offloads the cost of "dynamic" dispatch (on the - // amount to rotate) to the opcode dispatcher, and generates optimized code for the - // concrete cases we actually have - bytecode::Instruction::Rotate2 => self.execute_rotate(2), - bytecode::Instruction::Rotate3 => self.execute_rotate(3), bytecode::Instruction::BuildString { size } => { let s = self .pop_multiple(size.get(arg) as usize) @@ -1674,13 +1669,6 @@ impl ExecutingFrame<'_> { } } - #[inline(always)] - fn execute_rotate(&mut self, amount: usize) -> FrameResult { - let i = self.state.stack.len() - amount; - self.state.stack[i..].rotate_right(1); - Ok(None) - } - fn execute_subscript(&mut self, vm: &VirtualMachine) -> FrameResult { let b_ref = self.pop_value(); let a_ref = self.pop_value(); From 3a04abddd822628adb410dc1d0d274a5cad0b976 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Fri, 28 Nov 2025 16:44:39 +0200 Subject: [PATCH 3/4] Fix jit --- crates/jit/tests/common.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/crates/jit/tests/common.rs b/crates/jit/tests/common.rs index a88d3207f2a..5dafeaeb807 100644 --- a/crates/jit/tests/common.rs +++ b/crates/jit/tests/common.rs @@ -164,18 +164,6 @@ impl StackMachine { self.stack.push(StackValue::Function(func)); } } - Instruction::Duplicate => { - let value = self.stack.last().unwrap().clone(); - self.stack.push(value); - } - Instruction::Rotate2 => { - let i = self.stack.len() - 2; - self.stack[i..].rotate_right(1); - } - Instruction::Rotate3 => { - let i = self.stack.len() - 3; - self.stack[i..].rotate_right(1); - } Instruction::ReturnConst { idx } => { let idx = idx.get(arg); self.stack.push(constants[idx as usize].clone().into()); From bce1c660e5a1ab4d2fb4ecc74cf3bc677697a49a Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Fri, 28 Nov 2025 16:59:25 +0200 Subject: [PATCH 4/4] Update snapshot --- ...ython_codegen__compile__tests__nested_double_async_with.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap index 82c02d5ea34..dc624c7b6a1 100644 --- a/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap +++ b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap @@ -49,7 +49,7 @@ expression: "compile_exec(\"\\\nfor stop_exc in (StopIteration('spam'), StopAsyn 39 WithCleanupFinish 40 PopBlock 41 Jump (59) - >> 42 Duplicate + >> 42 CopyItem (1) 6 43 LoadNameAny (7, Exception) 44 TestOperation (ExceptionMatch)