[turbo] Reduce CallWithSpread where iteration is not observable.

Where the arguments have already been inlined, we can replace these
calls with a direct call. We have to make sure that the iteration over
the arguments is not observable.

Also factor out the large chunk of logic shared with
ReduceJSConstructWithSpread.

BUG=v8:5932

Change-Id: I6c4fac670028fbd8ca82c4474d4392231573bc49
Reviewed-on: https://chromium-review.googlesource.com/439329Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43039}

src/compiler/js-call-reducer.cc

@@ -26,6 +26,8 @@ Reduction JSCallReducer::Reduce(Node* node) {
       return ReduceJSConstructWithSpread(node);
     case IrOpcode::kJSCall:
       return ReduceJSCall(node);
+    case IrOpcode::kJSCallWithSpread:
+      return ReduceJSCallWithSpread(node);
     default:
       break;
   }
@@ -423,6 +425,79 @@ Reduction JSCallReducer::ReduceCallApiFunction(
   return Changed(node);
 }
 
+Reduction JSCallReducer::ReduceSpreadCall(Node* node, int arity) {
+  DCHECK(node->opcode() == IrOpcode::kJSCallWithSpread ||
+         node->opcode() == IrOpcode::kJSConstructWithSpread);
+
+  // Do check to make sure we can actually avoid iteration.
+  if (!isolate()->initial_array_iterator_prototype_map()->is_stable()) {
+    return NoChange();
+  }
+
+  Node* spread = NodeProperties::GetValueInput(node, arity);
+
+  // Check if spread is an arguments object, and {node} is the only value user
+  // of spread (except for value uses in frame states).
+  if (spread->opcode() != IrOpcode::kJSCreateArguments) return NoChange();
+  for (Edge edge : spread->use_edges()) {
+    if (edge.from()->opcode() == IrOpcode::kStateValues) continue;
+    if (!NodeProperties::IsValueEdge(edge)) continue;
+    if (edge.from() == node) continue;
+    return NoChange();
+  }
+
+  // Get to the actual frame state from which to extract the arguments;
+  // we can only optimize this in case the {node} was already inlined into
+  // some other function (and same for the {spread}).
+  CreateArgumentsType type = CreateArgumentsTypeOf(spread->op());
+  Node* frame_state = NodeProperties::GetFrameStateInput(spread);
+  Node* outer_state = frame_state->InputAt(kFrameStateOuterStateInput);
+  if (outer_state->opcode() != IrOpcode::kFrameState) return NoChange();
+  FrameStateInfo outer_info = OpParameter<FrameStateInfo>(outer_state);
+  if (outer_info.type() == FrameStateType::kArgumentsAdaptor) {
+    // Need to take the parameters from the arguments adaptor.
+    frame_state = outer_state;
+  }
+  FrameStateInfo state_info = OpParameter<FrameStateInfo>(frame_state);
+  int start_index = 0;
+  if (type == CreateArgumentsType::kMappedArguments) {
+    // Mapped arguments (sloppy mode) cannot be handled if they are aliased.
+    Handle<SharedFunctionInfo> shared;
+    if (!state_info.shared_info().ToHandle(&shared)) return NoChange();
+    if (shared->internal_formal_parameter_count() != 0) return NoChange();
+  } else if (type == CreateArgumentsType::kRestParameter) {
+    Handle<SharedFunctionInfo> shared;
+    if (!state_info.shared_info().ToHandle(&shared)) return NoChange();
+    start_index = shared->internal_formal_parameter_count();
+
+    // Only check the array iterator protector when we have a rest object.
+    if (!isolate()->IsArrayIteratorLookupChainIntact()) return NoChange();
+    // Add a code dependency on the array iterator protector.
+    dependencies()->AssumePropertyCell(factory()->array_iterator_protector());
+  }
+
+  dependencies()->AssumeMapStable(
+      isolate()->initial_array_iterator_prototype_map());
+
+  node->RemoveInput(arity--);
+
+  // Add the actual parameters to the {node}, skipping the receiver.
+  Node* const parameters = frame_state->InputAt(kFrameStateParametersInput);
+  for (int i = start_index + 1; i < state_info.parameter_count(); ++i) {
+    node->InsertInput(graph()->zone(), static_cast<int>(++arity),
+                      parameters->InputAt(i));
+  }
+
+  if (node->opcode() == IrOpcode::kJSCallWithSpread) {
+    NodeProperties::ChangeOp(
+        node, javascript()->Call(arity + 1, 7, VectorSlotPair()));
+  } else {
+    NodeProperties::ChangeOp(
+        node, javascript()->Construct(arity + 2, 7, VectorSlotPair()));
+  }
+  return Changed(node);
+}
+
 Reduction JSCallReducer::ReduceJSCall(Node* node) {
   DCHECK_EQ(IrOpcode::kJSCall, node->opcode());
   CallParameters const& p = CallParametersOf(node->op());
@@ -585,6 +660,15 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
   return NoChange();
 }
 
