[TurboFan] Array.prototype.reduce[Right] was missing a deopt point

We need a deopt point for the case when we fail to find an initial
element from which to begin the reduction step.

Bug: v8:7384
Change-Id: I5e476ddc433be690577677b018639c4c0c70809b
Reviewed-on: https://chromium-review.googlesource.com/906508Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51146}

src/builtins/builtins-array-gen.cc

@@ -2293,6 +2293,22 @@ TF_BUILTIN(ArrayReduceLoopContinuation, ArrayBuiltinsAssembler) {
       MissingPropertyMode::kSkip);
 }
 
+TF_BUILTIN(ArrayReducePreLoopEagerDeoptContinuation, ArrayBuiltinsAssembler) {
+  Node* context = Parameter(Descriptor::kContext);
+  Node* receiver = Parameter(Descriptor::kReceiver);
+  Node* callbackfn = Parameter(Descriptor::kCallbackFn);
+  Node* len = Parameter(Descriptor::kLength);
+
+  Callable stub(
+      Builtins::CallableFor(isolate(), Builtins::kArrayReduceLoopContinuation));
+  // Simulate starting the loop at 0, but ensuring that the accumulator is
+  // the hole. The continuation stub will search for the initial non-hole
+  // element, rightly throwing an exception if not found.
+  Return(CallStub(stub, context, receiver, callbackfn, UndefinedConstant(),
+                  TheHoleConstant(), receiver, SmiConstant(0), len,
+                  UndefinedConstant()));
+}
+
 TF_BUILTIN(ArrayReduceLoopEagerDeoptContinuation, ArrayBuiltinsAssembler) {
   Node* context = Parameter(Descriptor::kContext);
   Node* receiver = Parameter(Descriptor::kReceiver);
@@ -2383,6 +2399,23 @@ TF_BUILTIN(ArrayReduceRightLoopContinuation, ArrayBuiltinsAssembler) {
       ForEachDirection::kReverse);
 }
 
