Revert of [regexp] Use consistent map checks for fast paths (patchset #7...

Revert of [regexp] Use consistent map checks for fast paths (patchset #7 id:120001 of https://chromiumcodereview.appspot.com/2434983002/ )

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

Original issue's description:
> [regexp] Use consistent map checks for fast paths
>
> These map checks were implemented for TF code already. This CL makes
> sure that parts implemented in C++ follow the same logic, which is:
>
> An object is an unmodified regexp if:
> 1) it's a receiver,
> 2) its map is the initial regexp map,
> 3) its prototype is a receiver,
> 4) and its prototype's map is the initial prototype's initial map.
>
> We can now be smarter in @@replace and @@split since checking maps
> (unlike the previous check of RegExp.prototype.exec) is not observable,
> so we can perform fast-path checks at a time of our choosing.
>
> BUG=v8:5339,v8:5434,v8:5123

TBR=yangguo@chromium.org,jgruber@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5339,v8:5434,v8:5123

Review-Url: https://chromiumcodereview.appspot.com/2438283002
Cr-Commit-Position: refs/heads/master@{#40499}

src/builtins/builtins-regexp.cc

@@ -1290,17 +1290,23 @@ BUILTIN(RegExpPrototypeSplit) {
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, string,
                                      Object::ToString(isolate, string_obj));
 
-  if (RegExpUtils::IsUnmodifiedRegExp(isolate, recv)) {
-    RETURN_RESULT_OR_FAILURE(
-        isolate,
-        RegExpSplit(isolate, Handle<JSRegExp>::cast(recv), string, limit_obj));
-  }
-
   Handle<JSFunction> regexp_fun = isolate->regexp_function();
   Handle<Object> ctor;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, ctor, SpeciesConstructor(isolate, recv, regexp_fun));
 
+  if (recv->IsJSRegExp() && *ctor == *regexp_fun) {
+    Handle<Object> exec;
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, exec, JSObject::GetProperty(
+                           recv, factory->NewStringFromAsciiChecked("exec")));
+    if (RegExpUtils::IsBuiltinExec(exec)) {
+      RETURN_RESULT_OR_FAILURE(
+          isolate, RegExpSplit(isolate, Handle<JSRegExp>::cast(recv), string,
+                               limit_obj));
+    }
+  }
+
   Handle<Object> flags_obj;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, flags_obj, JSObject::GetProperty(recv, factory->flags_string()));

src/regexp/regexp-utils.cc

@@ -136,24 +136,13 @@ Maybe<bool> RegExpUtils::IsRegExp(Isolate* isolate, Handle<Object> object) {
   return Just(object->IsJSRegExp());
 }
 
