[builtins] Port String.prototype.includes to CSA

- Convert S.p.includes builtin from CPP to TFJ
  - Fast paths S.p.includes(str) and S.p.includes(str, smi)
- Add Runtime kStringIncludes
- Add StringIncludesIndexOfAssembler (Generate is based on
  StringPrototypeIndexOf builtin)
- S.p.includes and S.p.indexOf both use StringIncludesIndexOfAssembler

Quick measurements show 3x improvement for S.p.includes(str).
More about the measurements: https://gist.github.com/peterwmwong/7a2a96f3171a52f16ca8125a089f38e7

Bug: v8:6680
Change-Id: I79cb8dbe2b79e6df15aa734e128eee25c7e6aaf5
Reviewed-on: https://chromium-review.googlesource.com/620150Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47546}

AUTHORS

@@ -108,6 +108,7 @@ Paolo Giarrusso <p.giarrusso@gmail.com>
 Patrick Gansterer <paroga@paroga.com>
 Peter Rybin <peter.rybin@gmail.com>
 Peter Varga <pvarga@inf.u-szeged.hu>
+Peter Wong <peter.wm.wong@gmail.com>
 Paul Lind <plind44@gmail.com>
 Qiuyi Zhang <qiuyi.zqy@alibaba-inc.com>
 Rafal Krypa <rafal@krypa.net>

src/builtins/builtins-definitions.h

