Skip to content

Commit 8fb5bdb

Browse files
committed
LoadClosure as pseudo op
1 parent 9b5eefb commit 8fb5bdb

File tree

7 files changed

+34
-24
lines changed

7 files changed

+34
-24
lines changed

Lib/_opcode_metadata.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,6 @@
233233
'INSTRUMENTED_POP_JUMP_IF_NONE': 252,
234234
'INSTRUMENTED_POP_JUMP_IF_NOT_NONE': 253,
235235
'INSTRUMENTED_LINE': 254,
236-
'LOAD_CLOSURE': 255,
237236
'JUMP': 256,
238237
'JUMP_NO_INTERRUPT': 257,
239238
'RESERVED_258': 258,
@@ -246,6 +245,7 @@
246245
'SETUP_FINALLY': 265,
247246
'SETUP_WITH': 266,
248247
'STORE_FAST_MAYBE_NULL': 267,
248+
'LOAD_CLOSURE': 268,
249249
}
250250

251251
# CPython 3.13 compatible: opcodes < 44 have no argument

Lib/test/test__opcode.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ def check_bool_function_result(self, func, ops, expected):
1616
self.assertIsInstance(func(op), bool)
1717
self.assertEqual(func(op), expected)
1818

19-
@unittest.expectedFailure # TODO: RUSTPYTHON; Move LoadClosure to psudoes
2019
def test_invalid_opcodes(self):
2120
invalid = [-100, -1, 255, 512, 513, 1000]
2221
self.check_bool_function_result(_opcode.is_valid, invalid, False)

crates/codegen/src/compile.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4074,7 +4074,7 @@ impl Compiler {
40744074
}
40754075
};
40764076

4077-
emit!(self, Instruction::LoadClosure(idx.to_u32()));
4077+
emit!(self, PseudoInstruction::LoadClosure(idx.to_u32()));
40784078
}
40794079

40804080
// Build tuple of closure variables
@@ -4297,7 +4297,7 @@ impl Compiler {
42974297
.position(|var| *var == "__class__");
42984298

42994299
if let Some(classcell_idx) = classcell_idx {
4300-
emit!(self, Instruction::LoadClosure(classcell_idx.to_u32()));
4300+
emit!(self, PseudoInstruction::LoadClosure(classcell_idx.to_u32()));
43014301
emit!(self, Instruction::Copy { index: 1_u32 });
43024302
let classcell = self.name("__classcell__");
43034303
emit!(self, Instruction::StoreName(classcell));

crates/codegen/src/ir.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,13 @@ impl CodeInfo {
263263
info.arg = OpArg(encoded);
264264
info.instr = Instruction::LoadSuperAttr { arg: Arg::marker() }.into();
265265
}
266+
// LOAD_CLOSURE pseudo → LOAD_FAST (with varnames offset)
267+
PseudoInstruction::LoadClosure(idx) => {
268+
let varnames_len = varname_cache.len() as u32;
269+
let new_idx = varnames_len + idx.get(info.arg);
270+
info.arg = OpArg(new_idx);
271+
info.instr = Instruction::LoadFast(Arg::marker()).into();
272+
}
266273
PseudoInstruction::Jump { .. } => {
267274
// PseudoInstruction::Jump instructions are handled later
268275
}

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,6 @@ pub enum Instruction {
383383
InstrumentedPopJumpIfNone = 252, // Placeholder
384384
InstrumentedPopJumpIfNotNone = 253, // Placeholder
385385
InstrumentedLine = 254, // Placeholder
386-
// Pseudos (needs to be moved to `PseudoInstruction` enum.
387-
LoadClosure(Arg<NameIdx>) = 255, // TODO: Move to pseudos
388386
}
389387

390388
const _: () = assert!(mem::size_of::<Instruction>() == 1);
@@ -415,9 +413,6 @@ impl TryFrom<u8> for Instruction {
415413
let instrumented_start = u8::from(Self::InstrumentedResume);
416414
let instrumented_end = u8::from(Self::InstrumentedLine);
417415

418-
// TODO: Remove this; This instruction needs to be pseudo
419-
let load_closure = u8::from(Self::LoadClosure(Arg::marker()));
420-
421416
// RustPython-only opcodes (explicit list to avoid gaps like 125-127)
422417
let custom_ops: &[u8] = &[
423418
u8::from(Self::Break {
@@ -457,7 +452,6 @@ impl TryFrom<u8> for Instruction {
457452

458453
if (cpython_start..=cpython_end).contains(&value)
459454
|| value == resume_id
460-
|| value == load_closure
461455
|| custom_ops.contains(&value)
462456
|| (specialized_start..=specialized_end).contains(&value)
463457
|| (instrumented_start..=instrumented_end).contains(&value)
@@ -704,7 +698,6 @@ impl InstructionMetadata for Instruction {
704698
Self::StoreFastStoreFast { .. } => 0,
705699
Self::PopJumpIfNone { .. } => 0,
706700
Self::PopJumpIfNotNone { .. } => 0,
707-
Self::LoadClosure(_) => 1,
708701
Self::BinaryOpAddFloat => 0,
709702
Self::BinaryOpAddInt => 0,
710703
Self::BinaryOpAddUnicode => 0,
@@ -935,7 +928,6 @@ impl InstructionMetadata for Instruction {
935928
}
936929
Self::LoadBuildClass => w!(LOAD_BUILD_CLASS),
937930
Self::LoadFromDictOrDeref(i) => w!(LOAD_FROM_DICT_OR_DEREF, cell_name = i),
938-
Self::LoadClosure(i) => w!(LOAD_CLOSURE, cell_name = i),
939931
Self::LoadConst { idx } => fmt_const("LOAD_CONST", arg, f, idx),
940932
Self::LoadDeref(idx) => w!(LOAD_DEREF, cell_name = idx),
941933
Self::LoadFast(idx) => w!(LOAD_FAST, varname = idx),
@@ -1037,6 +1029,7 @@ pub enum PseudoInstruction {
10371029
SetupFinally = 265, // Placeholder
10381030
SetupWith = 266, // Placeholder
10391031
StoreFastMaybeNull = 267, // Placeholder
1032+
LoadClosure(Arg<NameIdx>) = 268,
10401033
}
10411034

10421035
const _: () = assert!(mem::size_of::<PseudoInstruction>() == 2);
@@ -1057,7 +1050,7 @@ impl TryFrom<u16> for PseudoInstruction {
10571050
let start = u16::from(Self::Jump {
10581051
target: Arg::marker(),
10591052
});
1060-
let end = u16::from(Self::StoreFastMaybeNull);
1053+
let end = u16::from(Self::LoadClosure(Arg::marker()));
10611054

10621055
if (start..=end).contains(&value) {
10631056
Ok(unsafe { mem::transmute::<u16, Self>(value) })
@@ -1093,6 +1086,7 @@ impl InstructionMetadata for PseudoInstruction {
10931086
Self::SetupWith => 0,
10941087
Self::StoreFastMaybeNull => 0,
10951088
Self::Reserved258 => 0,
1089+
Self::LoadClosure(_) => 1,
10961090
}
10971091
}
10981092

crates/stdlib/src/opcode.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ mod opcode {
125125
Ok(AnyInstruction::Real(
126126
Instruction::DeleteDeref(_)
127127
| Instruction::LoadFromDictOrDeref(_)
128-
| Instruction::LoadClosure(_)
129128
| Instruction::LoadDeref(_)
130129
| Instruction::StoreDeref(_)
131130
))

crates/vm/src/frame.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#[cfg(feature = "flame")]
2+
use crate::bytecode::InstructionMetadata;
13
use crate::{
24
AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
35
builtins::{
@@ -137,10 +139,24 @@ impl Frame {
137139
func_obj: Option<PyObjectRef>,
138140
vm: &VirtualMachine,
139141
) -> Self {
140-
let cells_frees = core::iter::repeat_with(|| PyCell::default().into_ref(&vm.ctx))
141-
.take(code.cellvars.len())
142-
.chain(closure.iter().cloned())
143-
.collect();
142+
let nlocals = code.varnames.len();
143+
let num_cells = code.cellvars.len();
144+
let nfrees = closure.len();
145+
146+
let cells_frees: Box<[PyCellRef]> =
147+
core::iter::repeat_with(|| PyCell::default().into_ref(&vm.ctx))
148+
.take(num_cells)
149+
.chain(closure.iter().cloned())
150+
.collect();
151+
152+
// Extend fastlocals to include varnames + cellvars + freevars (localsplus)
153+
let total_locals = nlocals + num_cells + nfrees;
154+
let mut fastlocals_vec: Vec<Option<PyObjectRef>> = vec![None; total_locals];
155+
156+
// Store cell objects at cellvars and freevars positions
157+
for (i, cell) in cells_frees.iter().enumerate() {
158+
fastlocals_vec[nlocals + i] = Some(cell.clone().into());
159+
}
144160

145161
let state = FrameState {
146162
stack: BoxVec::new(code.max_stackdepth as usize),
@@ -149,7 +165,7 @@ impl Frame {
149165
};
150166

151167
Self {
152-
fastlocals: PyMutex::new(vec![None; code.varnames.len()].into_boxed_slice()),
168+
fastlocals: PyMutex::new(fastlocals_vec.into_boxed_slice()),
153169
cells_frees,
154170
locals: scope.locals,
155171
globals: scope.globals,
@@ -1213,11 +1229,6 @@ impl ExecutingFrame<'_> {
12131229
});
12141230
Ok(None)
12151231
}
1216-
Instruction::LoadClosure(i) => {
1217-
let value = self.cells_frees[i.get(arg) as usize].clone();
1218-
self.push_value(value.into());
1219-
Ok(None)
1220-
}
12211232
Instruction::LoadConst { idx } => {
12221233
self.push_value(self.code.constants[idx.get(arg) as usize].clone().into());
12231234
Ok(None)

0 commit comments

Comments
 (0)