Skip to content

Commit fd75c97

Browse files
sygCommit Bot
syg
authored and
Commit Bot
committed
[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}
1 parent 2dd0296 commit fd75c97

10 files changed

Lines changed: 165 additions & 131 deletions

File tree

‎src/ast/ast.cc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,22 @@ Call::CallType Call::GetCallType() const {
935935
return OTHER_CALL;
936936
}
937937

938+
void Call::ComputeSpreadPosition() {
939+
int arguments_length = arguments_.length();
940+
int first_spread_index = 0;
941+
for (; first_spread_index < arguments_length; first_spread_index++) {
942+
if (arguments_.at(first_spread_index)->IsSpread()) break;
943+
}
944+
SpreadPosition position;
945+
if (first_spread_index == arguments_length - 1) {
946+
position = kHasFinalSpread;
947+
} else {
948+
DCHECK_LT(first_spread_index, arguments_length - 1);
949+
position = kHasNonFinalSpread;
950+
}
951+
bit_field_ |= SpreadPositionField::encode(position);
952+
}
953+
938954
CaseClause::CaseClause(Zone* zone, Expression* label,
939955
const ScopedPtrList<Statement>& statements)
940956
: label_(label), statements_(statements.ToConstVector(), zone) {}

‎src/ast/ast.h‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1635,6 +1635,12 @@ class Call final : public Expression {
16351635
return IsOptionalChainLinkField::decode(bit_field_);
16361636
}
16371637

