[builtins] implement Array.prototype[@@iterator] in TFJ builtins

src/builtins/builtins-array.cc

Index: src/builtins/builtins-array.cc
diff --git a/src/builtins/builtins-array.cc b/src/builtins/builtins-array.cc
index b5190e66bc9d1b1d9dc37e6b3bece61ccf5be399..7b8e5050e268b7fae474a0cd2f57d3f89dc38492 100644
--- a/src/builtins/builtins-array.cc
+++ b/src/builtins/builtins-array.cc
@@ -6,6 +6,7 @@
 #include "src/builtins/builtins-utils.h"
 
 #include "src/code-factory.h"
+#include "src/contexts.h"
 #include "src/elements.h"
 
 namespace v8 {
@@ -2067,5 +2068,796 @@ void Builtins::Generate_ArrayIndexOf(CodeStubAssembler* assembler) {
                                            array, search_element, start_from));
 }
 
+void Builtins::Generate_ArrayPrototypeValues(CodeStubAssembler* assembler) {

[Benedikt Meurer, 2016/10/13 05:07:19]

Do you plan to do a separate TypedArrayPrototypeValues? Because that just throws
if the receiver is not a typed array.

[caitp, 2016/10/13 22:32:43]

I've made it so that the triplet methods (keys/values/entries) can all share
common code (TypedArrays get their own version with the extra typechecking)

+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+
+  Variable var_array(assembler, MachineRepresentation::kTagged);
+  Variable var_result(assembler, MachineRepresentation::kTagged);

[Benedikt Meurer, 2016/10/13 05:07:20]

This seems to be unused.

[caitp, 2016/10/13 22:32:43]

I _think_ this is no longer an issue after the big refactoring, but I may be
wrong

+  Variable var_map(assembler, MachineRepresentation::kTagged);
+  Variable var_map_index(assembler, MachineType::PointerRepresentation());
+
+  Label call_to_object(assembler);
+  Label check_instance_type(assembler);
+  Label if_isfastarray(assembler);
+  Label if_isgeneric(assembler);
+  Label allocate_array_iterator(assembler);
+  Label allocate_typed_array_iterator(assembler);
+
+  Node* receiver = assembler->Parameter(0);

[caitp, 2016/10/13 00:13:36]

For reasons I don't completely understand, the for-of loop in PostNatives() in
src/js/prologue.js results in this being called with an undefined receiver.

This could be caused by the way this function is bootstrapped, but afaik it's
set up correctly in runtime-array.cc. Not entirely sure. Once that's fixed, most
of the Array-related tests should be passing, but the TypedArray stuff isn't
done yet.

[Benedikt Meurer, 2016/10/13 05:07:19]

Hm, this looks like a bug somewhere. You could replace that for-of loop with a
simple for loop to mitigate the problem if it's due to the funky JS+runtime
bootstrapping process. Maybe Yang can help here too.

[Yang, 2016/10/13 13:52:37]

Doesn't ring a bell, sorry. But I'm not all that surprised that things behave
weirdly while setting up stuff in the bootstrapper.

But maybe we could port everything in prologue.js to C++ \o/

[caitp, 2016/10/13 16:12:58]

It turns out, including the receiver in the internal formal parameter count
broke this. Fixed by changing the `1` to a `0` for the length parameter to
InstallBuiltin() calls in runtime-array.cc. I figured it was something like
that.

+  Node* context = assembler->Parameter(3);
+  Node* native_context = assembler->LoadNativeContext(context);
+
+  var_array.Bind(receiver);
+  var_result.Bind(assembler->UndefinedConstant());
+  var_map.Bind(assembler->UndefinedConstant());
+  var_map_index.Bind(assembler->IntPtrConstant(0));

[Benedikt Meurer, 2016/10/13 05:07:19]

You don't need to bind var_map / var_map_index here.

[caitp, 2016/10/13 22:32:43]

This still happens in the new version, in
CodeStubAssembler::CreateArrayIterator() --- due to check failures when it's not
initially bound (even though it's always bound before use).

+
+  assembler->Branch(assembler->TaggedIsSmi(var_array.value()), &call_to_object,
+                    &check_instance_type);
+  assembler->Bind(&check_instance_type);
+  {
+    var_map.Bind(assembler->LoadMap(var_array.value()));
+    Node* instance_type = assembler->LoadMapInstanceType(var_map.value());
+    assembler->GotoIf(
+        assembler->Int32LessThan(
+            instance_type, assembler->Int32Constant(FIRST_JS_RECEIVER_TYPE)),
+        &call_to_object);
+
+    assembler->GotoIf(
+        assembler->WordEqual(instance_type,
+                             assembler->Int32Constant(JS_TYPED_ARRAY_TYPE)),
+        &allocate_typed_array_iterator);
+
+    assembler->BranchIfFastJSArray(var_array.value(), context, &if_isfastarray,
+                                   &if_isgeneric);
+  }
+
+  assembler->Bind(&if_isfastarray);
+  {
+    Node* map_index = assembler->IntPtrAdd(

[Benedikt Meurer, 2016/10/13 05:07:19]

Neat! But please add some STATIC_ASSERTs to guard this and a comment explaining
what this does :-)

[caitp, 2016/10/13 22:32:43]

ah, I think I added some comments to this stuff, but I'll probably clarify it in
a future patch.

+        assembler->IntPtrConstant(Context::FAST_ARRAY_VALUE_ITERATOR_MAP_INDEX),
+        assembler->LoadMapElementsKind(assembler->LoadMap(var_array.value())));
+    assembler->Assert(assembler->IntPtrLessThan(
+        map_index, assembler->IntPtrConstant(
+                       Context::GENERIC_ARRAY_VALUE_ITERATOR_MAP_INDEX)));
+
+    var_map_index.Bind(map_index);
+    assembler->Goto(&allocate_array_iterator);
+  }
+
+  assembler->Bind(&call_to_object);
+  {
+    Callable callable = CodeFactory::ToObject(assembler->isolate());
+    Node* result = assembler->CallStub(callable, context, var_array.value());
+    var_map.Bind(assembler->LoadMap(result));
+    var_array.Bind(result);
+    assembler->Goto(&if_isgeneric);
+  }
+
+  assembler->Bind(&if_isgeneric);
+  {
+    Node* map_index =
+        assembler->IntPtrConstant(Context::FAST_ARRAY_VALUE_ITERATOR_MAP_INDEX);
+
+    var_map_index.Bind(map_index);
+    assembler->Goto(&allocate_array_iterator);
+  }
+
+  assembler->Bind(&allocate_array_iterator);
+  {
+    Node* iterator = assembler->Allocate(JSArrayIterator::kSize);

[Benedikt Meurer, 2016/10/13 05:07:19]

Can you add a helper method AllocateArrayIterator(map, array, array_map) for
this?

[caitp, 2016/10/13 22:32:44]

I've added CodeStubAssembler::AllocateJSArrayIterator() and
AllocateJSTypedArrayIterator()

+    Node* map = assembler->LoadFixedArrayElement(
+        native_context, var_map_index.value(), 0,
+        CodeStubAssembler::INTPTR_PARAMETERS);
+    assembler->StoreMapNoWriteBarrier(iterator, map);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSIteratorResult::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldRoot(iterator, JSIteratorResult::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kIteratedObjectOffset, var_array.value());
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kNextIndexOffset,
+        assembler->SmiConstant(Smi::FromInt(0)));
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kIteratedObjectMapOffset, var_map.value());
+    assembler->Return(iterator);
+  }
+
+  assembler->Bind(&allocate_typed_array_iterator);
+  {
+    Node* map_index = assembler->IntPtrAdd(
+        assembler->IntPtrConstant(
+            Context::UINT8_ARRAY_VALUE_ITERATOR_MAP_INDEX),
+        assembler->Int32Sub(assembler->LoadMapElementsKind(
+                                assembler->LoadMap(var_array.value())),
+                            assembler->Int32Constant(UINT8_ELEMENTS)));
+    Node* map = assembler->LoadFixedArrayElement(
+        native_context, map_index, 0, CodeStubAssembler::INTPTR_PARAMETERS);
+    Node* iterator = assembler->Allocate(JSTypedArrayIterator::kSize);
+    assembler->StoreMapNoWriteBarrier(iterator, map);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSTypedArrayIterator::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSTypedArrayIterator::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSTypedArrayIterator::kIteratedObjectOffset,
+        var_array.value());
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSTypedArrayIterator::kNextIndexOffset,
+        assembler->SmiConstant(Smi::FromInt(0)));
+    assembler->Return(iterator);
+  }
+}
+
+void Builtins::Generate_ArrayPrototypeEntries(CodeStubAssembler* assembler) {

[Benedikt Meurer, 2016/10/13 05:07:19]

Can you unify this with the ArrayPrototypeValues above? The only difference
seems to be the map index in the context.

[caitp, 2016/10/13 22:32:43]

Done in CodeStubAssembler::CreateArrayIterator()

+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+
+  Variable var_array(assembler, MachineRepresentation::kTagged);
+  Variable var_result(assembler, MachineRepresentation::kTagged);
+  Variable var_map(assembler, MachineRepresentation::kTagged);
+  Variable var_map_index(assembler, MachineType::PointerRepresentation());
+
+  Label call_to_object(assembler);
+  Label check_instance_type(assembler);
+  Label if_isfastarray(assembler);
+  Label if_isgeneric(assembler);
+  Label allocate_array_iterator(assembler);
+  Label allocate_typed_array_iterator(assembler);
+
+  Node* receiver = assembler->Parameter(0);
+  Node* context = assembler->Parameter(3);
+  Node* native_context = assembler->LoadNativeContext(context);
+
+  var_array.Bind(receiver);
+  var_result.Bind(assembler->UndefinedConstant());
+  var_map.Bind(assembler->UndefinedConstant());
+  var_map_index.Bind(assembler->IntPtrConstant(0));
+
+  assembler->Branch(assembler->TaggedIsSmi(var_array.value()), &call_to_object,
+                    &check_instance_type);
+  assembler->Bind(&check_instance_type);
+  {
+    var_map.Bind(assembler->LoadMap(var_array.value()));
+    Node* instance_type = assembler->LoadMapInstanceType(var_map.value());
+    assembler->GotoIf(
+        assembler->Int32LessThan(
+            instance_type, assembler->Int32Constant(FIRST_JS_RECEIVER_TYPE)),
+        &call_to_object);
+
+    assembler->GotoIf(
+        assembler->WordEqual(instance_type,
+                             assembler->Int32Constant(JS_TYPED_ARRAY_TYPE)),
+        &allocate_typed_array_iterator);
+
+    assembler->BranchIfFastJSArray(var_array.value(), context, &if_isfastarray,
+                                   &if_isgeneric);
+  }
+
+  assembler->Bind(&if_isfastarray);
+  {
+    Node* map_index = assembler->IntPtrAdd(
+        assembler->IntPtrConstant(
+            Context::FAST_ARRAY_KEY_VALUE_ITERATOR_MAP_INDEX),
+        assembler->LoadMapElementsKind(assembler->LoadMap(var_array.value())));
+    assembler->Assert(assembler->IntPtrLessThan(
+        map_index, assembler->IntPtrConstant(
+                       Context::GENERIC_ARRAY_KEY_VALUE_ITERATOR_MAP_INDEX)));
+
+    var_map_index.Bind(map_index);
+    assembler->Goto(&allocate_array_iterator);
+  }
+
+  assembler->Bind(&call_to_object);
+  {
+    Callable callable = CodeFactory::ToObject(assembler->isolate());
+    Node* result = assembler->CallStub(callable, context, var_array.value());
+    var_map.Bind(assembler->LoadMap(result));
+    var_array.Bind(result);
+    assembler->Goto(&if_isgeneric);
+  }
+
+  assembler->Bind(&if_isgeneric);
+  {
+    Node* map_index = assembler->IntPtrConstant(
+        Context::GENERIC_ARRAY_KEY_VALUE_ITERATOR_MAP_INDEX);
+    var_map_index.Bind(map_index);
+    assembler->Goto(&allocate_array_iterator);
+  }
+
+  assembler->Bind(&allocate_array_iterator);
+  {
+    Node* iterator = assembler->Allocate(JSArrayIterator::kSize);
+    Node* map = assembler->LoadFixedArrayElement(
+        native_context, var_map_index.value(), 0,
+        CodeStubAssembler::INTPTR_PARAMETERS);
+    assembler->StoreMapNoWriteBarrier(iterator, map);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSIteratorResult::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldRoot(iterator, JSIteratorResult::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kIteratedObjectOffset, var_array.value());
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kNextIndexOffset,
+        assembler->SmiConstant(Smi::FromInt(0)));
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kIteratedObjectMapOffset, var_map.value());
+    assembler->Return(iterator);
+  }
+
+  assembler->Bind(&allocate_typed_array_iterator);
+  {
+    Node* map_index = assembler->IntPtrAdd(
+        assembler->IntPtrConstant(
+            Context::UINT8_ARRAY_KEY_VALUE_ITERATOR_MAP_INDEX),
+        assembler->Int32Sub(assembler->LoadMapElementsKind(var_map.value()),
+                            assembler->Int32Constant(UINT8_ELEMENTS)));
+    Node* map = assembler->LoadFixedArrayElement(
+        native_context, map_index, 0, CodeStubAssembler::INTPTR_PARAMETERS);
+    Node* iterator = assembler->Allocate(JSTypedArrayIterator::kSize);
+    assembler->StoreMapNoWriteBarrier(iterator, map);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSTypedArrayIterator::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSTypedArrayIterator::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSTypedArrayIterator::kIteratedObjectOffset,
+        var_array.value());
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSTypedArrayIterator::kNextIndexOffset,
+        assembler->SmiConstant(Smi::FromInt(0)));
+    assembler->Return(iterator);
+  }
+}
+
+void Builtins::Generate_ArrayPrototypeKeys(CodeStubAssembler* assembler) {
+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+
+  Variable var_array(assembler, MachineRepresentation::kTagged);
+  Variable var_result(assembler, MachineRepresentation::kTagged);
+  Variable var_map(assembler, MachineRepresentation::kTagged);
+
+  Label call_to_object(assembler);
+  Label check_instance_type(assembler);
+  Label if_isfastarray(assembler);
+  Label if_isgeneric(assembler);
+  Label allocate_array_iterator(assembler);
+  Label allocate_typed_array_iterator(assembler);
+
+  Node* receiver = assembler->Parameter(0);
+  Node* context = assembler->Parameter(3);
+  Node* native_context = assembler->LoadNativeContext(context);
+
+  var_array.Bind(receiver);
+  var_result.Bind(assembler->UndefinedConstant());
+  var_map.Bind(assembler->UndefinedConstant());

[Benedikt Meurer, 2016/10/13 05:07:19]

You don't need to Bind var_result and var_map here. These bindings are
essentially dead code.

[caitp, 2016/10/13 22:32:43]

They are, but as I mentioned, they still hit CHECK failures if not bound

+
+  assembler->Branch(assembler->TaggedIsSmi(var_array.value()), &call_to_object,
+                    &check_instance_type);
+  assembler->Bind(&check_instance_type);
+  {
+    var_map.Bind(assembler->LoadMap(var_array.value()));
+    Node* instance_type = assembler->LoadMapInstanceType(var_map.value());
+    assembler->GotoIf(
+        assembler->Int32LessThan(
+            instance_type, assembler->Int32Constant(FIRST_JS_RECEIVER_TYPE)),
+        &call_to_object);
+
+    assembler->Branch(
+        assembler->WordEqual(instance_type,
+                             assembler->Int32Constant(JS_ARRAY_TYPE)),
+        &allocate_array_iterator, &allocate_typed_array_iterator);

[Benedikt Meurer, 2016/10/13 05:07:19]

You need to handle the generic as well. Also non-fast JSArray need to do
"length" access in each iteration.

[caitp, 2016/10/13 22:32:43]

I'm not sure it's possible to skip the length check, changing the length field
doesn't seem to guarantee that the map or elements kind changes (some length
changes always do, but some don't).

[Benedikt Meurer, 2016/10/14 03:46:50]

What I'm saying is you need to do the map check definitely in either case. But
for fast JSArray then map check is sufficient to know that it's safe to access
the JSArray::length field directly. For the generic case you need to use the
GetPropertyStub to read "length" in each iteration.

+  }
+
+  assembler->Bind(&call_to_object);
+  {
+    Callable callable = CodeFactory::ToObject(assembler->isolate());
+    Node* result = assembler->CallStub(callable, context, var_array.value());
+    var_array.Bind(result);
+    assembler->Goto(&allocate_array_iterator);
+  }
+
+  assembler->Bind(&allocate_array_iterator);
+  {
+    Node* map_index = assembler->IntPtrConstant(
+        Context::GENERIC_ARRAY_KEY_ITERATOR_MAP_INDEX);
+    Node* map = assembler->LoadFixedArrayElement(
+        native_context, map_index, 0, CodeStubAssembler::INTPTR_PARAMETERS);
+    Node* iterator = assembler->Allocate(JSArrayIterator::kSize);
+    assembler->StoreMapNoWriteBarrier(iterator, map);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSIteratorResult::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldRoot(iterator, JSIteratorResult::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kIteratedObjectOffset, var_array.value());
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kNextIndexOffset,
+        assembler->SmiConstant(Smi::FromInt(0)));
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSArrayIterator::kIteratedObjectMapOffset, var_map.value());
+    assembler->Return(iterator);
+  }
+
+  assembler->Bind(&allocate_typed_array_iterator);
+  {
+    Node* map_index =
+        assembler->IntPtrConstant(Context::TYPED_ARRAY_KEY_ITERATOR_MAP_INDEX);
+    Node* map = assembler->LoadFixedArrayElement(
+        native_context, map_index, 0, CodeStubAssembler::INTPTR_PARAMETERS);
+    Node* iterator = assembler->Allocate(JSTypedArrayIterator::kSize);
+    assembler->StoreMapNoWriteBarrier(iterator, map);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSTypedArrayIterator::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldRoot(iterator,
+                                    JSTypedArrayIterator::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSTypedArrayIterator::kIteratedObjectOffset,
+        var_array.value());
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSTypedArrayIterator::kNextIndexOffset,
+        assembler->SmiConstant(Smi::FromInt(0)));
+    assembler->Return(iterator);
+  }
+}
+
+void Builtins::Generate_ArrayIteratorPrototypeNext(
+    CodeStubAssembler* assembler) {
+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+
+  Variable var_value(assembler, MachineRepresentation::kTagged);
+  Variable var_done(assembler, MachineRepresentation::kTagged);
+  Variable var_result(assembler, MachineRepresentation::kTagged);
+
+  Label set_done_and_return(assembler);
+  Label did_set_done(assembler);
+  Label allocate_entry_if_needed(assembler);
+  Label allocate_iterator_result(assembler);
+  Label throw_bad_receiver(assembler);
+
+  Label if_isarray(assembler);
+  Label if_istypedarray(assembler);
+  Label if_slow(assembler);
+  Label if_slownotkeyiterator(assembler);
+
+  var_value.Bind(assembler->UndefinedConstant());
+  var_done.Bind(assembler->BooleanConstant(false));
+  var_result.Bind(assembler->UndefinedConstant());
+
+  Node* iterator = assembler->Parameter(0);
+  Node* context = assembler->Parameter(3);
+
+  // If O does not have all of the internal slots of an Array Iterator Instance
+  // (22.1.5.3), throw a TypeError exception
+  assembler->GotoIf(assembler->TaggedIsSmi(iterator), &throw_bad_receiver);
+  Node* instance_type = assembler->LoadInstanceType(iterator);
+  // FIXME: Use bitmasks instead of integer ranges

[Benedikt Meurer, 2016/10/13 05:07:19]

You can subtract the base and then do a single unsigned comparison. That way you
need only a single comparison, and don't need to fiddle with bitmasks.

[caitp, 2016/10/13 22:32:43]

Done, seems to be working nicely.

+  assembler->GotoIf(
+      assembler->Int32LessThan(
+          instance_type, assembler->Int32Constant(FIRST_ARRAY_ITERATOR_TYPE)),
+      &throw_bad_receiver);
+  assembler->GotoIf(
+      assembler->Int32GreaterThan(
+          instance_type, assembler->Int32Constant(LAST_ARRAY_ITERATOR_TYPE)),
+      &throw_bad_receiver);
+
+  Node* native_context = assembler->LoadNativeContext(context);
+
+  // Let a be O.[[IteratedObject]].
+  STATIC_ASSERT(JSArrayIterator::kIteratedObjectOffset ==
+                JSTypedArrayIterator::kIteratedObjectOffset);
+  Node* array = assembler->LoadObjectField(
+      iterator, JSArrayIterator::kIteratedObjectOffset);
+
+  Node* array_map = assembler->LoadMap(array);
+  Node* array_type = assembler->LoadMapInstanceType(array_map);
+
+  // If a is undefined, return CreateIterResultObject(undefined, true)
+  assembler->GotoIf(assembler->WordEqual(array, assembler->UndefinedConstant()),
+                    &did_set_done);
+
+  // Let index be O.[[ArrayIteratorNextIndex]].
+  STATIC_ASSERT(JSArrayIterator::kNextIndexOffset ==
+                JSTypedArrayIterator::kNextIndexOffset);
+  Node* index =
+      assembler->LoadObjectField(iterator, JSArrayIterator::kNextIndexOffset);
+
+  assembler->GotoIf(assembler->Word32Equal(

[Benedikt Meurer, 2016/10/13 05:07:19]

How about just having a single switch over all relevant instance types here?

[caitp, 2016/10/13 22:32:43]

Done

+                        array_type, assembler->Int32Constant(JS_ARRAY_TYPE)),
+                    &if_isarray);
+
+  assembler->Branch(
+      assembler->Word32Equal(array_type,
+                             assembler->Int32Constant(JS_TYPED_ARRAY_TYPE)),
+      &if_istypedarray, &if_slow);
+
+  assembler->Bind(&if_isarray);
+  {  // JSArray-specialized algorithm for fast JSArrays

[Benedikt Meurer, 2016/10/13 05:07:19]

You can only take the fast JSArray case if the array_map still matches the
recorded initial map, also for the key iterator as someone could have installed
a "length" getter or something like that.

[caitp, 2016/10/13 22:32:43]

I'll add some new interesting tests to ensure all of this in a followup

+    assembler->Assert(assembler->TaggedIsSmi(index));
+
+    Node* length = assembler->LoadObjectField(array, JSArray::kLengthOffset);
+    assembler->GotoIf(assembler->SmiAboveOrEqual(index, length),
+                      &set_done_and_return);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSStringIterator::kNextIndexOffset,
+        assembler->SmiAdd(index, assembler->SmiConstant(Smi::FromInt(1))));
+
+    Label if_keyiterator(assembler), if_notkeyiterator(assembler);
+
+    assembler->Branch(
+        assembler->WordEqual(
+            instance_type,
+            assembler->Int32Constant(JS_GENERIC_ARRAY_KEY_ITERATOR_TYPE)),
+        &if_keyiterator, &if_notkeyiterator);
+
+    assembler->Bind(&if_keyiterator);
+    {
+      var_value.Bind(index);
+      assembler->Goto(&allocate_iterator_result);
+    }
+
+    assembler->Bind(&if_notkeyiterator);
+    {
+      assembler->GotoIf(
+          assembler->WordEqual(instance_type,
+                               assembler->Int32Constant(
+                                   JS_GENERIC_ARRAY_KEY_VALUE_ITERATOR_TYPE)),
+          &if_slow);
+      assembler->GotoIf(
+          assembler->WordEqual(
+              instance_type,
+              assembler->Int32Constant(JS_GENERIC_ARRAY_VALUE_ITERATOR_TYPE)),
+          &if_slownotkeyiterator);
+      Node* orig_map = assembler->LoadObjectField(
+          iterator, JSArrayIterator::kIteratedObjectMapOffset);
+      assembler->GotoIf(assembler->WordNotEqual(orig_map, array_map),
+                        &if_slownotkeyiterator);
+
+      Node* elements = assembler->LoadElements(array);
+      static int32_t kInstanceType[] = {
+          JS_FAST_SMI_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_FAST_HOLEY_SMI_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_FAST_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_FAST_HOLEY_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_FAST_DOUBLE_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_FAST_HOLEY_DOUBLE_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+
+          JS_FAST_SMI_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_FAST_HOLEY_SMI_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_FAST_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_FAST_HOLEY_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_FAST_DOUBLE_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_FAST_HOLEY_DOUBLE_ARRAY_VALUE_ITERATOR_TYPE,
+      };
+
+      Label if_smiorobjects(assembler), if_doubles(assembler);
+      Label* instance_type_handlers[] = {
+          &if_smiorobjects, &if_smiorobjects, &if_smiorobjects,
+          &if_smiorobjects, &if_doubles,      &if_doubles,
+          &if_smiorobjects, &if_smiorobjects, &if_smiorobjects,
+          &if_smiorobjects, &if_doubles,      &if_doubles};
+
+      assembler->Switch(instance_type, &set_done_and_return, kInstanceType,
+                        instance_type_handlers, arraysize(kInstanceType));
+
+      assembler->Bind(&if_smiorobjects);
+      {
+        Node* value = assembler->LoadFixedArrayElement(
+            elements, index, 0, CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->UndefinedConstant());
+        assembler->GotoIf(
+            assembler->WordEqual(value, assembler->TheHoleConstant()),
+            &allocate_entry_if_needed);
+        var_value.Bind(value);
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+
+      assembler->Bind(&if_doubles);
+      {
+        Node* value = assembler->LoadFixedDoubleArrayElement(
+            elements, index, MachineType::Float64(), 0,
+            CodeStubAssembler::SMI_PARAMETERS, &allocate_entry_if_needed);
+        var_value.Bind(assembler->AllocateHeapNumberWithValue(value));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+    }
+  }
+
+  assembler->Bind(&if_istypedarray);
+  {  // Special-cased algorithm for JSTypedArrays
+    assembler->Assert(assembler->TaggedIsSmi(index));
+
+    Node* length = assembler->LoadObjectField(array, JSArray::kLengthOffset);
+    assembler->GotoIf(assembler->SmiAboveOrEqual(index, length),
+                      &set_done_and_return);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSStringIterator::kNextIndexOffset,
+        assembler->SmiAdd(index, assembler->SmiConstant(Smi::FromInt(1))));
+
+    Label if_keyiterator(assembler), if_notkeyiterator(assembler);
+
+    assembler->Branch(assembler->WordEqual(
+                          instance_type, assembler->Int32Constant(
+                                             JS_TYPED_ARRAY_KEY_ITERATOR_TYPE)),
+                      &if_keyiterator, &if_notkeyiterator);
+
+    assembler->Bind(&if_keyiterator);
+    {
+      var_value.Bind(index);
+      assembler->Goto(&allocate_iterator_result);
+    }
+
+    assembler->Bind(&if_notkeyiterator);
+    {
+      Node* elements = assembler->LoadElements(array);
+      Node* base_pointer = assembler->LoadObjectField(
+          elements, FixedTypedArrayBase::kBasePointerOffset);
+      Node* external_pointer = assembler->LoadObjectField(
+          elements, FixedTypedArrayBase::kExternalPointerOffset);
+      Node* data_pointer = assembler->IntPtrAdd(base_pointer, external_pointer);
+
+      static int32_t kInstanceType[] = {
+          JS_UINT8_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_INT8_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_UINT16_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_INT16_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_UINT32_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_INT32_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_FLOAT32_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_FLOAT64_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+          JS_UINT8_CLAMPED_ARRAY_KEY_VALUE_ITERATOR_TYPE,
+
+          JS_UINT8_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_INT8_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_UINT16_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_INT16_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_UINT32_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_INT32_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_FLOAT32_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_FLOAT64_ARRAY_VALUE_ITERATOR_TYPE,
+          JS_UINT8_CLAMPED_ARRAY_VALUE_ITERATOR_TYPE,
+      };
+
+      Label if_uint8(assembler), if_int8(assembler), if_uint16(assembler),
+          if_int16(assembler), if_uint32(assembler), if_int32(assembler),
+          if_float32(assembler), if_float64(assembler);
+      Label* instance_type_handlers[] = {
+          &if_uint8,   &if_int8,    &if_uint16,  &if_int16,  &if_uint32,
+          &if_int32,   &if_float32, &if_float64, &if_uint8,  &if_uint8,
+          &if_int8,    &if_uint16,  &if_int16,   &if_uint32, &if_int32,
+          &if_float32, &if_float64, &if_uint8};
+
+      assembler->Switch(instance_type, &set_done_and_return, kInstanceType,
+                        instance_type_handlers, arraysize(kInstanceType));
+
+      assembler->Bind(&if_uint8);
+      {
+        Node* value_uint8 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, UINT8_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->SmiFromWord(value_uint8));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+      assembler->Bind(&if_int8);
+      {
+        Node* value_int8 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, INT8_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->SmiFromWord(value_int8));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+      assembler->Bind(&if_uint16);
+      {
+        Node* value_uint16 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, UINT16_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->SmiFromWord(value_uint16));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+      assembler->Bind(&if_int16);
+      {
+        Node* value_int16 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, INT16_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->SmiFromWord(value_int16));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+      assembler->Bind(&if_uint32);
+      {
+        Node* value_uint32 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, UINT32_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->ChangeUint32ToTagged(value_uint32));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+      assembler->Bind(&if_int32);
+      {
+        Node* value_int32 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, INT32_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->ChangeInt32ToTagged(value_int32));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+      assembler->Bind(&if_float32);
+      {
+        Node* value_float32 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, FLOAT32_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->AllocateHeapNumberWithValue(
+            assembler->ChangeFloat32ToFloat64(value_float32)));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+      assembler->Bind(&if_float64);
+      {
+        Node* value_float64 = assembler->LoadFixedTypedArrayElement(
+            data_pointer, index, FLOAT64_ELEMENTS,
+            CodeStubAssembler::SMI_PARAMETERS);
+        var_value.Bind(assembler->AllocateHeapNumberWithValue(value_float64));
+        assembler->Goto(&allocate_entry_if_needed);
+      }
+    }
+  }
+
+  assembler->Bind(&if_slow);
+  {  // Generic objects
+    Label if_slowkeyiterator(assembler);
+    // Let len be ? ToLength(? Get(a, "length")).
+    Node* length;
+    {
+      Callable get_property = CodeFactory::GetProperty(assembler->isolate());
+      length = assembler->CallStub(
+          get_property, context, array,
+          assembler->HeapConstant(
+              assembler->isolate()->factory()->length_string()));
+      Callable to_length = CodeFactory::ToLength(assembler->isolate());
+      length = assembler->CallStub(to_length, context, length);
+    }
+
+    // If index ≥ len, then <goto set_done_and_return>
+    {
+      Label if_notdone(assembler), if_smi(assembler), if_heapnum(assembler);
+
+      assembler->Branch(assembler->TaggedIsSmi(length), &if_smi, &if_heapnum);
+
+      assembler->Bind(&if_smi);
+      {
+        // Are these shortcuts valid?
+        assembler->GotoUnless(assembler->TaggedIsSmi(index),
+                              &set_done_and_return);
+        assembler->Branch(assembler->SmiBelow(index, length), &if_notdone,
+                          &set_done_and_return);
+      }
+
+      assembler->Bind(&if_heapnum);
+      {
+        // Are these shortcuts valid?
+        assembler->GotoIf(assembler->TaggedIsSmi(index), &if_notdone);
+        assembler->Branch(
+            assembler->Float64LessThan(assembler->LoadHeapNumberValue(index),
+                                       assembler->LoadHeapNumberValue(length)),
+            &if_notdone, &set_done_and_return);
+      }
+
+      assembler->Bind(&if_notdone);
+      assembler->StoreObjectFieldNoWriteBarrier(
+          iterator, JSStringIterator::kNextIndexOffset,
+          assembler->NumberInc(index));
+
+      assembler->Branch(
+          assembler->WordEqual(
+              instance_type,
+              assembler->Int32Constant(JS_GENERIC_ARRAY_KEY_ITERATOR_TYPE)),
+          &if_slowkeyiterator, &if_slownotkeyiterator);
+    }
+
+    assembler->Bind(&if_slowkeyiterator);
+    {
+      var_value.Bind(index);
+      assembler->Goto(&allocate_iterator_result);
+    }
+
+    assembler->Bind(&if_slownotkeyiterator);
+    {
+      Callable callable = CodeFactory::GetProperty(assembler->isolate());
+      var_value.Bind(assembler->CallStub(callable, context, array, index));
+      assembler->Goto(&allocate_entry_if_needed);
+    }
+  }
+
+  assembler->Bind(&set_done_and_return);
+  {
+    assembler->StoreObjectFieldNoWriteBarrier(
+        iterator, JSTypedArrayIterator::kIteratedObjectOffset,
+        assembler->UndefinedConstant());
+    assembler->Goto(&did_set_done);
+    assembler->Bind(&did_set_done);
+    {
+      var_value.Bind(assembler->UndefinedConstant());
+      var_done.Bind(assembler->TrueConstant());
+      assembler->Goto(&allocate_iterator_result);
+    }
+  }
+
+  assembler->Bind(&allocate_entry_if_needed);
+  {
+    assembler->GotoIf(
+        assembler->Int32GreaterThan(
+            instance_type,
+            assembler->Int32Constant(LAST_ARRAY_KEY_VALUE_ITERATOR_TYPE)),
+        &allocate_iterator_result);
+    Node* elements = assembler->AllocateFixedArray(FAST_ELEMENTS,
+                                                   assembler->Int32Constant(2));
+    assembler->StoreFixedArrayElement(elements, assembler->Int32Constant(0),
+                                      index, SKIP_WRITE_BARRIER);
+    assembler->StoreFixedArrayElement(elements, assembler->Int32Constant(1),
+                                      var_value.value(), SKIP_WRITE_BARRIER);
+
+    Node* entry = assembler->Allocate(JSArray::kSize);
+    Node* map = assembler->LoadFixedArrayElement(
+        native_context,
+        assembler->IntPtrConstant(Context::JS_ARRAY_FAST_ELEMENTS_MAP_INDEX), 0,
+        CodeStubAssembler::INTPTR_PARAMETERS);
+
+    assembler->StoreMapNoWriteBarrier(entry, map);
+    assembler->StoreObjectFieldRoot(entry, JSArray::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(entry, JSArray::kElementsOffset,
+                                              elements);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        entry, JSArray::kLengthOffset, assembler->SmiConstant(Smi::FromInt(2)));
+
+    var_value.Bind(entry);
+    assembler->Goto(&allocate_iterator_result);
+  }
+
+  assembler->Bind(&allocate_iterator_result);
+  {
+    Node* result = assembler->Allocate(JSIteratorResult::kSize);
+    Node* map = assembler->LoadFixedArrayElement(
+        native_context,
+        assembler->IntPtrConstant(Context::ITERATOR_RESULT_MAP_INDEX), 0,
+        CodeStubAssembler::INTPTR_PARAMETERS);
+    assembler->StoreMapNoWriteBarrier(result, map);
+    assembler->StoreObjectFieldRoot(result, JSIteratorResult::kPropertiesOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldRoot(result, JSIteratorResult::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);
+    assembler->StoreObjectFieldNoWriteBarrier(
+        result, JSIteratorResult::kValueOffset, var_value.value());
+    assembler->StoreObjectFieldNoWriteBarrier(
+        result, JSIteratorResult::kDoneOffset, var_done.value());
+    assembler->Return(result);
+  }
+
+  assembler->Bind(&throw_bad_receiver);
+  {
+    // The {receiver} is not a valid JSArrayIterator or JSTypedArrayIterator.
+    Node* result = assembler->CallRuntime(
+        Runtime::kThrowIncompatibleMethodReceiver, context,
+        assembler->HeapConstant(assembler->factory()->NewStringFromAsciiChecked(
+            "Array Iterator.prototype.next", TENURED)),
+        iterator);
+    assembler->Return(result);
+  }
+}
+
 }  // namespace internal
 }  // namespace v8