[builtin] Further cleanup %ArrayIteratorPrototype%.next().

Refactor the ArrayIteratorPrototypeNext CSA builtin to handle the
JSArray element access in a dedicated helper macro, very similar
to how it's done for JSTypedArray's. Also add support for dictionary
elements to this helper macro using the existing dictionary access
logic in the CodeStubAssembler.

This improves the readability of the builtin significantly and the
performance of iterating arrays with dictionary elements goes up by
a factor of ~3.5x.

Bug: v8:8015, v8:8070
Change-Id: Ibfee760ea1e4bc0fffb42b232fb1d097b706bd1f
Reviewed-on: https://chromium-review.googlesource.com/1183305Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55283}

src/builtins/builtins-array-gen.cc

@@ -3523,13 +3523,9 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) {
 
   BIND(&if_array);
   {
-    // We can only handle fast elements here.
-    TNode<Int32T> elements_kind = LoadMapElementsKind(array_map);
-    GotoIfNot(IsFastElementsKind(elements_kind), &if_other);
-
     // Check that the {index} is within range for the {array}.
-    TNode<Smi> length = CAST(LoadJSArrayLength(CAST(array)));
-    GotoIfNot(SmiBelow(CAST(index), length), &set_done);
+    TNode<Number> length = LoadJSArrayLength(CAST(array));
+    GotoIfNumberGreaterThanOrEqual(index, length, &set_done);
     StoreObjectFieldNoWriteBarrier(iterator, JSArrayIterator::kNextIndexOffset,
                                    SmiInc(CAST(index)));
 
@@ -3541,69 +3537,19 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) {
                        Int32Constant(static_cast<int>(IterationKind::kKeys))),
            &allocate_iterator_result);
 
-    Node* elements = LoadElements(CAST(array));
-    Label if_packed(this), if_holey(this), if_packed_double(this),
-        if_holey_double(this), if_unknown_kind(this, Label::kDeferred);
-    int32_t kinds[] = {// Handled by if_packed.
-                       PACKED_SMI_ELEMENTS, PACKED_ELEMENTS,
-                       // Handled by if_holey.
-                       HOLEY_SMI_ELEMENTS, HOLEY_ELEMENTS,
-                       // Handled by if_packed_double.
-                       PACKED_DOUBLE_ELEMENTS,
-                       // Handled by if_holey_double.
-                       HOLEY_DOUBLE_ELEMENTS};
-    Label* labels[] = {// PACKED_{SMI,}_ELEMENTS
-                       &if_packed, &if_packed,
-                       // HOLEY_{SMI,}_ELEMENTS
-                       &if_holey, &if_holey,
-                       // PACKED_DOUBLE_ELEMENTS
-                       &if_packed_double,
-                       // HOLEY_DOUBLE_ELEMENTS
-                       &if_holey_double};
-    Switch(elements_kind, &if_unknown_kind, kinds, labels, arraysize(kinds));
-
-    BIND(&if_packed);
-    {
-      var_value.Bind(
-          LoadFixedArrayElement(CAST(elements), index, 0, SMI_PARAMETERS));
-      Goto(&allocate_entry_if_needed);
-    }
-
-    BIND(&if_holey);
-    {
-      Node* element =
-          LoadFixedArrayElement(CAST(elements), index, 0, SMI_PARAMETERS);
-      var_value.Bind(element);
-      GotoIfNot(WordEqual(element, TheHoleConstant()),
-                &allocate_entry_if_needed);
-      GotoIf(IsNoElementsProtectorCellInvalid(), &if_generic);
-      var_value.Bind(UndefinedConstant());
-      Goto(&allocate_entry_if_needed);
-    }
-
-    BIND(&if_packed_double);
-    {
-      Node* value = LoadFixedDoubleArrayElement(
-          elements, index, MachineType::Float64(), 0, SMI_PARAMETERS);
-      var_value.Bind(AllocateHeapNumberWithValue(value));
-      Goto(&allocate_entry_if_needed);
-    }
+    Label if_hole(this, Label::kDeferred);
+    TNode<Int32T> elements_kind = LoadMapElementsKind(array_map);
+    TNode<FixedArrayBase> elements = LoadElements(CAST(array));
+    var_value.Bind(LoadFixedArrayBaseElementAsTagged(
+        elements, CAST(index), elements_kind, &if_generic, &if_hole));
+    Goto(&allocate_entry_if_needed);
 
-    BIND(&if_holey_double);
+    BIND(&if_hole);
     {
-      Label if_hole(this, Label::kDeferred);
-      Node* value = LoadFixedDoubleArrayElement(
-          elements, index, MachineType::Float64(), 0, SMI_PARAMETERS, &if_hole);
-      var_value.Bind(AllocateHeapNumberWithValue(value));
-      Goto(&allocate_entry_if_needed);
-      BIND(&if_hole);
       GotoIf(IsNoElementsProtectorCellInvalid(), &if_generic);
       var_value.Bind(UndefinedConstant());
       Goto(&allocate_entry_if_needed);
     }
-
-    BIND(&if_unknown_kind);
-    Unreachable();
   }
 
   BIND(&if_other);

src/code-stub-assembler.cc

@@ -2383,6 +2383,72 @@ TNode<Float64T> CodeStubAssembler::LoadFixedDoubleArrayElement(
   return LoadDoubleWithHoleCheck(object, offset, if_hole, machine_type);
 }
 
