Commit fd75c97d authored by Shu-yu Guo's avatar Shu-yu Guo Committed by Commit Bot

[interpreter] Apply Reflect.apply transform in BytecodeGenerator

Calls with a spread expression in a non-final position get transformed
to calls to Reflect.apply. This transformation is currently done in
the parser, which does not compose well with other features (e.g.
direct eval checking, optional chaining).

Do this transform in the BytecodeGenerator instead.

Bug: v8:11573, v8:11558, v8:5690
Change-Id: I56c90a2036fe5b43e0897c57766f666bf72bc3a8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2765783
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73534}
parent 2dd02967
......@@ -935,6 +935,22 @@ Call::CallType Call::GetCallType() const {
return OTHER_CALL;
}
void Call::ComputeSpreadPosition() {
int arguments_length = arguments_.length();
int first_spread_index = 0;
for (; first_spread_index < arguments_length; first_spread_index++) {
if (arguments_.at(first_spread_index)->IsSpread()) break;
}
SpreadPosition position;
if (first_spread_index == arguments_length - 1) {
position = kHasFinalSpread;
} else {
DCHECK_LT(first_spread_index, arguments_length - 1);
position = kHasNonFinalSpread;
}
bit_field_ |= SpreadPositionField::encode(position);
}
CaseClause::CaseClause(Zone* zone, Expression* label,
const ScopedPtrList<Statement>& statements)
: label_(label), statements_(statements.ToConstVector(), zone) {}
......
......@@ -1635,6 +1635,12 @@ class Call final : public Expression {
return IsOptionalChainLinkField::decode(bit_field_);
}
enum SpreadPosition { kNoSpread, kHasFinalSpread, kHasNonFinalSpread };
SpreadPosition spread_position() const {
return SpreadPositionField::decode(bit_field_);
}
// TODO(syg): Remove this and its users.
bool only_last_arg_is_spread() {
return !arguments_.is_empty() && arguments_.last()->IsSpread();
}
......@@ -1669,7 +1675,7 @@ class Call final : public Expression {
friend Zone;
Call(Zone* zone, Expression* expression,
const ScopedPtrList<Expression>& arguments, int pos,
const ScopedPtrList<Expression>& arguments, int pos, bool has_spread,
PossiblyEval possibly_eval, bool optional_chain)
: Expression(pos, kCall),
expression_(expression),
......@@ -1677,7 +1683,9 @@ class Call final : public Expression {
bit_field_ |=
IsPossiblyEvalField::encode(possibly_eval == IS_POSSIBLY_EVAL) |
IsTaggedTemplateField::encode(false) |
IsOptionalChainLinkField::encode(optional_chain);
IsOptionalChainLinkField::encode(optional_chain) |
SpreadPositionField::encode(kNoSpread);
if (has_spread) ComputeSpreadPosition();
}
Call(Zone* zone, Expression* expression,
......@@ -1688,12 +1696,17 @@ class Call final : public Expression {
arguments_(arguments.ToConstVector(), zone) {
bit_field_ |= IsPossiblyEvalField::encode(false) |
IsTaggedTemplateField::encode(true) |
IsOptionalChainLinkField::encode(false);
IsOptionalChainLinkField::encode(false) |
SpreadPositionField::encode(kNoSpread);
}
// Only valid to be called if there is a spread in arguments_.
void ComputeSpreadPosition();
using IsPossiblyEvalField = Expression::NextBitField<bool, 1>;
using IsTaggedTemplateField = IsPossiblyEvalField::Next<bool, 1>;
using IsOptionalChainLinkField = IsTaggedTemplateField::Next<bool, 1>;
using SpreadPositionField = IsOptionalChainLinkField::Next<SpreadPosition, 2>;
Expression* expression_;
ZonePtrList<Expression> arguments_;
......@@ -3064,11 +3077,12 @@ class AstNodeFactory final {
Call* NewCall(Expression* expression,
const ScopedPtrList<Expression>& arguments, int pos,
bool has_spread,
Call::PossiblyEval possibly_eval = Call::NOT_EVAL,
bool optional_chain = false) {
DCHECK_IMPLIES(possibly_eval == Call::IS_POSSIBLY_EVAL, !optional_chain);
return zone_->New<Call>(zone_, expression, arguments, pos, possibly_eval,
optional_chain);
return zone_->New<Call>(zone_, expression, arguments, pos, has_spread,
possibly_eval, optional_chain);
}
Call* NewTaggedTemplate(Expression* expression,
......
......@@ -3111,6 +3111,8 @@ void BytecodeGenerator::BuildCreateArrayLiteral(
.StoreAccumulatorInRegister(index);
}
} else {
// TODO(v8:11582): Support allocating boilerplates here.
// In other cases, we prepare an empty array to be filled in below.
DCHECK(!elements->is_empty());
int literal_index = feedback_index(feedback_spec()->AddLiteralSlot());
......@@ -5027,17 +5029,30 @@ void BytecodeGenerator::VisitCall(Call* expr) {
return VisitCallSuper(expr);
}
// We compile the call differently depending on the presence of spreads and
// their positions.
//
// If there is only one spread and it is the final argument, there is a
// special CallWithSpread bytecode.
//
// If there is a non-final spread, we rewrite calls like
// callee(1, ...x, 2)
// to
// %reflect_apply(callee, receiver, [1, ...x, 2])
const Call::SpreadPosition spread_position = expr->spread_position();
// Grow the args list as we visit receiver / arguments to avoid allocating all
// the registers up-front. Otherwise these registers are unavailable during
// receiver / argument visiting and we can end up with memory leaks due to
// registers keeping objects alive.
Register callee = register_allocator()->NewRegister();
RegisterList args = register_allocator()->NewGrowableRegisterList();
// The callee is the first register in args for ease of calling %reflect_apply
// if we have a non-final spread. For all other cases it is popped from args
// before emitting the call below.
Register callee = register_allocator()->GrowRegisterList(&args);
bool implicit_undefined_receiver = false;
// When a call contains a spread, a Call AST node is only created if there is
// exactly one spread, and it is the last argument.
bool is_spread_call = expr->only_last_arg_is_spread();
bool optimize_as_one_shot = ShouldOptimizeAsOneShot();
// TODO(petermarshall): We have a lot of call bytecodes that are very similar,
......@@ -5057,7 +5072,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
}
case Call::GLOBAL_CALL: {
// Receiver is undefined for global calls.
if (!is_spread_call && !optimize_as_one_shot) {
if (spread_position == Call::kNoSpread && !optimize_as_one_shot) {
implicit_undefined_receiver = true;
} else {
// TODO(leszeks): There's no special bytecode for tail calls or spread
......@@ -5093,7 +5108,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
}
case Call::OTHER_CALL: {
// Receiver is undefined for other calls.
if (!is_spread_call && !optimize_as_one_shot) {
if (spread_position == Call::kNoSpread && !optimize_as_one_shot) {
implicit_undefined_receiver = true;
} else {
// TODO(leszeks): There's no special bytecode for tail calls or spread
......@@ -5142,25 +5157,51 @@ void BytecodeGenerator::VisitCall(Call* expr) {
BuildIncrementBlockCoverageCounterIfEnabled(right_range);
}
// Evaluate all arguments to the function call and store in sequential args
// registers.
VisitArguments(expr->arguments(), &args);
int receiver_arg_count = implicit_undefined_receiver ? 0 : 1;
CHECK_EQ(receiver_arg_count + expr->arguments()->length(),
args.register_count());
int receiver_arg_count = -1;
if (spread_position == Call::kHasNonFinalSpread) {
// If we're building %reflect_apply, build the array literal and put it in
// the 3rd argument.
DCHECK(!implicit_undefined_receiver);
DCHECK_EQ(args.register_count(), 2);
BuildCreateArrayLiteral(expr->arguments(), nullptr);
builder()->StoreAccumulatorInRegister(
register_allocator()->GrowRegisterList(&args));
} else {
// If we're not building %reflect_apply and don't need to build an array
// literal, pop the callee and evaluate all arguments to the function call
// and store in sequential args registers.
args = args.PopLeft();
VisitArguments(expr->arguments(), &args);
receiver_arg_count = implicit_undefined_receiver ? 0 : 1;
CHECK_EQ(receiver_arg_count + expr->arguments()->length(),
args.register_count());
}
// Resolve callee for a potential direct eval call. This block will mutate the
// callee value.
if (expr->is_possibly_eval() && expr->arguments()->length() > 0) {
RegisterAllocationScope inner_register_scope(this);
RegisterList runtime_call_args = register_allocator()->NewRegisterList(6);
// Set up arguments for ResolvePossiblyDirectEval by copying callee, source
// strings and function closure, and loading language and
// position.
Register first_arg = args[receiver_arg_count];
RegisterList runtime_call_args = register_allocator()->NewRegisterList(6);
// Move the first arg.
if (spread_position == Call::kHasNonFinalSpread) {
int feedback_slot_index =
feedback_index(feedback_spec()->AddKeyedLoadICSlot());
Register args_array = args[2];
builder()
->LoadLiteral(Smi::FromInt(0))
.LoadKeyedProperty(args_array, feedback_slot_index)
.StoreAccumulatorInRegister(runtime_call_args[1]);
} else {
// FIXME(v8:5690): Support final spreads for eval.
DCHECK_GE(receiver_arg_count, 0);
builder()->MoveRegister(args[receiver_arg_count], runtime_call_args[1]);
}
builder()
->MoveRegister(callee, runtime_call_args[0])
.MoveRegister(first_arg, runtime_call_args[1])
.MoveRegister(Register::function_closure(), runtime_call_args[2])
.LoadLiteral(Smi::FromEnum(language_mode()))
.StoreAccumulatorInRegister(runtime_call_args[3])
......@@ -5177,10 +5218,12 @@ void BytecodeGenerator::VisitCall(Call* expr) {
builder()->SetExpressionPosition(expr);
if (is_spread_call) {
if (spread_position == Call::kHasFinalSpread) {
DCHECK(!implicit_undefined_receiver);
builder()->CallWithSpread(callee, args,
feedback_index(feedback_spec()->AddCallICSlot()));
} else if (spread_position == Call::kHasNonFinalSpread) {
builder()->CallJSRuntime(Context::REFLECT_APPLY_INDEX, args);
} else if (optimize_as_one_shot) {
DCHECK(!implicit_undefined_receiver);
builder()->CallNoFeedback(callee, args);
......@@ -5203,10 +5246,20 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
SuperCallReference* super = expr->expression()->AsSuperCallReference();
const ZonePtrList<Expression>* args = expr->arguments();
int first_spread_index = 0;
for (; first_spread_index < args->length(); first_spread_index++) {
if (args->at(first_spread_index)->IsSpread()) break;
}
// We compile the super call differently depending on the presence of spreads
// and their positions.
//
// If there is only one spread and it is the final argument, there is a
// special ConstructWithSpread bytecode.
//
// It there is a non-final spread, we rewrite something like
// super(1, ...x, 2)
// to
// %reflect_construct(constructor, [1, ...x, 2], new_target)
//
// That is, we implement (non-last-arg) spreads in super calls via our
// mechanism for spreads in array literals.
const Call::SpreadPosition spread_position = expr->spread_position();
// Prepare the constructor to the super call.
Register this_function = VisitForRegisterValue(super->this_function_var());
......@@ -5215,14 +5268,7 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
->LoadAccumulatorWithRegister(this_function)
.GetSuperConstructor(constructor);
if (first_spread_index < expr->arguments()->length() - 1) {
// We rewrite something like
// super(1, ...x, 2)
// to
// %reflect_construct(constructor, [1, ...x, 2], new_target)
// That is, we implement (non-last-arg) spreads in super calls via our
// mechanism for spreads in array literals.
if (spread_position == Call::kHasNonFinalSpread) {
// First generate the array containing all arguments.
BuildCreateArrayLiteral(args, nullptr);
......@@ -5249,11 +5295,11 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
int feedback_slot_index = feedback_index(feedback_spec()->AddCallICSlot());
if (first_spread_index == expr->arguments()->length() - 1) {
if (spread_position == Call::kHasFinalSpread) {
builder()->ConstructWithSpread(constructor, args_regs,
feedback_slot_index);
} else {
DCHECK_EQ(first_spread_index, expr->arguments()->length());
DCHECK_EQ(spread_position, Call::kNoSpread);
// Call construct.
// TODO(turbofan): For now we do gather feedback on super constructor
// calls, utilizing the existing machinery to inline the actual call
......
......@@ -1186,6 +1186,7 @@ class ParserBase {
BlockT ParseClassStaticBlock(ClassInfo* class_info);
ObjectLiteralPropertyT ParseObjectPropertyDefinition(
ParsePropertyInfo* prop_info, bool* has_seen_proto);
// TODO(syg): Remove has_spread once SpreadCallNew is removed.
void ParseArguments(
ExpressionListT* args, bool* has_spread,
ParsingArrowHeadFlag maybe_arrow = kCertainlyNotArrowHead);
......@@ -3388,11 +3389,7 @@ ParserBase<Impl>::ParseLeftHandSideContinuation(ExpressionT result) {
return result;
}
if (has_spread) {
result = impl()->SpreadCall(result, args, pos, Call::NOT_EVAL, false);
} else {
result = factory()->NewCall(result, args, pos, Call::NOT_EVAL);
}
result = factory()->NewCall(result, args, pos, has_spread);
maybe_arrow.ValidateExpression();
......@@ -3486,13 +3483,8 @@ ParserBase<Impl>::ParseLeftHandSideContinuation(ExpressionT result) {
Call::PossiblyEval is_possibly_eval =
CheckPossibleEvalCall(result, is_optional, scope());
if (has_spread) {
result = impl()->SpreadCall(result, args, pos, is_possibly_eval,
is_optional);
} else {
result = factory()->NewCall(result, args, pos, is_possibly_eval,
is_optional);
}
result = factory()->NewCall(result, args, pos, has_spread,
is_possibly_eval, is_optional);
fni_.RemoveLastFunction();
break;
......
......@@ -69,7 +69,8 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
args.Add(spread_args);
Expression* super_call_ref = NewSuperCallReference(pos);
call = factory()->NewCall(super_call_ref, args, pos);
constexpr bool has_spread = true;
call = factory()->NewCall(super_call_ref, args, pos, has_spread);
}
body.Add(factory()->NewReturnStatement(call, pos));
}
......@@ -3373,47 +3374,10 @@ ArrayLiteral* Parser::ArrayLiteralFromListWithSpread(
return factory()->NewArrayLiteral(list, first_spread, kNoSourcePosition);
}
Expression* Parser::SpreadCall(Expression* function,
const ScopedPtrList<Expression>& args_list,
int pos, Call::PossiblyEval is_possibly_eval,
bool optional_chain) {
// Handle this case in BytecodeGenerator.
if (OnlyLastArgIsSpread(args_list) || function->IsSuperCallReference()) {
return factory()->NewCall(function, args_list, pos, Call::NOT_EVAL,
optional_chain);
}
ScopedPtrList<Expression> args(pointer_buffer());
if (function->IsProperty()) {
// Method calls
if (function->AsProperty()->IsSuperAccess()) {
Expression* home = ThisExpression();
args.Add(function);
args.Add(home);
} else {
Variable* temp = NewTemporary(ast_value_factory()->empty_string());
VariableProxy* obj = factory()->NewVariableProxy(temp);
Assignment* assign_obj = factory()->NewAssignment(
Token::ASSIGN, obj, function->AsProperty()->obj(), kNoSourcePosition);
function =
factory()->NewProperty(assign_obj, function->AsProperty()->key(),
kNoSourcePosition, optional_chain);
args.Add(function);
obj = factory()->NewVariableProxy(temp);
args.Add(obj);
}
} else {
// Non-method calls
args.Add(function);
args.Add(factory()->NewUndefinedLiteral(kNoSourcePosition));
}
args.Add(ArrayLiteralFromListWithSpread(args_list));
return factory()->NewCallRuntime(Context::REFLECT_APPLY_INDEX, args, pos);
}
Expression* Parser::SpreadCallNew(Expression* function,
const ScopedPtrList<Expression>& args_list,
int pos) {
// TODO(syg): Handle all spread cases in BytecodeGenerator.
if (OnlyLastArgIsSpread(args_list)) {
// Handle in BytecodeGenerator.
return factory()->NewCallNew(function, args_list, pos);
......
......@@ -493,10 +493,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
ArrayLiteral* ArrayLiteralFromListWithSpread(
const ScopedPtrList<Expression>& list);
Expression* SpreadCall(Expression* function,
const ScopedPtrList<Expression>& args, int pos,
Call::PossiblyEval is_possibly_eval,
bool optional_chain);
Expression* SpreadCallNew(Expression* function,
const ScopedPtrList<Expression>& args, int pos);
Expression* RewriteSuperCall(Expression* call_expression);
......
......@@ -645,6 +645,7 @@ class PreParserFactory {
}
PreParserExpression NewCall(PreParserExpression expression,
const PreParserExpressionList& arguments, int pos,
bool has_spread,
Call::PossiblyEval possibly_eval = Call::NOT_EVAL,
bool optional_chain = false) {
if (possibly_eval == Call::IS_POSSIBLY_EVAL) {
......@@ -1049,11 +1050,6 @@ class PreParser : public ParserBase<PreParser> {
}
V8_INLINE void SetAsmModule() {}
V8_INLINE PreParserExpression SpreadCall(const PreParserExpression& function,
const PreParserExpressionList& args,
int pos,
Call::PossiblyEval possibly_eval,
bool optional_chain);
V8_INLINE PreParserExpression
SpreadCallNew(const PreParserExpression& function,
const PreParserExpressionList& args, int pos);
......@@ -1682,14 +1678,6 @@ class PreParser : public ParserBase<PreParser> {
std::vector<void*> preparse_data_builder_buffer_;
};
PreParserExpression PreParser::SpreadCall(const PreParserExpression& function,
const PreParserExpressionList& args,
int pos,
Call::PossiblyEval possibly_eval,
bool optional_chain) {
return factory()->NewCall(function, args, pos, possibly_eval, optional_chain);
}
PreParserExpression PreParser::SpreadCallNew(
const PreParserExpression& function, const PreParserExpressionList& args,
int pos) {
......
......@@ -63,43 +63,47 @@ handlers: [
snippet: "
Math.max(0, ...[1, 2, 3], 4);
"
frame size: 8
frame size: 7
parameter count: 1
bytecode array length: 91
bytecode array length: 95
bytecodes: [
/* 34 S> */ B(LdaGlobal), U8(0), U8(0),
B(Star0),
B(LdaNamedProperty), R(0), U8(1), U8(2),
B(Star1),
B(CreateArrayLiteral), U8(2), U8(4), U8(37),
B(Star4),
B(LdaSmi), I8(1),
/* 39 E> */ B(LdaNamedProperty), R(1), U8(1), U8(2),
B(Star0),
B(CreateEmptyArrayLiteral), U8(4),
B(Star3),
/* 49 S> */ B(CreateArrayLiteral), U8(3), U8(5), U8(37),
B(Star7),
/* 49 E> */ B(GetIterator), R(7), U8(6), U8(8),
B(Mov), R(0), R(2),
B(LdaZero),
B(Star2),
B(LdaZero),
B(StaInArrayLiteral), R(3), R(2), U8(5),
B(Ldar), R(2),
B(Inc), U8(7),
B(Star2),
/* 49 S> */ B(CreateArrayLiteral), U8(2), U8(8), U8(37),
B(Star6),
/* 49 E> */ B(GetIterator), R(6), U8(9), U8(11),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
B(Star6),
B(LdaNamedProperty), R(6), U8(4), U8(10),
B(Star5),
B(CallProperty0), R(5), R(6), U8(19),
B(Star7),
B(LdaNamedProperty), R(5), U8(3), U8(13),
B(Star4),
B(CallProperty0), R(4), R(5), U8(19),
B(Star6),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(7), U8(1),
B(LdaNamedProperty), R(7), U8(5), U8(21),
B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(6), U8(1),
B(LdaNamedProperty), R(6), U8(4), U8(21),
B(JumpIfToBooleanTrue), U8(18),
B(LdaNamedProperty), R(7), U8(6), U8(12),
B(StaInArrayLiteral), R(4), R(3), U8(17),
B(Ldar), R(3),
B(Inc), U8(16),
B(Star3),
B(LdaNamedProperty), R(6), U8(5), U8(15),
B(StaInArrayLiteral), R(3), R(2), U8(5),
B(Ldar), R(2),
B(Inc), U8(7),
B(Star2),
B(JumpLoop), U8(31), I8(0),
B(LdaSmi), I8(4),
B(StaInArrayLiteral), R(4), R(3), U8(17),
B(Mov), R(4), R(3),
B(CallJSRuntime), U8(%reflect_apply), R(1), U8(3),
B(StaInArrayLiteral), R(3), R(2), U8(5),
B(Mov), R(3), R(2),
/* 39 E> */ B(CallJSRuntime), U8(%reflect_apply), R(0), U8(3),
B(LdaUndefined),
/* 64 S> */ B(Return),
]
......@@ -107,7 +111,6 @@ constant pool: [
ONE_BYTE_INTERNALIZED_STRING_TYPE ["Math"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["max"],
ARRAY_BOILERPLATE_DESCRIPTION_TYPE,
ARRAY_BOILERPLATE_DESCRIPTION_TYPE,
ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],
......
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Spread calls get rewritten to CallRuntime, which should be aware of optional
// chaining.
for (let nullish of [undefined, null]) {
const fn = nullish;
const n = nullish;
const o = {};
assertEquals(fn?.(...[], 1), undefined);
assertEquals(fn?.(...[], ...[]), undefined);
assertEquals(o.method?.(...[], 1), undefined);
assertEquals(n?.method(...[], 1), undefined);
}
......@@ -493,8 +493,6 @@
# https://bugs.chromium.org/p/v8/issues/detail?id=5690
'language/expressions/call/eval-spread': [FAIL],
'language/expressions/call/eval-spread-empty-leading': [FAIL],
'language/expressions/call/eval-spread-empty-trailing': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=7472
'intl402/NumberFormat/currency-digits': [FAIL],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment