Commit f48e7349 authored by Sigurd Schneider's avatar Sigurd Schneider Committed by Commit Bot

Revert "[parsing] inline ArrayLiteral creation for spread calls"

This reverts commit 93fc3841.

Reason for revert: may break node.js integration

Original change's description:
> [parsing] inline ArrayLiteral creation for spread calls
> 
> Instead of using runtime calls to generate the Array Literal passed to
> %reflect_call / %reflect_construct, we create an ArrayLiteral from the
> list of arguments, and perform spreads using the interpreter mechanism for
> spreading in ArrayLiterals (thus, the spreading becomes inline). This
> array literal is still passed to %reflect_call / %reflect_construct as
> before.
> 
> This cuts the runtime for bench-spread-call.js -> testSpread roughly in
> half, and will likely improve further once
> https://chromium-review.googlesource.com/c/v8/v8/+/915364 has landed.
> 
> BUG=v8:7446
> R=​neis@chromium.org, adamk@chromium.org
> 
> Change-Id: I74a6acd3a60aad422e4ac575275c7b567659d8ad
> Reviewed-on: https://chromium-review.googlesource.com/939587
> Commit-Queue: Georg Neis <neis@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#51678}

TBR=adamk@chromium.org,neis@chromium.org,caitp@igalia.com,bmeurer@chromium.org

