Revert "Add fast path for spreading primitive strings."

This reverts commit ef2a19a2.

Reason for revert: Broken layout tests: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/201392

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}

TBR=ulan@chromium.org,neis@chromium.org,sigurds@chromium.org,bmeurer@chromium.org,dhai@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:881273, v8:7980
Change-Id: I4868160b87bdebf9fd2ff346aefd4cdce23681a1
Reviewed-on: https://chromium-review.googlesource.com/c/1261022Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56365}

src/builtins/builtins-definitions.h

@@ -1127,7 +1127,6 @@ 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,7 +5,6 @@
 #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"
@@ -262,37 +261,20 @@ TF_BUILTIN(IterableToListMayPreserveHoles, IteratorBuiltinsAssembler) {
 }
 
 // This builtin loads the property Symbol.iterator as the iterator, and has a
-// 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.
+// 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.
 TF_BUILTIN(IterableToListWithSymbolLookup, IteratorBuiltinsAssembler) {
   TNode<Context> context = CAST(Parameter(Descriptor::kContext));
   TNode<Object> iterable = CAST(Parameter(Descriptor::kIterable));
 
-  Label slow_path(this), check_string(this);
-
-  GotoIfForceSlowPath(&slow_path);
-
-  GotoIfNot(IsFastJSArrayWithNoCustomIteration(iterable, context),
-            &check_string);
+  Label slow_path(this);
 
-  // Fast path for fast JSArray.
+  GotoIfNot(IsFastJSArrayWithNoCustomIteration(iterable, context), &slow_path);
+  // Here we are guaranteed that iterable is a fast JSArray with an original
+  // iterator.
   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,89 +2493,6 @@ 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;
-  TNode<IntPtrT> length = LoadStringLengthAsWord(string);
-
-  Node *array = nullptr, *elements = nullptr;
-  Node* array_map = LoadJSArrayElementsMap(kind, LoadNativeContext(context));
-  // Allocate both array and elements object, and initialize the JSArray.
-  std::tie(array, elements) = AllocateUninitializedJSArrayWithElements(
-      kind, array_map, SmiTag(length), nullptr, length);
-  // Put the array in a valid state because the loop below can trigger GC.
-  FillFixedArrayWithSmiZero(CAST(elements), length);
-
-  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});
-
-  // TODO(dhai): refactor the loop construction for string to reuse.
-  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,12 +23,8 @@ 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,7 +242,6 @@ 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)         \
@@ -258,6 +257,7 @@ 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,7 +150,6 @@ using v8::MemoryPressureLevel;
   V(TypedArraySpeciesProtector)             \
   V(PromiseSpeciesProtector)                \
   V(StaleRegister)                          \
-  V(StringIteratorProtector)                \
   V(StringLengthProtector)                  \
   V(StringTableMap)                         \
   V(SymbolMap)                              \

tools/v8heapconst.py

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