From a7aea59958bf0682d6ac0a04ef4d422e443e9b6c Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sun, 3 Dec 2023 23:53:21 +0100 Subject: [PATCH] Refactor vm calling convention to allow locals --- core/engine/src/builtins/function/mod.rs | 20 +++--- core/engine/src/builtins/generator/mod.rs | 30 +++----- core/engine/src/bytecompiler/mod.rs | 3 +- core/engine/src/module/synthetic.rs | 2 +- core/engine/src/vm/call_frame/mod.rs | 80 +++++++++++++++++----- core/engine/src/vm/mod.rs | 21 +++--- core/engine/src/vm/opcode/arguments.rs | 8 +-- core/engine/src/vm/opcode/generator/mod.rs | 38 ++-------- core/engine/src/vm/opcode/get/argument.rs | 10 ++- 9 files changed, 106 insertions(+), 106 deletions(-) diff --git a/core/engine/src/builtins/function/mod.rs b/core/engine/src/builtins/function/mod.rs index d8a85a016fc..cc466f1bfc0 100644 --- a/core/engine/src/builtins/function/mod.rs +++ b/core/engine/src/builtins/function/mod.rs @@ -938,10 +938,7 @@ pub(crate) fn function_call( context.vm.push_frame(frame); - let fp = context.vm.stack.len() - argument_count - CallFrame::FUNCTION_PROLOGUE; - context.vm.frame_mut().fp = fp as u32; - - let this = context.vm.stack[fp + CallFrame::THIS_POSITION].clone(); + let this = context.vm.frame().this(&context.vm); let lexical_this_mode = code.this_mode == ThisMode::Lexical; @@ -1013,8 +1010,6 @@ fn function_construct( let new_target = context.vm.pop(); - let at = context.vm.stack.len() - argument_count; - let this = if code.is_derived_constructor() { None } else { @@ -1042,7 +1037,10 @@ fn function_construct( context.vm.push_frame(frame); - context.vm.frame_mut().fp = at as u32 - 1; + let len = context.vm.stack.len(); + + // NOTE(HalidOdat): +1 because we insert `this` value below. + context.vm.frame_mut().fp = len as u32 + 1; let mut last_env = 0; @@ -1075,10 +1073,10 @@ fn function_construct( ); // Insert `this` value - context - .vm - .stack - .insert(at - 1, this.map(JsValue::new).unwrap_or_default()); + context.vm.stack.insert( + len - argument_count - 1, + this.map(JsValue::new).unwrap_or_default(), + ); Ok(CallValue::Ready) } diff --git a/core/engine/src/builtins/generator/mod.rs b/core/engine/src/builtins/generator/mod.rs index 75fb2705565..20175946b43 100644 --- a/core/engine/src/builtins/generator/mod.rs +++ b/core/engine/src/builtins/generator/mod.rs @@ -64,28 +64,20 @@ pub(crate) struct GeneratorContext { } impl GeneratorContext { - /// Creates a new `GeneratorContext` from the raw `Context` state components. - pub(crate) fn new(stack: Vec, call_frame: CallFrame) -> Self { - Self { - stack, - call_frame: Some(call_frame), - } - } - /// Creates a new `GeneratorContext` from the current `Context` state. pub(crate) fn from_current(context: &mut Context) -> Self { let mut frame = context.vm.frame().clone(); frame.environments = context.vm.environments.clone(); frame.realm = context.realm().clone(); - let fp = context.vm.frame().fp as usize; - let this = Self { - call_frame: Some(frame), - stack: context.vm.stack[fp..].to_vec(), - }; + let fp = frame.restore_fp() as usize; + let stack = context.vm.stack.split_off(fp); - context.vm.stack.truncate(fp); + frame.fp = CallFrame::FUNCTION_PROLOGUE + frame.argument_count; - this + Self { + call_frame: Some(frame), + stack, + } } /// Resumes execution with `GeneratorContext` as the current execution context. @@ -96,11 +88,11 @@ impl GeneratorContext { context: &mut Context, ) -> CompletionRecord { std::mem::swap(&mut context.vm.stack, &mut self.stack); - context - .vm - .push_frame(self.call_frame.take().expect("should have a call frame")); + let frame = self.call_frame.take().expect("should have a call frame"); + let fp = frame.fp; + context.vm.push_frame(frame); - context.vm.frame_mut().fp = 0; + context.vm.frame_mut().fp = fp; context.vm.frame_mut().set_exit_early(true); if let Some(value) = value { diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index dc13a911d36..889d2169d1a 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -327,8 +327,7 @@ impl<'ctx> ByteCompiler<'ctx> { params: FormalParameterList::default(), current_open_environments_count: 0, - // This starts at two because the first value is the `this` value, then function object. - current_stack_value_count: 2, + current_stack_value_count: 0, code_block_flags, handlers: ThinVec::default(), ic: Vec::default(), diff --git a/core/engine/src/module/synthetic.rs b/core/engine/src/module/synthetic.rs index 0f7f03f51c3..a7e47a9e4b4 100644 --- a/core/engine/src/module/synthetic.rs +++ b/core/engine/src/module/synthetic.rs @@ -324,7 +324,7 @@ impl SyntheticModule { // 11. Suspend moduleContext and remove it from the execution context stack. // 12. Resume the context that is now on the top of the execution context stack as the running execution context. let frame = context.vm.pop_frame().expect("there should be a frame"); - context.vm.stack.truncate(frame.fp as usize); + frame.restore_stack(&mut context.vm); // 13. Let pc be ! NewPromiseCapability(%Promise%). let (promise, ResolvingFunctions { resolve, reject }) = JsPromise::new_pending(context); diff --git a/core/engine/src/vm/call_frame/mod.rs b/core/engine/src/vm/call_frame/mod.rs index 1503e6f611f..92c923a963c 100644 --- a/core/engine/src/vm/call_frame/mod.rs +++ b/core/engine/src/vm/call_frame/mod.rs @@ -81,22 +81,48 @@ impl CallFrame { impl CallFrame { /// This is the size of the function prologue. /// - /// The position of the elements are relative to the [`CallFrame::fp`]. + /// The position of the elements are relative to the [`CallFrame::fp`] (frame pointer). /// /// ```text - /// --- frame pointer arguments - /// / __________________________/ - /// / / \ - /// | 0: this | 1: func | 2: arg1 | ... | (2 + N): argN | - /// \ / - /// ------------ - /// | - /// function prolugue + /// Setup by the caller + /// ┌─────────────────────────────────────────────────────────┐ ┌───── frame pointer + /// ▼ ▼ ▼ + /// | -(2 + N): this | -(1 + N): func | -N: arg1 | ... | -1: argN | 0: local1 | ... | K: localK | + /// ▲ ▲ ▲ ▲ ▲ ▲ + /// └──────────────────────────────┘ └──────────────────────┘ └─────────────────────────┘ + /// function prolugue arguments Setup by the callee /// ``` - pub(crate) const FUNCTION_PROLOGUE: usize = 2; - pub(crate) const THIS_POSITION: usize = 0; - pub(crate) const FUNCTION_POSITION: usize = 1; - pub(crate) const FIRST_ARGUMENT_POSITION: usize = 2; + /// + /// ### Example + /// + /// The following function calls, generate the following stack: + /// + /// ```JavaScript + /// function x(a) { + /// } + /// function y(b, c) { + /// return x(b + c) + /// } + /// + /// y(1, 2) + /// ``` + /// + /// ```text + /// caller prolugue caller arguments callee prolugue callee arguments + /// ______|____________ _____|_____ _________|_________ __|_ + /// / \ / \ / \ / \ + /// | 0: undefined | 1: y | 2: 1 | 3: 2 | 4: undefined | 5: x | 6: 3 | + /// / ^ + /// caller frame pointer --+ callee frame pointer + /// ``` + /// + /// Some questions: + /// + /// - Who is responsible for cleaning up the stack after a call? The caller. + /// + pub(crate) const FUNCTION_PROLOGUE: u32 = 2; + pub(crate) const THIS_POSITION: u32 = 2; + pub(crate) const FUNCTION_POSITION: u32 = 1; /// Creates a new `CallFrame` with the provided `CodeBlock`. pub(crate) fn new( @@ -142,19 +168,39 @@ impl CallFrame { } pub(crate) fn this(&self, vm: &Vm) -> JsValue { - let this_index = self.fp as usize + Self::THIS_POSITION; - vm.stack[this_index].clone() + let this_index = self.fp - self.argument_count - Self::THIS_POSITION; + vm.stack[this_index as usize].clone() } pub(crate) fn function(&self, vm: &Vm) -> Option { - let function_index = self.fp as usize + Self::FUNCTION_POSITION; - if let Some(object) = vm.stack[function_index].as_object() { + let function_index = self.fp - self.argument_count - Self::FUNCTION_POSITION; + if let Some(object) = vm.stack[function_index as usize].as_object() { return Some(object.clone()); } None } + pub(crate) fn arguments<'stack>(&self, vm: &'stack Vm) -> &'stack [JsValue] { + let fp = self.fp as usize; + let argument_count = self.argument_count as usize; + let arguments_start = fp - argument_count; + &vm.stack[arguments_start..fp] + } + + pub(crate) fn argument<'stack>(&self, index: usize, vm: &'stack Vm) -> Option<&'stack JsValue> { + self.arguments(vm).get(index) + } + + pub(crate) fn restore_fp(&self) -> u32 { + self.fp - self.argument_count - Self::FUNCTION_PROLOGUE + } + + pub(crate) fn restore_stack(&self, vm: &mut Vm) { + let fp = self.restore_fp(); + vm.stack.truncate(fp as usize); + } + /// Does this have the [`CallFrameFlags::EXIT_EARLY`] flag. pub(crate) fn exit_early(&self) -> bool { self.flags.contains(CallFrameFlags::EXIT_EARLY) diff --git a/core/engine/src/vm/mod.rs b/core/engine/src/vm/mod.rs index 9c54f049051..23767785b6d 100644 --- a/core/engine/src/vm/mod.rs +++ b/core/engine/src/vm/mod.rs @@ -167,20 +167,14 @@ impl Vm { pub(crate) fn push_frame_with_stack( &mut self, - mut frame: CallFrame, + frame: CallFrame, this: JsValue, function: JsValue, ) { - let current_stack_length = self.stack.len(); - frame.set_frame_pointer(current_stack_length as u32); - self.push(this); self.push(function); - std::mem::swap(&mut self.environments, &mut frame.environments); - std::mem::swap(&mut self.realm, &mut frame.realm); - - self.frames.push(frame); + self.push_frame(frame); } pub(crate) fn pop_frame(&mut self) -> Option { @@ -422,7 +416,7 @@ impl Context { break; } - fp = frame.fp as usize; + fp = frame.restore_fp() as usize; env_fp = frame.env_fp as usize; self.vm.pop_frame(); } @@ -449,7 +443,8 @@ impl Context { match result { CompletionType::Normal => {} CompletionType::Return => { - self.vm.stack.truncate(self.vm.frame().fp as usize); + let restore_fp = self.vm.frame().restore_fp() as usize; + self.vm.stack.truncate(restore_fp); let result = self.vm.take_return_value(); if self.vm.frame().exit_early() { @@ -460,7 +455,7 @@ impl Context { self.vm.pop_frame(); } CompletionType::Throw => { - let mut fp = self.vm.frame().fp; + let mut fp = self.vm.frame().restore_fp(); let mut env_fp = self.vm.frame().env_fp; if self.vm.frame().exit_early() { self.vm.environments.truncate(env_fp as usize); @@ -476,8 +471,8 @@ impl Context { self.vm.pop_frame(); while let Some(frame) = self.vm.frames.last_mut() { - fp = frame.fp; - env_fp = frame.fp; + fp = frame.restore_fp(); + env_fp = frame.env_fp; let pc = frame.pc; let exit_early = frame.exit_early(); diff --git a/core/engine/src/vm/opcode/arguments.rs b/core/engine/src/vm/opcode/arguments.rs index 2af9c57318f..7daf372ee31 100644 --- a/core/engine/src/vm/opcode/arguments.rs +++ b/core/engine/src/vm/opcode/arguments.rs @@ -1,6 +1,6 @@ use crate::{ builtins::function::arguments::{MappedArguments, UnmappedArguments}, - vm::{CallFrame, CompletionType}, + vm::CompletionType, Context, JsResult, }; @@ -19,7 +19,6 @@ impl Operation for CreateMappedArgumentsObject { const COST: u8 = 8; fn execute(context: &mut Context) -> JsResult { - let arguments_start = context.vm.frame().fp as usize + CallFrame::FIRST_ARGUMENT_POSITION; let function_object = context .vm .frame() @@ -27,7 +26,7 @@ impl Operation for CreateMappedArgumentsObject { .clone() .expect("there should be a function object"); let code = context.vm.frame().code_block().clone(); - let args = context.vm.stack[arguments_start..].to_vec(); + let args = context.vm.frame().arguments(&context.vm).to_vec(); let env = context.vm.environments.current(); let arguments = MappedArguments::new( @@ -55,8 +54,7 @@ impl Operation for CreateUnmappedArgumentsObject { const COST: u8 = 4; fn execute(context: &mut Context) -> JsResult { - let arguments_start = context.vm.frame().fp as usize + CallFrame::FIRST_ARGUMENT_POSITION; - let args = context.vm.stack[arguments_start..].to_vec(); + let args = context.vm.frame().arguments(&context.vm).to_vec(); let arguments = UnmappedArguments::new(&args, context); context.vm.push(arguments); Ok(CompletionType::Normal) diff --git a/core/engine/src/vm/opcode/generator/mod.rs b/core/engine/src/vm/opcode/generator/mod.rs index 3e660cd72b7..5d0150efdc3 100644 --- a/core/engine/src/vm/opcode/generator/mod.rs +++ b/core/engine/src/vm/opcode/generator/mod.rs @@ -13,7 +13,7 @@ use crate::{ vm::{ call_frame::GeneratorResumeKind, opcode::{Operation, ReThrow}, - CallFrame, CompletionType, + CompletionType, }, Context, JsError, JsObject, JsResult, JsValue, }; @@ -37,36 +37,12 @@ impl Operation for Generator { fn execute(context: &mut Context) -> JsResult { let r#async = context.vm.read::() != 0; - let frame = context.vm.frame(); - let code_block = frame.code_block().clone(); - let active_runnable = frame.active_runnable.clone(); - let active_function = frame.function(&context.vm); - let environments = frame.environments.clone(); - let realm = frame.realm.clone(); - let pc = frame.pc; - - let mut dummy_call_frame = CallFrame::new(code_block, active_runnable, environments, realm); - dummy_call_frame.pc = pc; - let mut call_frame = std::mem::replace(context.vm.frame_mut(), dummy_call_frame); - - context - .vm - .frame_mut() - .set_exit_early(call_frame.exit_early()); - - call_frame.environments = context.vm.environments.clone(); - call_frame.realm = context.realm().clone(); - - let fp = call_frame.fp as usize; - - let stack = context.vm.stack[fp..].to_vec(); - context.vm.stack.truncate(fp); - - call_frame.fp = 0; - + let active_function = context.vm.frame().function(&context.vm); let this_function_object = active_function.expect("active function should be set to the generator"); + let frame = GeneratorContext::from_current(context); + let proto = this_function_object .get(PROTOTYPE, context) .expect("generator must have a prototype property") @@ -88,7 +64,7 @@ impl Operation for Generator { proto, AsyncGenerator { state: AsyncGeneratorState::SuspendedStart, - context: Some(GeneratorContext::new(stack, call_frame)), + context: Some(frame), queue: VecDeque::new(), }, ) @@ -97,9 +73,7 @@ impl Operation for Generator { context.root_shape(), proto, crate::builtins::generator::Generator { - state: GeneratorState::SuspendedStart { - context: GeneratorContext::new(stack, call_frame), - }, + state: GeneratorState::SuspendedStart { context: frame }, }, ) }; diff --git a/core/engine/src/vm/opcode/get/argument.rs b/core/engine/src/vm/opcode/get/argument.rs index 77ab9324585..dcfd8cb6670 100644 --- a/core/engine/src/vm/opcode/get/argument.rs +++ b/core/engine/src/vm/opcode/get/argument.rs @@ -13,12 +13,10 @@ pub(crate) struct GetArgument; impl GetArgument { #[allow(clippy::unnecessary_wraps)] fn operation(context: &mut Context, index: usize) -> JsResult { - let fp = context.vm.frame().fp as usize; - let argument_index = fp + 2; - let argument_count = context.vm.frame().argument_count as usize; - - let value = context.vm.stack[argument_index..(argument_index + argument_count)] - .get(index) + let value = context + .vm + .frame() + .argument(index, &context.vm) .cloned() .unwrap_or_default(); context.vm.push(value);