[csa] Canonicalize empty elements in AllocateJSArray

Prior to this, AllocateJSArray would go ahead and allocate an empty
FixedArray as elements if passed any capacity that is not a compile-time
constant 0.

Things break later on since we rely on the fact that empty fixed arrays
are always canonicalize, and we use

  obj.elements == empty_fixed_array_constant

interchangeably with

  obj.elements.length == 0.

This CL introduces two new branches in AllocateJSArray: one if the
capacity is known to be non-zero; and another that explicitly
distinguishes between 0 and non-zero capacities.

Bug: chromium:760790
Change-Id: I7c22b19ce9ce15a46f91b0f75e6b4a1ff3a29a0f
Reviewed-on: https://chromium-review.googlesource.com/645959
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47776}

src/builtins/builtins-proxy-gen.cc

@@ -75,18 +75,18 @@ Node* ProxiesCodeStubAssembler::AllocateProxy(Node* target, Node* handler,
 
 Node* ProxiesCodeStubAssembler::AllocateJSArrayForCodeStubArguments(
     Node* context, CodeStubArguments& args, Node* argc, ParameterMode mode) {
-  Node* array = nullptr;
-  Node* elements = nullptr;
   Node* native_context = LoadNativeContext(context);
   Node* array_map = LoadJSArrayElementsMap(PACKED_ELEMENTS, native_context);
   Node* argc_smi = ParameterToTagged(argc, mode);
-  std::tie(array, elements) = AllocateUninitializedJSArrayWithElements(
-      PACKED_ELEMENTS, array_map, argc_smi, nullptr, argc, INTPTR_PARAMETERS);
 
-  VARIABLE(index, MachineType::PointerRepresentation());
-  index.Bind(IntPtrConstant(FixedArrayBase::kHeaderSize - kHeapObjectTag));
+  Node* array = AllocateJSArray(PACKED_ELEMENTS, array_map, argc, argc_smi,
+                                nullptr, mode);
+  Node* elements = LoadElements(array);
+
+  VARIABLE(index, MachineType::PointerRepresentation(),
+           IntPtrConstant(FixedArrayBase::kHeaderSize - kHeapObjectTag));
   VariableList list({&index}, zone());
-  args.ForEach(list, [this, elements, &index](Node* arg) {
+  args.ForEach(list, [=, &index](Node* arg) {
     StoreNoWriteBarrier(MachineRepresentation::kTagged, elements, index.value(),
                         arg);
     Increment(&index, kPointerSize);

src/code-stub-assembler.cc

@@ -227,6 +227,21 @@ bool CodeStubAssembler::IsIntPtrOrSmiConstantZero(Node* test) {
   return false;
 }
 
+bool CodeStubAssembler::TryGetIntPtrOrSmiConstantValue(Node* maybe_constant,
+                                                       int* value) {
+  int32_t int32_constant;
+  if (ToInt32Constant(maybe_constant, int32_constant)) {
+    *value = int32_constant;
+    return true;
+  }
+  Smi* smi_constant;
+  if (ToSmiConstant(maybe_constant, smi_constant)) {
+    *value = Smi::ToInt(smi_constant);
+    return true;
+  }
+  return false;
+}
+
 Node* CodeStubAssembler::IntPtrRoundUpToPowerOfTwo32(Node* value) {
   Comment("IntPtrRoundUpToPowerOfTwo32");
   CSA_ASSERT(this, UintPtrLessThanOrEqual(value, IntPtrConstant(0x80000000u)));
@@ -2447,8 +2462,10 @@ CodeStubAssembler::AllocateUninitializedJSArrayWithElements(
                                  : Heap::kFixedArrayMapRootIndex;
   DCHECK(Heap::RootIsImmortalImmovable(elements_map_index));
   StoreMapNoWriteBarrier(elements, elements_map_index);
+  Node* capacity_smi = ParameterToTagged(capacity, capacity_mode);
+  CSA_ASSERT(this, SmiGreaterThan(capacity_smi, SmiConstant(0)));
   StoreObjectFieldNoWriteBarrier(elements, FixedArray::kLengthOffset,
-                                 ParameterToTagged(capacity, capacity_mode));
+                                 capacity_smi);
   return {array, elements};
 }
 
@@ -2486,6 +2503,7 @@ Node* CodeStubAssembler::AllocateJSArray(ElementsKind kind, Node* array_map,
   CSA_SLOW_ASSERT(this, TaggedIsPositiveSmi(length));
   CSA_SLOW_ASSERT(this, MatchesParameterMode(capacity, capacity_mode));
 
+  int capacity_as_constant;
   Node *array = nullptr, *elements = nullptr;
   if (IsIntPtrOrSmiConstantZero(capacity)) {
     // Array is empty. Use the shared empty fixed array instead of allocating a
@@ -2494,14 +2512,49 @@ Node* CodeStubAssembler::AllocateJSArray(ElementsKind kind, Node* array_map,
                                                         allocation_site);
     StoreObjectFieldRoot(array, JSArray::kElementsOffset,
                          Heap::kEmptyFixedArrayRootIndex);
+  } else if (TryGetIntPtrOrSmiConstantValue(capacity, &capacity_as_constant) &&
+             capacity_as_constant > 0) {
+    // Allocate both array and elements object, and initialize the JSArray.
+    std::tie(array, elements) = AllocateUninitializedJSArrayWithElements(
+        kind, array_map, length, allocation_site, capacity, capacity_mode);
+    // Fill in the elements with holes.
+    FillFixedArrayWithValue(kind, elements,
+                            IntPtrOrSmiConstant(0, capacity_mode), capacity,
+                            Heap::kTheHoleValueRootIndex, capacity_mode);
   } else {
+    Label out(this), empty(this), nonempty(this);
+    VARIABLE(var_array, MachineRepresentation::kTagged);
+
+    Branch(SmiEqual(ParameterToTagged(capacity, capacity_mode), SmiConstant(0)),
+           &empty, &nonempty);
+
+    BIND(&empty);
+    {
+      // Array is empty. Use the shared empty fixed array instead of allocating
+      // a new one.
+      var_array.Bind(AllocateUninitializedJSArrayWithoutElements(
+          kind, array_map, length, allocation_site));
+      StoreObjectFieldRoot(var_array.value(), JSArray::kElementsOffset,
+                           Heap::kEmptyFixedArrayRootIndex);
+      Goto(&out);
+    }
+
+    BIND(&nonempty);
+    {
       // Allocate both array and elements object, and initialize the JSArray.
+      Node* array;
       std::tie(array, elements) = AllocateUninitializedJSArrayWithElements(
           kind, array_map, length, allocation_site, capacity, capacity_mode);
+      var_array.Bind(array);
       // Fill in the elements with holes.
       FillFixedArrayWithValue(kind, elements,
                               IntPtrOrSmiConstant(0, capacity_mode), capacity,
                               Heap::kTheHoleValueRootIndex, capacity_mode);
+      Goto(&out);
+    }
+
+    BIND(&out);
+    array = var_array.value();
   }
 
   return array;
@@ -7101,11 +7154,8 @@ void CodeStubAssembler::TransitionElementsKind(Node* object, Node* map,
     Comment("Non-simple map transition");
     Node* elements = LoadElements(object);
 
-    Node* empty_fixed_array =
-        HeapConstant(isolate()->factory()->empty_fixed_array());
-
     Label done(this);
-    GotoIf(WordEqual(elements, empty_fixed_array), &done);
+    GotoIf(WordEqual(elements, EmptyFixedArrayConstant()), &done);
 
     // TODO(ishell): Use OptimalParameterMode().
     ParameterMode mode = INTPTR_PARAMETERS;
@@ -7113,6 +7163,8 @@ void CodeStubAssembler::TransitionElementsKind(Node* object, Node* map,
     Node* array_length =
         is_jsarray ? SmiUntag(LoadFastJSArrayLength(object)) : elements_length;
 
+    CSA_ASSERT(this, WordNotEqual(elements_length, IntPtrConstant(0)));
+
     GrowElementsCapacity(object, elements, from_kind, to_kind, array_length,
                          elements_length, mode, bailout);
     Goto(&done);

src/code-stub-assembler.h

@@ -178,6 +178,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* IntPtrOrSmiConstant(int value, ParameterMode mode);
 
   bool IsIntPtrOrSmiConstantZero(Node* test);
+  bool TryGetIntPtrOrSmiConstantValue(Node* maybe_constant, int* value);
 
   // Round the 32bits payload of the provided word up to the next power of two.
   Node* IntPtrRoundUpToPowerOfTwo32(Node* value);

test/mjsunit/regress/regress-760790.js

+// Copyright 2017 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.
+
+function g() {
+  var a = Array(0);
+  a[0]++;
+}
+g();
+g();
+g();