[turbofan] Generalized OOB support for KeyedLoadIC.

This extends the support in TurboFan and the ICs for OOB loads to also
apply to typed arrays and receivers whose prototype chain is protected
by the "no elements" protector (aka the Array protector). TurboFan will
generate code to materialize undefined instead when it sees a load that
has the OOB bit set and add an appropriate code dependency on the global
protector. For typed arrays it doesn't even need to check the global
protector since elements are never looked up in the prototype chain
for typed arrays.

In the simple micro-benchmark from the bug we go from

  testInBounds: 103 ms.
  testOutOfBounds: 289 ms.

to

  testInBounds: 103 ms.
  testOutOfBounds: 102 ms.

which fixes the 3x slowdown and thus addresses the performance cliff. In
general it's still beneficial to make sure that you don't access out of
bounds, especially once we introduce a bounds check elimination pass to
TurboFan.

This also seems to improve the jQuery benchmark on the Speedometer test
suite by like 1-2% on average. And the SixSpeed rest benchmarks go from

  rest-es5: 25 ms.
  rest-es6: 23 ms.

to

  rest-es5: 6 ms.
  rest-es6: 4 ms.

so a solid 5.7x improvement there.

Bug: v8:6936, v8:7014, v8:7027
Change-Id: Ie99699c69cc40057512e72fd40ae28107216c423
Reviewed-on: https://chromium-review.googlesource.com/750089
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49095}

src/code-stub-assembler.cc

@@ -805,18 +805,35 @@ void CodeStubAssembler::BranchIfPrototypesHaveNoElements(
     Node* prototype = LoadMapPrototype(map);
     GotoIf(IsNull(prototype), definitely_no_elements);
     Node* prototype_map = LoadMap(prototype);
+    Node* prototype_instance_type = LoadMapInstanceType(prototype_map);
+
     // Pessimistically assume elements if a Proxy, Special API Object,
     // or JSValue wrapper is found on the prototype chain. After this
     // instance type check, it's not necessary to check for interceptors or
     // access checks.
-    GotoIf(Int32LessThanOrEqual(LoadMapInstanceType(prototype_map),
+    Label if_custom(this, Label::kDeferred), if_notcustom(this);
+    Branch(Int32LessThanOrEqual(prototype_instance_type,
                                 Int32Constant(LAST_CUSTOM_ELEMENTS_RECEIVER)),
-           possibly_elements);
-    Node* prototype_elements = LoadElements(prototype);
-    var_map.Bind(prototype_map);
-    GotoIf(WordEqual(prototype_elements, empty_fixed_array), &loop_body);
-    Branch(WordEqual(prototype_elements, empty_slow_element_dictionary),
-           &loop_body, possibly_elements);
+           &if_custom, &if_notcustom);
+
+    BIND(&if_custom);
+    {
+      // For string JSValue wrappers we still support the checks as long
+      // as they wrap the empty string.
+      GotoIfNot(InstanceTypeEqual(prototype_instance_type, JS_VALUE_TYPE),
+                possibly_elements);
+      Node* prototype_value = LoadJSValueValue(prototype);
+      Branch(IsEmptyString(prototype_value), &if_notcustom, possibly_elements);
+    }
+
+    BIND(&if_notcustom);
+    {
+      Node* prototype_elements = LoadElements(prototype);
+      var_map.Bind(prototype_map);
+      GotoIf(WordEqual(prototype_elements, empty_fixed_array), &loop_body);
+      Branch(WordEqual(prototype_elements, empty_slow_element_dictionary),
+             &loop_body, possibly_elements);
+    }
   }
 }
 

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

@@ -153,12 +153,10 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
                       Handle<FunctionTemplateInfo> function_template_info);
 
   // Construct the appropriate subgraph for element access.
