[ic] Support negative indices for typed array OOB accesses.

Extend the current OOB support for typed arrays to also handle the
negative integer indices in the fast-path. This is safe because in
ECMAScript we never look up integer indexed properties (including
negative indices) on typed arrays in the prototype chain.

This reduces the performance cliff shown in the benchmark on the
relevant bug from

  console.timeEnd: Runtime deopt, 596.185000
  console.timeEnd: Runtime deopt, 1444.289000
  console.timeEnd: Runtime deopt, 1445.191000
  console.timeEnd: Runtime deopt, 1443.008000

to

  console.timeEnd: Runtime deopt, 590.017000
  console.timeEnd: Runtime deopt, 784.899000
  console.timeEnd: Runtime deopt, 792.428000
  console.timeEnd: Runtime deopt, 786.740000

which corresponds to a 2x improvement overall. It's not for free,
especially not in this benchmark, but the cliff isn't as bad as
it was previously.

Bug: v8:7027
Change-Id: Icf8a7ee87bb7ebc54f82c1b9166fc5e78c12bc0e
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/911574Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51222}

src/code-stub-assembler.cc

@@ -7952,13 +7952,13 @@ void CodeStubAssembler::EmitElementStore(Node* object, Node* key, Node* value,
         parameter_mode);
 
     if (store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
-      // Skip the store if we write beyond the length.
-      GotoIfNot(IntPtrLessThan(key, length), &done);
-      // ... but bailout if the key is negative.
+      // Skip the store if we write beyond the length or
+      // to a property with a negative integer index.
+      GotoIfNot(UintPtrLessThan(key, length), &done);
     } else {
       DCHECK_EQ(STANDARD_STORE, store_mode);
+      GotoIfNot(UintPtrLessThan(key, length), bailout);
     }
-    GotoIfNot(UintPtrLessThan(key, length), bailout);
 
     // Backing store = external_pointer + base_pointer.
     Node* external_pointer =

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

