Reland "[ic] Improve performance of KeyedStoreIC on literal-based arrays."

This is a reland of 181ac2b0 that fixes
the issue with load elimination.

Original change's description:
> [ic] Improve performance of KeyedStoreIC on literal-based arrays.
>
> In mode STORE_AND_GROW_NO_TRANSITION, the handler for elements stores
> used to bail out when seeing a COW array, even if the store that
> installed the handler had been operating on the very same array.
>
> This CL adds support for COW arrays to the mode (and renames it to
> STORE_AND_GROW_NO_TRANSITION_HANDLE_COW).
>
> Bug: v8:7334
> Change-Id: I6a15e8c1ff8d4ad4d5b8fc447745dce5d146c67c
> Reviewed-on: https://chromium-review.googlesource.com/876014
> Commit-Queue: Georg Neis <neis@chromium.org>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50840}

TBR=bmeurer@chromium.org

Bug: v8:7334, chromium:805768
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I3d9c1b08583e08d68a1d30242a25e4a2190c8c55
Reviewed-on: https://chromium-review.googlesource.com/886261
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50885}

src/code-stub-assembler.cc

@@ -7847,14 +7847,14 @@ void CodeStubAssembler::EmitElementStore(Node* object, Node* key, Node* value,
                                          KeyedAccessStoreMode store_mode,
                                          Label* bailout) {
   CSA_ASSERT(this, Word32BinaryNot(IsJSProxy(object)));
+
   Node* elements = LoadElements(object);
-  if (IsSmiOrObjectElementsKind(elements_kind) &&
-      store_mode != STORE_NO_TRANSITION_HANDLE_COW) {
-    // Bailout in case of COW elements.
-    GotoIf(WordNotEqual(LoadMap(elements),
-                        LoadRoot(Heap::kFixedArrayMapRootIndex)),
-           bailout);
+  if (!IsSmiOrObjectElementsKind(elements_kind)) {
+    CSA_ASSERT(this, Word32BinaryNot(IsFixedCOWArrayMap(LoadMap(elements))));
+  } else if (!IsCOWHandlingStoreMode(store_mode)) {
+    GotoIf(IsFixedCOWArrayMap(LoadMap(elements)), bailout);
   }
+
   // TODO(ishell): introduce TryToIntPtrOrSmi() and use OptimalParameterMode().
   ParameterMode parameter_mode = INTPTR_PARAMETERS;
   key = TryToIntptr(key, bailout);
@@ -7920,25 +7920,30 @@ void CodeStubAssembler::EmitElementStore(Node* object, Node* key, Node* value,
   }
 
   if (IsGrowStoreMode(store_mode)) {
-    elements = CheckForCapacityGrow(object, elements, elements_kind, length,
-                                    key, parameter_mode, is_jsarray, bailout);
+    elements =
+        CheckForCapacityGrow(object, elements, elements_kind, store_mode,
+                             length, key, parameter_mode, is_jsarray, bailout);
   } else {
     GotoIfNot(UintPtrLessThan(key, length), bailout);
+  }
 
-    if ((store_mode == STORE_NO_TRANSITION_HANDLE_COW) &&
-        IsSmiOrObjectElementsKind(elements_kind)) {
-      elements = CopyElementsOnWrite(object, elements, elements_kind, length,
-                                     parameter_mode, bailout);
-    }
+  // If we didn't grow {elements}, it might still be COW, in which case we
+  // copy it now.
+  if (!IsSmiOrObjectElementsKind(elements_kind)) {
+    CSA_ASSERT(this, Word32BinaryNot(IsFixedCOWArrayMap(LoadMap(elements))));
+  } else if (IsCOWHandlingStoreMode(store_mode)) {
+    elements = CopyElementsOnWrite(object, elements, elements_kind, length,
+                                   parameter_mode, bailout);
   }
+
+  CSA_ASSERT(this, Word32BinaryNot(IsFixedCOWArrayMap(LoadMap(elements))));
   StoreElement(elements, elements_kind, key, value, parameter_mode);
 }
 
-Node* CodeStubAssembler::CheckForCapacityGrow(Node* object, Node* elements,
-                                              ElementsKind kind, Node* length,
-                                              Node* key, ParameterMode mode,
-                                              bool is_js_array,
-                                              Label* bailout) {
+Node* CodeStubAssembler::CheckForCapacityGrow(
+    Node* object, Node* elements, ElementsKind kind,
+    KeyedAccessStoreMode store_mode, Node* length, Node* key,
+    ParameterMode mode, bool is_js_array, Label* bailout) {
   VARIABLE(checked_elements, MachineRepresentation::kTagged);
   Label grow_case(this), no_grow_case(this), done(this);
 
@@ -7946,6 +7951,7 @@ Node* CodeStubAssembler::CheckForCapacityGrow(Node* object, Node* elements,
   if (IsHoleyOrDictionaryElementsKind(kind)) {
     condition = UintPtrGreaterThanOrEqual(key, length);
   } else {
+    // We don't support growing here unless the value is being appended.
     condition = WordEqual(key, length);
   }
   Branch(condition, &grow_case, &no_grow_case);
@@ -7954,20 +7960,18 @@ Node* CodeStubAssembler::CheckForCapacityGrow(Node* object, Node* elements,
   {
     Node* current_capacity =
         TaggedToParameter(LoadFixedArrayBaseLength(elements), mode);
-
     checked_elements.Bind(elements);
-
     Label fits_capacity(this);
     GotoIf(UintPtrLessThan(key, current_capacity), &fits_capacity);
+
     {
       Node* new_elements = TryGrowElementsCapacity(
           object, elements, kind, key, current_capacity, mode, bailout);
-
       checked_elements.Bind(new_elements);
       Goto(&fits_capacity);
     }
-    BIND(&fits_capacity);
 
+    BIND(&fits_capacity);
     if (is_js_array) {
       Node* new_length = IntPtrAdd(key, IntPtrOrSmiConstant(1, mode));
       StoreObjectFieldNoWriteBarrier(object, JSArray::kLengthOffset,
@@ -7994,15 +7998,12 @@ Node* CodeStubAssembler::CopyElementsOnWrite(Node* object, Node* elements,
   VARIABLE(new_elements_var, MachineRepresentation::kTagged, elements);
   Label done(this);
 
-  GotoIfNot(
-      WordEqual(LoadMap(elements), LoadRoot(Heap::kFixedCOWArrayMapRootIndex)),
-      &done);
+  GotoIfNot(IsFixedCOWArrayMap(LoadMap(elements)), &done);
   {
     Node* capacity =
         TaggedToParameter(LoadFixedArrayBaseLength(elements), mode);
     Node* new_elements = GrowElementsCapacity(object, elements, kind, kind,
                                               length, capacity, mode, bailout);
-
     new_elements_var.Bind(new_elements);
     Goto(&done);
   }

src/code-stub-assembler.h

@@ -1734,8 +1734,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                         KeyedAccessStoreMode store_mode, Label* bailout);
 
   Node* CheckForCapacityGrow(Node* object, Node* elements, ElementsKind kind,
-                             Node* length, Node* key, ParameterMode mode,
-                             bool is_js_array, Label* bailout);
+                             KeyedAccessStoreMode store_mode, Node* length,
+                             Node* key, ParameterMode mode, bool is_js_array,
+                             Label* bailout);
 
   Node* CopyElementsOnWrite(Node* object, Node* elements, ElementsKind kind,
                             Node* length, ParameterMode mode, Label* bailout);

src/code-stubs.cc

@@ -541,12 +541,13 @@ void StoreFastElementStub::GenerateAheadOfTime(Isolate* isolate) {
   StoreFastElementStub(isolate, false, HOLEY_ELEMENTS, STANDARD_STORE)
       .GetCode();
   StoreFastElementStub(isolate, false, HOLEY_ELEMENTS,
-                       STORE_AND_GROW_NO_TRANSITION)
+                       STORE_AND_GROW_NO_TRANSITION_HANDLE_COW)
       .GetCode();
   for (int i = FIRST_FAST_ELEMENTS_KIND; i <= LAST_FAST_ELEMENTS_KIND; i++) {
     ElementsKind kind = static_cast<ElementsKind>(i);
     StoreFastElementStub(isolate, true, kind, STANDARD_STORE).GetCode();
-    StoreFastElementStub(isolate, true, kind, STORE_AND_GROW_NO_TRANSITION)
+    StoreFastElementStub(isolate, true, kind,
+                         STORE_AND_GROW_NO_TRANSITION_HANDLE_COW)
         .GetCode();
   }
 }

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

@@ -2246,10 +2246,11 @@ JSNativeContextSpecialization::BuildElementAccess(
         simplified()->LoadField(AccessBuilder::ForJSObjectElements()), receiver,
         effect, control);
 
-    // Don't try to store to a copy-on-write backing store.
+    // Don't try to store to a copy-on-write backing store (unless supported by
+    // the store mode).
     if (access_mode == AccessMode::kStore &&
         IsSmiOrObjectElementsKind(elements_kind) &&
-        store_mode != STORE_NO_TRANSITION_HANDLE_COW) {
+        !IsCOWHandlingStoreMode(store_mode)) {
       effect = graph()->NewNode(
           simplified()->CheckMaps(
               CheckMapsFlag::kNone,
@@ -2459,6 +2460,15 @@ JSNativeContextSpecialization::BuildElementAccess(
             simplified()->MaybeGrowFastElements(mode, VectorSlotPair()),
             receiver, elements, index, elements_length, effect, control);
 
+        // If we didn't grow {elements}, it might still be COW, in which case we
+        // copy it now.
+        if (IsSmiOrObjectElementsKind(elements_kind) &&
+            store_mode == STORE_AND_GROW_NO_TRANSITION_HANDLE_COW) {
+          elements = effect =
+              graph()->NewNode(simplified()->EnsureWritableFastElements(),
+                               receiver, elements, effect, control);
+        }
+
         // Also update the "length" property if {receiver} is a JSArray.
         if (receiver_is_jsarray) {
           Node* check =

src/compiler/load-elimination.cc

@@ -821,9 +821,11 @@ Reduction LoadElimination::ReduceMaybeGrowFastElements(Node* node) {
     state = state->SetMaps(
         node, ZoneHandleSet<Map>(factory()->fixed_double_array_map()), zone());
   } else {
-    // We know that the resulting elements have the fixed array map.
-    state = state->SetMaps(
-        node, ZoneHandleSet<Map>(factory()->fixed_array_map()), zone());
+    // We know that the resulting elements have the fixed array map or the COW
+    // version thereof (if we didn't grow and it was already COW before).
+    ZoneHandleSet<Map> fixed_array_maps(factory()->fixed_array_map());
+    fixed_array_maps.insert(factory()->fixed_cow_array_map(), zone());
+    state = state->SetMaps(node, fixed_array_maps, zone());
   }
   // Kill the previous elements on {object}.
   state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset),

src/ic/ic.cc

@@ -58,12 +58,18 @@ const char* GetModifier(KeyedAccessLoadMode mode) {
 }
 
 const char* GetModifier(KeyedAccessStoreMode mode) {
-  if (mode == STORE_NO_TRANSITION_HANDLE_COW) return ".COW";
-  if (mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
-    return ".IGNORE_OOB";
+  switch (mode) {
+    case STORE_NO_TRANSITION_HANDLE_COW:
+      return ".COW";
+    case STORE_AND_GROW_NO_TRANSITION_HANDLE_COW:
+      return ".STORE+COW";
+    case STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS:
+      return ".IGNORE_OOB";
+    default:
+      break;
   }
-  if (IsGrowStoreMode(mode)) return ".GROW";
-  return "";
+  DCHECK(!IsCOWHandlingStoreMode(mode));
+  return IsGrowStoreMode(mode) ? ".GROW" : "";
 }
 
 }  // namespace
@@ -1692,7 +1698,7 @@ void KeyedStoreIC::UpdateStoreElement(Handle<Map> receiver_map,
     }
     if (receiver_map.is_identical_to(previous_receiver_map) &&
         old_store_mode == STANDARD_STORE &&
-        (store_mode == STORE_AND_GROW_NO_TRANSITION ||
+        (store_mode == STORE_AND_GROW_NO_TRANSITION_HANDLE_COW ||
          store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS ||
          store_mode == STORE_NO_TRANSITION_HANDLE_COW)) {
       // A "normal" IC that handles stores can switch to a version that can
@@ -1790,7 +1796,7 @@ Handle<Map> KeyedStoreIC::ComputeTransitionedMap(
     // Fall through
     case STORE_NO_TRANSITION_HANDLE_COW:
     case STANDARD_STORE:
-    case STORE_AND_GROW_NO_TRANSITION:
+    case STORE_AND_GROW_NO_TRANSITION_HANDLE_COW:
       return map;
   }
   UNREACHABLE();
@@ -1799,7 +1805,7 @@ Handle<Map> KeyedStoreIC::ComputeTransitionedMap(
 Handle<Object> KeyedStoreIC::StoreElementHandler(
     Handle<Map> receiver_map, KeyedAccessStoreMode store_mode) {
   DCHECK(store_mode == STANDARD_STORE ||
-         store_mode == STORE_AND_GROW_NO_TRANSITION ||
+         store_mode == STORE_AND_GROW_NO_TRANSITION_HANDLE_COW ||
          store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS ||
          store_mode == STORE_NO_TRANSITION_HANDLE_COW);
   DCHECK(!receiver_map->DictionaryElementsInPrototypeChainOnly());
@@ -1840,7 +1846,7 @@ void KeyedStoreIC::StoreElementPolymorphicHandlers(
     MapHandles* receiver_maps, ObjectHandles* handlers,
     KeyedAccessStoreMode store_mode) {
   DCHECK(store_mode == STANDARD_STORE ||
-         store_mode == STORE_AND_GROW_NO_TRANSITION ||
+         store_mode == STORE_AND_GROW_NO_TRANSITION_HANDLE_COW ||
          store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS ||
          store_mode == STORE_NO_TRANSITION_HANDLE_COW);
 
@@ -1915,7 +1921,7 @@ static KeyedAccessStoreMode GetStoreMode(Handle<JSObject> receiver,
         return STORE_AND_GROW_TRANSITION_TO_OBJECT;
       }
     }
-    return STORE_AND_GROW_NO_TRANSITION;
+    return STORE_AND_GROW_NO_TRANSITION_HANDLE_COW;
   } else {
     // Handle only in-bounds elements accesses.
     if (receiver->HasSmiElements()) {

src/objects.h

@@ -184,7 +184,7 @@ enum KeyedAccessStoreMode {
   STANDARD_STORE,
   STORE_TRANSITION_TO_OBJECT,
   STORE_TRANSITION_TO_DOUBLE,
-  STORE_AND_GROW_NO_TRANSITION,
+  STORE_AND_GROW_NO_TRANSITION_HANDLE_COW,
   STORE_AND_GROW_TRANSITION_TO_OBJECT,
   STORE_AND_GROW_TRANSITION_TO_DOUBLE,
   STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS,
@@ -204,21 +204,25 @@ static inline bool IsTransitionStoreMode(KeyedAccessStoreMode store_mode) {
          store_mode == STORE_AND_GROW_TRANSITION_TO_DOUBLE;
 }
 
+static inline bool IsCOWHandlingStoreMode(KeyedAccessStoreMode store_mode) {
+  return store_mode == STORE_NO_TRANSITION_HANDLE_COW ||
+         store_mode == STORE_AND_GROW_NO_TRANSITION_HANDLE_COW;
+}
 
 static inline KeyedAccessStoreMode GetNonTransitioningStoreMode(
     KeyedAccessStoreMode store_mode) {
   if (store_mode >= STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
     return store_mode;
   }
-  if (store_mode >= STORE_AND_GROW_NO_TRANSITION) {
-    return STORE_AND_GROW_NO_TRANSITION;
+  if (store_mode >= STORE_AND_GROW_NO_TRANSITION_HANDLE_COW) {
+    return STORE_AND_GROW_NO_TRANSITION_HANDLE_COW;
   }
   return STANDARD_STORE;
 }
 
 
 static inline bool IsGrowStoreMode(KeyedAccessStoreMode store_mode) {
-  return store_mode >= STORE_AND_GROW_NO_TRANSITION &&
+  return store_mode >= STORE_AND_GROW_NO_TRANSITION_HANDLE_COW &&
          store_mode <= STORE_AND_GROW_TRANSITION_TO_DOUBLE;
 }
 

test/mjsunit/keyed-store-array-literal.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
+
+
+function f1() {
+  const x = [,];
+  x[1] = 42;
+  assertEquals([undefined, 42], x);
+}
+
+f1();
+f1();
+%OptimizeFunctionOnNextCall(f1);
+f1();
+f1();
+
+
+function f2() {
+  const x = [0];
+  for (const y of [1, 2, 3, 4]) {
+    x[x.length] = y;
+  }
+  assertEquals([0, 1, 2, 3, 4], x);
+}
+
+f2();
+f2();
+%OptimizeFunctionOnNextCall(f2);
+f2();
+f2();
+
+
+function f3() {
+  const x = [0];
+  for (const y of [1.1, {}]) {
+    x[x.length] = y;
+  }
+  assertEquals([0, 1.1, {}], x);
+}
+
+f3();
+f3();
+%OptimizeFunctionOnNextCall(f3);
+f3();
+f3();
+
+
+function f4(x) {
+  x[x.length] = x.length;
+}
+
+let x1 = [];
+f4(x1);
+assertEquals([0], x1);
+f4(x1);
+assertEquals([0, 1], x1);
+%OptimizeFunctionOnNextCall(f4);
+f4(x1);
+assertEquals([0, 1, 2], x1);
+f4(x1);
+assertEquals([0, 1, 2, 3], x1);
+
+let x2 = {length: 42};
+f4(x2);
+assertEquals(42, x2[42]);
+f4(x2);
+assertEquals(42, x2[42]);
+%OptimizeFunctionOnNextCall(f4);
+f4(x2);
+assertEquals(42, x2[42]);
+f4(x2);
+assertEquals(42, x2[42]);

test/mjsunit/regress/regress-805768.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
+
+function foo() {
+  var a = [''];
+  print(a[0]);
+  return a;
+}
+
+function bar(a) { a[0] = "bazinga!"; }
+
+for (var i = 0; i < 5; i++) bar([]);
+
+%OptimizeFunctionOnNextCall(bar);
+bar(foo());
+assertEquals([''], foo());