[turbofan] Make sure we use only serialized elements kind transitions.

Currently, we call the MapRef::AsElementsKind method on an initial
map multiple times (from JSCreateLowering::ReduceJSCreateArray).
However, this does not does not play well with the heap copier/broker,
which only expectes AsElementsKind to be called on initial maps.

This CL makes sure we only call AsElementsKind once (on the initial map).

Bug: chromium:890620
Change-Id: If44421d3900abb7629ea8f789a005b8d8ebaf881
Reviewed-on: https://chromium-review.googlesource.com/1253105Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56307}

src/compiler/js-create-lowering.cc

@@ -475,7 +475,8 @@ Reduction JSCreateLowering::ReduceJSCreateGeneratorObject(Node* node) {
 // Constructs an array with a variable {length} when no upper bound
 // is known for the capacity.
 Reduction JSCreateLowering::ReduceNewArray(
-    Node* node, Node* length, MapRef initial_map, PretenureFlag pretenure,
+    Node* node, Node* length, MapRef initial_map, ElementsKind elements_kind,
+    PretenureFlag pretenure,
     const SlackTrackingPrediction& slack_tracking_prediction) {
   DCHECK_EQ(IrOpcode::kJSCreateArray, node->opcode());
   Node* effect = NodeProperties::GetEffectInput(node);
@@ -484,8 +485,8 @@ Reduction JSCreateLowering::ReduceNewArray(
   // Constructing an Array via new Array(N) where N is an unsigned
   // integer, always creates a holey backing store.
   ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING(
-      initial_map, initial_map.AsElementsKind(
-                       GetHoleyElementsKind(initial_map.elements_kind())));
+      initial_map,
+      initial_map.AsElementsKind(GetHoleyElementsKind(elements_kind)));
 
   // Check that the {limit} is an unsigned integer in the valid range.
   // This has to be kept in sync with src/runtime/runtime-array.cc,
@@ -524,7 +525,7 @@ Reduction JSCreateLowering::ReduceNewArray(
 // upper bound is known for the {capacity}.
 Reduction JSCreateLowering::ReduceNewArray(
     Node* node, Node* length, int capacity, MapRef initial_map,
-    PretenureFlag pretenure,
+    ElementsKind elements_kind, PretenureFlag pretenure,
     const SlackTrackingPrediction& slack_tracking_prediction) {
   DCHECK(node->opcode() == IrOpcode::kJSCreateArray ||
          node->opcode() == IrOpcode::kJSCreateEmptyLiteralArray);
@@ -532,12 +533,11 @@ Reduction JSCreateLowering::ReduceNewArray(
   Node* control = NodeProperties::GetControlInput(node);
 
   // Determine the appropriate elements kind.
-  ElementsKind elements_kind = initial_map.elements_kind();
   if (NodeProperties::GetType(length).Max() > 0.0) {
     elements_kind = GetHoleyElementsKind(elements_kind);
-    ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING(
-        initial_map, initial_map.AsElementsKind(elements_kind));
   }
+  ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING(
+      initial_map, initial_map.AsElementsKind(elements_kind));
   DCHECK(IsFastElementsKind(elements_kind));
 
   // Setup elements and properties.
@@ -569,15 +569,16 @@ Reduction JSCreateLowering::ReduceNewArray(
 
 Reduction JSCreateLowering::ReduceNewArray(
     Node* node, std::vector<Node*> values, MapRef initial_map,
-    PretenureFlag pretenure,
+    ElementsKind elements_kind, PretenureFlag pretenure,
     const SlackTrackingPrediction& slack_tracking_prediction) {
   DCHECK_EQ(IrOpcode::kJSCreateArray, node->opcode());
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
   // Determine the appropriate elements kind.
-  ElementsKind elements_kind = initial_map.elements_kind();
   DCHECK(IsFastElementsKind(elements_kind));
+  ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING(
+      initial_map, initial_map.AsElementsKind(elements_kind));
 
   // Check {values} based on the {elements_kind}. These checks are guarded
   // by the {elements_kind} feedback on the {site}, so it's safe to just
@@ -663,10 +664,9 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) {
       bool can_inline_call = false;
 
       // Check if we have a feedback {site} on the {node}.
+      ElementsKind elements_kind = initial_map.elements_kind();
       if (site_ref) {
-        ElementsKind elements_kind = site_ref->GetElementsKind();
-        ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING(
-            initial_map, initial_map.AsElementsKind(elements_kind));
+        elements_kind = site_ref->GetElementsKind();
         can_inline_call = site_ref->CanInlineCall();
         pretenure = dependencies()->DependOnPretenureMode(*site_ref);
         dependencies()->DependOnElementsKind(*site_ref);
@@ -677,7 +677,8 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) {
       if (arity == 0) {
         Node* length = jsgraph()->ZeroConstant();
         int capacity = JSArray::kPreallocatedArrayElements;
-        return ReduceNewArray(node, length, capacity, initial_map, pretenure,
+        return ReduceNewArray(node, length, capacity, initial_map,
+                              elements_kind, pretenure,
                               slack_tracking_prediction);
       } else if (arity == 1) {
         Node* length = NodeProperties::GetValueInput(node, 2);
@@ -685,26 +686,25 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) {
         if (!length_type.Maybe(Type::Number())) {
           // Handle the single argument case, where we know that the value
           // cannot be a valid Array length.
-          ElementsKind elements_kind = initial_map.elements_kind();
           elements_kind = GetMoreGeneralElementsKind(
               elements_kind, IsHoleyElementsKind(elements_kind)
                                  ? HOLEY_ELEMENTS
                                  : PACKED_ELEMENTS);
-          ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING(
-              initial_map, initial_map.AsElementsKind(elements_kind));
           return ReduceNewArray(node, std::vector<Node*>{length}, initial_map,
-                                pretenure, slack_tracking_prediction);
+                                elements_kind, pretenure,
+                                slack_tracking_prediction);
         }
         if (length_type.Is(Type::SignedSmall()) && length_type.Min() >= 0 &&
             length_type.Max() <= kElementLoopUnrollLimit &&
             length_type.Min() == length_type.Max()) {
           int capacity = static_cast<int>(length_type.Max());
-          return ReduceNewArray(node, length, capacity, initial_map, pretenure,
+          return ReduceNewArray(node, length, capacity, initial_map,
+                                elements_kind, pretenure,
                                 slack_tracking_prediction);
         }
         if (length_type.Maybe(Type::UnsignedSmall()) && can_inline_call) {
-          return ReduceNewArray(node, length, initial_map, pretenure,
-                                slack_tracking_prediction);
+          return ReduceNewArray(node, length, initial_map, elements_kind,
+                                pretenure, slack_tracking_prediction);
         }
       } else if (arity <= JSArray::kInitialMaxFastElementArray) {
         // Gather the values to store into the newly created array.
@@ -728,7 +728,6 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) {
         }
 
         // Try to figure out the ideal elements kind statically.
-        ElementsKind elements_kind = initial_map.elements_kind();
         if (values_all_smis) {
           // Smis can be stored with any elements kind.
         } else if (values_all_numbers) {
@@ -749,10 +748,8 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) {
           // we cannot inline this invocation of the Array constructor here.
           return NoChange();
         }
-        ASSIGN_RETURN_NO_CHANGE_IF_DATA_MISSING(
-            initial_map, initial_map.AsElementsKind(elements_kind));
-        return ReduceNewArray(node, values, initial_map, pretenure,
-                              slack_tracking_prediction);
+        return ReduceNewArray(node, values, initial_map, elements_kind,
+                              pretenure, slack_tracking_prediction);
       }
     }
   }
@@ -1097,7 +1094,8 @@ Reduction JSCreateLowering::ReduceJSCreateEmptyLiteralArray(Node* node) {
     DCHECK(!initial_map.IsInobjectSlackTrackingInProgress());
     SlackTrackingPrediction slack_tracking_prediction(
         initial_map, initial_map.instance_size());
-    return ReduceNewArray(node, length, 0, initial_map, pretenure,
+    return ReduceNewArray(node, length, 0, initial_map,
+                          initial_map.elements_kind(), pretenure,
                           slack_tracking_prediction);
   }
   return NoChange();

src/compiler/js-create-lowering.h

@@ -67,15 +67,16 @@ class V8_EXPORT_PRIVATE JSCreateLowering final
   Reduction ReduceJSCreateBlockContext(Node* node);
   Reduction ReduceJSCreateGeneratorObject(Node* node);
   Reduction ReduceNewArray(
-      Node* node, Node* length, MapRef initial_map, PretenureFlag pretenure,
+      Node* node, Node* length, MapRef initial_map, ElementsKind elements_kind,
+      PretenureFlag pretenure,
       const SlackTrackingPrediction& slack_tracking_prediction);
   Reduction ReduceNewArray(
       Node* node, Node* length, int capacity, MapRef initial_map,
-      PretenureFlag pretenure,
+      ElementsKind elements_kind, PretenureFlag pretenure,
       const SlackTrackingPrediction& slack_tracking_prediction);
   Reduction ReduceNewArray(
       Node* node, std::vector<Node*> values, MapRef initial_map,
-      PretenureFlag pretenure,
+      ElementsKind elements_kind, PretenureFlag pretenure,
       const SlackTrackingPrediction& slack_tracking_prediction);
   Reduction ReduceJSCreateObject(Node* node);
 

test/mjsunit/compiler/regress-890620.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
+
+var a = 42;
+
+function g(n) {
+  while (n > 0) {
+    a = new Array(n);
+    n--;
+  }
+}
+
+g(1);
+
+function f() {
+  g();
+}
+
+f();
+%OptimizeFunctionOnNextCall(f);
+f();
+assertEquals(1, a.length);