From 90544fe1f44281acb55326cdd9ffed5b1f944d57 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Fri, 28 Jul 2023 00:07:26 +0200 Subject: [PATCH] Use main stack for calling - Move `return_value` to stack --- boa_engine/src/builtins/eval/mod.rs | 3 ++ boa_engine/src/builtins/generator/mod.rs | 13 +++-- boa_engine/src/builtins/json/mod.rs | 2 + boa_engine/src/bytecompiler/mod.rs | 5 +- boa_engine/src/module/source.rs | 3 ++ boa_engine/src/script.rs | 3 ++ boa_engine/src/vm/call_frame/mod.rs | 6 --- boa_engine/src/vm/code_block.rs | 53 +++++++------------ boa_engine/src/vm/mod.rs | 12 ++++- boa_engine/src/vm/opcode/await/mod.rs | 6 ++- .../src/vm/opcode/control_flow/return.rs | 4 +- boa_engine/src/vm/opcode/generator/mod.rs | 15 ++++-- 12 files changed, 73 insertions(+), 52 deletions(-) diff --git a/boa_engine/src/builtins/eval/mod.rs b/boa_engine/src/builtins/eval/mod.rs index ab3e25b1b8d..6e20cc9076e 100644 --- a/boa_engine/src/builtins/eval/mod.rs +++ b/boa_engine/src/builtins/eval/mod.rs @@ -258,6 +258,9 @@ impl Eval { .vm .push_frame(CallFrame::new(code_block).with_env_fp(env_fp)); context.realm().resize_global_env(); + + // Push return value. + context.vm.push(JsValue::undefined()); let record = context.run(); context.vm.pop_frame(); diff --git a/boa_engine/src/builtins/generator/mod.rs b/boa_engine/src/builtins/generator/mod.rs index cfca602d79c..ea9a636e4d7 100644 --- a/boa_engine/src/builtins/generator/mod.rs +++ b/boa_engine/src/builtins/generator/mod.rs @@ -85,13 +85,18 @@ impl GeneratorContext { /// Creates a new `GeneratorContext` from the current `Context` state. pub(crate) fn from_current(context: &mut Context<'_>) -> Self { - Self { + let fp = context.vm.frame().fp as usize; + let this = Self { environments: context.vm.environments.clone(), call_frame: Some(context.vm.frame().clone()), - stack: context.vm.stack.clone(), + stack: context.vm.stack[fp..].to_vec(), active_function: context.vm.active_function.clone(), realm: context.realm().clone(), - } + }; + + context.vm.stack.truncate(fp); + + this } /// Resumes execution with `GeneratorContext` as the current execution context. @@ -109,6 +114,8 @@ impl GeneratorContext { .vm .push_frame(self.call_frame.take().expect("should have a call frame")); + context.vm.frame_mut().fp = 0; + if let Some(value) = value { context.vm.push(value); } diff --git a/boa_engine/src/builtins/json/mod.rs b/boa_engine/src/builtins/json/mod.rs index 67fd9a41bce..cfa603ce463 100644 --- a/boa_engine/src/builtins/json/mod.rs +++ b/boa_engine/src/builtins/json/mod.rs @@ -133,6 +133,8 @@ impl Json { .vm .push_frame(CallFrame::new(code_block).with_env_fp(env_fp)); context.realm().resize_global_env(); + // Push return value. + context.vm.push(JsValue::undefined()); let record = context.run(); context.vm.pop_frame(); diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index f10e050082f..4fe966fd9dc 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -300,7 +300,10 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { params: FormalParameterList::default(), compile_environments: Vec::default(), current_open_environments_count: 0, - current_stack_value_count: 0, + + // Note: We set it to one so we don't pop return value, + // which is allocated after frame pointer. + current_stack_value_count: 1, code_block_flags, handlers: ThinVec::default(), diff --git a/boa_engine/src/module/source.rs b/boa_engine/src/module/source.rs index 21e74a9e100..b5ca4429c1c 100644 --- a/boa_engine/src/module/source.rs +++ b/boa_engine/src/module/source.rs @@ -1763,6 +1763,9 @@ impl SourceTextModule { // 10. Else, // a. Assert: capability is a PromiseCapability Record. // b. Perform AsyncBlockStart(capability, module.[[ECMAScriptCode]], moduleContext). + + // Push return value. + context.vm.push(JsValue::undefined()); let result = context.run(); std::mem::swap(&mut context.vm.environments, &mut environments); diff --git a/boa_engine/src/script.rs b/boa_engine/src/script.rs index 226b9306610..fc5fa08645c 100644 --- a/boa_engine/src/script.rs +++ b/boa_engine/src/script.rs @@ -150,6 +150,9 @@ impl Script { // TODO: Here should be https://tc39.es/ecma262/#sec-globaldeclarationinstantiation + // Push return value. + context.vm.push(JsValue::undefined()); + self.realm().resize_global_env(); let record = context.run(); context.vm.pop_frame(); diff --git a/boa_engine/src/vm/call_frame/mod.rs b/boa_engine/src/vm/call_frame/mod.rs index 430840434e9..9597e3a28ac 100644 --- a/boa_engine/src/vm/call_frame/mod.rs +++ b/boa_engine/src/vm/call_frame/mod.rs @@ -36,11 +36,6 @@ pub struct CallFrame { /// How many iterations a loop has done. pub(crate) loop_iteration_count: u64, - - /// The value that is returned from the function. - // - // TODO(HalidOdat): Remove this and put into the stack, maybe before frame pointer. - pub(crate) return_value: JsValue, } /// ---- `CallFrame` public API ---- @@ -68,7 +63,6 @@ impl CallFrame { iterators: ThinVec::new(), binding_stack: Vec::new(), loop_iteration_count: 0, - return_value: JsValue::undefined(), } } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 5c2a235919a..828d61570cc 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -1231,22 +1231,7 @@ impl JsObject { } let argument_count = args.len(); - - // Push function arguments to the stack. - let mut args = if code.params.as_ref().len() > args.len() { - let mut v = args.to_vec(); - v.extend(vec![ - JsValue::Undefined; - code.params.as_ref().len() - args.len() - ]); - v - } else { - args.to_vec() - }; - args.reverse(); - let mut stack = args; - - std::mem::swap(&mut context.vm.stack, &mut stack); + let parameters_count = code.params.as_ref().len(); let frame = CallFrame::new(code) .with_argument_count(argument_count as u32) @@ -1256,6 +1241,15 @@ impl JsObject { context.vm.push_frame(frame); + // Push return value. + context.vm.push(JsValue::undefined()); + + // Push function arguments to the stack. + for _ in argument_count..parameters_count { + context.vm.push(JsValue::undefined()); + } + context.vm.stack.extend(args.iter().rev().cloned()); + let result = context .run() .consume() @@ -1263,7 +1257,6 @@ impl JsObject { context.vm.pop_frame().expect("frame must exist"); std::mem::swap(&mut environments, &mut context.vm.environments); - std::mem::swap(&mut context.vm.stack, &mut stack); std::mem::swap(&mut context.vm.active_runnable, &mut script_or_module); result @@ -1438,22 +1431,7 @@ impl JsObject { } let argument_count = args.len(); - - // Push function arguments to the stack. - let args = if code.params.as_ref().len() > args.len() { - let mut v = args.to_vec(); - v.extend(vec![ - JsValue::Undefined; - code.params.as_ref().len() - args.len() - ]); - v - } else { - args.to_vec() - }; - - for arg in args.iter().rev() { - context.vm.push(arg.clone()); - } + let parameters_count = code.params.as_ref().len(); let has_binding_identifier = code.has_binding_identifier(); @@ -1465,6 +1443,15 @@ impl JsObject { .with_env_fp(environments_len as u32), ); + // Push return value. + context.vm.push(JsValue::undefined()); + + // Push function arguments to the stack. + for _ in argument_count..parameters_count { + context.vm.push(JsValue::undefined()); + } + context.vm.stack.extend(args.iter().rev().cloned()); + let record = context.run(); context.vm.pop_frame(); diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index 8ddef1b24de..06f05c3d94d 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -181,6 +181,16 @@ impl Vm { true } + + pub(crate) fn get_return_value(&self) -> JsValue { + let fp = self.frame().fp; + self.stack[fp as usize].clone() + } + + pub(crate) fn set_return_value(&mut self, value: JsValue) { + let fp = self.frame().fp; + self.stack[fp as usize] = value; + } } #[derive(Debug, Clone, Copy, PartialEq)] @@ -328,8 +338,8 @@ impl Context<'_> { match result { Ok(CompletionType::Normal) => {} Ok(CompletionType::Return) => { + let execution_result = self.vm.get_return_value(); self.vm.stack.truncate(self.vm.frame().fp as usize); - let execution_result = self.vm.frame_mut().return_value.clone(); return CompletionRecord::Normal(execution_result); } Ok(CompletionType::Throw) => { diff --git a/boa_engine/src/vm/opcode/await/mod.rs b/boa_engine/src/vm/opcode/await/mod.rs index f952580c39b..c39e42128a4 100644 --- a/boa_engine/src/vm/opcode/await/mod.rs +++ b/boa_engine/src/vm/opcode/await/mod.rs @@ -189,14 +189,16 @@ impl Operation for CompletePromiseCapability { .call(&JsValue::undefined(), &[error.to_opaque(context)], context) .expect("cannot fail per spec"); } else { - let return_value = context.vm.frame().return_value.clone(); + let return_value = context.vm.get_return_value(); promise_capability .resolve() .call(&JsValue::undefined(), &[return_value], context) .expect("cannot fail per spec"); }; - context.vm.frame_mut().return_value = promise_capability.promise().clone().into(); + context + .vm + .set_return_value(promise_capability.promise().clone().into()); Ok(CompletionType::Normal) } diff --git a/boa_engine/src/vm/opcode/control_flow/return.rs b/boa_engine/src/vm/opcode/control_flow/return.rs index 17e88983906..8db9cab8707 100644 --- a/boa_engine/src/vm/opcode/control_flow/return.rs +++ b/boa_engine/src/vm/opcode/control_flow/return.rs @@ -31,7 +31,7 @@ impl Operation for GetReturnValue { const INSTRUCTION: &'static str = "INST - GetReturnValue"; fn execute(context: &mut Context<'_>) -> JsResult { - let value = context.vm.frame().return_value.clone(); + let value = context.vm.get_return_value(); context.vm.push(value); Ok(CompletionType::Normal) } @@ -50,7 +50,7 @@ impl Operation for SetReturnValue { fn execute(context: &mut Context<'_>) -> JsResult { let value = context.vm.pop(); - context.vm.frame_mut().return_value = value; + context.vm.set_return_value(value); Ok(CompletionType::Normal) } } diff --git a/boa_engine/src/vm/opcode/generator/mod.rs b/boa_engine/src/vm/opcode/generator/mod.rs index 34d4242a7a9..41ce8dd0013 100644 --- a/boa_engine/src/vm/opcode/generator/mod.rs +++ b/boa_engine/src/vm/opcode/generator/mod.rs @@ -16,7 +16,7 @@ use crate::{ opcode::{Operation, ReThrow}, CallFrame, CompletionType, }, - Context, JsError, JsObject, JsResult, + Context, JsError, JsObject, JsResult, JsValue, }; pub(crate) use yield_stm::*; @@ -41,7 +41,14 @@ impl Operation for Generator { let pc = context.vm.frame().pc; let mut dummy_call_frame = CallFrame::new(code_block); dummy_call_frame.pc = pc; - let call_frame = std::mem::replace(context.vm.frame_mut(), dummy_call_frame); + let mut call_frame = std::mem::replace(context.vm.frame_mut(), dummy_call_frame); + + 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 this_function_object = context .vm @@ -69,7 +76,6 @@ impl Operation for Generator { &mut context.vm.environments, EnvironmentStack::new(global_environement), ); - let stack = std::mem::take(&mut context.vm.stack); let data = if r#async { ObjectData::async_generator(AsyncGenerator { @@ -154,7 +160,8 @@ impl Operation for AsyncGeneratorClose { .expect("must have item in queue"); drop(generator_object_mut); - let return_value = std::mem::take(&mut context.vm.frame_mut().return_value); + let return_value = context.vm.get_return_value(); + context.vm.set_return_value(JsValue::undefined()); if let Some(error) = context.vm.pending_exception.take() { AsyncGenerator::complete_step(&next, Err(error), true, None, context);