From 70c87f714950db24588edb3e632c10ee4cfae17c 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 1/5] 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); From e41cc9982b6558a18bea03b79a9b330ce4626b66 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sun, 10 Dec 2023 17:07:07 +0100 Subject: [PATCH 2/5] Move async generator object to the stack --- core/engine/src/builtins/generator/mod.rs | 13 ++++++- core/engine/src/bytecompiler/mod.rs | 11 ++++++ core/engine/src/vm/call_frame/mod.rs | 35 +++++++++++++++--- core/engine/src/vm/code_block.rs | 10 +++++ core/engine/src/vm/mod.rs | 11 ++++++ core/engine/src/vm/opcode/await/mod.rs | 18 ++++----- core/engine/src/vm/opcode/generator/mod.rs | 37 +++++++++++-------- .../src/vm/opcode/generator/yield_stm.rs | 9 ++--- 8 files changed, 107 insertions(+), 37 deletions(-) diff --git a/core/engine/src/builtins/generator/mod.rs b/core/engine/src/builtins/generator/mod.rs index 20175946b43..a06515cc3b3 100644 --- a/core/engine/src/builtins/generator/mod.rs +++ b/core/engine/src/builtins/generator/mod.rs @@ -20,7 +20,7 @@ use crate::{ string::common::StaticJsStrings, symbol::JsSymbol, value::JsValue, - vm::{CallFrame, CompletionRecord, GeneratorResumeKind}, + vm::{CallFrame, CallFrameFlags, CompletionRecord, GeneratorResumeKind}, Context, JsArgs, JsData, JsError, JsResult, JsString, }; use boa_gc::{custom_trace, Finalize, Trace}; @@ -74,6 +74,10 @@ impl GeneratorContext { frame.fp = CallFrame::FUNCTION_PROLOGUE + frame.argument_count; + // NOTE: Since we get a pre-built call frame with stack, and we reuse them. + // So we don't need to push the locals in subsuquent calls. + frame.flags |= CallFrameFlags::LOCALS_ALREADY_PUSHED; + Self { call_frame: Some(frame), stack, @@ -107,6 +111,13 @@ impl GeneratorContext { assert!(self.call_frame.is_some()); result } + + /// Returns the async generator object, if the function that this [`GeneratorContext`] is from an async generator, [`None`] otherwise. + pub(crate) fn async_generator_object(&self) -> Option { + self.call_frame + .as_ref() + .and_then(|frame| frame.async_generator_object(&self.stack)) + } } /// The internal representation of a `Generator` object. diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index 889d2169d1a..6b5af7b381d 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -255,6 +255,8 @@ pub struct ByteCompiler<'ctx> { /// The number of arguments expected. pub(crate) length: u32, + pub(crate) locals_count: u32, + /// \[\[ThisMode\]\] pub(crate) this_mode: ThisMode, @@ -327,6 +329,7 @@ impl<'ctx> ByteCompiler<'ctx> { params: FormalParameterList::default(), current_open_environments_count: 0, + locals_count: 0, current_stack_value_count: 0, code_block_flags, handlers: ThinVec::default(), @@ -1520,9 +1523,17 @@ impl<'ctx> ByteCompiler<'ctx> { } self.r#return(false); + if self.is_async_generator() { + self.locals_count += 1; + } + for handler in &mut self.handlers { + handler.stack_count += self.locals_count; + } + CodeBlock { name: self.function_name, length: self.length, + locals_count: self.locals_count, this_mode: self.this_mode, params: self.params, bytecode: self.bytecode.into_boxed_slice(), diff --git a/core/engine/src/vm/call_frame/mod.rs b/core/engine/src/vm/call_frame/mod.rs index 92c923a963c..40c66087551 100644 --- a/core/engine/src/vm/call_frame/mod.rs +++ b/core/engine/src/vm/call_frame/mod.rs @@ -25,6 +25,9 @@ bitflags::bitflags! { /// Was this [`CallFrame`] created from the `__construct__()` internal object method? const CONSTRUCT = 0b0000_0010; + + /// Does this [`CallFrame`] need to push local variables on [`Vm::push_frame()`]. + const LOCALS_ALREADY_PUSHED = 0b0000_0100; } } @@ -40,10 +43,6 @@ pub struct CallFrame { pub(crate) argument_count: u32, pub(crate) promise_capability: Option, - // When an async generator is resumed, the generator object is needed - // to fulfill the steps 4.e-j in [AsyncGeneratorStart](https://tc39.es/ecma262/#sec-asyncgeneratorstart). - pub(crate) async_generator: Option, - // Iterators and their `[[Done]]` flags that must be closed when an abrupt completion is thrown. pub(crate) iterators: ThinVec, @@ -123,6 +122,7 @@ impl CallFrame { pub(crate) const FUNCTION_PROLOGUE: u32 = 2; pub(crate) const THIS_POSITION: u32 = 2; pub(crate) const FUNCTION_POSITION: u32 = 1; + pub(crate) const ASYNC_GENERATOR_OBJECT_LOCAL_INDEX: u32 = 0; /// Creates a new `CallFrame` with the provided `CodeBlock`. pub(crate) fn new( @@ -138,7 +138,6 @@ impl CallFrame { env_fp: 0, argument_count: 0, promise_capability: None, - async_generator: None, iterators: ThinVec::new(), binding_stack: Vec::new(), loop_iteration_count: 0, @@ -201,6 +200,28 @@ impl CallFrame { vm.stack.truncate(fp as usize); } + /// Returns the async generator object, if the function that this [`CallFrame`] is from an async generator, [`None`] otherwise. + pub(crate) fn async_generator_object(&self, stack: &[JsValue]) -> Option { + if !self.code_block().is_async_generator() { + return None; + } + + self.local(Self::ASYNC_GENERATOR_OBJECT_LOCAL_INDEX, stack) + .as_object() + .cloned() + } + + /// Returns the local at the given index. + /// + /// # Panics + /// + /// If the index is out of bounds. + pub(crate) fn local<'stack>(&self, index: u32, stack: &'stack [JsValue]) -> &'stack JsValue { + debug_assert!(index < self.code_block().locals_count); + let at = self.fp + index; + &stack[at as usize] + } + /// Does this have the [`CallFrameFlags::EXIT_EARLY`] flag. pub(crate) fn exit_early(&self) -> bool { self.flags.contains(CallFrameFlags::EXIT_EARLY) @@ -213,6 +234,10 @@ impl CallFrame { pub(crate) fn construct(&self) -> bool { self.flags.contains(CallFrameFlags::CONSTRUCT) } + /// Does this [`CallFrame`] need to push local variables on [`Vm::push_frame()`]. + pub(crate) fn locals_already_pushed(&self) -> bool { + self.flags.contains(CallFrameFlags::LOCALS_ALREADY_PUSHED) + } } /// ---- `CallFrame` stack methods ---- diff --git a/core/engine/src/vm/code_block.rs b/core/engine/src/vm/code_block.rs index 1ceec0f8ae7..fda1250d50d 100644 --- a/core/engine/src/vm/code_block.rs +++ b/core/engine/src/vm/code_block.rs @@ -177,6 +177,8 @@ pub struct CodeBlock { /// The number of arguments expected. pub(crate) length: u32, + pub(crate) locals_count: u32, + /// \[\[ThisMode\]\] pub(crate) this_mode: ThisMode, @@ -216,6 +218,7 @@ impl CodeBlock { name, flags: Cell::new(flags), length, + locals_count: 0, this_mode: ThisMode::Global, params: FormalParameterList::default(), handlers: ThinVec::default(), @@ -286,6 +289,13 @@ impl CodeBlock { self.flags.get().contains(CodeBlockFlags::IS_GENERATOR) } + /// Returns true if this function a async generator function. + pub(crate) fn is_async_generator(&self) -> bool { + self.flags + .get() + .contains(CodeBlockFlags::IS_ASYNC | CodeBlockFlags::IS_GENERATOR) + } + /// Returns true if this function an async function. pub(crate) fn is_ordinary(&self) -> bool { !self.is_async() && !self.is_generator() diff --git a/core/engine/src/vm/mod.rs b/core/engine/src/vm/mod.rs index 23767785b6d..94c12a8a0b3 100644 --- a/core/engine/src/vm/mod.rs +++ b/core/engine/src/vm/mod.rs @@ -162,6 +162,17 @@ impl Vm { std::mem::swap(&mut self.environments, &mut frame.environments); std::mem::swap(&mut self.realm, &mut frame.realm); + // NOTE: We need to check if we already pushed the locals, + // since generator-like functions push the same call + // frame with pre-built stack. + if !frame.locals_already_pushed() { + let locals_count = frame.code_block().locals_count; + self.stack.resize_with( + current_stack_length + locals_count as usize, + JsValue::undefined, + ); + } + self.frames.push(frame); } diff --git a/core/engine/src/vm/opcode/await/mod.rs b/core/engine/src/vm/opcode/await/mod.rs index cbad140abd3..7d02bddccb4 100644 --- a/core/engine/src/vm/opcode/await/mod.rs +++ b/core/engine/src/vm/opcode/await/mod.rs @@ -49,17 +49,16 @@ impl Operation for Await { // d. Resume the suspended evaluation of asyncContext using NormalCompletion(value) as the result of the operation that suspended it. let mut gen = captures.borrow_mut().take().expect("should only run once"); + // NOTE: We need to get the object before resuming, since it could clear the stack. + let async_generator = gen.async_generator_object(); + gen.resume( Some(args.get_or_undefined(0).clone()), GeneratorResumeKind::Normal, context, ); - if let Some(async_generator) = gen - .call_frame - .as_ref() - .and_then(|f| f.async_generator.clone()) - { + if let Some(async_generator) = async_generator { async_generator .downcast_mut::() .expect("must be async generator") @@ -92,17 +91,16 @@ impl Operation for Await { let mut gen = captures.borrow_mut().take().expect("should only run once"); + // NOTE: We need to get the object before resuming, since it could clear the stack. + let async_generator = gen.async_generator_object(); + gen.resume( Some(args.get_or_undefined(0).clone()), GeneratorResumeKind::Throw, context, ); - if let Some(async_generator) = gen - .call_frame - .as_ref() - .and_then(|f| f.async_generator.clone()) - { + if let Some(async_generator) = async_generator { async_generator .downcast_mut::() .expect("must be async generator") diff --git a/core/engine/src/vm/opcode/generator/mod.rs b/core/engine/src/vm/opcode/generator/mod.rs index 5d0150efdc3..8dbe8a525ba 100644 --- a/core/engine/src/vm/opcode/generator/mod.rs +++ b/core/engine/src/vm/opcode/generator/mod.rs @@ -41,7 +41,7 @@ impl Operation for Generator { let this_function_object = active_function.expect("active function should be set to the generator"); - let frame = GeneratorContext::from_current(context); + let mut frame = GeneratorContext::from_current(context); let proto = this_function_object .get(PROTOTYPE, context) @@ -64,7 +64,7 @@ impl Operation for Generator { proto, AsyncGenerator { state: AsyncGeneratorState::SuspendedStart, - context: Some(frame), + context: None, queue: VecDeque::new(), }, ) @@ -73,23 +73,29 @@ impl Operation for Generator { context.root_shape(), proto, crate::builtins::generator::Generator { - state: GeneratorState::SuspendedStart { context: frame }, + state: GeneratorState::Completed, }, ) }; if r#async { - let gen_clone = generator.clone(); + let fp = frame + .call_frame + .as_ref() + .map_or(0, |frame| frame.fp as usize); + frame.stack[fp] = generator.clone().into(); + let mut gen = generator .downcast_mut::() .expect("must be object here"); - let gen_context = gen.context.as_mut().expect("must exist"); - // TODO: try to move this to the context itself. - gen_context - .call_frame - .as_mut() - .expect("should have a call frame initialized") - .async_generator = Some(gen_clone); + + gen.context = Some(frame); + } else { + let mut gen = generator + .downcast_mut::() + .expect("must be object here"); + + gen.state = GeneratorState::SuspendedStart { context: frame }; } context.vm.set_return_value(generator.into()); @@ -111,14 +117,13 @@ impl Operation for AsyncGeneratorClose { fn execute(context: &mut Context) -> JsResult { // Step 3.e-g in [AsyncGeneratorStart](https://tc39.es/ecma262/#sec-asyncgeneratorstart) - let generator_object = context + let async_generator_object = context .vm .frame() - .async_generator - .clone() + .async_generator_object(&context.vm.stack) .expect("There should be a object"); - let mut gen = generator_object + let mut gen = async_generator_object .downcast_mut::() .expect("must be async generator"); @@ -136,7 +141,7 @@ impl Operation for AsyncGeneratorClose { } else { AsyncGenerator::complete_step(&next, Ok(return_value), true, None, context); } - AsyncGenerator::drain_queue(&generator_object, context); + AsyncGenerator::drain_queue(&async_generator_object, context); Ok(CompletionType::Normal) } diff --git a/core/engine/src/vm/opcode/generator/yield_stm.rs b/core/engine/src/vm/opcode/generator/yield_stm.rs index 35dbfc923ed..1cc692061a6 100644 --- a/core/engine/src/vm/opcode/generator/yield_stm.rs +++ b/core/engine/src/vm/opcode/generator/yield_stm.rs @@ -38,14 +38,13 @@ impl Operation for AsyncGeneratorYield { fn execute(context: &mut Context) -> JsResult { let value = context.vm.pop(); - let async_gen = context + let async_generator_object = context .vm .frame() - .async_generator - .clone() + .async_generator_object(&context.vm.stack) .expect("`AsyncGeneratorYield` must only be called inside async generators"); let completion = Ok(value); - let next = async_gen + let next = async_generator_object .downcast_mut::() .expect("must be async generator object") .queue @@ -55,7 +54,7 @@ impl Operation for AsyncGeneratorYield { // TODO: 7. Let previousContext be the second to top element of the execution context stack. AsyncGenerator::complete_step(&next, completion, false, None, context); - let mut generator_object_mut = async_gen.borrow_mut(); + let mut generator_object_mut = async_generator_object.borrow_mut(); let gen = generator_object_mut .downcast_mut::() .expect("must be async generator object"); From 887a3f454a4f2b511d82b190abd9a5b014faff68 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Wed, 20 Dec 2023 12:19:02 +0100 Subject: [PATCH 3/5] Apply review and some clean-up --- core/engine/src/builtins/function/mod.rs | 6 +- core/engine/src/builtins/generator/mod.rs | 10 ++-- core/engine/src/vm/call_frame/mod.rs | 58 ++++++++++--------- core/engine/src/vm/mod.rs | 21 ++++--- core/engine/src/vm/opcode/generator/mod.rs | 2 +- .../src/vm/opcode/rest_parameter/mod.rs | 6 +- 6 files changed, 57 insertions(+), 46 deletions(-) diff --git a/core/engine/src/builtins/function/mod.rs b/core/engine/src/builtins/function/mod.rs index cc466f1bfc0..45b3358c1af 100644 --- a/core/engine/src/builtins/function/mod.rs +++ b/core/engine/src/builtins/function/mod.rs @@ -1035,12 +1035,12 @@ fn function_construct( .with_env_fp(env_fp) .with_flags(CallFrameFlags::CONSTRUCT); - context.vm.push_frame(frame); - let len = context.vm.stack.len(); + context.vm.push_frame(frame); + // NOTE(HalidOdat): +1 because we insert `this` value below. - context.vm.frame_mut().fp = len as u32 + 1; + context.vm.frame_mut().rp = len as u32 + 1; let mut last_env = 0; diff --git a/core/engine/src/builtins/generator/mod.rs b/core/engine/src/builtins/generator/mod.rs index a06515cc3b3..c9760405df5 100644 --- a/core/engine/src/builtins/generator/mod.rs +++ b/core/engine/src/builtins/generator/mod.rs @@ -69,13 +69,13 @@ impl GeneratorContext { let mut frame = context.vm.frame().clone(); frame.environments = context.vm.environments.clone(); frame.realm = context.realm().clone(); - let fp = frame.restore_fp() as usize; + let fp = frame.fp() as usize; let stack = context.vm.stack.split_off(fp); - frame.fp = CallFrame::FUNCTION_PROLOGUE + frame.argument_count; + frame.rp = CallFrame::FUNCTION_PROLOGUE + frame.argument_count; // NOTE: Since we get a pre-built call frame with stack, and we reuse them. - // So we don't need to push the locals in subsuquent calls. + // So we don't need to push the locals in subsequent calls. frame.flags |= CallFrameFlags::LOCALS_ALREADY_PUSHED; Self { @@ -93,10 +93,10 @@ impl GeneratorContext { ) -> CompletionRecord { std::mem::swap(&mut context.vm.stack, &mut self.stack); let frame = self.call_frame.take().expect("should have a call frame"); - let fp = frame.fp; + let rp = frame.rp; context.vm.push_frame(frame); - context.vm.frame_mut().fp = fp; + context.vm.frame_mut().rp = rp; context.vm.frame_mut().set_exit_early(true); if let Some(value) = value { diff --git a/core/engine/src/vm/call_frame/mod.rs b/core/engine/src/vm/call_frame/mod.rs index 40c66087551..856a339f9fc 100644 --- a/core/engine/src/vm/call_frame/mod.rs +++ b/core/engine/src/vm/call_frame/mod.rs @@ -36,11 +36,10 @@ bitflags::bitflags! { pub struct CallFrame { pub(crate) code_block: Gc, pub(crate) pc: u32, - pub(crate) fp: u32, - pub(crate) env_fp: u32, - // Tracks the number of environments in environment entry. - // On abrupt returns this is used to decide how many environments need to be pop'ed. + /// The register pointer, points to the first register in the stack. + pub(crate) rp: u32, pub(crate) argument_count: u32, + pub(crate) env_fp: u32, pub(crate) promise_capability: Option, // Iterators and their `[[Done]]` flags that must be closed when an abrupt completion is thrown. @@ -80,16 +79,18 @@ impl CallFrame { impl CallFrame { /// This is the size of the function prologue. /// - /// The position of the elements are relative to the [`CallFrame::fp`] (frame pointer). + /// The position of the elements are relative to the [`CallFrame::fp`] (register pointer). /// /// ```text /// Setup by the caller - /// ┌─────────────────────────────────────────────────────────┐ ┌───── frame pointer + /// ┌─────────────────────────────────────────────────────────┐ ┌───── register pointer /// ▼ ▼ ▼ /// | -(2 + N): this | -(1 + N): func | -N: arg1 | ... | -1: argN | 0: local1 | ... | K: localK | /// ▲ ▲ ▲ ▲ ▲ ▲ /// └──────────────────────────────┘ └──────────────────────┘ └─────────────────────────┘ - /// function prolugue arguments Setup by the callee + /// function prologue arguments Setup by the callee + /// ▲ + /// └─ Frame pointer /// ``` /// /// ### Example @@ -107,17 +108,22 @@ impl CallFrame { /// ``` /// /// ```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 + /// caller prologue caller arguments callee prologue callee arguments + /// ┌─────────────────┐ ┌─────────┐ ┌─────────────────┐ ┌──────┐ + /// ▼ ▼ ▼ ▼ │ ▼ ▼ ▼ + /// | 0: undefined | 1: y | 2: 1 | 3: 2 | 4: undefined | 5: x | 6: 3 | + /// ▲ ▲ ▲ + /// │ caller register pointer ────┤ │ + /// │ │ callee register pointer + /// │ callee frame pointer + /// │ + /// └───── caller frame pointer + /// /// ``` /// /// Some questions: /// - /// - Who is responsible for cleaning up the stack after a call? The caller. + /// - Who is responsible for cleaning up the stack after a call? The rust caller. /// pub(crate) const FUNCTION_PROLOGUE: u32 = 2; pub(crate) const THIS_POSITION: u32 = 2; @@ -134,7 +140,7 @@ impl CallFrame { Self { code_block, pc: 0, - fp: 0, + rp: 0, env_fp: 0, argument_count: 0, promise_capability: None, @@ -167,12 +173,12 @@ impl CallFrame { } pub(crate) fn this(&self, vm: &Vm) -> JsValue { - let this_index = self.fp - self.argument_count - Self::THIS_POSITION; + let this_index = self.rp - 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 - self.argument_count - Self::FUNCTION_POSITION; + let function_index = self.rp - self.argument_count - Self::FUNCTION_POSITION; if let Some(object) = vm.stack[function_index as usize].as_object() { return Some(object.clone()); } @@ -181,22 +187,22 @@ impl CallFrame { } pub(crate) fn arguments<'stack>(&self, vm: &'stack Vm) -> &'stack [JsValue] { - let fp = self.fp as usize; + let rp = self.rp as usize; let argument_count = self.argument_count as usize; - let arguments_start = fp - argument_count; - &vm.stack[arguments_start..fp] + let arguments_start = rp - argument_count; + &vm.stack[arguments_start..rp] } 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 fp(&self) -> u32 { + self.rp - self.argument_count - Self::FUNCTION_PROLOGUE } pub(crate) fn restore_stack(&self, vm: &mut Vm) { - let fp = self.restore_fp(); + let fp = self.fp(); vm.stack.truncate(fp as usize); } @@ -218,7 +224,7 @@ impl CallFrame { /// If the index is out of bounds. pub(crate) fn local<'stack>(&self, index: u32, stack: &'stack [JsValue]) -> &'stack JsValue { debug_assert!(index < self.code_block().locals_count); - let at = self.fp + index; + let at = self.rp + index; &stack[at as usize] } @@ -242,8 +248,8 @@ impl CallFrame { /// ---- `CallFrame` stack methods ---- impl CallFrame { - pub(crate) fn set_frame_pointer(&mut self, pointer: u32) { - self.fp = pointer; + pub(crate) fn set_register_pointer(&mut self, pointer: u32) { + self.rp = pointer; } } diff --git a/core/engine/src/vm/mod.rs b/core/engine/src/vm/mod.rs index 94c12a8a0b3..cf02d8b0b30 100644 --- a/core/engine/src/vm/mod.rs +++ b/core/engine/src/vm/mod.rs @@ -158,7 +158,7 @@ impl Vm { pub(crate) fn push_frame(&mut self, mut frame: CallFrame) { let current_stack_length = self.stack.len(); - frame.set_frame_pointer(current_stack_length as u32); + frame.set_register_pointer(current_stack_length as u32); std::mem::swap(&mut self.environments, &mut frame.environments); std::mem::swap(&mut self.realm, &mut frame.realm); @@ -210,7 +210,7 @@ impl Vm { let catch_address = handler.handler(); let environment_sp = frame.env_fp + handler.environment_count; - let sp = frame.fp + handler.stack_count; + let sp = frame.rp + handler.stack_count; // Go to handler location. frame.pc = catch_address; @@ -325,7 +325,12 @@ impl Context { let result = self.execute_instruction(f); let duration = instant.elapsed(); - let fp = self.vm.frames.last().map(|frame| frame.fp as usize); + let fp = self + .vm + .frames + .last() + .map(CallFrame::fp) + .map(|fp| fp as usize); let stack = { let mut stack = String::from("[ "); @@ -427,7 +432,7 @@ impl Context { break; } - fp = frame.restore_fp() as usize; + fp = frame.fp() as usize; env_fp = frame.env_fp as usize; self.vm.pop_frame(); } @@ -454,8 +459,8 @@ impl Context { match result { CompletionType::Normal => {} CompletionType::Return => { - let restore_fp = self.vm.frame().restore_fp() as usize; - self.vm.stack.truncate(restore_fp); + let fp = self.vm.frame().fp() as usize; + self.vm.stack.truncate(fp); let result = self.vm.take_return_value(); if self.vm.frame().exit_early() { @@ -466,7 +471,7 @@ impl Context { self.vm.pop_frame(); } CompletionType::Throw => { - let mut fp = self.vm.frame().restore_fp(); + let mut fp = self.vm.frame().fp(); let mut env_fp = self.vm.frame().env_fp; if self.vm.frame().exit_early() { self.vm.environments.truncate(env_fp as usize); @@ -482,7 +487,7 @@ impl Context { self.vm.pop_frame(); while let Some(frame) = self.vm.frames.last_mut() { - fp = frame.restore_fp(); + fp = frame.fp(); env_fp = frame.env_fp; let pc = frame.pc; let exit_early = frame.exit_early(); diff --git a/core/engine/src/vm/opcode/generator/mod.rs b/core/engine/src/vm/opcode/generator/mod.rs index 8dbe8a525ba..989c0c45f13 100644 --- a/core/engine/src/vm/opcode/generator/mod.rs +++ b/core/engine/src/vm/opcode/generator/mod.rs @@ -82,7 +82,7 @@ impl Operation for Generator { let fp = frame .call_frame .as_ref() - .map_or(0, |frame| frame.fp as usize); + .map_or(0, |frame| frame.rp as usize); frame.stack[fp] = generator.clone().into(); let mut gen = generator diff --git a/core/engine/src/vm/opcode/rest_parameter/mod.rs b/core/engine/src/vm/opcode/rest_parameter/mod.rs index 280c2951df9..eda0764c610 100644 --- a/core/engine/src/vm/opcode/rest_parameter/mod.rs +++ b/core/engine/src/vm/opcode/rest_parameter/mod.rs @@ -17,11 +17,11 @@ impl Operation for RestParameterInit { const COST: u8 = 6; fn execute(context: &mut Context) -> JsResult { - let arg_count = context.vm.frame().argument_count as usize; + let argument_count = context.vm.frame().argument_count as usize; let param_count = context.vm.frame().code_block().params.as_ref().len(); - let array = if arg_count >= param_count { - let rest_count = arg_count - param_count + 1; + let array = if argument_count >= param_count { + let rest_count = argument_count - param_count + 1; let args = context.vm.pop_n_values(rest_count); Array::create_array_from_list(args, context) } else { From a5628f37122b47b23587d5a6d27f9c5f7ff07392 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Wed, 20 Dec 2023 12:30:44 +0100 Subject: [PATCH 4/5] Add todo for storing fp in CallFrame --- core/engine/src/vm/call_frame/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/engine/src/vm/call_frame/mod.rs b/core/engine/src/vm/call_frame/mod.rs index 856a339f9fc..3cf30cac408 100644 --- a/core/engine/src/vm/call_frame/mod.rs +++ b/core/engine/src/vm/call_frame/mod.rs @@ -37,6 +37,10 @@ pub struct CallFrame { pub(crate) code_block: Gc, pub(crate) pc: u32, /// The register pointer, points to the first register in the stack. + /// + // TODO: Check if storing the frame pointer instead of argument count and computing the + // argument count based on the pointers would be better for accessing the arguments + // and the elements before the register pointer. pub(crate) rp: u32, pub(crate) argument_count: u32, pub(crate) env_fp: u32, From 8d41a22ca85ba7587d91a10e63ba1eda386609a3 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Wed, 20 Dec 2023 12:53:58 +0100 Subject: [PATCH 5/5] Rename ASYNC_GENERATOR_OBJECT_LOCAL_INDEX to ASYNC_GENERATOR_OBJECT_REGISTER_INDEX --- core/engine/src/vm/call_frame/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/engine/src/vm/call_frame/mod.rs b/core/engine/src/vm/call_frame/mod.rs index 3cf30cac408..97645e007ef 100644 --- a/core/engine/src/vm/call_frame/mod.rs +++ b/core/engine/src/vm/call_frame/mod.rs @@ -132,7 +132,7 @@ impl CallFrame { pub(crate) const FUNCTION_PROLOGUE: u32 = 2; pub(crate) const THIS_POSITION: u32 = 2; pub(crate) const FUNCTION_POSITION: u32 = 1; - pub(crate) const ASYNC_GENERATOR_OBJECT_LOCAL_INDEX: u32 = 0; + pub(crate) const ASYNC_GENERATOR_OBJECT_REGISTER_INDEX: u32 = 0; /// Creates a new `CallFrame` with the provided `CodeBlock`. pub(crate) fn new( @@ -216,7 +216,7 @@ impl CallFrame { return None; } - self.local(Self::ASYNC_GENERATOR_OBJECT_LOCAL_INDEX, stack) + self.local(Self::ASYNC_GENERATOR_OBJECT_REGISTER_INDEX, stack) .as_object() .cloned() }