-bool RegExpUtils::IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj) {
-  // TODO(ishell): Update this check once map changes for constant field
-  // tracking are landing.
+bool RegExpUtils::IsBuiltinExec(Handle<Object> exec) {
+  if (!exec->IsJSFunction()) return false;
 
-  if (!obj->IsJSReceiver()) return false;
+  Code* code = Handle<JSFunction>::cast(exec)->code();
+  if (code == nullptr) return false;
 
-  JSReceiver* recv = JSReceiver::cast(*obj);
-
-  // Check the receiver's map.
-  Handle<JSFunction> regexp_function = isolate->regexp_function();
-  if (recv->map() != regexp_function->initial_map()) return false;
-
-  // Check the receiver's prototype's map.
-  Object* proto = recv->map()->prototype();
-  if (!proto->IsJSReceiver()) return false;
-
-  Handle<Map> initial_proto_initial_map = isolate->regexp_prototype_map();
-  return (JSReceiver::cast(proto)->map() == *initial_proto_initial_map);
+  return (code->builtin_index() == Builtins::kRegExpPrototypeExec);
 }
 
 int RegExpUtils::AdvanceStringIndex(Isolate* isolate, Handle<String> string,

src/regexp/regexp-utils.h

@@ -34,9 +34,8 @@ class RegExpUtils : public AllStatic {
   // Includes checking of the match property.
   static Maybe<bool> IsRegExp(Isolate* isolate, Handle<Object> object);
 
-  // Checks whether the given object is an unmodified JSRegExp instance.
-  // Neither the object's map, nor its prototype's map may be modified.
-  static bool IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj);
+  // Checks whether exec is identical to the initial RegExp.prototype.exec.
+  static bool IsBuiltinExec(Handle<Object> exec);
 
   // ES#sec-advancestringindex
   // AdvanceStringIndex ( S, index, unicode )

src/runtime/runtime-regexp.cc

@@ -1500,13 +1500,6 @@ RUNTIME_FUNCTION(Runtime_RegExpReplace) {
 
   string = String::Flatten(string);
 
-  // Fast-path for unmodified JSRegExps.
-  if (RegExpUtils::IsUnmodifiedRegExp(isolate, recv)) {
-    RETURN_RESULT_OR_FAILURE(
-        isolate, RegExpReplace(isolate, Handle<JSRegExp>::cast(recv), string,
-                               replace_obj));
-  }
-
   const int length = string->length();
   const bool functional_replace = replace_obj->IsCallable();
 
@@ -1534,14 +1527,35 @@ RUNTIME_FUNCTION(Runtime_RegExpReplace) {
                                 RegExpUtils::SetLastIndex(isolate, recv, 0));
   }
 
+  // TODO(jgruber): Here and at all other fast path checks, rely on map checks
+  // instead.
+  // TODO(jgruber): We could speed up the fast path by checking flags
+  // afterwards, but that would violate the spec (which states that exec is
+  // accessed after global and unicode).
+  // TODO(adamk): this fast path is wrong as we doesn't ensure that 'exec'
+  // is actually a data property on RegExp.prototype.
+  Handle<Object> exec = factory->undefined_value();
+  if (recv->IsJSRegExp()) {
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, exec, JSObject::GetProperty(
+                           recv, factory->NewStringFromAsciiChecked("exec")));
+    if (RegExpUtils::IsBuiltinExec(exec)) {
+      RETURN_RESULT_OR_FAILURE(
+          isolate, RegExpReplace(isolate, Handle<JSRegExp>::cast(recv), string,
+                                 replace_obj));
+    }
+  }
+
   Zone zone(isolate->allocator(), ZONE_NAME);
   ZoneVector<Handle<Object>> results(&zone);
 
   while (true) {
     Handle<Object> result;
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-        isolate, result, RegExpUtils::RegExpExec(isolate, recv, string,
-                                                 factory->undefined_value()));
+        isolate, result, RegExpUtils::RegExpExec(isolate, recv, string, exec));
+
+    // Ensure exec will be read again on the next loop through.
+    exec = factory->undefined_value();
 
     if (result->IsNull(isolate)) break;
 

test/mjsunit/es6/regexp-flags.js

@@ -45,12 +45,8 @@ assertEquals(4, get_count);
 // Overridden flag getters affect string.replace
 // TODO(adamk): Add more tests here once we've switched
 // to use [[OriginalFlags]] in more cases.
-// TODO(jgruber): This exact case actually causes an infinite loop in the spec
-// (@@replace sees global = true while BuiltinExec sees global = false).
-// Comment the test for now and remove / fix once this has been resolved on
-// the spec side.
-//assertEquals(expected, string.replace(r3, "X"));
-//assertEquals(5, get_count);
+assertEquals(expected, string.replace(r3, "X"));
+assertEquals(5, get_count);
 
 
 function testName(name) {

test/test262/test262.status

@@ -105,6 +105,10 @@
   'built-ins/RegExp/prototype/Symbol.replace/y-init-lastindex': [FAIL],
   'built-ins/RegExp/prototype/Symbol.replace/y-set-lastindex': [FAIL],
 
+  # https://bugs.chromium.org/p/v8/issues/detail?id=5123
+  'built-ins/RegExp/prototype/Symbol.replace/coerce-global': [FAIL],
+  'built-ins/RegExp/prototype/Symbol.replace/coerce-unicode': [FAIL],
+
   ###### END REGEXP SUBCLASSING SECTION ######
 
   # https://code.google.com/p/v8/issues/detail?id=4360