+Reduction JSCallReducer::ReduceJSCallWithSpread(Node* node) {
+  DCHECK_EQ(IrOpcode::kJSCallWithSpread, node->opcode());
+  CallWithSpreadParameters const& p = CallWithSpreadParametersOf(node->op());
+  DCHECK_LE(3u, p.arity());
+  int arity = static_cast<int>(p.arity() - 1);
+
+  return ReduceSpreadCall(node, arity);
+}
+
 Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
   DCHECK_EQ(IrOpcode::kJSConstruct, node->opcode());
   ConstructParameters const& p = ConstructParametersOf(node->op());
@@ -707,70 +791,7 @@ Reduction JSCallReducer::ReduceJSConstructWithSpread(Node* node) {
   DCHECK_LE(3u, p.arity());
   int arity = static_cast<int>(p.arity() - 2);
 
-  // Do check to make sure we can actually avoid iteration.
-  if (!isolate()->initial_array_iterator_prototype_map()->is_stable()) {
-    return NoChange();
-  }
-
-  Node* spread = NodeProperties::GetValueInput(node, arity);
-
-  // Check if spread is an arguments object, and {node} is the only value user
-  // of spread (except for value uses in frame states).
-  if (spread->opcode() != IrOpcode::kJSCreateArguments) return NoChange();
-  for (Edge edge : spread->use_edges()) {
-    if (edge.from()->opcode() == IrOpcode::kStateValues) continue;
-    if (!NodeProperties::IsValueEdge(edge)) continue;
-    if (edge.from() == node) continue;
-    return NoChange();
-  }
-
-  // Get to the actual frame state from which to extract the arguments;
-  // we can only optimize this in case the {node} was already inlined into
-  // some other function (and same for the {spread}).
-  CreateArgumentsType type = CreateArgumentsTypeOf(spread->op());
-  Node* frame_state = NodeProperties::GetFrameStateInput(spread);
-  Node* outer_state = frame_state->InputAt(kFrameStateOuterStateInput);
-  if (outer_state->opcode() != IrOpcode::kFrameState) return NoChange();
-  FrameStateInfo outer_info = OpParameter<FrameStateInfo>(outer_state);
-  if (outer_info.type() == FrameStateType::kArgumentsAdaptor) {
-    // Need to take the parameters from the arguments adaptor.
-    frame_state = outer_state;
-  }
-  FrameStateInfo state_info = OpParameter<FrameStateInfo>(frame_state);
-  int start_index = 0;
-  if (type == CreateArgumentsType::kMappedArguments) {
-    // Mapped arguments (sloppy mode) cannot be handled if they are aliased.
-    Handle<SharedFunctionInfo> shared;
-    if (!state_info.shared_info().ToHandle(&shared)) return NoChange();
-    if (shared->internal_formal_parameter_count() != 0) return NoChange();
-  } else if (type == CreateArgumentsType::kRestParameter) {
-    Handle<SharedFunctionInfo> shared;
-    if (!state_info.shared_info().ToHandle(&shared)) return NoChange();
-    start_index = shared->internal_formal_parameter_count();
-
-    // Only check the array iterator protector when we have a rest object.
-    if (!isolate()->IsArrayIteratorLookupChainIntact()) return NoChange();
-    // Add a code dependency on the array iterator protector.
-    dependencies()->AssumePropertyCell(factory()->array_iterator_protector());
-  }
-
-  dependencies()->AssumeMapStable(
-      isolate()->initial_array_iterator_prototype_map());
-
-  // Remove the spread input from the {node}.
-  node->RemoveInput(arity--);
-
-  // Add the actual parameters to the {node}, skipping the receiver.
-  Node* const parameters = frame_state->InputAt(kFrameStateParametersInput);
-  for (int i = start_index + 1; i < state_info.parameter_count(); ++i) {
-    node->InsertInput(graph()->zone(), static_cast<int>(++arity),
-                      parameters->InputAt(i));
-  }
-
-  NodeProperties::ChangeOp(
-      node, javascript()->Construct(arity + 2, 7, VectorSlotPair()));
-
-  return Changed(node);
+  return ReduceSpreadCall(node, arity);
 }
 
 Graph* JSCallReducer::graph() const { return jsgraph()->graph(); }

src/compiler/js-call-reducer.h