+TNode<Object> CodeStubAssembler::LoadFixedArrayBaseElementAsTagged(
+    TNode<FixedArrayBase> elements, TNode<Smi> index,
+    TNode<Int32T> elements_kind, Label* if_accessor, Label* if_hole) {
+  TVARIABLE(Object, var_result);
+  Label done(this), if_packed(this), if_holey(this), if_packed_double(this),
+      if_holey_double(this), if_dictionary(this, Label::kDeferred);
+
+  int32_t kinds[] = {// Handled by if_packed.
+                     PACKED_SMI_ELEMENTS, PACKED_ELEMENTS,
+                     // Handled by if_holey.
+                     HOLEY_SMI_ELEMENTS, HOLEY_ELEMENTS,
+                     // Handled by if_packed_double.
+                     PACKED_DOUBLE_ELEMENTS,
+                     // Handled by if_holey_double.
+                     HOLEY_DOUBLE_ELEMENTS};
+  Label* labels[] = {// PACKED_{SMI,}_ELEMENTS
+                     &if_packed, &if_packed,
+                     // HOLEY_{SMI,}_ELEMENTS
+                     &if_holey, &if_holey,
+                     // PACKED_DOUBLE_ELEMENTS
+                     &if_packed_double,
+                     // HOLEY_DOUBLE_ELEMENTS
+                     &if_holey_double};
+  Switch(elements_kind, &if_dictionary, kinds, labels, arraysize(kinds));
+
+  BIND(&if_packed);
+  {
+    var_result =
+        LoadFixedArrayElement(CAST(elements), index, 0, SMI_PARAMETERS);
+    Goto(&done);
+  }
+
+  BIND(&if_holey);
+  {
+    var_result =
+        LoadFixedArrayElement(CAST(elements), index, 0, SMI_PARAMETERS);
+    Branch(WordEqual(var_result.value(), TheHoleConstant()), if_hole, &done);
+  }
+
+  BIND(&if_packed_double);
+  {
+    var_result = AllocateHeapNumberWithValue(LoadFixedDoubleArrayElement(
+        CAST(elements), index, MachineType::Float64(), 0, SMI_PARAMETERS));
+    Goto(&done);
+  }
+
+  BIND(&if_holey_double);
+  {
+    var_result = AllocateHeapNumberWithValue(LoadFixedDoubleArrayElement(
+        CAST(elements), index, MachineType::Float64(), 0, SMI_PARAMETERS,
+        if_hole));
+    Goto(&done);
+  }
+
+  BIND(&if_dictionary);
+  {
+    CSA_ASSERT(this, IsDictionaryElementsKind(elements_kind));
+    var_result = BasicLoadNumberDictionaryElement(
+        CAST(elements), SmiToIntPtr(index), if_accessor, if_hole);
+    Goto(&done);
+  }
+
+  BIND(&done);
+  return var_result.value();
+}
+
 TNode<Float64T> CodeStubAssembler::LoadDoubleWithHoleCheck(
     SloppyTNode<Object> base, SloppyTNode<IntPtrT> offset, Label* if_hole,
     MachineType machine_type) {

src/code-stub-assembler.h

@@ -1050,6 +1050,16 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                                        SMI_PARAMETERS);
   }
 
+  // Load an array element from a FixedArray, FixedDoubleArray or a
+  // NumberDictionary (depending on the |elements_kind|) and return
+  // it as a tagged value. Assumes that the |index| passed a length
+  // check before. Bails out to |if_accessor| if the element that
+  // was found is an accessor, or to |if_hole| if the element at
+  // the given |index| is not found in |elements|.
+  TNode<Object> LoadFixedArrayBaseElementAsTagged(
+      TNode<FixedArrayBase> elements, TNode<Smi> index,
+      TNode<Int32T> elements_kind, Label* if_accessor, Label* if_hole);
+
   // Load a feedback slot from a FeedbackVector.
   TNode<MaybeObject> LoadFeedbackVectorSlot(
       Node* object, Node* index, int additional_offset = 0,
@@ -1874,6 +1884,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   bool IsFastElementsKind(ElementsKind kind) {
     return v8::internal::IsFastElementsKind(kind);
   }
+  TNode<BoolT> IsDictionaryElementsKind(TNode<Int32T> elements_kind) {
+    return ElementsKindEqual(elements_kind, Int32Constant(DICTIONARY_ELEMENTS));
+  }
   TNode<BoolT> IsDoubleElementsKind(TNode<Int32T> elements_kind);
   bool IsDoubleElementsKind(ElementsKind kind) {
     return v8::internal::IsDoubleElementsKind(kind);

test/mjsunit/es6/array-iterator.js

@@ -252,3 +252,16 @@ function TestNonOwnSlots() {
   }, TypeError);
 }
 TestNonOwnSlots();
+
+function TestForDictionaryArray() {
+  var array = [];
+  array[1024] = 'c';
+  assertTrue(%HasDictionaryElements(array));
+  var iterator = array[Symbol.iterator]();
+  for (var i = 0; i < 1024; ++i) {
+    assertIteratorResult(void 0, false, iterator.next());
+  }
+  assertIteratorResult('c', false, iterator.next());
+  assertIteratorResult(void 0, true, iterator.next());
+}
+TestForDictionaryArray();