Reland "Add fast path for spreading primitive strings."

This is a reland of ef2a19a2.
Use AllocateJSArray to avoid allocating an empty fixed array.

Original change's description:
> Add fast path for spreading primitive strings.
>
> This improves the performance on primitive strings of
> IterableToListWithSymbolLookup, which implements the
> CreateArrayFromIterable bytecode. The fast path is only
> taken if the string iterator protector is valid (that is,
> String.prototype[Symbol.iterator] and
> String.prototype[Symbol.iterator]().next are untouched).
>
> This brings spreading of primitive strings closer to the
> performance of the string iterator optimizations.
> (see https://docs.google.com/document/d/13z1fvRVpe_oEroplXEEX0a3WK94fhXorHjcOMsDmR-8/).
>
> Bug: chromium:881273, v8:7980
> Change-Id: Ic8d8619da2f2afcc9346203613a844f62653fd7a
> Reviewed-on: https://chromium-review.googlesource.com/1243110
> Commit-Queue: Hai Dang <dhai@google.com>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56329}

Bug: chromium:881273, v8:7980
Change-Id: I746c57ddfc300e1032057b5125bc824adf5c2cd3
Reviewed-on: https://chromium-review.googlesource.com/c/1267497
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56438}

src/builtins/builtins-definitions.h

