[regexp] Move RegExp.prototype[@@search] to TF

src/builtins/builtins-regexp.cc

Index: src/builtins/builtins-regexp.cc
diff --git a/src/builtins/builtins-regexp.cc b/src/builtins/builtins-regexp.cc
index de2528dc59bfa21c65ba924ae91c64828ad9e7bc..94c0075bf066182620dd81624cba363dadbb516a 100644
--- a/src/builtins/builtins-regexp.cc
+++ b/src/builtins/builtins-regexp.cc
@@ -170,34 +170,45 @@ BUILTIN(RegExpPrototypeCompile) {
 
 namespace {
 
+compiler::Node* FastLoadLastIndex(CodeStubAssembler* a, compiler::Node* context,
+                                  compiler::Node* regexp) {
+  // Load the in-object field.
+  static const int field_offset =
+      JSRegExp::kSize + JSRegExp::kLastIndexFieldIndex * kPointerSize;
+  return a->LoadObjectField(regexp, field_offset);
+}
+
+compiler::Node* SlowLoadLastIndex(CodeStubAssembler* a, compiler::Node* context,
+                                  compiler::Node* regexp) {
+  // Load through the GetProperty stub.
+  typedef compiler::Node Node;
+
+  Node* const name =
+      a->HeapConstant(a->isolate()->factory()->lastIndex_string());
+  Callable getproperty_callable = CodeFactory::GetProperty(a->isolate());
+  return a->CallStub(getproperty_callable, context, regexp, name);
+}
+
 compiler::Node* LoadLastIndex(CodeStubAssembler* a, compiler::Node* context,
                               compiler::Node* has_initialmap,
                               compiler::Node* regexp) {
   typedef CodeStubAssembler::Variable Variable;
   typedef CodeStubAssembler::Label Label;
-  typedef compiler::Node Node;
 
   Variable var_value(a, MachineRepresentation::kTagged);
 
-  Label out(a), if_unmodified(a), if_modified(a, Label::kDeferred);
+  Label out(a), if_unmodified(a), if_modified(a);
   a->Branch(has_initialmap, &if_unmodified, &if_modified);
 
   a->Bind(&if_unmodified);
   {
-    // Load the in-object field.
-    static const int field_offset =
-        JSRegExp::kSize + JSRegExp::kLastIndexFieldIndex * kPointerSize;
-    var_value.Bind(a->LoadObjectField(regexp, field_offset));
+    var_value.Bind(FastLoadLastIndex(a, context, regexp));
     a->Goto(&out);
   }
 
   a->Bind(&if_modified);
   {
-    // Load through the GetProperty stub.
-    Node* const name =
-        a->HeapConstant(a->isolate()->factory()->lastIndex_string());
-    Callable getproperty_callable = CodeFactory::GetProperty(a->isolate());
-    var_value.Bind(a->CallStub(getproperty_callable, context, regexp, name));
+    var_value.Bind(SlowLoadLastIndex(a, context, regexp));
     a->Goto(&out);
   }
 
@@ -215,13 +226,25 @@ void FastStoreLastIndex(CodeStubAssembler* a, compiler::Node* context,
   a->StoreObjectField(regexp, field_offset, value);
 }
 
+void SlowStoreLastIndex(CodeStubAssembler* a, compiler::Node* context,
+                        compiler::Node* regexp, compiler::Node* value) {
+  // Store through runtime.
+  // TODO(ishell): Use SetPropertyStub here once available.
+  typedef compiler::Node Node;
+
+  Node* const name =
+      a->HeapConstant(a->isolate()->factory()->lastIndex_string());
+  Node* const language_mode = a->SmiConstant(Smi::FromInt(STRICT));
+  a->CallRuntime(Runtime::kSetProperty, context, regexp, name, value,
+                 language_mode);
+}
+
 void StoreLastIndex(CodeStubAssembler* a, compiler::Node* context,
                     compiler::Node* has_initialmap, compiler::Node* regexp,
                     compiler::Node* value) {
   typedef CodeStubAssembler::Label Label;
-  typedef compiler::Node Node;
 
-  Label out(a), if_unmodified(a), if_modified(a, Label::kDeferred);
+  Label out(a), if_unmodified(a), if_modified(a);
   a->Branch(has_initialmap, &if_unmodified, &if_modified);
 
   a->Bind(&if_unmodified);
@@ -232,13 +255,7 @@ void StoreLastIndex(CodeStubAssembler* a, compiler::Node* context,
 
   a->Bind(&if_modified);
   {
-    // Store through runtime.
-    // TODO(ishell): Use SetPropertyStub here once available.
-    Node* const name =
-        a->HeapConstant(a->isolate()->factory()->lastIndex_string());
-    Node* const language_mode = a->SmiConstant(Smi::FromInt(STRICT));
-    a->CallRuntime(Runtime::kSetProperty, context, regexp, name, value,
-                   language_mode);
+    SlowStoreLastIndex(a, context, regexp, value);
     a->Goto(&out);
   }
 
@@ -1144,55 +1161,155 @@ BUILTIN(RegExpPrototypeMatch) {
 
 // ES#sec-regexp.prototype-@@search
 // RegExp.prototype [ @@search ] ( string )
-BUILTIN(RegExpPrototypeSearch) {
-  HandleScope scope(isolate);
-  CHECK_RECEIVER(JSReceiver, recv, "RegExp.prototype.@@search");
+void Builtins::Generate_RegExpPrototypeSearch(CodeStubAssembler* a) {
+  typedef CodeStubAssembler::Label Label;
+  typedef compiler::Node Node;
 
-  Handle<Object> string_obj = args.atOrUndefined(isolate, 1);
+  Isolate* const isolate = a->isolate();
 
-  Handle<String> string;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, string,
-                                     Object::ToString(isolate, string_obj));
+  Node* const maybe_receiver = a->Parameter(0);
+  Node* const maybe_string = a->Parameter(1);
+  Node* const context = a->Parameter(4);
 
-  Handle<Object> previous_last_index_obj;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, previous_last_index_obj,
-                                     RegExpUtils::GetLastIndex(isolate, recv));
+  Node* const smi_zero = a->SmiConstant(Smi::kZero);
 
-  if (!previous_last_index_obj->SameValue(Smi::kZero)) {
-    RETURN_FAILURE_ON_EXCEPTION(isolate,
-                                RegExpUtils::SetLastIndex(isolate, recv, 0));
-  }
+  // Ensure {maybe_receiver} is a JSReceiver.
+  Node* const map =
+      ThrowIfNotJSReceiver(a, isolate, context, maybe_receiver,
+                           MessageTemplate::kIncompatibleMethodReceiver,
+                           "RegExp.prototype.@@search");
+  Node* const receiver = maybe_receiver;
 
-  Handle<Object> result;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, result,
-      RegExpUtils::RegExpExec(isolate, recv, string,
-                              isolate->factory()->undefined_value()));
-
-  Handle<Object> current_last_index_obj;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, current_last_index_obj,
-                                     RegExpUtils::GetLastIndex(isolate, recv));
-
-  const bool is_last_index_unchanged =
-      current_last_index_obj->SameValue(*previous_last_index_obj);
-  if (!is_last_index_unchanged) {
-    if (previous_last_index_obj->IsSmi()) {
-      RETURN_FAILURE_ON_EXCEPTION(
-          isolate,
-          RegExpUtils::SetLastIndex(
-              isolate, recv, Smi::cast(*previous_last_index_obj)->value()));
-    } else {
-      RETURN_FAILURE_ON_EXCEPTION(
-          isolate,
-          Object::SetProperty(recv, isolate->factory()->lastIndex_string(),
-                              previous_last_index_obj, STRICT));
+  // Convert {maybe_string} to a String.
+  Node* const string = a->ToString(context, maybe_string);
+
+  Label fast_path(a), slow_path(a);
+  BranchIfFastPath(a, context, map, &fast_path, &slow_path);
+
+  a->Bind(&fast_path);
+  {
+    // Grab the initial value of last index.
+    Node* const previous_last_index = FastLoadLastIndex(a, context, receiver);

[Igor Sheludko, 2016/11/11 16:13:21]

Given that the only difference between fast and slow paths is the way the
lastIndex property is accessed I think it makes sense to factor out the code
that generates one branch to a helper function.

[jgruber, 2016/11/13 13:00:09]

That helper function exists and is used elsewhere (also in the first version of
this CL). The reason we now use separate fast and slow paths is that merging the
two paths would require repeated map checks to ensure we're still on the fast
path. 

Looking at this again, we can also skip the call to RegExpExec on the fast path,
calling directly into RegExpPrototypeExecInternal instead.

[Igor Sheludko, 2016/11/13 18:27:59]

I meant something like this:
  BranchIfFastPath(...)
  Bind(&fast_path);
  GenerateXYZBody(fast: true)
  Bind(&slow_path);
  GenerateXYZBody(fast: false);

[jgruber, 2016/11/15 10:02:29]

Done.

+
+    // Ensure last index is 0.
+    {
+      Label next(a);
+      a->GotoIf(a->SameValue(previous_last_index, smi_zero, context), &next);

[Igor Sheludko, 2016/11/11 16:13:20]

According to my understanding of a spec we must always call Set. It could be
observable if we pass non-regexp receiver with getter/setter lastIndex.

For the fast case the store is not observable but I'm not sure how much do you
save here given the complexity of a SameValue implementation.

[jgruber, 2016/11/13 13:00:09]

Changes to @@search are about to be merged into the spec, and this CL implements
those semantics. See https://github.com/tc39/ecma262/pull/627. That pull request
ensures Set is only called when necessary.

Sorry about not making this clearer. I discussed it in reply to Yang's comments
above, but I'll add it to this CL's commit message.
True - but the common case, when both args to SameValue are Smis, should skip
most of the complication. 

[Igor Sheludko, 2016/11/13 18:27:59]

Ah. Sorry, I didn't notice that. 
But still the idea is that for the fast case we know that the lastIndex is a
data property and therefore the access is not observable and it will probably be
faster to skip the SaveValue and always write zero.

[jgruber, 2016/11/15 10:02:29]

Done.

[Yang, 2016/11/15 13:03:43]

Yeah. I agree. Always writing zero avoids the complexity introduced by SameValue
and the branch. Note that this would also make code size a bit smaller.

+
+      FastStoreLastIndex(a, context, receiver, smi_zero);
+      a->Goto(&next);
+
+      a->Bind(&next);
+    }
+
+    // Call exec.
+    Node* const match_indices = RegExpExec(a, context, receiver, string);
+
+    // Reset last index if necessary.
+    {
+      Label next(a);
+
+      Node* const current_last_index = FastLoadLastIndex(a, context, receiver);
+
+      a->GotoIf(a->SameValue(current_last_index, previous_last_index, context),

[Igor Sheludko, 2016/11/11 16:13:20]

Same here.

[jgruber, 2016/11/13 13:00:09]

See above.

+                &next);
+
+      FastStoreLastIndex(a, context, receiver, previous_last_index);
+      a->Goto(&next);
+
+      a->Bind(&next);
+    }
+
+    // Return -1 if no match was found.
+    {
+      Label next(a);
+      a->GotoUnless(a->WordEqual(match_indices, a->NullConstant()), &next);
+      a->Return(a->SmiConstant(Smi::FromInt(-1)));

[Igor Sheludko, 2016/11/11 16:13:20]

SmiConstant(-1)

[jgruber, 2016/11/13 13:00:09]

Nice, is this a recent change? Will fix.

[Igor Sheludko, 2016/11/13 18:27:59]

Yes.

[jgruber, 2016/11/15 10:02:29]

Done.

+      a->Bind(&next);
+    }
+
+    // Return the index of the match.
+    {
+      Label fast_result(a), slow_result(a, Label::kDeferred);
+
+      Node* const native_context = a->LoadNativeContext(context);
+      Node* const initial_regexp_result_map = a->LoadContextElement(
+          native_context, Context::REGEXP_RESULT_MAP_INDEX);
+      Node* const match_indices_map = a->LoadMap(match_indices);
+
+      a->Branch(a->WordEqual(match_indices_map, initial_regexp_result_map),
+                &fast_result, &slow_result);
+
+      a->Bind(&fast_result);
+      {
+        Node* const index =
+            a->LoadObjectField(match_indices, JSRegExpResult::kIndexOffset,
+                               MachineType::AnyTagged());
+        a->Return(index);
+      }
+
+      a->Bind(&slow_result);
+      {
+        Node* const name = a->HeapConstant(isolate->factory()->index_string());
+        Callable getproperty_callable = CodeFactory::GetProperty(a->isolate());
+        Node* const index =
+            a->CallStub(getproperty_callable, context, match_indices, name);
+        a->Return(index);
+      }
     }
   }
 
-  if (result->IsNull(isolate)) return Smi::FromInt(-1);
+  a->Bind(&slow_path);
+  {
+    // Grab the initial value of last index.
+    Node* const previous_last_index = SlowLoadLastIndex(a, context, receiver);
+
+    // Ensure last index is 0.
+    {
+      Label next(a);
+      a->GotoIf(a->SameValue(previous_last_index, smi_zero, context), &next);

[Igor Sheludko, 2016/11/11 16:13:21]

According to my understanding of a spec we must always call Set. It could be
observable if we pass non-regexp receiver with getter/setter lastIndex.

[jgruber, 2016/11/13 13:00:09]

See above.

+
+      SlowStoreLastIndex(a, context, receiver, smi_zero);
+      a->Goto(&next);
+
+      a->Bind(&next);
+    }
+
+    // Call exec.
+    Node* const match_indices = RegExpExec(a, context, receiver, string);
+
+    // Reset last index if necessary.
+    {
+      Label next(a);
+
+      Node* const current_last_index = SlowLoadLastIndex(a, context, receiver);
 
-  RETURN_RESULT_OR_FAILURE(
-      isolate, Object::GetProperty(result, isolate->factory()->index_string()));
+      a->GotoIf(a->SameValue(current_last_index, previous_last_index, context),

[Igor Sheludko, 2016/11/11 16:13:21]

Same here.

[jgruber, 2016/11/13 13:00:09]

See above.

+                &next);
+
+      SlowStoreLastIndex(a, context, receiver, previous_last_index);
+      a->Goto(&next);
+
+      a->Bind(&next);
+    }
+
+    // Return -1 if no match was found.
+    {
+      Label next(a);
+      a->GotoUnless(a->WordEqual(match_indices, a->NullConstant()), &next);
+      a->Return(a->SmiConstant(Smi::FromInt(-1)));
+      a->Bind(&next);
+    }
+
+    // Return the index of the match.

[Igor Sheludko, 2016/11/11 16:13:20]

I guess in this case the match_indices could still be in fast mode.

[jgruber, 2016/11/13 13:00:09]

Yes, but I decided against complicating the implementation here for small
performance gains on the slow path. I can be convinced otherwise though if
anyone has strong opinions on this.

[Igor Sheludko, 2016/11/13 18:27:59]

Maybe if you follow the GenerateXYZBody idea mentioned above it will not be too
complicated :)

[jgruber, 2016/11/15 10:02:29]

Done.

+    {
+      Node* const name = a->HeapConstant(isolate->factory()->index_string());
+      Callable getproperty_callable = CodeFactory::GetProperty(a->isolate());
+      Node* const index =
+          a->CallStub(getproperty_callable, context, match_indices, name);
+      a->Return(index);
+    }
+  }
 }
 
 namespace {