Revert of [builtins] Migrate String.prototype.charCodeAt and...

Revert of [builtins] Migrate String.prototype.charCodeAt and String.prototype.charAt to TurboFan. (patchset #3 id:40001 of https://codereview.chromium.org/1868963002/ )

Reason for revert:
Please rebaseline:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/5992

Original issue's description:
> [builtins] Migrate String.prototype.charCodeAt and String.prototype.charAt to TurboFan.
>
> Make these builtins fast by default w/o relying on the
> %_StringCharCodeAt and %_StringCharAt intrinsics, which we cannot deal
> with well in TurboFan (and ignition).
>
> CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux_nosnap_dbg
> R=epertoso@chromium.org
>
> Committed: https://crrev.com/43d3331d87f68ea7efa1b58f559f33f60e5dacfb
> Cr-Commit-Position: refs/heads/master@{#35350}
>
> Committed: https://crrev.com/253a16f690ab593a95249c34ded9df477ad9a54c
> Cr-Commit-Position: refs/heads/master@{#35402}

TBR=epertoso@chromium.org,bmeurer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1877263002

Cr-Commit-Position: refs/heads/master@{#35410}

src/bootstrapper.cc

@@ -1255,22 +1255,6 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
                                    attribs);
       string_map->AppendDescriptor(&d);
     }
-
-    // Create the %StringPrototype%
-    Handle<JSValue> prototype =
-        Handle<JSValue>::cast(factory->NewJSObject(string_fun, TENURED));
-    prototype->set_value(isolate->heap()->empty_string());
-    Accessors::FunctionSetPrototype(string_fun, prototype).Assert();
-
-    // Install the "constructor" property on the {prototype}.
-    JSObject::AddProperty(prototype, factory->constructor_string(), string_fun,
-                          DONT_ENUM);
-
-    // Install the String.prototype methods.
-    SimpleInstallFunction(prototype, "charAt", Builtins::kStringPrototypeCharAt,
-                          1, true);
-    SimpleInstallFunction(prototype, "charCodeAt",
-                          Builtins::kStringPrototypeCharCodeAt, 1, true);
   }
 
   {

src/builtins.cc

@@ -4187,199 +4187,6 @@ BUILTIN(ObjectProtoToString) {
   return *result;
 }
 
-// -----------------------------------------------------------------------------
-// ES6 section 21.1 String Objects
-
-// ES6 section 21.1.3.1 String.prototype.charAt ( pos )
-void Builtins::Generate_StringPrototypeCharAt(
-    compiler::CodeStubAssembler* assembler) {
-  typedef compiler::CodeStubAssembler::Label Label;
-  typedef compiler::Node Node;
-  typedef compiler::CodeStubAssembler::Variable Variable;
-
-  Node* receiver = assembler->Parameter(0);
-  Node* position = assembler->Parameter(1);
-  Node* context = assembler->Parameter(4);
-
-  // Check that {receiver} is coercible to Object and convert it to a String.
-  receiver =
-      assembler->ToThisString(context, receiver, "String.prototype.charAt");
-
-  // Convert the {position} to a Smi and check that it's in bounds of the
-  // {receiver}.
-  // TODO(bmeurer): Find an abstraction for this!
-  {
-    // Check if the {position} is already a Smi.
-    Variable var_position(assembler, MachineRepresentation::kTagged);
-    var_position.Bind(position);
-    Label if_positionissmi(assembler),
-        if_positionisnotsmi(assembler, Label::kDeferred);
-    assembler->Branch(assembler->WordIsSmi(position), &if_positionissmi,
-                      &if_positionisnotsmi);
-    assembler->Bind(&if_positionisnotsmi);
-    {
-      // Convert the {position} to an Integer via the ToIntegerStub.
-      Callable callable = CodeFactory::ToInteger(assembler->isolate());
-      Node* index = assembler->CallStub(callable, context, position);
-
-      // Check if the resulting {index} is now a Smi.
-      Label if_indexissmi(assembler, Label::kDeferred),
-          if_indexisnotsmi(assembler, Label::kDeferred);
-      assembler->Branch(assembler->WordIsSmi(index), &if_indexissmi,
-                        &if_indexisnotsmi);
-
-      assembler->Bind(&if_indexissmi);
-      {
-        var_position.Bind(index);
-        assembler->Goto(&if_positionissmi);
-      }
-
-      assembler->Bind(&if_indexisnotsmi);
-      {
-        // The ToIntegerStub canonicalizes everything in Smi range to Smi
-        // representation, so any HeapNumber returned is not in Smi range.
-        // The only exception here is -0.0, which we treat as 0.
-        Node* index_value = assembler->LoadHeapNumberValue(index);
-        Label if_indexiszero(assembler, Label::kDeferred),
-            if_indexisnotzero(assembler, Label::kDeferred);
-        assembler->Branch(assembler->Float64Equal(
-                              index_value, assembler->Float64Constant(0.0)),
-                          &if_indexiszero, &if_indexisnotzero);
-
-        assembler->Bind(&if_indexiszero);
-        {
-          var_position.Bind(assembler->SmiConstant(Smi::FromInt(0)));
-          assembler->Goto(&if_positionissmi);
-        }
-
-        assembler->Bind(&if_indexisnotzero);
-        {
-          // The {index} is some other integral Number, that is definitely
-          // neither -0.0 nor in Smi range.
-          assembler->Return(assembler->EmptyStringConstant());
-        }
-      }
-    }
-    assembler->Bind(&if_positionissmi);
-    position = var_position.value();
-
-    // Determine the actual length of the {receiver} String.
-    Node* receiver_length =
-        assembler->LoadObjectField(receiver, String::kLengthOffset);
-
-    // Return "" if the Smi {position} is outside the bounds of the {receiver}.
-    Label if_positioninbounds(assembler),
-        if_positionnotinbounds(assembler, Label::kDeferred);
-    assembler->Branch(assembler->SmiAboveOrEqual(position, receiver_length),
-                      &if_positionnotinbounds, &if_positioninbounds);
-    assembler->Bind(&if_positionnotinbounds);
-    assembler->Return(assembler->EmptyStringConstant());
-    assembler->Bind(&if_positioninbounds);
-  }
-
-  // Load the character code at the {position} from the {receiver}.
-  Node* code = assembler->StringCharCodeAt(receiver, position);
-
-  // And return the single character string with only that {code}.
-  Node* result = assembler->StringFromCharCode(code);
-  assembler->Return(result);
-}
-
-// ES6 section 21.1.3.2 String.prototype.charCodeAt ( pos )
-void Builtins::Generate_StringPrototypeCharCodeAt(
-    compiler::CodeStubAssembler* assembler) {
-  typedef compiler::CodeStubAssembler::Label Label;
-  typedef compiler::Node Node;
-  typedef compiler::CodeStubAssembler::Variable Variable;
-
-  Node* receiver = assembler->Parameter(0);
-  Node* position = assembler->Parameter(1);
-  Node* context = assembler->Parameter(4);
-
-  // Check that {receiver} is coercible to Object and convert it to a String.
-  receiver =
-      assembler->ToThisString(context, receiver, "String.prototype.charCodeAt");
-
-  // Convert the {position} to a Smi and check that it's in bounds of the
-  // {receiver}.
-  // TODO(bmeurer): Find an abstraction for this!
-  {
-    // Check if the {position} is already a Smi.
-    Variable var_position(assembler, MachineRepresentation::kTagged);
-    var_position.Bind(position);
-    Label if_positionissmi(assembler),
-        if_positionisnotsmi(assembler, Label::kDeferred);
-    assembler->Branch(assembler->WordIsSmi(position), &if_positionissmi,
-                      &if_positionisnotsmi);
-    assembler->Bind(&if_positionisnotsmi);
-    {
-      // Convert the {position} to an Integer via the ToIntegerStub.
-      Callable callable = CodeFactory::ToInteger(assembler->isolate());
-      Node* index = assembler->CallStub(callable, context, position);
-
-      // Check if the resulting {index} is now a Smi.
-      Label if_indexissmi(assembler, Label::kDeferred),
-          if_indexisnotsmi(assembler, Label::kDeferred);
-      assembler->Branch(assembler->WordIsSmi(index), &if_indexissmi,
-                        &if_indexisnotsmi);
-
-      assembler->Bind(&if_indexissmi);
-      {
-        var_position.Bind(index);
-        assembler->Goto(&if_positionissmi);
-      }
-
-      assembler->Bind(&if_indexisnotsmi);
-      {
-        // The ToIntegerStub canonicalizes everything in Smi range to Smi
-        // representation, so any HeapNumber returned is not in Smi range.
-        // The only exception here is -0.0, which we treat as 0.
-        Node* index_value = assembler->LoadHeapNumberValue(index);
-        Label if_indexiszero(assembler, Label::kDeferred),
-            if_indexisnotzero(assembler, Label::kDeferred);
-        assembler->Branch(assembler->Float64Equal(
-                              index_value, assembler->Float64Constant(0.0)),
-                          &if_indexiszero, &if_indexisnotzero);
-
-        assembler->Bind(&if_indexiszero);
-        {
-          var_position.Bind(assembler->SmiConstant(Smi::FromInt(0)));
-          assembler->Goto(&if_positionissmi);
-        }
-
-        assembler->Bind(&if_indexisnotzero);
-        {
-          // The {index} is some other integral Number, that is definitely
-          // neither -0.0 nor in Smi range.
-          assembler->Return(assembler->NaNConstant());
-        }
-      }
-    }
-    assembler->Bind(&if_positionissmi);
-    position = var_position.value();
-
-    // Determine the actual length of the {receiver} String.
-    Node* receiver_length =
-        assembler->LoadObjectField(receiver, String::kLengthOffset);
-
-    // Return NaN if the Smi {position} is outside the bounds of the {receiver}.
-    Label if_positioninbounds(assembler),
-        if_positionnotinbounds(assembler, Label::kDeferred);
-    assembler->Branch(assembler->SmiAboveOrEqual(position, receiver_length),
-                      &if_positionnotinbounds, &if_positioninbounds);
-    assembler->Bind(&if_positionnotinbounds);
-    assembler->Return(assembler->NaNConstant());
-    assembler->Bind(&if_positioninbounds);
-  }
-
-  // Load the character at the {position} from the {receiver}.
-  Node* value = assembler->StringCharCodeAt(receiver, position);
-  Node* result = assembler->SmiFromWord32(value);
-  assembler->Return(result);
-}
-
-// -----------------------------------------------------------------------------
-// ES6 section 21.1 ArrayBuffer Objects
 
 // ES6 section 24.1.2.1 ArrayBuffer ( length ) for the [[Call]] case.
 BUILTIN(ArrayBufferConstructor) {

src/builtins.h

@@ -313,9 +313,7 @@ inline bool operator&(BuiltinExtraArguments lhs, BuiltinExtraArguments rhs) {
   V(MathRound, 2)                \
   V(MathSqrt, 2)                 \
   V(MathTrunc, 2)                \
-  V(ObjectHasOwnProperty, 2)     \
-  V(StringPrototypeCharAt, 2)    \
-  V(StringPrototypeCharCodeAt, 2)
+  V(ObjectHasOwnProperty, 2)
 
 // Define list of builtin handlers implemented in assembly.
 #define BUILTIN_LIST_H(V)                    \
@@ -631,13 +629,6 @@ class Builtins {
   static void Generate_ObjectHasOwnProperty(
       compiler::CodeStubAssembler* assembler);
 
-  // ES6 section 21.1.3.1 String.prototype.charAt ( pos )
-  static void Generate_StringPrototypeCharAt(
-      compiler::CodeStubAssembler* assembler);
-  // ES6 section 21.1.3.2 String.prototype.charCodeAt ( pos )
-  static void Generate_StringPrototypeCharCodeAt(
-      compiler::CodeStubAssembler* assembler);
-
   static void Generate_StringConstructor(MacroAssembler* masm);
   static void Generate_StringConstructor_ConstructStub(MacroAssembler* masm);
   static void Generate_OnStackReplacement(MacroAssembler* masm);

src/compiler/code-stub-assembler.h

@@ -166,10 +166,7 @@ class CodeStubAssembler {
   Node* ExternalConstant(ExternalReference address);
   Node* Float64Constant(double value);
   Node* BooleanMapConstant();
-  Node* EmptyStringConstant();
   Node* HeapNumberMapConstant();
-  Node* NaNConstant();
-  Node* NoContextConstant();
   Node* NullConstant();
   Node* UndefinedConstant();
 
@@ -285,11 +282,9 @@ class CodeStubAssembler {
   Node* SmiTag(Node* value);
   // Untag a Smi value as a Word.
   Node* SmiUntag(Node* value);
-  Node* SmiToWord(Node* value) { return SmiUntag(value); }
 
   // Smi conversions.
   Node* SmiToFloat64(Node* value);
-  Node* SmiFromWord32(Node* value);
   Node* SmiToWord32(Node* value);
 
   // Smi operations.
@@ -298,7 +293,6 @@ class CodeStubAssembler {
   Node* SmiSub(Node* a, Node* b);
   Node* SmiSubWithOverflow(Node* a, Node* b);
   Node* SmiEqual(Node* a, Node* b);
-  Node* SmiAboveOrEqual(Node* a, Node* b);
   Node* SmiLessThan(Node* a, Node* b);
   Node* SmiLessThanOrEqual(Node* a, Node* b);
   Node* SmiMin(Node* a, Node* b);
@@ -318,10 +312,6 @@ class CodeStubAssembler {
   // Load a field from an object on the heap.
   Node* LoadObjectField(Node* object, int offset,
                         MachineType rep = MachineType::AnyTagged());
-  // Store a field to an object on the heap.
-  Node* StoreObjectFieldNoWriteBarrier(
-      Node* object, int offset, Node* value,
-      MachineRepresentation rep = MachineRepresentation::kTagged);
   // Load the floating point value of a HeapNumber.
   Node* LoadHeapNumberValue(Node* object);
   // Store the floating point value of a HeapNumber.
@@ -351,19 +341,12 @@ class CodeStubAssembler {
 
   // Allocate an object of the given size.
   Node* Allocate(int size, AllocationFlags flags = kNone);
-  Node* InnerAllocate(Node* previous, int offset);
   // Allocate a HeapNumber without initializing its value.
   Node* AllocateHeapNumber();
   // Allocate a HeapNumber with a specific value.
   Node* AllocateHeapNumberWithValue(Node* value);
-  // Allocate a SeqOneByteString with the given length.
-  Node* AllocateSeqOneByteString(int length);
-  // Allocate a SeqTwoByteString with the given length.
-  Node* AllocateSeqTwoByteString(int length);
 
   // Store an array element to a FixedArray.
-  Node* StoreFixedArrayElementInt32Index(Node* object, Node* index,
-                                         Node* value);
   Node* StoreFixedArrayElementNoWriteBarrier(Node* object, Node* index,
                                              Node* value);
   // Load the Map of an HeapObject.
@@ -392,17 +375,6 @@ class CodeStubAssembler {
   Node* TruncateTaggedToFloat64(Node* context, Node* value);
   Node* TruncateTaggedToWord32(Node* context, Node* value);
 
-  // Type conversions.
-  // Throws a TypeError for {method_name} if {value} is not coercible to Object,
-  // or returns the {value} converted to a String otherwise.
-  Node* ToThisString(Node* context, Node* value, char const* method_name);
-
-  // String helpers.
-  // Load a character from a String (might flatten a ConsString).
-  Node* StringCharCodeAt(Node* string, Node* smi_index);
-  // Return the single character string with only {code}.
-  Node* StringFromCharCode(Node* code);
-
   // Branching helpers.
   // TODO(danno): Can we be more cleverish wrt. edge-split?
   void BranchIf(Node* condition, Label* if_true, Label* if_false);

src/js/messages.js

@@ -37,6 +37,7 @@ var ObjectHasOwnProperty;
 var ObjectToString = utils.ImportNow("object_to_string");
 var Script = utils.ImportNow("Script");
 var stackTraceSymbol = utils.ImportNow("stack_trace_symbol");
+var StringCharAt;
 var StringIndexOf;
 var StringSubstring;
 var SymbolToString;
@@ -56,6 +57,7 @@ utils.Import(function(from) {
   Int8x16ToString = from.Int8x16ToString;
   ObjectDefineProperty = from.ObjectDefineProperty;
   ObjectHasOwnProperty = from.ObjectHasOwnProperty;
+  StringCharAt = from.StringCharAt;
   StringIndexOf = from.StringIndexOf;
   StringSubstring = from.StringSubstring;
   SymbolToString = from.SymbolToString;
@@ -253,7 +255,6 @@ function ScriptLineFromPosition(position) {
   return -1;
 }
 
-
 /**
  * Get information on a specific source position.
  * @param {number} position The source position
@@ -271,7 +272,7 @@ function ScriptLocationFromPosition(position,
   var line_ends = this.line_ends;
   var start = line == 0 ? 0 : line_ends[line - 1] + 1;
   var end = line_ends[line];
-  if (end > 0 && %_StringCharAt(this.source, end - 1) === '\r') {
+  if (end > 0 && %_Call(StringCharAt, this.source, end - 1) == '\r') {
     end--;
   }
   var column = position - start;

src/js/string.js

@@ -57,6 +57,30 @@ function StringValueOf() {
 }
 
 
+// ECMA-262, section 15.5.4.4
+function StringCharAtJS(pos) {
+  CHECK_OBJECT_COERCIBLE(this, "String.prototype.charAt");
+
+  var result = %_StringCharAt(this, pos);
+  if (%_IsSmi(result)) {
+    result = %_StringCharAt(TO_STRING(this), TO_INTEGER(pos));
+  }
+  return result;
+}
+
+
+// ECMA-262 section 15.5.4.5
+function StringCharCodeAtJS(pos) {
+  CHECK_OBJECT_COERCIBLE(this, "String.prototype.charCodeAt");
+
+  var result = %_StringCharCodeAt(this, pos);
+  if (!%_IsSmi(result)) {
+    result = %_StringCharCodeAt(TO_STRING(this), TO_INTEGER(pos));
+  }
+  return result;
+}
+
+
 // ECMA-262, section 15.5.4.6
 function StringConcat(other /* and more */) {  // length == 1
   "use strict";
@@ -833,6 +857,13 @@ function StringRaw(callSite) {
 
 // -------------------------------------------------------------------
 
+// Set the String function and constructor.
+%FunctionSetPrototype(GlobalString, new GlobalString());
+
+// Set up the constructor property on the String prototype object.
+%AddNamedProperty(
+    GlobalString.prototype, "constructor", GlobalString, DONT_ENUM);
+
 // Set up the non-enumerable functions on the String object.
 utils.InstallFunctions(GlobalString, DONT_ENUM, [
   "fromCharCode", StringFromCharCode,
@@ -844,6 +875,8 @@ utils.InstallFunctions(GlobalString, DONT_ENUM, [
 utils.InstallFunctions(GlobalString.prototype, DONT_ENUM, [
   "valueOf", StringValueOf,
   "toString", StringToString,
+  "charAt", StringCharAtJS,
+  "charCodeAt", StringCharCodeAtJS,
   "codePointAt", StringCodePointAt,
   "concat", StringConcat,
   "endsWith", StringEndsWith,
@@ -889,6 +922,7 @@ utils.InstallFunctions(GlobalString.prototype, DONT_ENUM, [
 
 utils.Export(function(to) {
   to.ExpandReplacement = ExpandReplacement;
+  to.StringCharAt = StringCharAtJS;
   to.StringIndexOf = StringIndexOf;
   to.StringLastIndexOf = StringLastIndexOf;
   to.StringMatch = StringMatchJS;

src/runtime/runtime-internal.cc

@@ -431,13 +431,6 @@ RUNTIME_FUNCTION(Runtime_ThrowCalledNonCallable) {
       isolate, NewTypeError(MessageTemplate::kCalledNonCallable, callsite));
 }
 
-RUNTIME_FUNCTION(Runtime_ThrowCalledOnNullOrUndefined) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
-  THROW_NEW_ERROR_RETURN_FAILURE(
-      isolate, NewTypeError(MessageTemplate::kCalledOnNullOrUndefined, name));
-}
 
 RUNTIME_FUNCTION(Runtime_ThrowConstructedNonConstructable) {
   HandleScope scope(isolate);

src/runtime/runtime-strings.cc

@@ -1273,13 +1273,6 @@ RUNTIME_FUNCTION(Runtime_StringCharAt) {
   return __RT_impl_Runtime_StringCharFromCode(Arguments(1, &code), isolate);
 }
 
-RUNTIME_FUNCTION(Runtime_ExternalStringGetChar) {
-  SealHandleScope shs(isolate);
-  DCHECK_EQ(2, args.length());
-  CONVERT_ARG_CHECKED(ExternalString, string, 0);
-  CONVERT_INT32_ARG_CHECKED(index, 1);
-  return Smi::FromInt(string->Get(index));
-}
 
 RUNTIME_FUNCTION(Runtime_OneByteSeqStringGetChar) {
   SealHandleScope shs(isolate);

src/runtime/runtime.h

@@ -312,7 +312,6 @@ namespace internal {
   F(ThrowConstructedNonConstructable, 1, 1)         \
   F(ThrowDerivedConstructorReturnedNonObject, 0, 1) \
   F(ThrowCalledNonCallable, 1, 1)                   \
-  F(ThrowCalledOnNullOrUndefined, 1, 1)             \
   F(CreateListFromArrayLike, 1, 1)                  \
   F(IncrementUseCounter, 1, 1)                      \
   F(GetOrdinaryHasInstance, 0, 1)                   \
@@ -859,7 +858,6 @@ namespace internal {
   F(FlattenString, 1, 1)                  \
   F(StringCharFromCode, 1, 1)             \
   F(StringCharAt, 2, 1)                   \
-  F(ExternalStringGetChar, 2, 1)          \
   F(OneByteSeqStringGetChar, 2, 1)        \
   F(OneByteSeqStringSetChar, 3, 1)        \
   F(TwoByteSeqStringGetChar, 2, 1)        \