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

This reverts commit 181ac2b0.

Reason for revert: TF changes break 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=neis@chromium.org,ishell@chromium.org,bmeurer@chromium.org

Change-Id: Id841d91b12d199045e0a9c4ddae2c2ead20b5e21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7334
Reviewed-on: https://chromium-review.googlesource.com/885814Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50860}

src/code-stub-assembler.cc

@@ -7794,14 +7794,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)) {
-    CSA_ASSERT(this, Word32BinaryNot(IsFixedCOWArrayMap(LoadMap(elements))));
-  } else if (!IsCOWHandlingStoreMode(store_mode)) {
-    GotoIf(IsFixedCOWArrayMap(LoadMap(elements)), bailout);
+  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);
   }
-
   // TODO(ishell): introduce TryToIntPtrOrSmi() and use OptimalParameterMode().
   ParameterMode parameter_mode = INTPTR_PARAMETERS;
   key = TryToIntptr(key, bailout);
@@ -7867,30 +7867,25 @@ void CodeStubAssembler::EmitElementStore(Node* object, Node* key, Node* value,
   }
 
   if (IsGrowStoreMode(store_mode)) {
-    elements =
-        CheckForCapacityGrow(object, elements, elements_kind, store_mode,
-                             length, key, parameter_mode, is_jsarray, bailout);
+    elements = CheckForCapacityGrow(object, elements, elements_kind, length,
+                                    key, parameter_mode, is_jsarray, bailout);
   } else {
     GotoIfNot(UintPtrLessThan(key, length), 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);
+    if ((store_mode == STORE_NO_TRANSITION_HANDLE_COW) &&
+        IsSmiOrObjectElementsKind(elements_kind)) {
+      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,
-    KeyedAccessStoreMode store_mode, Node* length, Node* key,
-    ParameterMode mode, bool is_js_array, Label* bailout) {
+Node* CodeStubAssembler::CheckForCapacityGrow(Node* object, Node* elements,
+                                              ElementsKind kind, 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);
 
@@ -7898,7 +7893,6 @@ Node* CodeStubAssembler::CheckForCapacityGrow(
   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);
@@ -7907,18 +7901,20 @@ Node* CodeStubAssembler::CheckForCapacityGrow(
   {
     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);
+
     if (is_js_array) {
       Node* new_length = IntPtrAdd(key, IntPtrOrSmiConstant(1, mode));
       StoreObjectFieldNoWriteBarrier(object, JSArray::kLengthOffset,
@@ -7945,12 +7941,15 @@ Node* CodeStubAssembler::CopyElementsOnWrite(Node* object, Node* elements,
   VARIABLE(new_elements_var, MachineRepresentation::kTagged, elements);
   Label done(this);
 
-  GotoIfNot(IsFixedCOWArrayMap(LoadMap(elements)), &done);
+  GotoIfNot(
+      WordEqual(LoadMap(elements), LoadRoot(Heap::kFixedCOWArrayMapRootIndex)),
+      &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

@@ -1725,9 +1725,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                         KeyedAccessStoreMode store_mode, Label* bailout);
 
   Node* CheckForCapacityGrow(Node* object, Node* elements, ElementsKind kind,
-                             KeyedAccessStoreMode store_mode, Node* length,
-                             Node* key, ParameterMode mode, bool is_js_array,
-                             Label* bailout);
+                             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,13 +541,12 @@ void StoreFastElementStub::GenerateAheadOfTime(Isolate* isolate) {
   StoreFastElementStub(isolate, false, HOLEY_ELEMENTS, STANDARD_STORE)
       .GetCode();
   StoreFastElementStub(isolate, false, HOLEY_ELEMENTS,
-                       STORE_AND_GROW_NO_TRANSITION_HANDLE_COW)
+                       STORE_AND_GROW_NO_TRANSITION)
       .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_HANDLE_COW)
+    StoreFastElementStub(isolate, true, kind, STORE_AND_GROW_NO_TRANSITION)
         .GetCode();
   }
 }

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

@@ -2246,11 +2246,10 @@ JSNativeContextSpecialization::BuildElementAccess(
         simplified()->LoadField(AccessBuilder::ForJSObjectElements()), receiver,
         effect, control);
 
-    // Don't try to store to a copy-on-write backing store (unless supported by
-    // the store mode).
+    // Don't try to store to a copy-on-write backing store.
     if (access_mode == AccessMode::kStore &&
         IsSmiOrObjectElementsKind(elements_kind) &&
-        !IsCOWHandlingStoreMode(store_mode)) {
+        store_mode != STORE_NO_TRANSITION_HANDLE_COW) {
       effect = graph()->NewNode(
           simplified()->CheckMaps(
               CheckMapsFlag::kNone,
@@ -2460,15 +2459,6 @@ 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/ic/ic.cc

@@ -58,18 +58,12 @@ const char* GetModifier(KeyedAccessLoadMode mode) {
 }
 
 const char* GetModifier(KeyedAccessStoreMode mode) {
-  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 (mode == STORE_NO_TRANSITION_HANDLE_COW) return ".COW";
+  if (mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
+    return ".IGNORE_OOB";
   }
-  DCHECK(!IsCOWHandlingStoreMode(mode));
-  return IsGrowStoreMode(mode) ? ".GROW" : "";
+  if (IsGrowStoreMode(mode)) return ".GROW";
+  return "";
 }
 
 }  // namespace
@@ -1698,7 +1692,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_HANDLE_COW ||
+        (store_mode == STORE_AND_GROW_NO_TRANSITION ||
          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
@@ -1796,7 +1790,7 @@ Handle<Map> KeyedStoreIC::ComputeTransitionedMap(
     // Fall through
     case STORE_NO_TRANSITION_HANDLE_COW:
     case STANDARD_STORE:
-    case STORE_AND_GROW_NO_TRANSITION_HANDLE_COW:
+    case STORE_AND_GROW_NO_TRANSITION:
       return map;
   }
   UNREACHABLE();
@@ -1805,7 +1799,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_HANDLE_COW ||
+         store_mode == STORE_AND_GROW_NO_TRANSITION ||
          store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS ||
          store_mode == STORE_NO_TRANSITION_HANDLE_COW);
   DCHECK(!receiver_map->DictionaryElementsInPrototypeChainOnly());
@@ -1846,7 +1840,7 @@ void KeyedStoreIC::StoreElementPolymorphicHandlers(
     MapHandles* receiver_maps, ObjectHandles* handlers,
     KeyedAccessStoreMode store_mode) {
   DCHECK(store_mode == STANDARD_STORE ||
-         store_mode == STORE_AND_GROW_NO_TRANSITION_HANDLE_COW ||
+         store_mode == STORE_AND_GROW_NO_TRANSITION ||
          store_mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS ||
          store_mode == STORE_NO_TRANSITION_HANDLE_COW);
 
@@ -1921,7 +1915,7 @@ static KeyedAccessStoreMode GetStoreMode(Handle<JSObject> receiver,
         return STORE_AND_GROW_TRANSITION_TO_OBJECT;
       }
     }
-    return STORE_AND_GROW_NO_TRANSITION_HANDLE_COW;
+    return STORE_AND_GROW_NO_TRANSITION;
   } 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_HANDLE_COW,
+  STORE_AND_GROW_NO_TRANSITION,
   STORE_AND_GROW_TRANSITION_TO_OBJECT,
   STORE_AND_GROW_TRANSITION_TO_DOUBLE,
   STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS,
@@ -204,25 +204,21 @@ 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_HANDLE_COW) {
-    return STORE_AND_GROW_NO_TRANSITION_HANDLE_COW;
+  if (store_mode >= STORE_AND_GROW_NO_TRANSITION) {
+    return STORE_AND_GROW_NO_TRANSITION;
   }
   return STANDARD_STORE;
 }
 
 
 static inline bool IsGrowStoreMode(KeyedAccessStoreMode store_mode) {
-  return store_mode >= STORE_AND_GROW_NO_TRANSITION_HANDLE_COW &&
+  return store_mode >= STORE_AND_GROW_NO_TRANSITION &&
          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]);