Change-Id: I4730077591bce0e5e7b2ce7d59678e8b7135cc08
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7446
Reviewed-on: https://chromium-review.googlesource.com/945769Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51682}
parent 9ca27e18
......@@ -57,6 +57,7 @@ enum ContextLookupFlags {
V(REFLECT_CONSTRUCT_INDEX, JSFunction, reflect_construct) \
V(REFLECT_DEFINE_PROPERTY_INDEX, JSFunction, reflect_define_property) \
V(REFLECT_DELETE_PROPERTY_INDEX, JSFunction, reflect_delete_property) \
V(SPREAD_ARGUMENTS_INDEX, JSFunction, spread_arguments) \
V(SPREAD_ITERABLE_INDEX, JSFunction, spread_iterable) \
V(MATH_FLOOR_INDEX, JSFunction, math_floor) \
V(MATH_POW_INDEX, JSFunction, math_pow) \
......
......@@ -12,6 +12,22 @@ var InternalArray = utils.InternalArray;
// -------------------------------------------------------------------
function SpreadArguments() {
var count = arguments.length;
var args = new InternalArray();
for (var i = 0; i < count; ++i) {
var array = arguments[i];
var length = array.length;
for (var j = 0; j < length; ++j) {
args.push(array[j]);
}
}
return args;
}
function SpreadIterable(collection) {
if (IS_NULL_OR_UNDEFINED(collection)) {
throw %make_type_error(kNotIterable, collection);
......@@ -28,6 +44,7 @@ function SpreadIterable(collection) {
// Exports
%InstallToContext([
"spread_arguments", SpreadArguments,
"spread_iterable", SpreadIterable,
]);
......
......@@ -3564,52 +3564,96 @@ bool OnlyLastArgIsSpread(ZoneList<Expression*>* args) {
} // namespace
ArrayLiteral* Parser::ArrayLiteralFromListWithSpread(
ZoneList<Expression*>* Parser::PrepareSpreadArguments(
ZoneList<Expression*>* list) {
// If there's only a single spread argument, a fast path using CallWithSpread
// is taken.
DCHECK_LT(1, list->length());
ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
if (list->length() == 1) {
// Spread-call with single spread argument produces an InternalArray
// containing the values from the array.
//
// Function is called or constructed with the produced array of arguments
//
// EG: Apply(Func, Spread(spread0))
ZoneList<Expression*>* spread_list =
new (zone()) ZoneList<Expression*>(0, zone());
spread_list->Add(list->at(0)->AsSpread()->expression(), zone());
args->Add(factory()->NewCallRuntime(Runtime::kSpreadIterablePrepare,
spread_list, kNoSourcePosition),
zone());
return args;
} else {
// Spread-call with multiple arguments produces array literals for each
// sequences of unspread arguments, and converts each spread iterable to
// an Internal array. Finally, all of these produced arrays are flattened
// into a single InternalArray, containing the arguments for the call.
//
// EG: Apply(Func, Flatten([unspread0, unspread1], Spread(spread0),
// Spread(spread1), [unspread2, unspread3]))
int i = 0;
int n = list->length();
while (i < n) {
if (!list->at(i)->IsSpread()) {
ZoneList<Expression*>* unspread =
new (zone()) ZoneList<Expression*>(1, zone());
// Push array of unspread parameters
while (i < n && !list->at(i)->IsSpread()) {
unspread->Add(list->at(i++), zone());
}
args->Add(factory()->NewArrayLiteral(unspread, kNoSourcePosition),
zone());
// The arguments of the spread call become a single ArrayLiteral.
int first_spread = 0;
for (; first_spread < list->length() && !list->at(first_spread)->IsSpread();
++first_spread) {
}
if (i == n) break;
}
DCHECK_LT(first_spread, list->length());
return factory()->NewArrayLiteral(list, first_spread, kNoSourcePosition);
// Push eagerly spread argument
ZoneList<Expression*>* spread_list =
new (zone()) ZoneList<Expression*>(1, zone());
spread_list->Add(list->at(i++)->AsSpread()->expression(), zone());
args->Add(factory()->NewCallRuntime(Context::SPREAD_ITERABLE_INDEX,
spread_list, kNoSourcePosition),
zone());
}
list = new (zone()) ZoneList<Expression*>(1, zone());
list->Add(factory()->NewCallRuntime(Context::SPREAD_ARGUMENTS_INDEX, args,
kNoSourcePosition),
zone());
return list;
}
UNREACHABLE();
}
Expression* Parser::SpreadCall(Expression* function,
ZoneList<Expression*>* args_list, int pos,
ZoneList<Expression*>* args, int pos,
Call::PossiblyEval is_possibly_eval) {
// Handle this case in BytecodeGenerator.
if (OnlyLastArgIsSpread(args_list)) {
return factory()->NewCall(function, args_list, pos);
if (OnlyLastArgIsSpread(args)) {
return factory()->NewCall(function, args, pos);
}
if (function->IsSuperCallReference()) {
// Super calls
// $super_constructor = %_GetSuperConstructor(<this-function>)
// %reflect_construct($super_constructor, args, new.target)
ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());
args = PrepareSpreadArguments(args);
ZoneList<Expression*>* tmp = new (zone()) ZoneList<Expression*>(1, zone());
tmp->Add(function->AsSuperCallReference()->this_function_var(), zone());
args->Add(factory()->NewCallRuntime(Runtime::kInlineGetSuperConstructor,
tmp, pos),
zone());
args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
Expression* super_constructor = factory()->NewCallRuntime(
Runtime::kInlineGetSuperConstructor, tmp, pos);
args->InsertAt(0, super_constructor, zone());
args->Add(function->AsSuperCallReference()->new_target_var(), zone());
return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args,
pos);
} else {
ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());
args = PrepareSpreadArguments(args);
if (function->IsProperty()) {
// Method calls
if (function->AsProperty()->IsSuperAccess()) {
Expression* home = ThisExpression(kNoSourcePosition);
args->Add(function, zone());
args->Add(home, zone());
args->InsertAt(0, function, zone());
args->InsertAt(1, home, zone());
} else {
Variable* temp = NewTemporary(ast_value_factory()->empty_string());
VariableProxy* obj = factory()->NewVariableProxy(temp);
......@@ -3618,29 +3662,28 @@ Expression* Parser::SpreadCall(Expression* function,
kNoSourcePosition);
function = factory()->NewProperty(
assign_obj, function->AsProperty()->key(), kNoSourcePosition);
args->Add(function, zone());
args->InsertAt(0, function, zone());
obj = factory()->NewVariableProxy(temp);
args->Add(obj, zone());
args->InsertAt(1, obj, zone());
}
} else {
// Non-method calls
args->Add(function, zone());
args->Add(factory()->NewUndefinedLiteral(kNoSourcePosition), zone());
args->InsertAt(0, function, zone());
args->InsertAt(1, factory()->NewUndefinedLiteral(kNoSourcePosition),
zone());
}
args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
return factory()->NewCallRuntime(Context::REFLECT_APPLY_INDEX, args, pos);
}
}
Expression* Parser::SpreadCallNew(Expression* function,
ZoneList<Expression*>* args_list, int pos) {
if (OnlyLastArgIsSpread(args_list)) {
ZoneList<Expression*>* args, int pos) {
if (OnlyLastArgIsSpread(args)) {
// Handle in BytecodeGenerator.
return factory()->NewCallNew(function, args_list, pos);
return factory()->NewCallNew(function, args, pos);
}
ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
args->Add(function, zone());
args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
args = PrepareSpreadArguments(args);
args->InsertAt(0, function, zone());
return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);
}
......
......@@ -502,7 +502,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
Expression* CloseTemplateLiteral(TemplateLiteralState* state, int start,
Expression* tag);
ArrayLiteral* ArrayLiteralFromListWithSpread(ZoneList<Expression*>* list);
ZoneList<Expression*>* PrepareSpreadArguments(ZoneList<Expression*>* list);
Expression* SpreadCall(Expression* function, ZoneList<Expression*>* args,
int pos, Call::PossiblyEval is_possibly_eval);
Expression* SpreadCallNew(Expression* function, ZoneList<Expression*>* args,
......
......@@ -794,5 +794,23 @@ RUNTIME_FUNCTION(Runtime_ArrayIndexOf) {
return Smi::FromInt(-1);
}
RUNTIME_FUNCTION(Runtime_SpreadIterablePrepare) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(Object, spread, 0);
// Iterate over the spread if we need to.
if (spread->IterationHasObservableEffects()) {
Handle<JSFunction> spread_iterable_function = isolate->spread_iterable();
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, spread,
Execution::Call(isolate, spread_iterable_function,
isolate->factory()->undefined_value(), 1, &spread));
}
return *spread;
}
} // namespace internal
} // namespace v8
......@@ -51,7 +51,8 @@ namespace internal {
F(ArrayIsArray, 1, 1) \
F(ArraySpeciesConstructor, 1, 1) \
F(ArrayIncludes_Slow, 3, 1) \
F(ArrayIndexOf, 3, 1)
F(ArrayIndexOf, 3, 1) \
F(SpreadIterablePrepare, 1, 1)
#define FOR_EACH_INTRINSIC_ATOMICS(F) \
F(AtomicsExchange, 3, 1) \
......
......@@ -618,7 +618,7 @@ TEST(BytecodeGraphBuilderCallRuntime) {
{factory->true_value(), BytecodeGraphTester::NewObject("[1, 2, 3]")}},
{"function f(arg0) { return %Add(arg0, 2) }\nf(1)",
{factory->NewNumberFromInt(5), factory->NewNumberFromInt(3)}},
{"function f(arg0) { return %spread_iterable(arg0).length }\nf([])",
{"function f(arg0) { return %spread_arguments(arg0).length }\nf([])",
{factory->NewNumberFromInt(3),
BytecodeGraphTester::NewObject("[1, 2, 3]")}},
};
......
......@@ -65,9 +65,9 @@ handlers: [
snippet: "
Math.max(0, ...[1, 2, 3], 4);
"
frame size: 11
frame size: 6
parameter count: 1
bytecode array length: 109
bytecode array length: 51
bytecodes: [
/* 30 E> */ B(StackCheck),
/* 34 S> */ B(LdaGlobal), U8(0), U8(0),
......@@ -75,34 +75,16 @@ bytecodes: [
B(LdaNamedProperty), R(0), U8(1), U8(2),
B(Star), R(1),
B(CreateArrayLiteral), U8(2), U8(4), U8(37),
B(Star), R(3),
B(CreateArrayLiteral), U8(3), U8(5), U8(37),
B(Star), R(4),
B(CallJSRuntime), U8(%spread_iterable), R(4), U8(1),
B(Star), R(4),
/* 49 S> */ B(CreateArrayLiteral), U8(3), U8(5), U8(37),
B(Star), R(9),
B(LdaNamedProperty), R(9), U8(4), U8(6),
B(Star), R(10),
B(CallProperty0), R(10), R(9), U8(8),
B(CreateArrayLiteral), U8(4), U8(6), U8(37),
B(Star), R(5),
B(CallJSRuntime), U8(%spread_arguments), R(3), U8(3),
B(Star), R(3),
B(Mov), R(0), R(2),
B(Mov), R(4), R(5),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
B(Star), R(8),
B(LdaNamedProperty), R(8), U8(5), U8(10),
B(Star), R(7),
B(CallProperty0), R(7), R(8), U8(12),
B(Star), R(6),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(6), U8(1),
B(LdaNamedProperty), R(6), U8(6), U8(14),
B(JumpIfToBooleanTrue), U8(16),
B(LdaNamedProperty), R(6), U8(7), U8(16),
B(Star), R(6),
B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
B(JumpLoop), U8(30), I8(0),
B(LdaSmi), I8(4),
B(Star), R(6),
B(Mov), R(4), R(5),
B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
B(Mov), R(5), R(3),
B(CallJSRuntime), U8(%reflect_apply), R(1), U8(3),
B(LdaUndefined),
/* 64 S> */ B(Return),
......@@ -112,10 +94,7 @@ constant pool: [
ONE_BYTE_INTERNALIZED_STRING_TYPE ["max"],
TUPLE2_TYPE,
TUPLE2_TYPE,
SYMBOL_TYPE,
ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],
TUPLE2_TYPE,
]
handlers: [
]
......
......@@ -84,9 +84,9 @@ snippet: "
class A { constructor(...args) { this.args = args; } }
new A(0, ...[1, 2, 3], 4);
"
frame size: 11
frame size: 6
parameter count: 1
bytecode array length: 124
bytecode array length: 66
bytecodes: [
/* 30 E> */ B(StackCheck),
B(LdaTheHole),
......@@ -101,33 +101,15 @@ bytecodes: [
B(Mov), R(4), R(0),
B(Mov), R(0), R(1),
/* 89 S> */ B(CreateArrayLiteral), U8(2), U8(1), U8(37),
B(Star), R(3),
B(CreateArrayLiteral), U8(3), U8(2), U8(37),
B(Star), R(4),
B(CallJSRuntime), U8(%spread_iterable), R(4), U8(1),
B(Star), R(4),
/* 101 S> */ B(CreateArrayLiteral), U8(3), U8(2), U8(37),
B(Star), R(9),
B(LdaNamedProperty), R(9), U8(4), U8(3),
B(Star), R(10),
B(CallProperty0), R(10), R(9), U8(5),
B(Mov), R(4), R(5),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
B(Star), R(8),
B(LdaNamedProperty), R(8), U8(5), U8(7),
B(Star), R(7),
B(CallProperty0), R(7), R(8), U8(9),
B(Star), R(6),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(6), U8(1),
B(LdaNamedProperty), R(6), U8(6), U8(11),
B(JumpIfToBooleanTrue), U8(16),
B(LdaNamedProperty), R(6), U8(7), U8(13),
B(Star), R(6),
B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
B(JumpLoop), U8(30), I8(0),
B(LdaSmi), I8(4),
B(Star), R(6),
B(Mov), R(4), R(5),
B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
B(Mov), R(5), R(3),
B(CreateArrayLiteral), U8(4), U8(3), U8(37),
B(Star), R(5),
B(CallJSRuntime), U8(%spread_arguments), R(3), U8(3),
B(Star), R(3),
B(CallJSRuntime), U8(%reflect_construct), R(2), U8(2),
B(LdaUndefined),
/* 116 S> */ B(Return),
......@@ -137,10 +119,7 @@ constant pool: [
SHARED_FUNCTION_INFO_TYPE,
TUPLE2_TYPE,
TUPLE2_TYPE,
SYMBOL_TYPE,
ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],
TUPLE2_TYPE,
]
handlers: [
]
......
......@@ -91,9 +91,9 @@ snippet: "
test = new B(1, 2, 3).constructor;
})();
"
frame size: 13
frame size: 8
parameter count: 1
bytecode array length: 121
bytecode array length: 60
bytecodes: [
B(CreateRestParameter),
B(Star), R(2),
......@@ -103,32 +103,13 @@ bytecodes: [
/* 140 S> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(closure), U8(1),
B(Star), R(4),
B(CreateArrayLiteral), U8(0), U8(0), U8(37),
/* 152 S> */ B(Star), R(6),
B(LdaNamedProperty), R(2), U8(1), U8(1),
B(Star), R(12),
B(CallProperty0), R(12), R(2), U8(3),
B(Mov), R(2), R(11),
B(Mov), R(6), R(7),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
B(Star), R(10),
B(LdaNamedProperty), R(10), U8(2), U8(5),
B(Star), R(9),
B(CallProperty0), R(9), R(10), U8(7),
B(Star), R(8),
B(JumpIfJSReceiver), U8(7),
B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(8), U8(1),
B(LdaNamedProperty), R(8), U8(3), U8(9),
B(JumpIfToBooleanTrue), U8(16),
B(LdaNamedProperty), R(8), U8(4), U8(11),
B(Star), R(8),
B(CallRuntime), U16(Runtime::kAppendElement), R(7), U8(2),
B(JumpLoop), U8(30), I8(0),
B(LdaSmi), I8(1),
B(Star), R(8),
B(Mov), R(6), R(7),
B(CallRuntime), U16(Runtime::kAppendElement), R(7), U8(2),
B(Mov), R(7), R(5),
B(Star), R(5),
/* 152 E> */ B(CallJSRuntime), U8(%spread_iterable), R(2), U8(1),
B(Star), R(6),
B(CreateArrayLiteral), U8(1), U8(1), U8(37),
B(Star), R(7),
B(CallJSRuntime), U8(%spread_arguments), R(5), U8(3),
B(Star), R(5),
B(Mov), R(0), R(6),
/* 140 E> */ B(CallJSRuntime), U8(%reflect_construct), R(4), U8(3),
B(Star), R(4),
......@@ -141,10 +122,7 @@ bytecodes: [
]
constant pool: [
TUPLE2_TYPE,
SYMBOL_TYPE,
ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],
TUPLE2_TYPE,
]
handlers: [
]
......
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