Skip to content

Commit

Permalink
Avoid reversing arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
HalidOdat committed Oct 9, 2023
1 parent 8b8e0b9 commit ebe2471
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 72 deletions.
1 change: 0 additions & 1 deletion boa_engine/src/bytecompiler/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ impl ByteCompiler<'_, '_> {
if class.super_ref().is_some() {
compiler.emit_opcode(Opcode::SuperCallDerived);
} else {
compiler.emit_opcode(Opcode::RestParameterPop);
compiler.emit_opcode(Opcode::PushUndefined);
}
compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR;
Expand Down
7 changes: 3 additions & 4 deletions boa_engine/src/bytecompiler/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,9 +985,11 @@ impl ByteCompiler<'_, '_> {
// a. Perform ? IteratorBindingInitialization of formals with arguments iteratorRecord and undefined.
// 26. Else,
// a. Perform ? IteratorBindingInitialization of formals with arguments iteratorRecord and env.
for parameter in formals.as_ref() {
for (i, parameter) in formals.as_ref().iter().enumerate() {
if parameter.is_rest_param() {
self.emit_opcode(Opcode::RestParameterInit);
} else {
self.emit_with_varying_operand(Opcode::GetArgument, i as u32);
}
match parameter.variable().binding() {
Binding::Identifier(ident) => {
Expand All @@ -1012,9 +1014,6 @@ impl ByteCompiler<'_, '_> {
}
}
}
if !formals.has_rest_parameter() {
self.emit_opcode(Opcode::RestParameterPop);
}

if generator {
self.emit(Opcode::Generator, &[Operand::U8(self.in_async().into())]);
Expand Down
4 changes: 1 addition & 3 deletions boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,9 +803,7 @@ impl Context<'_> {
}

pub(crate) fn push_n_arguments(&mut self, arguments: &[JsValue]) {
for argument in arguments {
self.vm.push(argument.clone());
}
self.vm.stack.extend_from_slice(arguments);
}
}

Expand Down
23 changes: 0 additions & 23 deletions boa_engine/src/object/internal_methods/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ pub(crate) fn function_call(
drop(object);

let env_fp = environments.len() as u32;
let parameters_count = code.params.as_ref().len();

let mut frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
.with_argument_count(arguments_count as u32)
Expand Down Expand Up @@ -155,16 +154,6 @@ pub(crate) fn function_call(
.put_lexical_value(env_index, 0, arguments_obj.into());
}

// Push function arguments to the stack.
if arguments_count < parameters_count {
let n = parameters_count - arguments_count;
context
.vm
.stack
.extend(std::iter::repeat_with(JsValue::undefined).take(n));
}
context.vm.stack[at..].reverse();

// The call is pending, not complete.
Ok(false)
}
Expand Down Expand Up @@ -225,8 +214,6 @@ fn function_construct(
None
};

let parameters_count = code.params.as_ref().len();

let mut frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
.with_argument_count(argument_count as u32)
.with_env_fp(env_fp);
Expand Down Expand Up @@ -315,16 +302,6 @@ fn function_construct(
.stack
.insert(at - 1, this.map(JsValue::new).unwrap_or_default());

// Push function arguments to the stack.
if argument_count < parameters_count {
let n = parameters_count - argument_count;
context
.vm
.stack
.extend(std::iter::repeat_with(JsValue::undefined).take(n));
}
context.vm.stack[(at + 1)..].reverse();

Ok(false)
}

Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,8 @@ impl CodeBlock {
| Instruction::SuperCall {
argument_count: value,
}
| Instruction::ConcatToString { value_count: value } => value.value().to_string(),
| Instruction::ConcatToString { value_count: value }
| Instruction::GetArgument { index: value } => value.value().to_string(),
Instruction::PushDeclarativeEnvironment {
compile_environments_index,
} => compile_environments_index.value().to_string(),
Expand Down Expand Up @@ -552,7 +553,6 @@ impl CodeBlock {
| Instruction::RequireObjectCoercible
| Instruction::ValueNotNullOrUndefined
| Instruction::RestParameterInit
| Instruction::RestParameterPop
| Instruction::PushValueToArray
| Instruction::PushElisionToArray
| Instruction::PushIteratorToArray
Expand Down
4 changes: 2 additions & 2 deletions boa_engine/src/vm/flowgraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ impl CodeBlock {
| Instruction::Call { .. }
| Instruction::New { .. }
| Instruction::SuperCall { .. }
| Instruction::ConcatToString { .. } => {
| Instruction::ConcatToString { .. }
| Instruction::GetArgument { .. } => {
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 @@ -425,7 +426,6 @@ impl CodeBlock {
| Instruction::RequireObjectCoercible
| Instruction::ValueNotNullOrUndefined
| Instruction::RestParameterInit
| Instruction::RestParameterPop
| Instruction::PushValueToArray
| Instruction::PushElisionToArray
| Instruction::PushIteratorToArray
Expand Down
7 changes: 2 additions & 5 deletions boa_engine/src/vm/opcode/environment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,8 @@ impl Operation for SuperCallDerived {
const INSTRUCTION: &'static str = "INST - SuperCallDerived";

fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
let argument_count = context.vm.frame().argument_count;
let mut arguments = Vec::with_capacity(argument_count as usize);
for _ in 0..argument_count {
arguments.push(context.vm.pop());
}
let argument_count = context.vm.frame().argument_count as usize;
let arguments = context.pop_n_arguments(argument_count);

let this_env = context
.vm
Expand Down
47 changes: 47 additions & 0 deletions boa_engine/src/vm/opcode/get/argument.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use crate::{
vm::{opcode::Operation, CompletionType},
Context, JsResult,
};

/// `GetArgument` implements the Opcode Operation for `Opcode::GetArgument`
///
/// Operation:
/// - Get i-th argument of the current frame.
#[derive(Debug, Clone, Copy)]
pub(crate) struct GetArgument;

impl GetArgument {
#[allow(clippy::unnecessary_wraps)]
fn operation(context: &mut Context<'_>, index: usize) -> JsResult<CompletionType> {
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)
.cloned()
.unwrap_or_default();
context.vm.push(value);
Ok(CompletionType::Normal)
}
}

impl Operation for GetArgument {
const NAME: &'static str = "GetArgument";
const INSTRUCTION: &'static str = "INST - GetArgument";

fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
let index = context.vm.read::<u8>() as usize;
Self::operation(context, index)
}

fn execute_with_u16_operands(context: &mut Context<'_>) -> JsResult<CompletionType> {
let index = context.vm.read::<u16>() as usize;
Self::operation(context, index)
}

fn execute_with_u32_operands(context: &mut Context<'_>) -> JsResult<CompletionType> {
let index = context.vm.read::<u32>() as usize;
Self::operation(context, index)
}
}
2 changes: 2 additions & 0 deletions boa_engine/src/vm/opcode/get/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
pub(crate) mod argument;
pub(crate) mod function;
pub(crate) mod generator;
pub(crate) mod name;
pub(crate) mod private;
pub(crate) mod property;

pub(crate) use argument::*;
pub(crate) use function::*;
pub(crate) use generator::*;
pub(crate) use name::*;
Expand Down
16 changes: 9 additions & 7 deletions boa_engine/src/vm/opcode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,15 @@ generate_opcodes! {
/// Stack: **=>**
ThrowMutateImmutable { index: VaryingOperand },

/// Get i-th argument of the current frame.
///
/// Returns `undefined` if `arguments.len()` < `index`.
///
/// Operands: index: `u32`
///
/// Stack: **=>** value
GetArgument { index: VaryingOperand },

/// Find a binding on the environment chain and push its value.
///
/// Operands: index: `u32`
Expand Down Expand Up @@ -1919,13 +1928,6 @@ generate_opcodes! {
/// Stack: `argument_1` .. `argument_n` **=>** `array`
RestParameterInit,

/// Pop the remaining arguments of a function.
///
/// Operands:
///
/// Stack: `argument_1` .. `argument_n` **=>**
RestParameterPop,

/// Yields from the current generator execution.
///
/// Operands:
Expand Down
26 changes: 1 addition & 25 deletions boa_engine/src/vm/opcode/rest_parameter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,39 +24,15 @@ impl Operation for RestParameterInit {
for _ in 0..rest_count {
args.push(context.vm.pop());
}
args.reverse();
let array = Array::create_array_from_list(args, context);

context.vm.push(array);
} else {
context.vm.pop();

let array =
Array::array_create(0, None, context).expect("could not create an empty array");
context.vm.push(array);
}
Ok(CompletionType::Normal)
}
}

/// `RestParameterPop` implements the Opcode Operation for `Opcode::RestParameterPop`
///
/// Operation:
/// - Pop the remaining arguments of a function.
#[derive(Debug, Clone, Copy)]
pub(crate) struct RestParameterPop;

impl Operation for RestParameterPop {
const NAME: &'static str = "RestParameterPop";
const INSTRUCTION: &'static str = "INST - RestParameterPop";

fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
let arg_count = context.vm.frame().argument_count;
let param_count = context.vm.frame().code_block().params.as_ref().len() as u32;
if arg_count > param_count {
for _ in 0..(arg_count - param_count) {
context.vm.pop();
}
}
Ok(CompletionType::Normal)
}
}

0 comments on commit ebe2471

Please sign in to comment.