@@ -1123,6 +1123,7 @@ namespace internal {
   /* StringIterator */                                                         \
   /* ES6 #sec-%stringiteratorprototype%.next */                                \
   TFJ(StringIteratorPrototypeNext, 0, kReceiver)                               \
+  TFS(StringToList, kSource)                                                   \
                                                                                \
   /* Symbol */                                                                 \
   /* ES #sec-symbol-constructor */                                             \

src/builtins/builtins-iterator-gen.cc

@@ -5,6 +5,7 @@
 #include "src/builtins/builtins-iterator-gen.h"
 #include "src/builtins/growable-fixed-array-gen.h"
 
+#include "src/builtins/builtins-string-gen.h"
 #include "src/builtins/builtins-utils-gen.h"
 #include "src/builtins/builtins.h"
 #include "src/code-stub-assembler.h"
@@ -261,20 +262,37 @@ TF_BUILTIN(IterableToListMayPreserveHoles, IteratorBuiltinsAssembler) {
 }
 
 // This builtin loads the property Symbol.iterator as the iterator, and has a
-// fast path for fast arrays. In case of fast holey arrays, holes will be
-// converted to undefined to reflect iteration semantics. Note that replacement
-// by undefined is only correct when the NoElements protector is valid.
+// fast path for fast arrays and another one for strings. These fast paths will
+// only be taken if Symbol.iterator and the Iterator prototype are not modified
+// in a way that changes the original iteration behavior.
+// * In case of fast holey arrays, holes will be converted to undefined to
+// reflect iteration semantics. Note that replacement by undefined is only
+// correct when the NoElements protector is valid.
 TF_BUILTIN(IterableToListWithSymbolLookup, IteratorBuiltinsAssembler) {
   TNode<Context> context = CAST(Parameter(Descriptor::kContext));
   TNode<Object> iterable = CAST(Parameter(Descriptor::kIterable));
 
-  Label slow_path(this);
+  Label slow_path(this), check_string(this);
 
-  GotoIfNot(IsFastJSArrayWithNoCustomIteration(iterable, context), &slow_path);
-  // Here we are guaranteed that iterable is a fast JSArray with an original
-  // iterator.
+  GotoIfForceSlowPath(&slow_path);
+
+  GotoIfNot(IsFastJSArrayWithNoCustomIteration(iterable, context),
+            &check_string);
+
+  // Fast path for fast JSArray.
   TailCallBuiltin(Builtins::kCloneFastJSArrayFillingHoles, context, iterable);
 
+  BIND(&check_string);
+  {
+    StringBuiltinsAssembler string_assembler(state());
+    GotoIfNot(string_assembler.IsStringPrimitiveWithNoCustomIteration(iterable,
+                                                                      context),
+              &slow_path);
+
+    // Fast path for strings.
+    TailCallBuiltin(Builtins::kStringToList, context, iterable);
+  }
+
   BIND(&slow_path);
   {
     TNode<Object> iterator_fn = GetIteratorMethod(context, iterable);

src/builtins/builtins-string-gen.cc

@@ -2493,6 +2493,85 @@ TF_BUILTIN(StringIteratorPrototypeNext, StringBuiltinsAssembler) {
   }
 }
 
+TNode<BoolT> StringBuiltinsAssembler::IsStringPrimitiveWithNoCustomIteration(
+    TNode<Object> object, TNode<Context> context) {
+  Label if_false(this, Label::kDeferred), exit(this);
+  TVARIABLE(BoolT, var_result);
+
+  GotoIf(TaggedIsSmi(object), &if_false);
+  GotoIfNot(IsString(CAST(object)), &if_false);
+
+  // Check that the String iterator hasn't been modified in a way that would
+  // affect iteration.
+  Node* protector_cell = LoadRoot(RootIndex::kStringIteratorProtector);
+  DCHECK(isolate()->heap()->string_iterator_protector()->IsPropertyCell());
+  var_result =
+      WordEqual(LoadObjectField(protector_cell, PropertyCell::kValueOffset),
+                SmiConstant(Isolate::kProtectorValid));
+  Goto(&exit);
+
+  BIND(&if_false);
+  {
+    var_result = Int32FalseConstant();
+    Goto(&exit);
+  }
+
+  BIND(&exit);
+  return var_result.value();
+}
+
+TNode<JSArray> StringBuiltinsAssembler::StringToList(TNode<Context> context,
+                                                     TNode<String> string) {
+  CSA_ASSERT(this, IsStringPrimitiveWithNoCustomIteration(string, context));
+  const ElementsKind kind = PACKED_ELEMENTS;
+  const TNode<IntPtrT> length = LoadStringLengthAsWord(string);
+
+  Node* const array_map =
+      LoadJSArrayElementsMap(kind, LoadNativeContext(context));
+  Node* const array = AllocateJSArray(kind, array_map, length, SmiTag(length));
+  Node* const elements = LoadElements(array);
+
+  const int first_element_offset = FixedArray::kHeaderSize - kHeapObjectTag;
+  TNode<IntPtrT> first_to_element_offset =
+      ElementOffsetFromIndex(IntPtrConstant(0), kind, INTPTR_PARAMETERS, 0);
+  VARIABLE(
+      var_offset, MachineType::PointerRepresentation(),
+      IntPtrAdd(first_to_element_offset, IntPtrConstant(first_element_offset)));
+  TVARIABLE(IntPtrT, var_position, IntPtrConstant(0));
+  Label done(this), next_codepoint(this, {&var_position, &var_offset});
+
+  Goto(&next_codepoint);
+
+  BIND(&next_codepoint);
+  {
+    // Loop condition.
+    GotoIfNot(IntPtrLessThan(var_position.value(), length), &done);
+    const UnicodeEncoding encoding = UnicodeEncoding::UTF16;
+    TNode<Int32T> ch =
+        LoadSurrogatePairAt(string, length, var_position.value(), encoding);
+    TNode<String> value = StringFromSingleCodePoint(ch, encoding);
+
+    Store(elements, var_offset.value(), value);
+
+    // Increment the position.
+    TNode<IntPtrT> ch_length = LoadStringLengthAsWord(value);
+    var_position = IntPtrAdd(var_position.value(), ch_length);
+    // Increment the array offset and continue the loop.
+    var_offset.Bind(
+        IntPtrAdd(var_offset.value(), IntPtrConstant(kPointerSize)));
+    Goto(&next_codepoint);
+  }
+
+  BIND(&done);
+  return UncheckedCast<JSArray>(array);
+}
+
+TF_BUILTIN(StringToList, StringBuiltinsAssembler) {
+  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
+  TNode<String> string = CAST(Parameter(Descriptor::kSource));
+  Return(StringToList(context, string));
+}
+
 // -----------------------------------------------------------------------------
 // ES6 section B.2.3 Additional Properties of the String.prototype object
 

src/builtins/builtins-string-gen.h

@@ -23,8 +23,12 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
                         Node* rhs, Node* rhs_instance_type,
                         TNode<IntPtrT> length, Label* if_equal,
                         Label* if_not_equal, Label* if_indirect);
+  TNode<BoolT> IsStringPrimitiveWithNoCustomIteration(TNode<Object> object,
+                                                      TNode<Context> context);
 
  protected:
+  TNode<JSArray> StringToList(TNode<Context> context, TNode<String> string);
+
   void StringEqual_Loop(Node* lhs, Node* lhs_instance_type,
                         MachineType lhs_type, Node* rhs,
                         Node* rhs_instance_type, MachineType rhs_type,

src/compiler/js-heap-broker.h

@@ -242,6 +242,7 @@ class ContextRef : public HeapObjectRef {
   V(JSFunction, promise_function)             \
   V(Map, fast_aliased_arguments_map)          \
   V(Map, initial_array_iterator_map)          \
+  V(Map, initial_string_iterator_map)         \
   V(Map, iterator_result_map)                 \
   V(Map, js_array_holey_double_elements_map)  \
   V(Map, js_array_holey_elements_map)         \
@@ -257,7 +258,6 @@ class ContextRef : public HeapObjectRef {
   V(Map, sloppy_arguments_map)                \
   V(Map, slow_object_with_null_prototype_map) \
   V(Map, strict_arguments_map)                \
-  V(Map, initial_string_iterator_map)         \
   V(ScriptContextTable, script_context_table)
 
 class NativeContextRef : public ContextRef {

src/heap/heap.h

@@ -150,6 +150,7 @@ using v8::MemoryPressureLevel;
   V(TypedArraySpeciesProtector)             \
   V(PromiseSpeciesProtector)                \
   V(StaleRegister)                          \
+  V(StringIteratorProtector)                \
   V(StringLengthProtector)                  \
   V(StringTableMap)                         \
   V(SymbolMap)                              \

test/mjsunit/es6/string-iterator2.js

@@ -4,7 +4,9 @@
 
 // Flags: --allow-natives-syntax --no-stress-opt
 
-// Tests for primitive strings.
+// Tests for spreading primitive strings.
+
+assertEquals([...''], []);
 
 var str = 'ott';
 assertEquals(['o', 't', 't'], [...str]);

tools/v8heapconst.py

@@ -378,6 +378,7 @@ KNOWN_OBJECTS = {
   ("OLD_SPACE", 0x02361): "StringLengthProtector",
   ("OLD_SPACE", 0x02371): "ArrayIteratorProtector",
   ("OLD_SPACE", 0x02399): "ArrayBufferNeuteringProtector",
+  ("OLD_SPACE", 0x02421): "StringIteratorProtector",
 }
 
 # List of known V8 Frame Markers.