@@ -2104,12 +2104,18 @@ JSNativeContextSpecialization::BuildElementAccess(
 
     if (load_mode == LOAD_IGNORE_OUT_OF_BOUNDS ||
         store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
-      // 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
+      // Only check that the {index} is in Signed32 range. We do the actual
+      // bounds check below and just skip the property access if it's out of
       // bounds for the {receiver}.
       index = effect = graph()->NewNode(
-          simplified()->CheckBounds(VectorSlotPair()), index,
-          jsgraph()->Constant(Smi::kMaxValue), effect, control);
+          simplified()->SpeculativeToNumber(NumberOperationHint::kSigned32,
+                                            VectorSlotPair()),
+          index, effect, control);
+
+      // Cast the {index} to Unsigned32 range, so that the bounds checks
+      // below are performed on unsigned values, which means that all the
+      // Negative32 values are treated as out-of-bounds.
+      index = graph()->NewNode(simplified()->NumberToUint32(), index);
     } else {
       // Check that the {index} is in the valid range for the {receiver}.
       index = effect =

src/compiler/type-cache.h

@@ -95,8 +95,8 @@ class TypeCache final {
   // [0, kMaxUInt32].
   Type* const kJSArrayLengthType = Type::Unsigned32();
 
-  // The JSTyped::length property always contains a tagged number in the range
-  // [0, kMaxSmiValue].
+  // The JSTypedArray::length property always contains a tagged number in the
+  // range [0, kMaxSmiValue].
   Type* const kJSTypedArrayLengthType = Type::UnsignedSmall();
 
   // The String::length property always contains a smi in the range

src/ic/accessor-assembler.cc

@@ -300,18 +300,18 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
       Comment("out of bounds elements access");
       Label return_undefined(this);
 
-      // Negative indices aren't valid array indices (according to
-      // the ECMAScript specification), and are stored as properties
-      // in V8, not elements. So we cannot handle them here.
-      GotoIf(IntPtrLessThan(intptr_index, IntPtrConstant(0)), miss);
-
       // 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.
+      // Negative indices aren't valid array indices (according to
+      // the ECMAScript specification), and are stored as properties
+      // in V8, not elements. So we cannot handle them here, except
+      // in case of typed arrays, where integer indexed properties
+      // aren't looked up in the prototype chain.
       GotoIf(IsJSTypedArray(holder), &return_undefined);
+      GotoIf(IntPtrLessThan(intptr_index, IntPtrConstant(0)), miss);
 
       // For all other receivers we need to check that the prototype chain
       // doesn't contain any elements.
@@ -1803,10 +1803,12 @@ void AccessorAssembler::GenericElementLoad(Node* receiver, Node* receiver_map,
   BIND(&if_oob);
   {
     Comment("out of bounds");
-    // Negative keys can't take the fast OOB path.
-    GotoIf(IntPtrLessThan(index, IntPtrConstant(0)), slow);
     // Positive OOB indices are effectively the same as hole loads.
-    Goto(&if_element_hole);
+    GotoIf(IntPtrGreaterThanOrEqual(index, IntPtrConstant(0)),
+           &if_element_hole);
+    // Negative keys can't take the fast OOB path, except for typed arrays.
+    GotoIfNot(InstanceTypeEqual(instance_type, JS_TYPED_ARRAY_TYPE), slow);
+    Return(UndefinedConstant());
   }
 
   BIND(&if_element_hole);

src/ic/ic.cc

@@ -1201,8 +1201,14 @@ MaybeHandle<Object> KeyedLoadIC::Load(Handle<Object> object,
                                Object);
   } else if (FLAG_use_ic && !object->IsAccessCheckNeeded() &&
              !object->IsJSValue()) {
-    if ((object->IsJSReceiver() || object->IsString()) &&
-        key->ToArrayIndex(&index)) {
+    // For regular JSReceiver or String {object}s the {key} must be a positive
+    // array index, for JSTypedArray {object}s we can also support negative
+    // {key}s which we just map into the [2*31,2*32-1] range (via a bit_cast).
+    // This is valid since JSTypedArray::length is always a Smi.
+    if (((object->IsJSReceiver() || object->IsString()) &&
+         key->ToArrayIndex(&index)) ||
+        (object->IsJSTypedArray() &&
+         key->ToInt32(bit_cast<int32_t*>(&index)))) {
       KeyedAccessLoadMode load_mode = GetLoadMode(object, index);
       UpdateLoadElement(Handle<HeapObject>::cast(object), load_mode);
       if (is_vector_set()) {
@@ -2005,7 +2011,13 @@ MaybeHandle<Object> KeyedStoreIC::Store(Handle<Object> object,
     old_receiver_map = handle(receiver->map(), isolate());
     is_arguments = receiver->IsJSArgumentsObject();
     bool is_proxy = receiver->IsJSProxy();
-    key_is_valid_index = key->IsSmi() && Smi::ToInt(*key) >= 0;
+    // For JSTypedArray {object}s we can handle negative indices as OOB
+    // accesses, since integer indexed properties are never looked up
+    // on the prototype chain. For this we simply map the negative {key}s
+    // to the [2**31,2**32-1] range, which is safe since JSTypedArray::length
+    // is always an unsigned Smi.
+    key_is_valid_index =
+        key->IsSmi() && (Smi::ToInt(*key) >= 0 || object->IsJSTypedArray());
     if (!is_arguments && !is_proxy) {
       if (key_is_valid_index) {
         uint32_t index = static_cast<uint32_t>(Smi::ToInt(*key));

src/ic/keyed-store-generic.cc

@@ -428,9 +428,10 @@ void KeyedStoreGenericAssembler::StoreElementWithCapacity(
 void KeyedStoreGenericAssembler::EmitGenericElementStore(
     Node* receiver, Node* receiver_map, Node* instance_type, Node* intptr_index,
     Node* value, Node* context, Label* slow) {
-  Label if_fast(this), if_in_bounds(this), if_increment_length_by_one(this),
-      if_bump_length_with_gap(this), if_grow(this), if_nonfast(this),
-      if_typed_array(this), if_dictionary(this);
+  Label if_fast(this), if_in_bounds(this), if_out_of_bounds(this),
+      if_increment_length_by_one(this), if_bump_length_with_gap(this),
+      if_grow(this), if_nonfast(this), if_typed_array(this),
+      if_dictionary(this);
   Node* elements = LoadElements(receiver);
   Node* elements_kind = LoadMapElementsKind(receiver_map);
   Branch(IsFastElementsKind(elements_kind), &if_fast, &if_nonfast);
@@ -440,7 +441,8 @@ void KeyedStoreGenericAssembler::EmitGenericElementStore(
   GotoIf(InstanceTypeEqual(instance_type, JS_ARRAY_TYPE), &if_array);
   {
     Node* capacity = SmiUntag(LoadFixedArrayBaseLength(elements));
-    Branch(UintPtrLessThan(intptr_index, capacity), &if_in_bounds, &if_grow);
+    Branch(UintPtrLessThan(intptr_index, capacity), &if_in_bounds,
+           &if_out_of_bounds);
   }
   BIND(&if_array);
   {
@@ -459,6 +461,16 @@ void KeyedStoreGenericAssembler::EmitGenericElementStore(
                              kDontChangeLength);
   }
 
+  BIND(&if_out_of_bounds);
+  {
+    // Integer indexed out-of-bounds accesses to typed arrays are simply
+    // ignored, since we never look up integer indexed properties on the
+    // prototypes of typed arrays. For all other types, we may need to
+    // grow the backing store.
+    GotoIfNot(InstanceTypeEqual(instance_type, JS_TYPED_ARRAY_TYPE), &if_grow);
+    Return(value);
+  }
+
   BIND(&if_increment_length_by_one);
   {
     StoreElementWithCapacity(receiver, receiver_map, elements, elements_kind,