@@ -898,7 +898,8 @@ namespace internal {
   /* ES6 #sec-string.prototype.endswith */                                     \
   CPP(StringPrototypeEndsWith)                                                 \
   /* ES6 #sec-string.prototype.includes */                                     \
-  CPP(StringPrototypeIncludes)                                                 \
+  TFJ(StringPrototypeIncludes,                                                 \
+      SharedFunctionInfo::kDontAdaptArgumentsSentinel)                         \
   /* ES6 #sec-string.prototype.indexof */                                      \
   TFJ(StringPrototypeIndexOf, SharedFunctionInfo::kDontAdaptArgumentsSentinel) \
   /* ES6 #sec-string.prototype.lastindexof */                                  \

src/builtins/builtins-string-gen.cc

@@ -769,9 +769,8 @@ TF_BUILTIN(StringPrototypeConcat, CodeStubAssembler) {
 }
 
 void StringBuiltinsAssembler::StringIndexOf(
-    Node* const subject_string, Node* const subject_instance_type,
-    Node* const search_string, Node* const search_instance_type,
-    Node* const position, std::function<void(Node*)> f_return) {
+    Node* const subject_string, Node* const search_string, Node* const position,
+    std::function<void(Node*)> f_return) {
   CSA_ASSERT(this, IsString(subject_string));
   CSA_ASSERT(this, IsString(search_string));
   CSA_ASSERT(this, TaggedIsSmi(position));
@@ -949,84 +948,89 @@ TF_BUILTIN(StringIndexOf, StringBuiltinsAssembler) {
   Node* receiver = Parameter(Descriptor::kReceiver);
   Node* search_string = Parameter(Descriptor::kSearchString);
   Node* position = Parameter(Descriptor::kPosition);
-
-  Node* instance_type = LoadInstanceType(receiver);
-  Node* search_string_instance_type = LoadInstanceType(search_string);
-
-  StringIndexOf(receiver, instance_type, search_string,
-                search_string_instance_type, position,
+  StringIndexOf(receiver, search_string, position,
                 [this](Node* result) { this->Return(result); });
 }
 
+// ES6 String.prototype.includes(searchString [, position])
+// #sec-string.prototype.includes
+TF_BUILTIN(StringPrototypeIncludes, StringIncludesIndexOfAssembler) {
+  Generate(kIncludes);
+}
+
 // ES6 String.prototype.indexOf(searchString [, position])
 // #sec-string.prototype.indexof
-TF_BUILTIN(StringPrototypeIndexOf, StringBuiltinsAssembler) {
-  VARIABLE(search_string, MachineRepresentation::kTagged);
-  VARIABLE(position, MachineRepresentation::kTagged);
-  Label call_runtime(this), call_runtime_unchecked(this), argc_0(this),
-      no_argc_0(this), argc_1(this), no_argc_1(this), argc_2(this),
-      fast_path(this), return_minus_1(this);
+TF_BUILTIN(StringPrototypeIndexOf, StringIncludesIndexOfAssembler) {
+  Generate(kIndexOf);
+}
 
+void StringIncludesIndexOfAssembler::Generate(SearchVariant variant) {
   // TODO(ishell): use constants from Descriptor once the JSFunction linkage
   // arguments are reordered.
   Node* argc = Parameter(BuiltinDescriptor::kArgumentsCount);
-  Node* context = Parameter(BuiltinDescriptor::kContext);
-
+  Node* const context = Parameter(BuiltinDescriptor::kContext);
   CodeStubArguments arguments(this, ChangeInt32ToIntPtr(argc));
-  Node* receiver = arguments.GetReceiver();
+  Node* const receiver = arguments.GetReceiver();
   // From now on use word-size argc value.
   argc = arguments.GetLength();
 
-  GotoIf(IntPtrEqual(argc, IntPtrConstant(0)), &argc_0);
+  VARIABLE(var_search_string, MachineRepresentation::kTagged);
+  VARIABLE(var_position, MachineRepresentation::kTagged);
+  Label argc_1(this), argc_2(this), call_runtime(this, Label::kDeferred),
+      fast_path(this);
+
   GotoIf(IntPtrEqual(argc, IntPtrConstant(1)), &argc_1);
-  Goto(&argc_2);
-  BIND(&argc_0);
+  GotoIf(IntPtrGreaterThan(argc, IntPtrConstant(1)), &argc_2);
   {
     Comment("0 Argument case");
-    Node* undefined = UndefinedConstant();
-    search_string.Bind(undefined);
-    position.Bind(undefined);
+    CSA_ASSERT(this, IntPtrEqual(argc, IntPtrConstant(0)));
+    Node* const undefined = UndefinedConstant();
+    var_search_string.Bind(undefined);
+    var_position.Bind(undefined);
     Goto(&call_runtime);
   }
   BIND(&argc_1);
   {
     Comment("1 Argument case");
-    search_string.Bind(arguments.AtIndex(0));
-    position.Bind(SmiConstant(0));
+    var_search_string.Bind(arguments.AtIndex(0));
+    var_position.Bind(SmiConstant(0));
     Goto(&fast_path);
   }
   BIND(&argc_2);
   {
     Comment("2 Argument case");
-    search_string.Bind(arguments.AtIndex(0));
-    position.Bind(arguments.AtIndex(1));
-    GotoIfNot(TaggedIsSmi(position.value()), &call_runtime);
+    var_search_string.Bind(arguments.AtIndex(0));
+    var_position.Bind(arguments.AtIndex(1));
+    GotoIfNot(TaggedIsSmi(var_position.value()), &call_runtime);
     Goto(&fast_path);
   }
-
   BIND(&fast_path);
   {
     Comment("Fast Path");
+    Node* const search = var_search_string.value();
+    Node* const position = var_position.value();
     GotoIf(TaggedIsSmi(receiver), &call_runtime);
-    Node* needle = search_string.value();
-    GotoIf(TaggedIsSmi(needle), &call_runtime);
-
-    Node* instance_type = LoadInstanceType(receiver);
-    GotoIfNot(IsStringInstanceType(instance_type), &call_runtime);
-
-    Node* needle_instance_type = LoadInstanceType(needle);
-    GotoIfNot(IsStringInstanceType(needle_instance_type), &call_runtime);
-
-    StringIndexOf(
-        receiver, instance_type, needle, needle_instance_type, position.value(),
-        [&arguments](Node* result) { arguments.PopAndReturn(result); });
+    GotoIf(TaggedIsSmi(search), &call_runtime);
+    GotoIfNot(IsString(receiver), &call_runtime);
+    GotoIfNot(IsString(search), &call_runtime);
+
+    StringIndexOf(receiver, search, position, [&](Node* result) {
+      CSA_ASSERT(this, TaggedIsSmi(result));
+      arguments.PopAndReturn((variant == kIndexOf)
+                                 ? result
+                                 : SelectBooleanConstant(SmiGreaterThanOrEqual(
+                                       result, SmiConstant(0))));
+    });
   }
-
   BIND(&call_runtime);
   {
     Comment("Call Runtime");
-    Node* result = CallRuntime(Runtime::kStringIndexOf, context, receiver,
-                               search_string.value(), position.value());
+    Runtime::FunctionId runtime = variant == kIndexOf
+                                      ? Runtime::kStringIndexOf
+                                      : Runtime::kStringIncludes;
+    Node* const result =
+        CallRuntime(runtime, context, receiver, var_search_string.value(),
+                    var_position.value());
     arguments.PopAndReturn(result);
   }
 }

src/builtins/builtins-string-gen.h

@@ -58,11 +58,8 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
                                      SloppyTNode<Smi> index,
                                      UnicodeEncoding encoding);
 
-  void StringIndexOf(Node* const subject_string,
-                     Node* const subject_instance_type,
-                     Node* const search_string,
-                     Node* const search_instance_type, Node* const position,
-                     std::function<void(Node*)> f_return);
+  void StringIndexOf(Node* const subject_string, Node* const search_string,
+                     Node* const position, std::function<void(Node*)> f_return);
 
   Node* IndexOfDollarChar(Node* const context, Node* const string);
 
@@ -94,6 +91,17 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
                                  CodeStubArguments* args = nullptr);
 };
 
+class StringIncludesIndexOfAssembler : public StringBuiltinsAssembler {
+ public:
+  explicit StringIncludesIndexOfAssembler(compiler::CodeAssemblerState* state)
+      : StringBuiltinsAssembler(state) {}
+
+ protected:
+  enum SearchVariant { kIncludes, kIndexOf };
+
+  void Generate(SearchVariant variant);
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/builtins/builtins-string.cc

@@ -172,38 +172,6 @@ BUILTIN(StringPrototypeEndsWith) {
   return isolate->heap()->true_value();
 }
 
-// ES6 section 21.1.3.7
-// String.prototype.includes ( searchString [ , position ] )
-BUILTIN(StringPrototypeIncludes) {
-  HandleScope handle_scope(isolate);
-  TO_THIS_STRING(str, "String.prototype.includes");
-
-  // Check if the search string is a regExp and fail if it is.
-  Handle<Object> search = args.atOrUndefined(isolate, 1);
-  Maybe<bool> is_reg_exp = RegExpUtils::IsRegExp(isolate, search);
-  if (is_reg_exp.IsNothing()) {
-    DCHECK(isolate->has_pending_exception());
-    return isolate->heap()->exception();
-  }
-  if (is_reg_exp.FromJust()) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate, NewTypeError(MessageTemplate::kFirstArgumentNotRegExp,
-                              isolate->factory()->NewStringFromStaticChars(
-                                  "String.prototype.includes")));
-  }
-  Handle<String> search_string;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, search_string,
-                                     Object::ToString(isolate, search));
-  Handle<Object> position;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, position,
-      Object::ToInteger(isolate, args.atOrUndefined(isolate, 2)));
-
-  uint32_t index = str->ToValidIndex(*position);
-  int index_in_str = String::IndexOf(isolate, str, search_string, index);
-  return *isolate->factory()->ToBoolean(index_in_str != -1);
-}
-
 // ES6 section 21.1.3.9
 // String.prototype.lastIndexOf ( searchString [ , position ] )
 BUILTIN(StringPrototypeLastIndexOf) {

src/debug/debug-evaluate.cc

@@ -300,6 +300,7 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
   /* Strings */                      \
   V(StringCharCodeAt)                \
   V(StringIndexOf)                   \
+  V(StringIncludes)                  \
   V(StringReplaceOneCharWithString)  \
   V(SubString)                       \
   V(RegExpInternalReplace)           \

src/runtime/runtime-strings.cc

@@ -9,6 +9,7 @@
 #include "src/counters.h"
 #include "src/objects-inl.h"
 #include "src/regexp/jsregexp-inl.h"
+#include "src/regexp/regexp-utils.h"
 #include "src/string-builder.h"
 #include "src/string-search.h"
 
@@ -136,6 +137,49 @@ RUNTIME_FUNCTION(Runtime_StringReplaceOneCharWithString) {
   return isolate->StackOverflow();
 }
 
+// ES6 #sec-string.prototype.includes
+// String.prototype.includes(searchString [, position])
+RUNTIME_FUNCTION(Runtime_StringIncludes) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(3, args.length());
+
+  Handle<Object> receiver = args.at(0);
+  if (receiver->IsNullOrUndefined(isolate)) {
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewTypeError(MessageTemplate::kCalledOnNullOrUndefined,
+                              isolate->factory()->NewStringFromAsciiChecked(
+                                  "String.prototype.includes")));
+  }
+  Handle<String> receiver_string;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver_string,
+                                     Object::ToString(isolate, receiver));
+
+  // Check if the search string is a regExp and fail if it is.
+  Handle<Object> search = args.at(1);
+  Maybe<bool> is_reg_exp = RegExpUtils::IsRegExp(isolate, search);
+  if (is_reg_exp.IsNothing()) {
+    DCHECK(isolate->has_pending_exception());
+    return isolate->heap()->exception();
+  }
+  if (is_reg_exp.FromJust()) {
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewTypeError(MessageTemplate::kFirstArgumentNotRegExp,
+                              isolate->factory()->NewStringFromStaticChars(
+                                  "String.prototype.includes")));
+  }
+  Handle<String> search_string;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, search_string,
+                                     Object::ToString(isolate, args.at(1)));
+  Handle<Object> position;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, position,
+                                     Object::ToInteger(isolate, args.at(2)));
+
+  uint32_t index = receiver_string->ToValidIndex(*position);
+  int index_in_str =
+      String::IndexOf(isolate, receiver_string, search_string, index);
+  return *isolate->factory()->ToBoolean(index_in_str != -1);
+}
+
 // ES6 #sec-string.prototype.indexof
 // String.prototype.indexOf(searchString [, position])
 RUNTIME_FUNCTION(Runtime_StringIndexOf) {

src/runtime/runtime.h

@@ -513,6 +513,7 @@ namespace internal {
 #define FOR_EACH_INTRINSIC_STRINGS(F)     \
   F(GetSubstitution, 5, 1)                \
   F(StringReplaceOneCharWithString, 3, 1) \
+  F(StringIncludes, 3, 1)                 \
   F(StringIndexOf, 3, 1)                  \
   F(StringIndexOfUnchecked, 3, 1)         \
   F(StringLastIndexOf, 2, 1)              \

test/mjsunit/messages.js

@@ -73,6 +73,10 @@ test(function() {
   Array.prototype.shift.call(null);
 }, "Array.prototype.shift called on null or undefined", TypeError);
 
+test(function() {
+  String.prototype.includes.call(null);
+}, "String.prototype.includes called on null or undefined", TypeError);
+
 // kCannotFreezeArrayBufferView
 test(function() {
   Object.freeze(new Uint16Array(1));
@@ -122,6 +126,11 @@ test(function() {
 }, "First argument to String.prototype.startsWith " +
    "must not be a regular expression", TypeError);
 
+test(function() {
+  "a".includes(/a/);
+}, "First argument to String.prototype.includes " +
+   "must not be a regular expression", TypeError);
+
 // kFlagsGetterNonObject
 test(function() {
   Object.getOwnPropertyDescriptor(RegExp.prototype, "flags").get.call(1);