[turbofan] Add support for updating feedback via CheckBounds

Add support for disallowing speculation upon deoptimize from
a CheckBound node, and use this in the case of array builtins
in js-call-reducer to prevent deoptimization loops.

Bug: v8:7127
Change-Id: I04cf655b10178d2938d2f0ee6b336601fab6463b
Reviewed-on: https://chromium-review.googlesource.com/822195
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50097}

src/compiler/effect-control-linearizer.cc

@@ -1312,9 +1312,11 @@ Node* EffectControlLinearizer::LowerTruncateTaggedToFloat64(Node* node) {
 Node* EffectControlLinearizer::LowerCheckBounds(Node* node, Node* frame_state) {
   Node* index = node->InputAt(0);
   Node* limit = node->InputAt(1);
+  const CheckParameters& params = CheckParametersOf(node->op());
 
   Node* check = __ Uint32LessThan(index, limit);
-  __ DeoptimizeIfNot(DeoptimizeReason::kOutOfBounds, check, frame_state);
+  __ DeoptimizeIfNot(DeoptimizeReason::kOutOfBounds, check, frame_state,
+                     params.feedback());
   return index;
 }
 

src/compiler/js-call-reducer.cc

@@ -902,7 +902,8 @@ Reduction JSCallReducer::ReduceArrayForEach(Handle<JSFunction> function,
                                                receiver_maps, p.feedback()),
                        receiver, effect, control);
 
-  Node* element = SafeLoadElement(kind, receiver, control, &effect, &k);
+  Node* element =
+      SafeLoadElement(kind, receiver, control, &effect, &k, p.feedback());
 
   Node* next_k =
       graph()->NewNode(simplified()->NumberAdd(), k, jsgraph()->Constant(1));
@@ -1109,7 +1110,8 @@ Reduction JSCallReducer::ReduceArrayMap(Handle<JSFunction> function,
                                                receiver_maps, p.feedback()),
                        receiver, effect, control);
 
-  Node* element = SafeLoadElement(kind, receiver, control, &effect, &k);
+  Node* element =
+      SafeLoadElement(kind, receiver, control, &effect, &k, p.feedback());
 
   Node* next_k =
       graph()->NewNode(simplified()->NumberAdd(), k, jsgraph()->OneConstant());
@@ -1311,7 +1313,8 @@ Reduction JSCallReducer::ReduceArrayFilter(Handle<JSFunction> function,
                                                receiver_maps, p.feedback()),
                        receiver, effect, control);
 
-  Node* element = SafeLoadElement(kind, receiver, control, &effect, &k);
+  Node* element =
+      SafeLoadElement(kind, receiver, control, &effect, &k, p.feedback());
 
   Node* next_k =
       graph()->NewNode(simplified()->NumberAdd(), k, jsgraph()->OneConstant());
@@ -1497,7 +1500,8 @@ Reduction JSCallReducer::ReduceArrayFind(Handle<JSFunction> function,
   }
 
   // Load k-th element from receiver.
-  Node* element = SafeLoadElement(kind, receiver, control, &effect, &k);
+  Node* element =
+      SafeLoadElement(kind, receiver, control, &effect, &k, p.feedback());
 
   // Increment k for the next iteration.
   Node* next_k = checkpoint_params[3] =
@@ -1675,14 +1679,15 @@ void JSCallReducer::RewirePostCallbackExceptionEdges(Node* check_throw,
 }
 
 Node* JSCallReducer::SafeLoadElement(ElementsKind kind, Node* receiver,
-                                     Node* control, Node** effect, Node** k) {
+                                     Node* control, Node** effect, Node** k,
+                                     const VectorSlotPair& feedback) {
   // Make sure that the access is still in bounds, since the callback could have
   // changed the array's size.
   Node* length = *effect = graph()->NewNode(
       simplified()->LoadField(AccessBuilder::ForJSArrayLength(kind)), receiver,
       *effect, control);
-  *k = *effect = graph()->NewNode(simplified()->CheckBounds(), *k, length,
+  *k = *effect = graph()->NewNode(simplified()->CheckBounds(feedback), *k,
-                                  *effect, control);
+                                  length, *effect, control);
 
   // Reload the elements pointer before calling the callback, since the previous
   // callback might have resized the array causing the elements buffer to be

src/compiler/js-call-reducer.h

@@ -110,7 +110,8 @@ class JSCallReducer final : public AdvancedReducer {
   // Load receiver[k], first bounding k by receiver array length.
   // k is thusly changed, and the effect is changed as well.
   Node* SafeLoadElement(ElementsKind kind, Node* receiver, Node* control,
-                        Node** effect, Node** k);
+                        Node** effect, Node** k,
+                        const VectorSlotPair& feedback);
 
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }

src/compiler/js-create-lowering.cc

@@ -525,7 +525,7 @@ Reduction JSCreateLowering::ReduceNewArray(Node* node, Node* length,
   // This has to be kept in sync with src/runtime/runtime-array.cc,
   // where this limit is protected.
   length = effect = graph()->NewNode(
-      simplified()->CheckBounds(), length,
+      simplified()->CheckBounds(VectorSlotPair()), length,
       jsgraph()->Constant(JSArray::kInitialMaxFastElementArray), effect,
       control);
 

src/compiler/js-native-context-specialization.cc

@@ -2126,12 +2126,13 @@ JSNativeContextSpecialization::BuildElementAccess(
       // Check that the {index} is a valid array index, we do the actual
       // bounds check below and just skip the store below if it's out of
       // bounds for the {receiver}.
-      index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
+      index = effect = graph()->NewNode(
-                                        jsgraph()->Constant(Smi::kMaxValue),
+          simplified()->CheckBounds(VectorSlotPair()), index,
-                                        effect, control);
+          jsgraph()->Constant(Smi::kMaxValue), effect, control);
     } else {
       // Check that the {index} is in the valid range for the {receiver}.
-      index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
+      index = effect =
+          graph()->NewNode(simplified()->CheckBounds(VectorSlotPair()), index,
                            length, effect, control);
     }
 
@@ -2272,12 +2273,13 @@ JSNativeContextSpecialization::BuildElementAccess(
       // Check that the {index} is a valid array index, we do the actual
       // bounds check below and just skip the store below if it's out of
       // bounds for the {receiver}.
-      index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
+      index = effect = graph()->NewNode(
-                                        jsgraph()->Constant(Smi::kMaxValue),
+          simplified()->CheckBounds(VectorSlotPair()), index,
-                                        effect, control);
+          jsgraph()->Constant(Smi::kMaxValue), effect, control);
     } else {
       // Check that the {index} is in the valid range for the {receiver}.
-      index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
+      index = effect =
+          graph()->NewNode(simplified()->CheckBounds(VectorSlotPair()), index,
                            length, effect, control);
     }
 
@@ -2430,7 +2432,8 @@ JSNativeContextSpecialization::BuildElementAccess(
                                    jsgraph()->Constant(JSObject::kMaxGap))
                 : graph()->NewNode(simplified()->NumberAdd(), length,
                                    jsgraph()->OneConstant());
-        index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
+        index = effect =
+            graph()->NewNode(simplified()->CheckBounds(VectorSlotPair()), index,
                              limit, effect, control);
 
         // Grow {elements} backing store if necessary.