@@ -55,9 +55,11 @@ class JSCallReducer final : public AdvancedReducer {
   Reduction ReduceFunctionPrototypeCall(Node* node);
   Reduction ReduceFunctionPrototypeHasInstance(Node* node);
   Reduction ReduceObjectPrototypeGetProto(Node* node);
+  Reduction ReduceSpreadCall(Node* node, int arity);
   Reduction ReduceJSConstruct(Node* node);
   Reduction ReduceJSConstructWithSpread(Node* node);
   Reduction ReduceJSCall(Node* node);
+  Reduction ReduceJSCallWithSpread(Node* node);
 
   enum HolderLookup { kHolderNotFound, kHolderIsReceiver, kHolderFound };
 

test/mjsunit/es6/call-with-spread-modify-array-iterator.js

+// Copyright 2017 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.
+
+// Flags: --allow-natives-syntax
+
+(function modifyArrayIterator() {
+  'use strict';
+
+  function maxWithZero(...args) {
+    return Math.max(0, ...args);
+  }
+
+  function testMax(x, y) {
+    return maxWithZero(x, y);
+  }
+
+  testMax(1, 2);
+  testMax(1, 2);
+  % OptimizeFunctionOnNextCall(testMax);
+  var r = testMax(1, 2);
+
+  assertEquals(2, r);
+
+  Object.defineProperty(Array.prototype, Symbol.iterator, {
+    value: function*
+        () {
+          yield 3;
+          yield 4;
+        },
+    configurable: true
+  });
+
+  var r2 = testMax(1, 2);
+
+  assertEquals(4, r2);
+})();

test/mjsunit/es6/call-with-spread-modify-next.js

+// Copyright 2017 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.
+
+// Flags: --allow-natives-syntax
+
+(function modifyNext() {
+  'use strict';
+
+  var a = [];
+  var ai = a[Symbol.iterator]();
+
+  var original_next = ai.__proto__['next'];
+
+  function maxWithZero(...args) {
+    return Math.max(0, ...args);
+  }
+
+  function testMax(x, y) {
+    return maxWithZero(x, y);
+  }
+
+  testMax(1, 2);
+  testMax(1, 2);
+  % OptimizeFunctionOnNextCall(testMax);
+  var r = testMax(1, 2);
+
+  assertEquals(2, r);
+
+  var called = 0;
+  Object.defineProperty(ai.__proto__, 'next', {
+    get: function() {
+      called++;
+      return original_next;
+    }
+  });
+
+  var r2 = testMax(1, 2);
+
+  assertEquals(3, called);
+  assertEquals(2, r2);
+})();

test/mjsunit/es6/call-with-spread.js

+// Copyright 2017 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.
+
+// Flags: --allow-natives-syntax
+
+(function() {
+  'use strict';
+
+  function testBaselineAndOpt(func) {
+    func(-1, -2);
+    func(-1, -2);
+    % OptimizeFunctionOnNextCall(func);
+    return func(-1, -2);
+  }
+
+  // Rest parameters
+
+  function RestMax(...args) {
+    return Math.max(...args);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return RestMax(x, y);
+  });
+  assertEquals(r, -1);
+
+  function RestMaxWithZero(...args) {
+    return Math.max(0, ...args);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return RestMaxWithZero(x, y);
+  });
+  assertEquals(r, 0);
+
+  function RestMaxOneArg(x, ...args) {
+    return Math.max(-10, ...args);
+  }
+
+  r = testBaselineAndOpt(function(x, y) {
+    return RestMaxOneArg(x, y);
+  });
+  assertEquals(r, -2);
+
+  // Strict Arguments Object
+
+  function ArgumentsMax() {
+    return Math.max(...arguments);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return ArgumentsMax(x, y);
+  });
+  assertEquals(r, -1);
+
+  function ArgumentsMaxWithZero() {
+    return Math.max(0, ...arguments);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return ArgumentsMaxWithZero(x, y);
+  });
+  assertEquals(r, 0);
+
+  function ArgumentsMaxOneArg(x) {
+    return Math.max(-10, ...arguments);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return ArgumentsMaxOneArg(x, y);
+  });
+  assertEquals(r, -1);
+
+})();
+
+(function() {
+  function testBaselineAndOpt(func) {
+    func(-1, -2);
+    func(-1, -2);
+    % OptimizeFunctionOnNextCall(func);
+    return func(-1, -2);
+  }
+
+  // Sloppy Arguments Object
+
+  function ArgumentsMax() {
+    return Math.max(...arguments);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return ArgumentsMax(x, y);
+  });
+  assertEquals(r, -1);
+
+  function ArgumentsMaxWithZero() {
+    return Math.max(0, ...arguments);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return ArgumentsMaxWithZero(x, y);
+  });
+  assertEquals(r, 0);
+
+  function ArgumentsMaxOneArg(x) {
+    return Math.max(-10, ...arguments);
+  }
+
+  var r = testBaselineAndOpt(function(x, y) {
+    return ArgumentsMaxOneArg(x, y);
+  });
+  assertEquals(r, -1);
+})();