Skip to content

deps: V8: cherry-pick fd75c97d3f56 #38455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
targos wants to merge 1 commit into from
Closed

deps: V8: cherry-pick fd75c97d3f56 #38455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
deps: V8: cherry-pick fd75c97d3f56 
Original commit message:

    [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 <[email protected]>
    Commit-Queue: Leszek Swirski <[email protected]>
    Reviewed-by: Ross McIlroy <[email protected]>
    Reviewed-by: Leszek Swirski <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#73534}

Refs: v8/v8@fd75c97
  • Loading branch information
@targos
targos committed May 1, 2021
commit a72b7e08be23d68fda054fcee2ff22778b5f7da2
2 changes: 1 addition & 1 deletion common.gypi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.10',
'v8_embedder_string': '-node.11',

##### V8 defaults for Node.js #####

Expand Down
16 changes: 16 additions & 0 deletions deps/v8/src/ast/ast.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
24 changes: 19 additions & 5 deletions deps/v8/src/ast/ast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -1669,15 +1675,17 @@ 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),
arguments_(arguments.ToConstVector(), zone) {
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,
Expand All @@ -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_;
Expand Down Expand Up @@ -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,
Expand Down
106 changes: 76 additions & 30 deletions deps/v8/src/interpreter/bytecode-generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3106,6 +3106,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());
Expand Down Expand Up @@ -5022,17 +5024,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,
Expand All @@ -5052,7 +5067,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
Expand Down Expand Up @@ -5088,7 +5103,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
Expand Down Expand Up @@ -5137,25 +5152,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])
Expand All @@ -5172,10 +5213,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);
Expand All @@ -5198,10 +5241,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());
Expand All @@ -5210,14 +5263,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);

Expand All @@ -5244,11 +5290,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
Expand Down
16 changes: 4 additions & 12 deletions deps/v8/src/parsing/parser-base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -3392,11 +3393,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();

Expand Down Expand Up @@ -3490,13 +3487,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;
Expand Down
42 changes: 3 additions & 39 deletions deps/v8/src/parsing/parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -3371,47 +3372,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);
Expand Down
4 changes: 0 additions & 4 deletions deps/v8/src/parsing/parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading