[turbofan] Remove unnecessary prototype checks for element access.

We don't need to add stability dependencies on JSObject prototypes when
storing to an element, because we do the map check (and thereby guard
the elements kind) and we also properly deoptimize on holes if the array
protector is not usable.

R=verwaest@chromium.org
BUG=chromium:616709

Review-Url: https://codereview.chromium.org/2198833002
Cr-Commit-Position: refs/heads/master@{#38355}

src/compiler/access-info.cc

@@ -61,11 +61,8 @@ std::ostream& operator<<(std::ostream& os, AccessMode access_mode) {
 ElementAccessInfo::ElementAccessInfo() {}
 
 ElementAccessInfo::ElementAccessInfo(MapList const& receiver_maps,
-                                     ElementsKind elements_kind,
-                                     MaybeHandle<JSObject> holder)
-    : elements_kind_(elements_kind),
-      holder_(holder),
-      receiver_maps_(receiver_maps) {}
+                                     ElementsKind elements_kind)
+    : elements_kind_(elements_kind), receiver_maps_(receiver_maps) {}
 
 // static
 PropertyAccessInfo PropertyAccessInfo::NotFound(MapList const& receiver_maps,
@@ -182,29 +179,8 @@ bool AccessInfoFactory::ComputeElementAccessInfo(
     Handle<Map> map, AccessMode access_mode, ElementAccessInfo* access_info) {
   // Check if it is safe to inline element access for the {map}.
   if (!CanInlineElementAccess(map)) return false;
-
   ElementsKind const elements_kind = map->elements_kind();
-
-  // Certain (monomorphic) stores need a prototype chain check because shape
-  // changes could allow callbacks on elements in the chain that are not
-  // compatible with monomorphic keyed stores.
-  MaybeHandle<JSObject> holder;
-  if (access_mode == AccessMode::kStore && map->prototype()->IsJSObject()) {
-    for (PrototypeIterator i(map); !i.IsAtEnd(); i.Advance()) {
-      Handle<JSReceiver> prototype =
-          PrototypeIterator::GetCurrent<JSReceiver>(i);
-      if (!prototype->IsJSObject()) return false;
-      // TODO(bmeurer): We do not currently support unstable prototypes.
-      // We might want to revisit the way we handle certain keyed stores
-      // because this whole prototype chain check is essential a hack,
-      // and I'm not sure that it is correct at all with dictionaries in
-      // the prototype chain.
-      if (!prototype->map()->is_stable()) return false;
-      holder = Handle<JSObject>::cast(prototype);
-    }
-  }
-
-  *access_info = ElementAccessInfo(MapList{map}, elements_kind, holder);
+  *access_info = ElementAccessInfo(MapList{map}, elements_kind);
   return true;
 }
 

src/compiler/access-info.h

@@ -35,10 +35,8 @@ typedef std::vector<std::pair<Handle<Map>, Handle<Map>>> MapTransitionList;
 class ElementAccessInfo final {
  public:
   ElementAccessInfo();
-  ElementAccessInfo(MapList const& receiver_maps, ElementsKind elements_kind,
-                    MaybeHandle<JSObject> holder);
+  ElementAccessInfo(MapList const& receiver_maps, ElementsKind elements_kind);
 
-  MaybeHandle<JSObject> holder() const { return holder_; }
   ElementsKind elements_kind() const { return elements_kind_; }
   MapList const& receiver_maps() const { return receiver_maps_; }
   MapTransitionList& transitions() { return transitions_; }
@@ -46,7 +44,6 @@ class ElementAccessInfo final {
 
  private:
   ElementsKind elements_kind_;
-  MaybeHandle<JSObject> holder_;
   MapList receiver_maps_;
   MapTransitionList transitions_;
 };

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

@@ -581,14 +581,6 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
         }
       }
 
-      // Certain stores need a prototype chain check because shape changes
-      // could allow callbacks on elements in the prototype chain that are
-      // not compatible with (monomorphic) keyed stores.
-      Handle<JSObject> holder;
-      if (access_info.holder().ToHandle(&holder)) {
-        AssumePrototypesStable(receiver_maps, native_context, holder);
-      }
-
       // Access the actual element.
       ValueEffectControl continuation = BuildElementAccess(
           this_receiver, this_index, this_value, this_effect, this_control,
@@ -956,12 +948,6 @@ JSNativeContextSpecialization::BuildElementAccess(
     Node* receiver, Node* index, Node* value, Node* effect, Node* control,
     Handle<Context> native_context, ElementAccessInfo const& access_info,
     AccessMode access_mode, KeyedAccessStoreMode store_mode) {
-  // Determine actual holder and perform prototype chain checks.
-  Handle<JSObject> holder;
-  if (access_info.holder().ToHandle(&holder)) {
-    AssumePrototypesStable(access_info.receiver_maps(), native_context, holder);
-  }
-
   // TODO(bmeurer): We currently specialize based on elements kind. We should
   // also be able to properly support strings and other JSObjects here.
   ElementsKind elements_kind = access_info.elements_kind();
@@ -1133,16 +1119,7 @@ JSNativeContextSpecialization::BuildElementAccess(
         // Perform the hole check on the result.
         CheckTaggedHoleMode mode = CheckTaggedHoleMode::kNeverReturnHole;
         // Check if we are allowed to turn the hole into undefined.
-        // TODO(bmeurer): We might check the JSArray map from a different
-        // context here; may need reinvestigation.
-        if (receiver_maps.size() == 1 &&
-            receiver_maps[0].is_identical_to(
-                handle(isolate()->get_initial_js_array_map(elements_kind))) &&
-            isolate()->IsFastArrayConstructorPrototypeChainIntact()) {
-          // Add a code dependency on the array protector cell.
-          dependencies()->AssumePrototypeMapsStable(
-              receiver_maps[0], isolate()->initial_object_prototype());
-          dependencies()->AssumePropertyCell(factory()->array_protector());
+        if (CanTreatHoleAsUndefined(receiver_maps, native_context)) {
           // Turn the hole into undefined.
           mode = CheckTaggedHoleMode::kConvertHoleToUndefined;
         }
@@ -1152,16 +1129,7 @@ JSNativeContextSpecialization::BuildElementAccess(
         // Perform the hole check on the result.
         CheckFloat64HoleMode mode = CheckFloat64HoleMode::kNeverReturnHole;
         // Check if we are allowed to return the hole directly.
-        // TODO(bmeurer): We might check the JSArray map from a different
-        // context here; may need reinvestigation.
-        if (receiver_maps.size() == 1 &&
-            receiver_maps[0].is_identical_to(
-                handle(isolate()->get_initial_js_array_map(elements_kind))) &&
-            isolate()->IsFastArrayConstructorPrototypeChainIntact()) {
-          // Add a code dependency on the array protector cell.
-          dependencies()->AssumePrototypeMapsStable(
-              receiver_maps[0], isolate()->initial_object_prototype());
-          dependencies()->AssumePropertyCell(factory()->array_protector());
+        if (CanTreatHoleAsUndefined(receiver_maps, native_context)) {
           // Return the signaling NaN hole directly if all uses are truncating.
           mode = CheckFloat64HoleMode::kAllowReturnHole;
         }
@@ -1258,6 +1226,42 @@ void JSNativeContextSpecialization::AssumePrototypesStable(
   }
 }
 
+bool JSNativeContextSpecialization::CanTreatHoleAsUndefined(
+    std::vector<Handle<Map>> const& receiver_maps,
+    Handle<Context> native_context) {
+  // Check if the array prototype chain is intact.
+  if (!isolate()->IsFastArrayConstructorPrototypeChainIntact()) return false;
+
+  // Make sure both the initial Array and Object prototypes are stable.
+  Handle<JSObject> initial_array_prototype(
+      native_context->initial_array_prototype(), isolate());
+  Handle<JSObject> initial_object_prototype(
+      native_context->initial_object_prototype(), isolate());
+  if (!initial_array_prototype->map()->is_stable() ||
+      !initial_object_prototype->map()->is_stable()) {
+    return false;
+  }
+
+  // Check if all {receiver_maps} either have the initial Array.prototype
+  // or the initial Object.prototype as their prototype, as those are
+  // guarded by the array protector cell.
+  for (Handle<Map> map : receiver_maps) {
+    if (map->prototype() != *initial_array_prototype &&
+        map->prototype() != *initial_object_prototype) {
+      return false;
+    }
+  }
+
+  // Install code dependencies on the prototype maps.
+  for (Handle<Map> map : receiver_maps) {
+    dependencies()->AssumePrototypeMapsStable(map, initial_object_prototype);
+  }
+
+  // Install code dependency on the array protector cell.
+  dependencies()->AssumePropertyCell(factory()->array_protector());
+  return true;
+}
+
 bool JSNativeContextSpecialization::ExtractReceiverMaps(
     Node* receiver, Node* effect, FeedbackNexus const& nexus,
     MapHandleList* receiver_maps) {

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

@@ -127,6 +127,12 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
                               Handle<Context> native_context,
                               Handle<JSObject> holder);
 
+  // Checks if we can turn the hole into undefined when loading an element
+  // from an object with one of the {receiver_maps}; sets up appropriate
+  // code dependencies and might use the array protector cell.
+  bool CanTreatHoleAsUndefined(std::vector<Handle<Map>> const& receiver_maps,
+                               Handle<Context> native_context);
+
   // Extract receiver maps from {nexus} and filter based on {receiver} if
   // possible.
   bool ExtractReceiverMaps(Node* receiver, Node* effect,

test/mjsunit/regress/regress-crbug-616709-1.js

+// Copyright 2016 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
+
+// Make the Object prototype have dictionary properties.
+for (var i = 0; i < 2000; i++) {
+  Object.prototype['X'+i] = true;
+}
+
+function boom(a1) {
+  return a1[0];
+}
+
+var a = new Array(1);
+a[0] = 0.1;
+boom(a);
+boom(a);
+%OptimizeFunctionOnNextCall(boom);
+boom(a);

test/mjsunit/regress/regress-crbug-616709-2.js

+// Copyright 2016 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
+
+// Make the Array prototype have dictionary properties.
+for (var i = 0; i < 2000; i++) {
+  Array.prototype['X'+i] = true;
+}
+
+function boom(a1) {
+  return a1[0];
+}
+
+var a = new Array(1);
+a[0] = 0.1;
+boom(a);
+boom(a);
+%OptimizeFunctionOnNextCall(boom);
+boom(a);