+TF_BUILTIN(ArrayReduceRightPreLoopEagerDeoptContinuation,
+           ArrayBuiltinsAssembler) {
+  Node* context = Parameter(Descriptor::kContext);
+  Node* receiver = Parameter(Descriptor::kReceiver);
+  Node* callbackfn = Parameter(Descriptor::kCallbackFn);
+  Node* len = Parameter(Descriptor::kLength);
+
+  Callable stub(Builtins::CallableFor(
+      isolate(), Builtins::kArrayReduceRightLoopContinuation));
+  // Simulate starting the loop at 0, but ensuring that the accumulator is
+  // the hole. The continuation stub will search for the initial non-hole
+  // element, rightly throwing an exception if not found.
+  Return(CallStub(stub, context, receiver, callbackfn, UndefinedConstant(),
+                  TheHoleConstant(), receiver, SmiConstant(0), len,
+                  UndefinedConstant()));
+}
+
 TF_BUILTIN(ArrayReduceRightLoopEagerDeoptContinuation, ArrayBuiltinsAssembler) {
   Node* context = Parameter(Descriptor::kContext);
   Node* receiver = Parameter(Descriptor::kReceiver);

src/builtins/builtins-definitions.h

@@ -312,6 +312,7 @@ namespace internal {
   /* ES6 #sec-array.prototype.reduce */                                        \
   TFS(ArrayReduceLoopContinuation, kReceiver, kCallbackFn, kThisArg,           \
       kAccumulator, kObject, kInitialK, kLength, kTo)                          \
+  TFJ(ArrayReducePreLoopEagerDeoptContinuation, 2, kCallbackFn, kLength)       \
   TFJ(ArrayReduceLoopEagerDeoptContinuation, 4, kCallbackFn, kInitialK,        \
       kLength, kAccumulator)                                                   \
   TFJ(ArrayReduceLoopLazyDeoptContinuation, 4, kCallbackFn, kInitialK,         \
@@ -320,6 +321,7 @@ namespace internal {
   /* ES6 #sec-array.prototype.reduceRight */                                   \
   TFS(ArrayReduceRightLoopContinuation, kReceiver, kCallbackFn, kThisArg,      \
       kAccumulator, kObject, kInitialK, kLength, kTo)                          \
+  TFJ(ArrayReduceRightPreLoopEagerDeoptContinuation, 2, kCallbackFn, kLength)  \
   TFJ(ArrayReduceRightLoopEagerDeoptContinuation, 4, kCallbackFn, kInitialK,   \
       kLength, kAccumulator)                                                   \
   TFJ(ArrayReduceRightLoopLazyDeoptContinuation, 4, kCallbackFn, kInitialK,    \

src/builtins/builtins.cc

@@ -226,8 +226,10 @@ bool Builtins::IsLazy(int index) {
     case kArrayEveryLoopLazyDeoptContinuation:     // https://crbug.com/v8/6786.
     case kArrayFilterLoopEagerDeoptContinuation:   // https://crbug.com/v8/6786.
     case kArrayFilterLoopLazyDeoptContinuation:    // https://crbug.com/v8/6786.
+    case kArrayReducePreLoopEagerDeoptContinuation:
     case kArrayReduceLoopEagerDeoptContinuation:   // https://crbug.com/v8/6786.
     case kArrayReduceLoopLazyDeoptContinuation:    // https://crbug.com/v8/6786.
+    case kArrayReduceRightPreLoopEagerDeoptContinuation:
     case kArrayReduceRightLoopEagerDeoptContinuation:
     case kArrayReduceRightLoopLazyDeoptContinuation:
     case kArraySomeLoopEagerDeoptContinuation:  // https://crbug.com/v8/6786.

src/compiler/js-call-reducer.cc

@@ -1066,22 +1066,20 @@ Reduction JSCallReducer::ReduceArrayReduce(Handle<JSFunction> function,
       left ? simplified()->NumberAdd() : simplified()->NumberSubtract();
   Node* k = initial_index;
 
-  Builtins::Name builtin_lazy =
-      left ? Builtins::kArrayReduceLoopLazyDeoptContinuation
-           : Builtins::kArrayReduceRightLoopLazyDeoptContinuation;
-
-  Builtins::Name builtin_eager =
-      left ? Builtins::kArrayReduceLoopEagerDeoptContinuation
-           : Builtins::kArrayReduceRightLoopEagerDeoptContinuation;
-
-  const std::vector<Node*> checkpoint_params({receiver, fncallback, k,
-                                              original_length,
-                                              jsgraph()->UndefinedConstant()});
-  const int stack_parameters = static_cast<int>(checkpoint_params.size());
-  Node* check_frame_state = CreateJavaScriptBuiltinContinuationFrameState(
-      jsgraph(), function, builtin_lazy, node->InputAt(0), context,
-      checkpoint_params.data(), stack_parameters - 1, outer_frame_state,
-      ContinuationFrameStateMode::LAZY);
+  Node* check_frame_state;
+  {
+    Builtins::Name builtin_lazy =
+        left ? Builtins::kArrayReduceLoopLazyDeoptContinuation
+             : Builtins::kArrayReduceRightLoopLazyDeoptContinuation;
+    const std::vector<Node*> checkpoint_params(
+        {receiver, fncallback, k, original_length,
+         jsgraph()->UndefinedConstant()});
+    const int stack_parameters = static_cast<int>(checkpoint_params.size());
+    check_frame_state = CreateJavaScriptBuiltinContinuationFrameState(
+        jsgraph(), function, builtin_lazy, node->InputAt(0), context,
+        checkpoint_params.data(), stack_parameters - 1, outer_frame_state,
+        ContinuationFrameStateMode::LAZY);
+  }
   Node* check_fail = nullptr;
   Node* check_throw = nullptr;
   // Check whether the given callback function is callable. Note that
@@ -1093,21 +1091,28 @@ Reduction JSCallReducer::ReduceArrayReduce(Handle<JSFunction> function,
   // Set initial accumulator value
   Node* cur = jsgraph()->TheHoleConstant();
 
-  Node* initial_element_frame_state =
-      CreateJavaScriptBuiltinContinuationFrameState(
-          jsgraph(), function, builtin_eager, node->InputAt(0), context,
-          checkpoint_params.data(), stack_parameters, outer_frame_state,
-          ContinuationFrameStateMode::EAGER);
-
   if (node->op()->ValueInputCount() > 3) {
     cur = NodeProperties::GetValueInput(node, 3);
   } else {
-    // Find first/last non holey element.
+    // Find first/last non holey element. In case the search fails, we need a
+    // deopt continuation.
+    Builtins::Name builtin_eager =
+        left ? Builtins::kArrayReducePreLoopEagerDeoptContinuation
+             : Builtins::kArrayReduceRightPreLoopEagerDeoptContinuation;
+    const std::vector<Node*> checkpoint_params(
+        {receiver, fncallback, original_length});
+    const int stack_parameters = static_cast<int>(checkpoint_params.size());
+    Node* find_first_element_frame_state =
+        CreateJavaScriptBuiltinContinuationFrameState(
+            jsgraph(), function, builtin_eager, node->InputAt(0), context,
+            checkpoint_params.data(), stack_parameters, outer_frame_state,
+            ContinuationFrameStateMode::EAGER);
+
     Node* vloop = k = WireInLoopStart(k, &control, &effect);
     Node* loop = control;
     Node* eloop = effect;
     effect = graph()->NewNode(common()->Checkpoint(),
-                              initial_element_frame_state, effect, control);
+                              find_first_element_frame_state, effect, control);
     Node* continue_test =
         left ? graph()->NewNode(simplified()->NumberLessThan(), k,
                                 original_length)
@@ -1161,6 +1166,9 @@ Reduction JSCallReducer::ReduceArrayReduce(Handle<JSFunction> function,
   control = if_true;
 
   {
+    Builtins::Name builtin_eager =
+        left ? Builtins::kArrayReduceLoopEagerDeoptContinuation
+             : Builtins::kArrayReduceRightLoopEagerDeoptContinuation;
     const std::vector<Node*> checkpoint_params(
         {receiver, fncallback, k, original_length, curloop});
     const int stack_parameters = static_cast<int>(checkpoint_params.size());
@@ -1204,6 +1212,9 @@ Reduction JSCallReducer::ReduceArrayReduce(Handle<JSFunction> function,
 
   Node* next_cur;
   {
+    Builtins::Name builtin_lazy =
+        left ? Builtins::kArrayReduceLoopLazyDeoptContinuation
+             : Builtins::kArrayReduceRightLoopLazyDeoptContinuation;
     const std::vector<Node*> checkpoint_params(
         {receiver, fncallback, next_k, original_length, curloop});
     const int stack_parameters = static_cast<int>(checkpoint_params.size());

test/mjsunit/optimized-reduce.js

+// Copyright 2018 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 --expose-gc --turbo-inline-array-builtins
+// Flags: --opt --no-always-opt
+
+// Make sure we gracefully handle the case of an empty array in
+// optimized code.
+(function() {
+  var nothingThere = function(only_holes) {
+    var a = [1,2,,3];  // holey smi array.
+    if (only_holes) {
+      a = [,,,];  // also a holey smi array.
+    }
+    return a.reduce((r,v,i,o)=>r+v);
+  }
+  nothingThere();
+  nothingThere();
+  %OptimizeFunctionOnNextCall(nothingThere);
+  assertThrows(() => nothingThere(true));
+})();
+
+// An error generated inside the callback includes reduce in it's
+// stack trace.
+(function() {
+  var re = /Array\.reduce/;
+  var alwaysThrows = function() {
+    var b = [,,,];
+    var result = 0;
+    var callback = function(r,v,i,o) {
+      return r + v;
+    };
+    b.reduce(callback);
+  }
+  try {
+    alwaysThrows();
+  } catch (e) {
+    assertTrue(re.exec(e.stack) !== null);
+  }
+  try { alwaysThrows(); } catch (e) {}
+  try { alwaysThrows(); } catch (e) {}
+  %OptimizeFunctionOnNextCall(alwaysThrows);
+  try {
+    alwaysThrows();
+  } catch (e) {
+    assertTrue(re.exec(e.stack) !== null);
+  }
+})();

test/mjsunit/optimized-reduceright.js

+// Copyright 2018 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 --expose-gc --turbo-inline-array-builtins
+// Flags: --opt --no-always-opt
+
+// Make sure we gracefully handle the case of an empty array in
+// optimized code.
+(function() {
+  var nothingThere = function(only_holes) {
+    var a = [1,2,,3];  // holey smi array.
+    if (only_holes) {
+      a = [,,,];  // also a holey smi array.
+    }
+    return a.reduceRight((r,v,i,o)=>r+v);
+  }
+  nothingThere();
+  nothingThere();
+  %OptimizeFunctionOnNextCall(nothingThere);
+  assertThrows(() => nothingThere(true));
+})();
+
+// An error generated inside the callback includes reduce in it's
+// stack trace.
+(function() {
+  var re = /Array\.reduceRight/;
+  var alwaysThrows = function() {
+    var b = [,,,];
+    var result = 0;
+    var callback = function(r,v,i,o) {
+      return r + v;
+    };
+    b.reduceRight(callback);
+  }
+  try {
+    alwaysThrows();
+  } catch (e) {
+    assertTrue(re.exec(e.stack) !== null);
+  }
+  try { alwaysThrows(); } catch (e) {}
+  try { alwaysThrows(); } catch (e) {}
+  %OptimizeFunctionOnNextCall(alwaysThrows);
+  try {
+    alwaysThrows();
+  } catch (e) {
+    assertTrue(re.exec(e.stack) !== null);
+  }
+})();