@@ -2492,9 +2495,9 @@ Node* JSNativeContextSpecialization::BuildIndexedStringLoad(
     dependencies()->AssumePropertyCell(factory()->no_elements_protector());
 
     // Ensure that the {index} is a valid String length.
-    index = *effect = graph()->NewNode(simplified()->CheckBounds(), index,
+    index = *effect = graph()->NewNode(
-                                       jsgraph()->Constant(String::kMaxLength),
+        simplified()->CheckBounds(VectorSlotPair()), index,
-                                       *effect, *control);
+        jsgraph()->Constant(String::kMaxLength), *effect, *control);
 
     // Load the single character string from {receiver} or yield
     // undefined if the {index} is not within the valid bounds.
@@ -2515,7 +2518,8 @@ Node* JSNativeContextSpecialization::BuildIndexedStringLoad(
                             vtrue, vfalse, *control);
   } else {
     // Ensure that {index} is less than {receiver} length.
-    index = *effect = graph()->NewNode(simplified()->CheckBounds(), index,
+    index = *effect =
+        graph()->NewNode(simplified()->CheckBounds(VectorSlotPair()), index,
                          length, *effect, *control);
 
     // Return the character from the {receiver} as single character string.

src/compiler/js-typed-lowering.cc

@@ -666,9 +666,9 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
     // has the additional benefit of not holding on to the lazy {frame_state}
     // and thus potentially reduces the number of live ranges and allows for
     // more truncations.
-    length = effect = graph()->NewNode(simplified()->CheckBounds(), length,
+    length = effect = graph()->NewNode(
-                                       jsgraph()->Constant(String::kMaxLength),
+        simplified()->CheckBounds(VectorSlotPair()), length,
-                                       effect, control);
+        jsgraph()->Constant(String::kMaxLength), effect, control);
   } else {
     // Check if we would overflow the allowed maximum string length.
     Node* check =

src/compiler/redundancy-elimination.cc

@@ -129,6 +129,9 @@ bool IsCompatibleCheck(Node const* a, Node const* b) {
     if (a->opcode() == IrOpcode::kCheckInternalizedString &&
         b->opcode() == IrOpcode::kCheckString) {
       // CheckInternalizedString(node) implies CheckString(node)
+    } else if (a->opcode() == IrOpcode::kCheckBounds &&
+               b->opcode() == IrOpcode::kCheckBounds) {
+      // CheckBounds are compatible independent of associated feedback.
     } else {
       return false;
     }

src/compiler/simplified-operator.cc

@@ -128,6 +128,21 @@ ConvertReceiverMode ConvertReceiverModeOf(Operator const* op) {
   return OpParameter<ConvertReceiverMode>(op);
 }
 
+bool operator==(CheckParameters const& lhs, CheckParameters const& rhs) {
+  return lhs.feedback() == rhs.feedback();
+}
+
+size_t hash_value(CheckParameters const& p) { return hash_value(p.feedback()); }
+
+std::ostream& operator<<(std::ostream& os, CheckParameters const& p) {
+  return os << p.feedback();
+}
+
+CheckParameters const& CheckParametersOf(Operator const* op) {
+  CHECK_EQ(IrOpcode::kCheckBounds, op->opcode());
+  return OpParameter<CheckParameters>(op);
+}
+
 size_t hash_value(CheckFloat64HoleMode mode) {
   return static_cast<size_t>(mode);
 }
@@ -648,7 +663,6 @@ DeoptimizeReason DeoptimizeReasonOf(const Operator* op) {
   V(SpeculativeNumberLessThanOrEqual)
 
 #define CHECKED_OP_LIST(V)               \
-  V(CheckBounds, 2, 1)                   \
   V(CheckHeapObject, 1, 1)               \
   V(CheckInternalizedString, 1, 1)       \
   V(CheckNumber, 1, 1)                   \
@@ -673,6 +687,8 @@ DeoptimizeReason DeoptimizeReasonOf(const Operator* op) {
   V(CheckedTaggedToTaggedSigned, 1, 1)   \
   V(CheckedTaggedToTaggedPointer, 1, 1)
 
+#define CHECKED_WITH_FEEDBACK_OP_LIST(V) V(CheckBounds, 2, 1)
+
 struct SimplifiedOperatorGlobalCache final {
 #define PURE(Name, properties, value_input_count, control_input_count)     \
   struct Name##Operator final : public Operator {                          \
@@ -695,6 +711,18 @@ struct SimplifiedOperatorGlobalCache final {
   CHECKED_OP_LIST(CHECKED)
 #undef CHECKED
 
+#define CHECKED_WITH_FEEDBACK(Name, value_input_count, value_output_count) \
+  struct Name##Operator final : public Operator1<CheckParameters> {        \
+    Name##Operator()                                                       \
+        : Operator1<CheckParameters>(                                      \
+              IrOpcode::k##Name, Operator::kFoldable | Operator::kNoThrow, \
+              #Name, value_input_count, 1, 1, value_output_count, 1, 0,    \
+              CheckParameters(VectorSlotPair())) {}                        \
+  };                                                                       \
+  Name##Operator k##Name;
+  CHECKED_WITH_FEEDBACK_OP_LIST(CHECKED_WITH_FEEDBACK)
+#undef CHECKED_WITH_FEEDBACK
+
   template <DeoptimizeReason kDeoptimizeReason>
   struct CheckIfOperator final : public Operator1<DeoptimizeReason> {
     CheckIfOperator()
@@ -940,6 +968,21 @@ GET_FROM_CACHE(FindOrderedHashMapEntryForInt32Key)
 GET_FROM_CACHE(LoadFieldByIndex)
 #undef GET_FROM_CACHE
 
+#define GET_FROM_CACHE_WITH_FEEDBACK(Name, value_input_count,               \
+                                     value_output_count)                    \
+  const Operator* SimplifiedOperatorBuilder::Name(                          \
+      const VectorSlotPair& feedback) {                                     \
+    if (!feedback.IsValid()) {                                              \
+      return &cache_.k##Name;                                               \
+    }                                                                       \
+    return new (zone()) Operator1<CheckParameters>(                         \
+        IrOpcode::k##Name, Operator::kFoldable | Operator::kNoThrow, #Name, \
+        value_input_count, 1, 1, value_output_count, 1, 0,                  \
+        CheckParameters(feedback));                                         \
+  }
+CHECKED_WITH_FEEDBACK_OP_LIST(GET_FROM_CACHE_WITH_FEEDBACK)
+#undef GET_FROM_CACHE_WITH_FEEDBACK
+
 const Operator* SimplifiedOperatorBuilder::RuntimeAbort(BailoutReason reason) {
   return new (zone()) Operator1<BailoutReason>(  // --
       IrOpcode::kRuntimeAbort,                   // opcode
@@ -1299,6 +1342,7 @@ const Operator* SimplifiedOperatorBuilder::TransitionAndStoreNonNumberElement(
 
 #undef PURE_OP_LIST
 #undef SPECULATIVE_NUMBER_BINOP_LIST
+#undef CHECKED_WITH_FEEDBACK_OP_LIST
 #undef CHECKED_OP_LIST
 #undef ACCESS_OP_LIST
 

src/compiler/simplified-operator.h

@@ -92,6 +92,28 @@ ExternalArrayType ExternalArrayTypeOf(const Operator* op) WARN_UNUSED_RESULT;
 // The ConvertReceiverMode is used as parameter by ConvertReceiver operators.
 ConvertReceiverMode ConvertReceiverModeOf(Operator const* op);
 
+// A the parameters for several Check nodes. The {feedback} parameter is
+// optional. If {feedback} references a valid CallIC slot and this MapCheck
+// fails, then speculation on that CallIC slot will be disabled.
+class CheckParameters final {
+ public:
+  explicit CheckParameters(const VectorSlotPair& feedback)
+      : feedback_(feedback) {}
+
+  VectorSlotPair const& feedback() const { return feedback_; }
+
+ private:
+  VectorSlotPair feedback_;
+};
+
+bool operator==(CheckParameters const&, CheckParameters const&);
+
+size_t hash_value(CheckParameters const&);
+
+std::ostream& operator<<(std::ostream&, CheckParameters const&);
+
+CheckParameters const& CheckParametersOf(Operator const*) WARN_UNUSED_RESULT;
+
 enum class CheckFloat64HoleMode : uint8_t {
   kNeverReturnHole,  // Never return the hole (deoptimize instead).
   kAllowReturnHole   // Allow to return the hole (signaling NaN).
@@ -443,7 +465,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* TruncateTaggedPointerToBit();
 
   const Operator* CheckIf(DeoptimizeReason deoptimize_reason);
-  const Operator* CheckBounds();
+  const Operator* CheckBounds(const VectorSlotPair& feedback);
   const Operator* CheckMaps(CheckMapsFlags, ZoneHandleSet<Map>,
                             const VectorSlotPair& = VectorSlotPair());
   const Operator* CompareMaps(ZoneHandleSet<Map>);

test/mjsunit/compiler/deopt-array-builtins.js

@@ -4,7 +4,9 @@
 
 // Flags: --allow-natives-syntax --opt
 
-(function testForEach() {
+/* Test MapCheck behavior */
+
+(function testForEachMapCheck() {
     function f(v,n,o) {
         Object.freeze(o);
     }
@@ -21,7 +23,7 @@
 })();
 
 
-(function testFind() {
+(function testFindMapCheck() {
     function f(v,n,o) {
         Object.freeze(o);
         return false;
@@ -38,7 +40,7 @@
     assertOptimized(g);
 })();
 
-(function testMap() {
+(function testMapMapCheck() {
     function f(v,n,o) {
         Object.freeze(o);
         return false;
@@ -55,7 +57,7 @@
     assertOptimized(g);
 })();
 
-(function testFilter() {
+(function testFilterMapCheck() {
     function f(v,n,o) {
         Object.freeze(o);
         return true;
@@ -71,3 +73,76 @@
     g();
     assertOptimized(g);
 })();
+
+
+/* Test CheckBounds behavior */
+
+(function testForEachCheckBounds() {
+    function f(v,n,o) {
+        o.length=2;
+    }
+    function g() {
+        [1,2,3].forEach(f);
+    }
+    g();
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    assertOptimized(g);
+})();
+
+
+(function testFindCheckBounds() {
+    function f(v,n,o) {
+        o.length=2;
+        return false;
+    }
+    function g() {
+        [1,2,3].find(f);
+    }
+    g();
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    assertOptimized(g);
+})();
+
+(function testMapCheckBounds() {
+    function f(v,n,o) {
+        o.length=2;
+        return false;
+    }
+    function g() {
+        [1,2,3].map(f);
+    }
+    g();
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    assertOptimized(g);
+})();
+
+(function testFilterCheckBounds() {
+    function f(v,n,o) {
+        o.length = 2;
+        return true;
+    }
+    function g() {
+        [1,2,3].filter(f);
+    }
+    g();
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    g();
+    %OptimizeFunctionOnNextCall(g);
+    g();
+    g();
+    assertOptimized(g);
+})();