1638+
enum SpreadPosition { kNoSpread, kHasFinalSpread, kHasNonFinalSpread };
1639+
SpreadPosition spread_position() const {
1640+
return SpreadPositionField::decode(bit_field_);
1641+
}
1642+
1643+
// TODO(syg): Remove this and its users.
16381644
bool only_last_arg_is_spread() {
16391645
return !arguments_.is_empty() && arguments_.last()->IsSpread();
16401646
}
@@ -1669,15 +1675,17 @@ class Call final : public Expression {
16691675
friend Zone;
16701676

16711677
Call(Zone* zone, Expression* expression,
1672-
const ScopedPtrList<Expression>& arguments, int pos,
1678+
const ScopedPtrList<Expression>& arguments, int pos, bool has_spread,
16731679
PossiblyEval possibly_eval, bool optional_chain)
16741680
: Expression(pos, kCall),
16751681
expression_(expression),
16761682
arguments_(arguments.ToConstVector(), zone) {
16771683
bit_field_ |=
16781684
IsPossiblyEvalField::encode(possibly_eval == IS_POSSIBLY_EVAL) |
16791685
IsTaggedTemplateField::encode(false) |
1680-
IsOptionalChainLinkField::encode(optional_chain);
1686+
IsOptionalChainLinkField::encode(optional_chain) |
1687+
SpreadPositionField::encode(kNoSpread);
1688+
if (has_spread) ComputeSpreadPosition();
16811689
}
16821690

16831691
Call(Zone* zone, Expression* expression,
@@ -1688,12 +1696,17 @@ class Call final : public Expression {
16881696
arguments_(arguments.ToConstVector(), zone) {
16891697
bit_field_ |= IsPossiblyEvalField::encode(false) |
16901698
IsTaggedTemplateField::encode(true) |
1691-
IsOptionalChainLinkField::encode(false);
1699+
IsOptionalChainLinkField::encode(false) |
1700+
SpreadPositionField::encode(kNoSpread);
16921701
}
16931702

1703+
// Only valid to be called if there is a spread in arguments_.
1704+
void ComputeSpreadPosition();
1705+
16941706
using IsPossiblyEvalField = Expression::NextBitField<bool, 1>;
16951707
using IsTaggedTemplateField = IsPossiblyEvalField::Next<bool, 1>;
16961708
using IsOptionalChainLinkField = IsTaggedTemplateField::Next<bool, 1>;
1709+
using SpreadPositionField = IsOptionalChainLinkField::Next<SpreadPosition, 2>;
16971710

16981711
Expression* expression_;
16991712
ZonePtrList<Expression> arguments_;
@@ -3064,11 +3077,12 @@ class AstNodeFactory final {
30643077

30653078
Call* NewCall(Expression* expression,
30663079
const ScopedPtrList<Expression>& arguments, int pos,
3080+
bool has_spread,
30673081
Call::PossiblyEval possibly_eval = Call::NOT_EVAL,
30683082
bool optional_chain = false) {
30693083
DCHECK_IMPLIES(possibly_eval == Call::IS_POSSIBLY_EVAL, !optional_chain);
3070-
return zone_->New<Call>(zone_, expression, arguments, pos, possibly_eval,
3071-
optional_chain);
3084+
return zone_->New<Call>(zone_, expression, arguments, pos, has_spread,
3085+
possibly_eval, optional_chain);
30723086
}
30733087

30743088
Call* NewTaggedTemplate(Expression* expression,

‎src/interpreter/bytecode-generator.cc‎

Lines changed: 76 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3111,6 +3111,8 @@ void BytecodeGenerator::BuildCreateArrayLiteral(
31113111
.StoreAccumulatorInRegister(index);
31123112
}
31133113
} else {
3114+
// TODO(v8:11582): Support allocating boilerplates here.
3115+
31143116
// In other cases, we prepare an empty array to be filled in below.
31153117
DCHECK(!elements->is_empty());
31163118
int literal_index = feedback_index(feedback_spec()->AddLiteralSlot());
@@ -5027,17 +5029,30 @@ void BytecodeGenerator::VisitCall(Call* expr) {
50275029
return VisitCallSuper(expr);
50285030
}
50295031

5032+
// We compile the call differently depending on the presence of spreads and
5033+
// their positions.
5034+
//
5035+
// If there is only one spread and it is the final argument, there is a
5036+
// special CallWithSpread bytecode.
5037+
//
5038+
// If there is a non-final spread, we rewrite calls like
5039+
// callee(1, ...x, 2)
5040+
// to
5041+
// %reflect_apply(callee, receiver, [1, ...x, 2])
5042+
const Call::SpreadPosition spread_position = expr->spread_position();
5043+
50305044
// Grow the args list as we visit receiver / arguments to avoid allocating all
50315045
// the registers up-front. Otherwise these registers are unavailable during
50325046
// receiver / argument visiting and we can end up with memory leaks due to
50335047
// registers keeping objects alive.
5034-
Register callee = register_allocator()->NewRegister();
50355048
RegisterList args = register_allocator()->NewGrowableRegisterList();
50365049

5050+
// The callee is the first register in args for ease of calling %reflect_apply
5051+
// if we have a non-final spread. For all other cases it is popped from args
5052+
// before emitting the call below.
5053+
Register callee = register_allocator()->GrowRegisterList(&args);
5054+
50375055
bool implicit_undefined_receiver = false;
5038-
// When a call contains a spread, a Call AST node is only created if there is
5039-
// exactly one spread, and it is the last argument.
5040-
bool is_spread_call = expr->only_last_arg_is_spread();
50415056
bool optimize_as_one_shot = ShouldOptimizeAsOneShot();
50425057

50435058
// TODO(petermarshall): We have a lot of call bytecodes that are very similar,
@@ -5057,7 +5072,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
50575072
}
50585073
case Call::GLOBAL_CALL: {
50595074
// Receiver is undefined for global calls.
5060-
if (!is_spread_call && !optimize_as_one_shot) {
5075+
if (spread_position == Call::kNoSpread && !optimize_as_one_shot) {
50615076
implicit_undefined_receiver = true;
50625077
} else {
50635078
// TODO(leszeks): There's no special bytecode for tail calls or spread
@@ -5093,7 +5108,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
50935108
}
50945109
case Call::OTHER_CALL: {
50955110
// Receiver is undefined for other calls.
5096-
if (!is_spread_call && !optimize_as_one_shot) {
5111+
if (spread_position == Call::kNoSpread && !optimize_as_one_shot) {
50975112
implicit_undefined_receiver = true;
50985113
} else {
50995114
// TODO(leszeks): There's no special bytecode for tail calls or spread
@@ -5142,25 +5157,51 @@ void BytecodeGenerator::VisitCall(Call* expr) {
51425157
BuildIncrementBlockCoverageCounterIfEnabled(right_range);
51435158
}
51445159

5145-
// Evaluate all arguments to the function call and store in sequential args
5146-
// registers.
5147-
VisitArguments(expr->arguments(), &args);
5148-
int receiver_arg_count = implicit_undefined_receiver ? 0 : 1;
5149-
CHECK_EQ(receiver_arg_count + expr->arguments()->length(),
5150-
args.register_count());
5160+
int receiver_arg_count = -1;
5161+
if (spread_position == Call::kHasNonFinalSpread) {
5162+
// If we're building %reflect_apply, build the array literal and put it in
5163+
// the 3rd argument.
5164+
DCHECK(!implicit_undefined_receiver);
5165+
DCHECK_EQ(args.register_count(), 2);
5166+
BuildCreateArrayLiteral(expr->arguments(), nullptr);
5167+
builder()->StoreAccumulatorInRegister(
5168+
register_allocator()->GrowRegisterList(&args));
5169+
} else {
5170+
// If we're not building %reflect_apply and don't need to build an array
5171+
// literal, pop the callee and evaluate all arguments to the function call
5172+
// and store in sequential args registers.
5173+
args = args.PopLeft();
5174+
VisitArguments(expr->arguments(), &args);
5175+
receiver_arg_count = implicit_undefined_receiver ? 0 : 1;
5176+
CHECK_EQ(receiver_arg_count + expr->arguments()->length(),
5177+
args.register_count());
5178+
}
51515179

51525180
// Resolve callee for a potential direct eval call. This block will mutate the
51535181
// callee value.
51545182
if (expr->is_possibly_eval() && expr->arguments()->length() > 0) {
51555183
RegisterAllocationScope inner_register_scope(this);
5184+
RegisterList runtime_call_args = register_allocator()->NewRegisterList(6);
51565185
// Set up arguments for ResolvePossiblyDirectEval by copying callee, source
51575186
// strings and function closure, and loading language and
51585187
// position.
5159-
Register first_arg = args[receiver_arg_count];
5160-
RegisterList runtime_call_args = register_allocator()->NewRegisterList(6);
5188+
5189+
// Move the first arg.
5190+
if (spread_position == Call::kHasNonFinalSpread) {
5191+
int feedback_slot_index =
5192+
feedback_index(feedback_spec()->AddKeyedLoadICSlot());
5193+
Register args_array = args[2];
5194+
builder()
5195+
->LoadLiteral(Smi::FromInt(0))
5196+
.LoadKeyedProperty(args_array, feedback_slot_index)
5197+
.StoreAccumulatorInRegister(runtime_call_args[1]);
5198+
} else {
5199+
// FIXME(v8:5690): Support final spreads for eval.
5200+
DCHECK_GE(receiver_arg_count, 0);
5201+
builder()->MoveRegister(args[receiver_arg_count], runtime_call_args[1]);
5202+
}
51615203
builder()
51625204
->MoveRegister(callee, runtime_call_args[0])
5163-
.MoveRegister(first_arg, runtime_call_args[1])
51645205
.MoveRegister(Register::function_closure(), runtime_call_args[2])
51655206
.LoadLiteral(Smi::FromEnum(language_mode()))
51665207
.StoreAccumulatorInRegister(runtime_call_args[3])
@@ -5177,10 +5218,12 @@ void BytecodeGenerator::VisitCall(Call* expr) {
51775218

51785219
builder()->SetExpressionPosition(expr);
51795220

5180-
if (is_spread_call) {
5221+
if (spread_position == Call::kHasFinalSpread) {
51815222
DCHECK(!implicit_undefined_receiver);
51825223
builder()->CallWithSpread(callee, args,
51835224
feedback_index(feedback_spec()->AddCallICSlot()));
5225+
} else if (spread_position == Call::kHasNonFinalSpread) {
5226+
builder()->CallJSRuntime(Context::REFLECT_APPLY_INDEX, args);
51845227
} else if (optimize_as_one_shot) {
51855228
DCHECK(!implicit_undefined_receiver);
51865229
builder()->CallNoFeedback(callee, args);
@@ -5203,10 +5246,20 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
52035246
SuperCallReference* super = expr->expression()->AsSuperCallReference();
52045247
const ZonePtrList<Expression>* args = expr->arguments();
52055248

5206-
int first_spread_index = 0;
5207-
for (; first_spread_index < args->length(); first_spread_index++) {
5208-
if (args->at(first_spread_index)->IsSpread()) break;
5209-
}
5249+
// We compile the super call differently depending on the presence of spreads
5250+
// and their positions.
5251+
//
5252+
// If there is only one spread and it is the final argument, there is a
5253+
// special ConstructWithSpread bytecode.
5254+
//
5255+
// It there is a non-final spread, we rewrite something like
5256+
// super(1, ...x, 2)
5257+
// to
5258+
// %reflect_construct(constructor, [1, ...x, 2], new_target)
5259+
//
5260+
// That is, we implement (non-last-arg) spreads in super calls via our
5261+
// mechanism for spreads in array literals.
5262+
const Call::SpreadPosition spread_position = expr->spread_position();
52105263

52115264
// Prepare the constructor to the super call.
52125265
Register this_function = VisitForRegisterValue(super->this_function_var());
@@ -5215,14 +5268,7 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
52155268
->LoadAccumulatorWithRegister(this_function)
52165269
.GetSuperConstructor(constructor);
52175270

5218-
if (first_spread_index < expr->arguments()->length() - 1) {
5219-
// We rewrite something like
5220-
// super(1, ...x, 2)
5221-
// to
5222-
// %reflect_construct(constructor, [1, ...x, 2], new_target)
5223-
// That is, we implement (non-last-arg) spreads in super calls via our
5224-
// mechanism for spreads in array literals.
5225-
5271+
if (spread_position == Call::kHasNonFinalSpread) {
52265272
// First generate the array containing all arguments.
52275273
BuildCreateArrayLiteral(args, nullptr);
52285274

@@ -5249,11 +5295,11 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
52495295

52505296
int feedback_slot_index = feedback_index(feedback_spec()->AddCallICSlot());
52515297

5252-
if (first_spread_index == expr->arguments()->length() - 1) {
5298+
if (spread_position == Call::kHasFinalSpread) {
52535299
builder()->ConstructWithSpread(constructor, args_regs,
52545300
feedback_slot_index);
52555301
} else {
5256-
DCHECK_EQ(first_spread_index, expr->arguments()->length());
5302+
DCHECK_EQ(spread_position, Call::kNoSpread);
52575303
// Call construct.
52585304
// TODO(turbofan): For now we do gather feedback on super constructor
52595305
// calls, utilizing the existing machinery to inline the actual call

‎src/parsing/parser-base.h‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,7 @@ class ParserBase {
11861186
BlockT ParseClassStaticBlock(ClassInfo* class_info);
11871187
ObjectLiteralPropertyT ParseObjectPropertyDefinition(
11881188
ParsePropertyInfo* prop_info, bool* has_seen_proto);
1189+
// TODO(syg): Remove has_spread once SpreadCallNew is removed.
11891190
void ParseArguments(
11901191
ExpressionListT* args, bool* has_spread,
11911192
ParsingArrowHeadFlag maybe_arrow = kCertainlyNotArrowHead);
@@ -3388,11 +3389,7 @@ ParserBase<Impl>::ParseLeftHandSideContinuation(ExpressionT result) {
33883389
return result;
33893390
}
33903391

3391-
if (has_spread) {
3392-
result = impl()->SpreadCall(result, args, pos, Call::NOT_EVAL, false);
3393-
} else {
3394-
result = factory()->NewCall(result, args, pos, Call::NOT_EVAL);
3395-
}
3392+
result = factory()->NewCall(result, args, pos, has_spread);
33963393

33973394
maybe_arrow.ValidateExpression();
33983395

@@ -3486,13 +3483,8 @@ ParserBase<Impl>::ParseLeftHandSideContinuation(ExpressionT result) {
34863483
Call::PossiblyEval is_possibly_eval =
34873484
CheckPossibleEvalCall(result, is_optional, scope());
34883485

3489-
if (has_spread) {
3490-
result = impl()->SpreadCall(result, args, pos, is_possibly_eval,
3491-
is_optional);
3492-
} else {
3493-
result = factory()->NewCall(result, args, pos, is_possibly_eval,
3494-
is_optional);
3495-
}
3486+
result = factory()->NewCall(result, args, pos, has_spread,
3487+
is_possibly_eval, is_optional);
34963488

34973489
fni_.RemoveLastFunction();
34983490
break;

‎src/parsing/parser.cc‎

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
6969

7070
args.Add(spread_args);
7171
Expression* super_call_ref = NewSuperCallReference(pos);
72-
call = factory()->NewCall(super_call_ref, args, pos);
72+
constexpr bool has_spread = true;
73+
call = factory()->NewCall(super_call_ref, args, pos, has_spread);
7374
}
7475
body.Add(factory()->NewReturnStatement(call, pos));
7576
}
@@ -3373,47 +3374,10 @@ ArrayLiteral* Parser::ArrayLiteralFromListWithSpread(
33733374
return factory()->NewArrayLiteral(list, first_spread, kNoSourcePosition);
33743375
}
33753376

3376-
Expression* Parser::SpreadCall(Expression* function,
3377-
const ScopedPtrList<Expression>& args_list,
3378-
int pos, Call::PossiblyEval is_possibly_eval,
3379-
bool optional_chain) {
3380-
// Handle this case in BytecodeGenerator.
3381-
if (OnlyLastArgIsSpread(args_list) || function->IsSuperCallReference()) {
3382-
return factory()->NewCall(function, args_list, pos, Call::NOT_EVAL,
3383-
optional_chain);
3384-
}
3385-
3386-
ScopedPtrList<Expression> args(pointer_buffer());
3387-
if (function->IsProperty()) {
3388-
// Method calls
3389-
if (function->AsProperty()->IsSuperAccess()) {
3390-
Expression* home = ThisExpression();
3391-
args.Add(function);
3392-
args.Add(home);
3393-
} else {
3394-
Variable* temp = NewTemporary(ast_value_factory()->empty_string());
3395-
VariableProxy* obj = factory()->NewVariableProxy(temp);
3396-
Assignment* assign_obj = factory()->NewAssignment(
3397-
Token::ASSIGN, obj, function->AsProperty()->obj(), kNoSourcePosition);
3398-
function =
3399-
factory()->NewProperty(assign_obj, function->AsProperty()->key(),
3400-
kNoSourcePosition, optional_chain);
3401-
args.Add(function);
3402-
obj = factory()->NewVariableProxy(temp);
3403-
args.Add(obj);
3404-
}
3405-
} else {
3406-
// Non-method calls
3407-
args.Add(function);
3408-
args.Add(factory()->NewUndefinedLiteral(kNoSourcePosition));
3409-
}
3410-
args.Add(ArrayLiteralFromListWithSpread(args_list));
3411-
return factory()->NewCallRuntime(Context::REFLECT_APPLY_INDEX, args, pos);
3412-
}
3413-
34143377
Expression* Parser::SpreadCallNew(Expression* function,
34153378
const ScopedPtrList<Expression>& args_list,
34163379
int pos) {
3380+
// TODO(syg): Handle all spread cases in BytecodeGenerator.
34173381
if (OnlyLastArgIsSpread(args_list)) {
34183382
// Handle in BytecodeGenerator.
34193383
return factory()->NewCallNew(function, args_list, pos);

‎src/parsing/parser.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -493,10 +493,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
493493

494494
ArrayLiteral* ArrayLiteralFromListWithSpread(
495495
const ScopedPtrList<Expression>& list);
496-
Expression* SpreadCall(Expression* function,
497-
const ScopedPtrList<Expression>& args, int pos,
498-
Call::PossiblyEval is_possibly_eval,
499-
bool optional_chain);
500496
Expression* SpreadCallNew(Expression* function,
501497
const ScopedPtrList<Expression>& args, int pos);
502498
Expression* RewriteSuperCall(Expression* call_expression);

0 commit comments

Comments
 (0)