-  ValueEffectControl BuildElementAccess(Node* receiver, Node* index,
-                                        Node* value, Node* effect,
-                                        Node* control,
-                                        ElementAccessInfo const& access_info,
-                                        AccessMode access_mode,
-                                        KeyedAccessStoreMode store_mode);
+  ValueEffectControl BuildElementAccess(
+      Node* receiver, Node* index, Node* value, Node* effect, Node* control,
+      ElementAccessInfo const& access_info, AccessMode access_mode,
+      KeyedAccessLoadMode load_mode, KeyedAccessStoreMode store_mode);
 
   // Construct appropriate subgraph to load from a String.
   Node* BuildIndexedStringLoad(Node* receiver, Node* index, Node* length,

src/feedback-vector.cc

@@ -859,27 +859,19 @@ Name* KeyedStoreICNexus::FindFirstName() const {
 }
 
 KeyedAccessLoadMode KeyedLoadICNexus::GetKeyedAccessLoadMode() const {
-  KeyedAccessLoadMode mode = STANDARD_LOAD;
   MapHandles maps;
   ObjectHandles handlers;
 
-  if (GetKeyType() == PROPERTY) return mode;
+  if (GetKeyType() == PROPERTY) return STANDARD_LOAD;
 
   ExtractMaps(&maps);
   FindHandlers(&handlers, static_cast<int>(maps.size()));
   for (Handle<Object> const& handler : handlers) {
-    if (handler->IsSmi()) {
-      int const raw_handler = Handle<Smi>::cast(handler)->value();
-      if (LoadHandler::KindBits::decode(raw_handler) ==
-              LoadHandler::kIndexedString &&
-          LoadHandler::AllowOutOfBoundsBits::decode(raw_handler)) {
-        mode = LOAD_IGNORE_OUT_OF_BOUNDS;
-        break;
-      }
-    }
+    KeyedAccessLoadMode mode = LoadHandler::GetKeyedAccessLoadMode(*handler);
+    if (mode != STANDARD_LOAD) return mode;
   }
 
-  return mode;
+  return STANDARD_LOAD;
 }
 
 KeyedAccessStoreMode KeyedStoreICNexus::GetKeyedAccessStoreMode() const {

src/ic/accessor-assembler.cc

@@ -230,12 +230,12 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
         IsSetWord<LoadHandler::IsJsArrayBits>(handler_word);
     Node* elements_kind =
         DecodeWord32FromWord<LoadHandler::ElementsKindBits>(handler_word);
-    Label if_hole(this), unimplemented_elements_kind(this);
-    Label* out_of_bounds = miss;
+    Label if_hole(this), unimplemented_elements_kind(this),
+        if_oob(this, Label::kDeferred);
     EmitElementLoad(holder, elements, elements_kind, intptr_index,
                     is_jsarray_condition, &if_hole, &rebox_double,
-                    &var_double_value, &unimplemented_elements_kind,
-                    out_of_bounds, miss, exit_point);
+                    &var_double_value, &unimplemented_elements_kind, &if_oob,
+                    miss, exit_point);
 
     BIND(&unimplemented_elements_kind);
     {
@@ -245,6 +245,28 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
       Goto(miss);
     }
 
+    BIND(&if_oob);
+    {
+      Comment("out of bounds elements access");
+      Label return_undefined(this);
+
+      // Check if we're allowed to handle OOB accesses.
+      Node* allow_out_of_bounds =
+          IsSetWord<LoadHandler::AllowOutOfBoundsBits>(handler_word);
+      GotoIfNot(allow_out_of_bounds, miss);
+
+      // For typed arrays we never lookup elements in the prototype chain.
+      GotoIf(IsJSTypedArray(holder), &return_undefined);
+
+      // For all other receivers we need to check that the prototype chain
+      // doesn't contain any elements.
+      BranchIfPrototypesHaveNoElements(LoadMap(holder), &return_undefined,
+                                       miss);
+
+      BIND(&return_undefined);
+      exit_point->Return(UndefinedConstant());
+    }
+
     BIND(&if_hole);
     {
       Comment("convert hole");

src/ic/handler-configuration-inl.h

@@ -100,18 +100,22 @@ Handle<Smi> LoadHandler::LoadNonExistent(Isolate* isolate) {
 Handle<Smi> LoadHandler::LoadElement(Isolate* isolate,
                                      ElementsKind elements_kind,
                                      bool convert_hole_to_undefined,
-                                     bool is_js_array) {
-  int config = KindBits::encode(kElement) |
-               ElementsKindBits::encode(elements_kind) |
-               ConvertHoleBits::encode(convert_hole_to_undefined) |
-               IsJsArrayBits::encode(is_js_array);
+                                     bool is_js_array,
+                                     KeyedAccessLoadMode load_mode) {
+  int config =
+      KindBits::encode(kElement) |
+      AllowOutOfBoundsBits::encode(load_mode == LOAD_IGNORE_OUT_OF_BOUNDS) |
+      ElementsKindBits::encode(elements_kind) |
+      ConvertHoleBits::encode(convert_hole_to_undefined) |
+      IsJsArrayBits::encode(is_js_array);
   return handle(Smi::FromInt(config), isolate);
 }
 
 Handle<Smi> LoadHandler::LoadIndexedString(Isolate* isolate,
-                                           bool allow_out_of_bounds) {
-  int config = KindBits::encode(kIndexedString) |
-               AllowOutOfBoundsBits::encode(allow_out_of_bounds);
+                                           KeyedAccessLoadMode load_mode) {
+  int config =
+      KindBits::encode(kIndexedString) |
+      AllowOutOfBoundsBits::encode(load_mode == LOAD_IGNORE_OUT_OF_BOUNDS);
   return handle(Smi::FromInt(config), isolate);
 }
 

src/ic/handler-configuration.cc

@@ -209,6 +209,20 @@ Handle<Object> LoadHandler::LoadFullChain(Isolate* isolate,
   return handler_array;
 }
 
+// static
+KeyedAccessLoadMode LoadHandler::GetKeyedAccessLoadMode(Object* handler) {
+  DisallowHeapAllocation no_gc;
+  if (handler->IsSmi()) {
+    int const raw_handler = Smi::cast(handler)->value();
+    Kind const kind = KindBits::decode(raw_handler);
+    if ((kind == kElement || kind == kIndexedString) &&
+        AllowOutOfBoundsBits::decode(raw_handler)) {
+      return LOAD_IGNORE_OUT_OF_BOUNDS;
+    }
+  }
+  return STANDARD_LOAD;
+}
+
 // static
 Handle<Object> StoreHandler::StoreElementTransition(
     Isolate* isolate, Handle<Map> receiver_map, Handle<Map> transition,

src/ic/handler-configuration.h

@@ -69,21 +69,22 @@ class LoadHandler {
   // Make sure we don't overflow the smi.
   STATIC_ASSERT(FieldOffsetBits::kNext <= kSmiValueSize);
 
+  //
+  // Encoding when KindBits contains kElement or kIndexedString.
+  //
+  class AllowOutOfBoundsBits : public BitField<bool, KindBits::kNext, 1> {};
+
   //
   // Encoding when KindBits contains kElement.
   //
-  class IsJsArrayBits : public BitField<bool, KindBits::kNext, 1> {};
+  class IsJsArrayBits : public BitField<bool, AllowOutOfBoundsBits::kNext, 1> {
+  };
   class ConvertHoleBits : public BitField<bool, IsJsArrayBits::kNext, 1> {};
   class ElementsKindBits
       : public BitField<ElementsKind, ConvertHoleBits::kNext, 8> {};
   // Make sure we don't overflow the smi.
   STATIC_ASSERT(ElementsKindBits::kNext <= kSmiValueSize);
 
-  //
-  // Encoding when KindBits contains kIndexedString.
-  //
-  class AllowOutOfBoundsBits : public BitField<bool, KindBits::kNext, 1> {};
-
   //
   // Encoding when KindBits contains kModuleExport.
   //
@@ -163,11 +164,15 @@ class LoadHandler {
   static inline Handle<Smi> LoadElement(Isolate* isolate,
                                         ElementsKind elements_kind,
                                         bool convert_hole_to_undefined,
-                                        bool is_js_array);
+                                        bool is_js_array,
+                                        KeyedAccessLoadMode load_mode);
 
   // Creates a Smi-handler for loading from a String.
   static inline Handle<Smi> LoadIndexedString(Isolate* isolate,
-                                              bool allow_out_of_bounds);
+                                              KeyedAccessLoadMode load_mode);
+
+  // Decodes the KeyedAccessLoadMode from a {handler}.
+  static KeyedAccessLoadMode GetKeyedAccessLoadMode(Object* handler);
 
  private:
   // Sets DoAccessCheckOnReceiverBits in given Smi-handler. The receiver

src/ic/ic.cc

@@ -996,6 +996,12 @@ static Handle<Object> TryConvertKey(Handle<Object> key, Isolate* isolate) {
   return key;
 }
 
+bool KeyedLoadIC::CanChangeToAllowOutOfBounds(Handle<Map> receiver_map) {
+  Handle<Object> handler;
+  return nexus()->FindHandlerForMap(receiver_map).ToHandle(&handler) &&
+         LoadHandler::GetKeyedAccessLoadMode(*handler) == STANDARD_LOAD;
+}
+
 void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver,
                                     KeyedAccessLoadMode load_mode) {
   Handle<Map> receiver_map(receiver->map(), isolate());
@@ -1041,12 +1047,11 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver,
   // Determine the list of receiver maps that this call site has seen,
   // adding the map that was just encountered.
   if (!AddOneReceiverMapIfMissing(&target_receiver_maps, receiver_map)) {
-    // If the {receiver_map} is a primitive String map, we can only get
-    // here if the access was out of bounds (and the IC was not in that
-    // state already). In that case just go on and update the handler
-    // appropriately below.
-    if (!receiver_map->IsStringMap() ||
-        load_mode != LOAD_IGNORE_OUT_OF_BOUNDS) {
+    // If the {receiver_map} previously had a handler that didn't handle
+    // out-of-bounds access, but can generally handle it, we can just go
+    // on and update the handler appropriately below.
+    if (load_mode != LOAD_IGNORE_OUT_OF_BOUNDS ||
+        !CanChangeToAllowOutOfBounds(receiver_map)) {
       // If the miss wasn't due to an unseen map, a polymorphic stub
       // won't help, use the generic stub.
       TRACE_GENERIC_IC("same map added twice");
@@ -1103,7 +1108,7 @@ Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map,
   if (elements_kind == DICTIONARY_ELEMENTS) {
     TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadElementDH);
     return LoadHandler::LoadElement(isolate(), elements_kind, false,
-                                    is_js_array);
+                                    is_js_array, load_mode);
   }
   DCHECK(IsFastElementsKind(elements_kind) ||
          IsFixedTypedArrayElementsKind(elements_kind));
@@ -1114,7 +1119,8 @@ Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map,
           isolate()->raw_native_context()->GetInitialJSArrayMap(elements_kind);
   TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadElementDH);
   return LoadHandler::LoadElement(isolate(), elements_kind,
-                                  convert_hole_to_undefined, is_js_array);
+                                  convert_hole_to_undefined, is_js_array,
+                                  load_mode);
 }
 
 void KeyedLoadIC::LoadElementPolymorphicHandlers(
@@ -1149,18 +1155,43 @@ bool IsOutOfBoundsAccess(Handle<Object> receiver, uint32_t index) {
     JSArray::cast(*receiver)->length()->ToArrayLength(&length);
   } else if (receiver->IsString()) {
     length = String::cast(*receiver)->length();
-  } else {
+  } else if (receiver->IsJSObject()) {
     length = JSObject::cast(*receiver)->elements()->length();
+  } else {
+    return false;
   }
   return index >= length;
 }
 
 KeyedAccessLoadMode GetLoadMode(Handle<Object> receiver, uint32_t index) {
-  if (receiver->IsString() && IsOutOfBoundsAccess(receiver, index)) {
-    Isolate* isolate = Handle<String>::cast(receiver)->GetIsolate();
-    if (isolate->IsFastArrayConstructorPrototypeChainIntact()) {
+  if (IsOutOfBoundsAccess(receiver, index)) {
+    if (receiver->IsJSTypedArray()) {
+      // For JSTypedArray we never lookup elements in the prototype chain.
       return LOAD_IGNORE_OUT_OF_BOUNDS;
     }
+
+    // For other {receiver}s we need to check the "no elements" protector.
+    Isolate* isolate = Handle<HeapObject>::cast(receiver)->GetIsolate();
+    if (isolate->IsFastArrayConstructorPrototypeChainIntact()) {
+      if (receiver->IsString()) {
+        // ToObject(receiver) will have the initial String.prototype.
+        return LOAD_IGNORE_OUT_OF_BOUNDS;
+      }
+      if (receiver->IsJSObject()) {
+        // For other JSObjects (including JSArrays) we can only continue if
+        // the {receiver}s prototype is either the initial Object.prototype
+        // or the initial Array.prototype, which are both guarded by the
+        // "no elements" protector checked above.
+        Handle<Object> receiver_prototype(
+            JSObject::cast(*receiver)->map()->prototype(), isolate);
+        if (isolate->IsInAnyContext(*receiver_prototype,
+                                    Context::INITIAL_ARRAY_PROTOTYPE_INDEX) ||
+            isolate->IsInAnyContext(*receiver_prototype,
+                                    Context::INITIAL_OBJECT_PROTOTYPE_INDEX)) {
+          return LOAD_IGNORE_OUT_OF_BOUNDS;
+        }
+      }
+    }
   }
   return STANDARD_LOAD;
 }

src/ic/ic.h

@@ -300,6 +300,11 @@ class KeyedLoadIC : public LoadIC {
   void LoadElementPolymorphicHandlers(MapHandles* receiver_maps,
                                       ObjectHandles* handlers,
                                       KeyedAccessLoadMode load_mode);
+
+  // Returns true if the receiver_map has a kElement or kIndexedString
+  // handler in the nexus currently but didn't yet allow out of bounds
+  // accesses.
+  bool CanChangeToAllowOutOfBounds(Handle<Map> receiver_map);
 };