Skip to content

Commit

Permalink
Move arguments object creation to opcode (boa-dev#3432)
Browse files Browse the repository at this point in the history
  • Loading branch information
@HalidOdat @ahaoboy
HalidOdat authored and ahaoboy committed Nov 15, 2023
1 parent dd2c9f9 commit 9e316ef
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 117 deletions.
38 changes: 23 additions & 15 deletions boa_engine/src/bytecompiler/declarations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::rc::Rc;
use crate::{
bytecompiler::{ByteCompiler, FunctionCompiler, FunctionSpec, NodeKind},
environments::CompileTimeEnvironment,
js_string,
vm::{create_function_object_fast, BindingOpcode, CodeBlockFlags, Opcode},
JsNativeError, JsResult,
};
Expand Down Expand Up @@ -285,6 +286,7 @@ impl ByteCompiler<'_, '_> {
let function = create_function_object_fast(code, self.context);

// c. Perform ? env.CreateGlobalFunctionBinding(fn, fo, false).
let name = js_string!(self.interner().resolve_expect(name.sym()).utf16());
self.context
.create_global_function_binding(name, function, false)?;
}
Expand Down Expand Up @@ -726,14 +728,17 @@ impl ByteCompiler<'_, '_> {
// c. If varEnv is a Global Environment Record, then
if var_env.is_global() {
// Ensures global functions are printed when generating the global flowgraph.
let _ = self.push_function_to_constants(code.clone());
let index = self.push_function_to_constants(code.clone());

// b. Let fo be InstantiateFunctionObject of f with arguments lexEnv and privateEnv.
let function = create_function_object_fast(code, self.context);
self.emit_with_varying_operand(Opcode::GetFunction, index);

// i. Perform ? varEnv.CreateGlobalFunctionBinding(fn, fo, true).
self.context
.create_global_function_binding(name, function, true)?;
let name_index = self.get_or_insert_name(name);
self.emit(
Opcode::CreateGlobalFunctionBinding,
&[Operand::Bool(true), Operand::Varying(name_index)],
);
}
// d. Else,
else {
Expand Down Expand Up @@ -915,14 +920,19 @@ impl ByteCompiler<'_, '_> {
// NOTE(HalidOdat): Has been moved up, so "arguments" gets registed as
// the first binding in the environment with index 0.
if arguments_object_needed {
// Note: This happens at runtime.
// a. If strict is true or simpleParameterList is false, then
// i. Let ao be CreateUnmappedArgumentsObject(argumentsList).
if strict || !formals.is_simple() {
// i. Let ao be CreateUnmappedArgumentsObject(argumentsList).
self.emit_opcode(Opcode::CreateUnmappedArgumentsObject);
}
// b. Else,
// i. NOTE: A mapped argument object is only provided for non-strict functions
// that don't have a rest parameter, any parameter
// default value initializers, or any destructured parameters.
// ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env).
else {
// i. NOTE: A mapped argument object is only provided for non-strict functions
// that don't have a rest parameter, any parameter
// default value initializers, or any destructured parameters.
// ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env).
self.emit_opcode(Opcode::CreateMappedArgumentsObject);
}

// c. If strict is true, then
if strict {
Expand All @@ -937,7 +947,8 @@ impl ByteCompiler<'_, '_> {
env.create_mutable_binding(arguments, false);
}

self.code_block_flags |= CodeBlockFlags::NEEDS_ARGUMENTS_OBJECT;
// e. Perform ! env.InitializeBinding("arguments", ao).
self.emit_binding(BindingOpcode::InitLexical, arguments);
}

// 21. For each String paramName of parameterNames, do
Expand All @@ -961,13 +972,10 @@ impl ByteCompiler<'_, '_> {

// 22. If argumentsObjectNeeded is true, then
if arguments_object_needed {
// MOVED: a-d.
// MOVED: a-e.
//
// NOTE(HalidOdat): Has been moved up, see comment above.

// Note: This happens at runtime.
// e. Perform ! env.InitializeBinding("arguments", ao).

// f. Let parameterBindings be the list-concatenation of parameterNames and « "arguments" ».
parameter_names.push(arguments);
}
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/bytecompiler/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ pub struct ByteCompiler<'ctx, 'host> {
json_parse: bool,

// TODO: remove when we separate scripts from the context
context: &'ctx mut Context<'host>,
pub(crate) context: &'ctx mut Context<'host>,

#[cfg(feature = "annex-b")]
annex_b_function_names: Vec<Identifier>,
Expand Down
5 changes: 2 additions & 3 deletions boa_engine/src/context/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ impl Context<'_> {
/// [spec]: https://tc39.es/ecma262/#sec-createglobalfunctionbinding
pub(crate) fn create_global_function_binding(
&mut self,
name: Identifier,
name: JsString,
function: JsObject,
configurable: bool,
) -> JsResult<()> {
Expand All @@ -727,8 +727,7 @@ impl Context<'_> {
let global_object = self.realm().global_object().clone();

// 3. Let existingProp be ? globalObject.[[GetOwnProperty]](N).
let name = PropertyKey::from(self.interner().resolve_expect(name.sym()).utf16());
let existing_prop = global_object.__get_own_property__(&name, self)?;
let existing_prop = global_object.__get_own_property__(&name.clone().into(), self)?;

// 4. If existingProp is undefined or existingProp.[[Configurable]] is true, then
let desc = if existing_prop.is_none()
Expand Down
68 changes: 1 addition & 67 deletions boa_engine/src/object/internal_methods/function.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
builtins::function::{arguments::Arguments, ThisMode},
builtins::function::ThisMode,
context::intrinsics::StandardConstructors,
environments::{FunctionSlots, ThisBindingStatus},
native_function::NativeFunctionObject,
Expand Down Expand Up @@ -121,39 +121,6 @@ pub(crate) fn function_call(
.push_lexical(code.constant_compile_time_environment(last_env));
}

// Taken from: `FunctionDeclarationInstantiation` abstract function.
//
// Spec: https://tc39.es/ecma262/#sec-functiondeclarationinstantiation
//
// 22. If argumentsObjectNeeded is true, then
if code.needs_arguments_object() {
// a. If strict is true or simpleParameterList is false, then
// i. Let ao be CreateUnmappedArgumentsObject(argumentsList).
// b. Else,
// i. NOTE: A mapped argument object is only provided for non-strict functions
// that don't have a rest parameter, any parameter
// default value initializers, or any destructured parameters.
// ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env).
let args = context.vm.stack[(fp + CallFrame::FIRST_ARGUMENT_POSITION)..].to_vec();
let arguments_obj = if code.strict() || !code.params.is_simple() {
Arguments::create_unmapped_arguments_object(&args, context)
} else {
let env = context.vm.environments.current();
Arguments::create_mapped_arguments_object(
function_object,
&code.params,
&args,
env.declarative_expect(),
context,
)
};
let env_index = context.vm.environments.len() as u32 - 1;
context
.vm
.environments
.put_lexical_value(env_index, 0, arguments_obj.into());
}

Ok(CallValue::Ready)
}

Expand Down Expand Up @@ -257,39 +224,6 @@ fn function_construct(
.push_lexical(code.constant_compile_time_environment(last_env));
}

// Taken from: `FunctionDeclarationInstantiation` abstract function.
//
// Spec: https://tc39.es/ecma262/#sec-functiondeclarationinstantiation
//
// 22. If argumentsObjectNeeded is true, then
if code.needs_arguments_object() {
// a. If strict is true or simpleParameterList is false, then
// i. Let ao be CreateUnmappedArgumentsObject(argumentsList).
// b. Else,
// i. NOTE: A mapped argument object is only provided for non-strict functions
// that don't have a rest parameter, any parameter
// default value initializers, or any destructured parameters.
// ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env).
let args = context.vm.stack[at..].to_vec();
let arguments_obj = if code.strict() || !code.params.is_simple() {
Arguments::create_unmapped_arguments_object(&args, context)
} else {
let env = context.vm.environments.current();
Arguments::create_mapped_arguments_object(
this_function_object,
&code.params,
&args,
env.declarative_expect(),
context,
)
};
let env_index = context.vm.environments.len() as u32 - 1;
context
.vm
.environments
.put_lexical_value(env_index, 0, arguments_obj.into());
}

// Insert `this` value
context
.vm
Expand Down
38 changes: 17 additions & 21 deletions boa_engine/src/vm/code_block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,17 @@ bitflags! {
/// Does this function have a parameters environment.
const PARAMETERS_ENV_BINDINGS = 0b0000_1000;

/// Does this function need a `"arguments"` object.
///
/// The `"arguments"` binding is the first binding.
const NEEDS_ARGUMENTS_OBJECT = 0b0001_0000;

/// The `[[ClassFieldInitializerName]]` internal slot.
const IN_CLASS_FIELD_INITIALIZER = 0b0010_0000;
const IN_CLASS_FIELD_INITIALIZER = 0b0001_0000;

/// `[[ConstructorKind]]`
const IS_DERIVED_CONSTRUCTOR = 0b0100_0000;
const IS_DERIVED_CONSTRUCTOR = 0b0010_0000;

const IS_ASYNC = 0b1000_0000;
const IS_GENERATOR = 0b0001_0000_0000;
const IS_ASYNC = 0b0100_0000;
const IS_GENERATOR = 0b0000_1000_0000;

/// Arrow and method functions don't have `"prototype"` property.
const HAS_PROTOTYPE_PROPERTY = 0b0010_0000_0000;
const HAS_PROTOTYPE_PROPERTY = 0b0001_0000_0000;

/// Trace instruction execution to `stdout`.
#[cfg(feature = "trace")]
Expand Down Expand Up @@ -233,13 +228,6 @@ impl CodeBlock {
.contains(CodeBlockFlags::PARAMETERS_ENV_BINDINGS)
}

/// Does this function need a `"arguments"` object.
pub(crate) fn needs_arguments_object(&self) -> bool {
self.flags
.get()
.contains(CodeBlockFlags::NEEDS_ARGUMENTS_OBJECT)
}

/// Does this function have the `[[ClassFieldInitializerName]]` internal slot set to non-empty value.
pub(crate) fn in_class_field_initializer(&self) -> bool {
self.flags
Expand Down Expand Up @@ -526,6 +514,15 @@ impl CodeBlock {
Instruction::CreateIteratorResult { done } => {
format!("done: {done}")
}
Instruction::CreateGlobalFunctionBinding {
name_index,
configurable,
} => {
let name = self
.constant_string(name_index.value() as usize)
.to_std_string_escaped();
format!("name: {name}, configurable: {configurable}")
}
Instruction::Pop
| Instruction::Dup
| Instruction::Swap
Expand Down Expand Up @@ -646,6 +643,8 @@ impl CodeBlock {
| Instruction::GetReturnValue
| Instruction::SetReturnValue
| Instruction::BindThisValue
| Instruction::CreateMappedArgumentsObject
| Instruction::CreateUnmappedArgumentsObject
| Instruction::Nop => String::new(),

Instruction::U16Operands
Expand Down Expand Up @@ -707,10 +706,7 @@ impl CodeBlock {
| Instruction::Reserved55
| Instruction::Reserved56
| Instruction::Reserved57
| Instruction::Reserved58
| Instruction::Reserved59
| Instruction::Reserved60
| Instruction::Reserved61 => unreachable!("Reserved opcodes are unrechable"),
| Instruction::Reserved58 => unreachable!("Reserved opcodes are unrechable"),
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions boa_engine/src/vm/flowgraph/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,9 @@ impl CodeBlock {
| Instruction::MaybeException
| Instruction::CheckReturn
| Instruction::BindThisValue
| Instruction::CreateMappedArgumentsObject
| Instruction::CreateUnmappedArgumentsObject
| Instruction::CreateGlobalFunctionBinding { .. }
| Instruction::Nop => {
graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None);
graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line);
Expand Down Expand Up @@ -517,10 +520,7 @@ impl CodeBlock {
| Instruction::Reserved55
| Instruction::Reserved56
| Instruction::Reserved57
| Instruction::Reserved58
| Instruction::Reserved59
| Instruction::Reserved60
| Instruction::Reserved61 => unreachable!("Reserved opcodes are unrechable"),
| Instruction::Reserved58 => unreachable!("Reserved opcodes are unrechable"),
}
}

Expand Down
64 changes: 64 additions & 0 deletions boa_engine/src/vm/opcode/arguments.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use crate::{
builtins::function::arguments::Arguments,
vm::{CallFrame, CompletionType},
Context, JsResult,
};

use super::Operation;

/// `CreateMappedArgumentsObject` implements the Opcode Operation for `Opcode::CreateMappedArgumentsObject`
///
/// Operation:
/// - TODO: doc
#[derive(Debug, Clone, Copy)]
pub(crate) struct CreateMappedArgumentsObject;

impl Operation for CreateMappedArgumentsObject {
const NAME: &'static str = "CreateMappedArgumentsObject";
const INSTRUCTION: &'static str = "INST - CreateMappedArgumentsObject";
const COST: u8 = 8;

fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
let arguments_start = context.vm.frame().fp as usize + CallFrame::FIRST_ARGUMENT_POSITION;
let function_object = context
.vm
.frame()
.function(&context.vm)
.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 env = context.vm.environments.current();
let arguments = Arguments::create_mapped_arguments_object(
&function_object,
&code.params,
&args,
env.declarative_expect(),
context,
);
context.vm.push(arguments);
Ok(CompletionType::Normal)
}
}

/// `CreateUnmappedArgumentsObject` implements the Opcode Operation for `Opcode::CreateUnmappedArgumentsObject`
///
/// Operation:
/// - TODO: doc
#[derive(Debug, Clone, Copy)]
pub(crate) struct CreateUnmappedArgumentsObject;

impl Operation for CreateUnmappedArgumentsObject {
const NAME: &'static str = "CreateUnmappedArgumentsObject";
const INSTRUCTION: &'static str = "INST - CreateUnmappedArgumentsObject";
const COST: u8 = 4;

fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
let arguments_start = context.vm.frame().fp as usize + CallFrame::FIRST_ARGUMENT_POSITION;
let args = context.vm.stack[arguments_start..].to_vec();
let arguments = Arguments::create_unmapped_arguments_object(&args, context);
context.vm.push(arguments);
Ok(CompletionType::Normal)
}
}
Loading

0 comments on commit 9e316ef

Please sign in to comment.