One instead of two runtime calls when initializing regexp.

R=ulan@chromium.org

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

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

src/compiler/linkage.cc

@@ -183,7 +183,7 @@ bool Linkage::NeedsFrameState(Runtime::FunctionId function) {
     case Runtime::kPreventExtensions:
     case Runtime::kPromiseRejectEvent:
     case Runtime::kPromiseRevokeReject:
-    case Runtime::kRegExpCompile:
+    case Runtime::kRegExpInitializeAndCompile:
     case Runtime::kRegExpExecMultiple:
     case Runtime::kResolvePossiblyDirectEval:
     case Runtime::kRunMicrotasks:

src/jsregexp.cc

@@ -58,28 +58,6 @@ MaybeHandle<Object> RegExpImpl::CreateRegExpLiteral(
 }
 
 
-static JSRegExp::Flags RegExpFlagsFromString(Handle<String> str) {
-  int flags = JSRegExp::NONE;
-  for (int i = 0; i < str->length(); i++) {
-    switch (str->Get(i)) {
-      case 'i':
-        flags |= JSRegExp::IGNORE_CASE;
-        break;
-      case 'g':
-        flags |= JSRegExp::GLOBAL;
-        break;
-      case 'm':
-        flags |= JSRegExp::MULTILINE;
-        break;
-      case 'y':
-        if (FLAG_harmony_regexps) flags |= JSRegExp::STICKY;
-        break;
-    }
-  }
-  return JSRegExp::Flags(flags);
-}
-
-
 MUST_USE_RESULT
 static inline MaybeHandle<Object> ThrowRegExpException(
     Handle<JSRegExp> re,
@@ -156,10 +134,9 @@ static bool HasFewDifferentCharacters(Handle<String> pattern) {
 
 MaybeHandle<Object> RegExpImpl::Compile(Handle<JSRegExp> re,
                                         Handle<String> pattern,
-                                        Handle<String> flag_str) {
+                                        JSRegExp::Flags flags) {
   Isolate* isolate = re->GetIsolate();
   Zone zone(isolate);
-  JSRegExp::Flags flags = RegExpFlagsFromString(flag_str);
   CompilationCache* compilation_cache = isolate->compilation_cache();
   MaybeHandle<FixedArray> maybe_cached =
       compilation_cache->LookupRegExp(pattern, flags);

src/jsregexp.h

@@ -46,10 +46,9 @@ class RegExpImpl {
   // generic data and choice of implementation - as well as what
   // the implementation wants to store in the data field.
   // Returns false if compilation fails.
-  MUST_USE_RESULT static MaybeHandle<Object> Compile(
-      Handle<JSRegExp> re,
-      Handle<String> pattern,
-      Handle<String> flags);
+  MUST_USE_RESULT static MaybeHandle<Object> Compile(Handle<JSRegExp> re,
+                                                     Handle<String> pattern,
+                                                     JSRegExp::Flags flags);
 
   // See ECMA-262 section 15.10.6.2.
   // This function calls the garbage collector if necessary.

src/regexp.js

@@ -30,46 +30,7 @@ function DoConstructRegExp(object, pattern, flags) {
   pattern = IS_UNDEFINED(pattern) ? '' : ToString(pattern);
   flags = IS_UNDEFINED(flags) ? '' : ToString(flags);
 
-  var global = false;
-  var ignoreCase = false;
-  var multiline = false;
-  var sticky = false;
-  for (var i = 0; i < flags.length; i++) {
-    var c = %_CallFunction(flags, i, StringCharAt);
-    switch (c) {
-      case 'g':
-        if (global) {
-          throw MakeSyntaxError("invalid_regexp_flags", [flags]);
-        }
-        global = true;
-        break;
-      case 'i':
-        if (ignoreCase) {
-          throw MakeSyntaxError("invalid_regexp_flags", [flags]);
-        }
-        ignoreCase = true;
-        break;
-      case 'm':
-        if (multiline) {
-          throw MakeSyntaxError("invalid_regexp_flags", [flags]);
-        }
-        multiline = true;
-        break;
-      case 'y':
-        if (!harmony_regexps || sticky) {
-          throw MakeSyntaxError("invalid_regexp_flags", [flags]);
-        }
-        sticky = true;
-        break;
-      default:
-        throw MakeSyntaxError("invalid_regexp_flags", [flags]);
-    }
-  }
-
-  %RegExpInitializeObject(object, pattern, global, ignoreCase, multiline, sticky);
-
-  // Call internal function to compile the pattern.
-  %RegExpCompile(object, pattern, flags);
+  %RegExpInitializeAndCompile(object, pattern, flags);
 }
 
 

src/runtime/runtime-regexp.cc

@@ -759,19 +759,6 @@ RUNTIME_FUNCTION(Runtime_StringSplit) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_RegExpCompile) {
-  HandleScope scope(isolate);
-  DCHECK(args.length() == 3);
-  CONVERT_ARG_HANDLE_CHECKED(JSRegExp, re, 0);
-  CONVERT_ARG_HANDLE_CHECKED(String, pattern, 1);
-  CONVERT_ARG_HANDLE_CHECKED(String, flags, 2);
-  Handle<Object> result;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result,
-                                     RegExpImpl::Compile(re, pattern, flags));
-  return *result;
-}
-
-
 RUNTIME_FUNCTION(Runtime_RegExpExecRT) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 4);
@@ -813,26 +800,65 @@ RUNTIME_FUNCTION(Runtime_RegExpConstructResult) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_RegExpInitializeObject) {
+static JSRegExp::Flags RegExpFlagsFromString(Handle<String> flags,
+                                             bool* success) {
+  uint32_t value = JSRegExp::NONE;
+  int length = flags->length();
+  // A longer flags string cannot be valid.
+  if (length > 4) return JSRegExp::Flags(0);
+  for (int i = 0; i < length; i++) {
+    uint32_t flag = JSRegExp::NONE;
+    switch (flags->Get(i)) {
+      case 'g':
+        flag = JSRegExp::GLOBAL;
+        break;
+      case 'i':
+        flag = JSRegExp::IGNORE_CASE;
+        break;
+      case 'm':
+        flag = JSRegExp::MULTILINE;
+        break;
+      case 'y':
+        if (!FLAG_harmony_regexps) return JSRegExp::Flags(0);
+        flag = JSRegExp::STICKY;
+        break;
+      default:
+        return JSRegExp::Flags(0);
+    }
+    // Duplicate flag.
+    if (value & flag) return JSRegExp::Flags(0);
+    value |= flag;
+  }
+  *success = true;
+  return JSRegExp::Flags(value);
+}
+
+
+RUNTIME_FUNCTION(Runtime_RegExpInitializeAndCompile) {
   HandleScope scope(isolate);
-  DCHECK(args.length() == 6);
+  DCHECK(args.length() == 3);
   CONVERT_ARG_HANDLE_CHECKED(JSRegExp, regexp, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, source, 1);
+  CONVERT_ARG_HANDLE_CHECKED(String, flags_string, 2);
+  Factory* factory = isolate->factory();
   // If source is the empty string we set it to "(?:)" instead as
   // suggested by ECMA-262, 5th, section 15.10.4.1.
-  if (source->length() == 0) source = isolate->factory()->query_colon_string();
-
-  CONVERT_ARG_HANDLE_CHECKED(Object, global, 2);
-  if (!global->IsTrue()) global = isolate->factory()->false_value();
-
-  CONVERT_ARG_HANDLE_CHECKED(Object, ignoreCase, 3);
-  if (!ignoreCase->IsTrue()) ignoreCase = isolate->factory()->false_value();
-
-  CONVERT_ARG_HANDLE_CHECKED(Object, multiline, 4);
-  if (!multiline->IsTrue()) multiline = isolate->factory()->false_value();
+  if (source->length() == 0) source = factory->query_colon_string();
+
+  bool success = false;
+  JSRegExp::Flags flags = RegExpFlagsFromString(flags_string, &success);
+  if (!success) {
+    Handle<FixedArray> element = factory->NewFixedArray(1);
+    element->set(0, *flags_string);
+    Handle<JSArray> args = factory->NewJSArrayWithElements(element);
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewSyntaxError("invalid_regexp_flags", args));
+  }
 
-  CONVERT_ARG_HANDLE_CHECKED(Object, sticky, 5);
-  if (!sticky->IsTrue()) sticky = isolate->factory()->false_value();
+  Handle<Object> global = factory->ToBoolean(flags.is_global());
+  Handle<Object> ignore_case = factory->ToBoolean(flags.is_ignore_case());
+  Handle<Object> multiline = factory->ToBoolean(flags.is_multiline());
+  Handle<Object> sticky = factory->ToBoolean(flags.is_sticky());
 
   Map* map = regexp->map();
   Object* constructor = map->constructor();
@@ -844,41 +870,43 @@ RUNTIME_FUNCTION(Runtime_RegExpInitializeObject) {
     // barrier.
     regexp->InObjectPropertyAtPut(JSRegExp::kGlobalFieldIndex, *global,
                                   SKIP_WRITE_BARRIER);
-    regexp->InObjectPropertyAtPut(JSRegExp::kIgnoreCaseFieldIndex, *ignoreCase,
+    regexp->InObjectPropertyAtPut(JSRegExp::kIgnoreCaseFieldIndex, *ignore_case,
                                   SKIP_WRITE_BARRIER);
     regexp->InObjectPropertyAtPut(JSRegExp::kMultilineFieldIndex, *multiline,
                                   SKIP_WRITE_BARRIER);
     regexp->InObjectPropertyAtPut(JSRegExp::kLastIndexFieldIndex,
                                   Smi::FromInt(0), SKIP_WRITE_BARRIER);
-    return *regexp;
+  } else {
+    // Map has changed, so use generic, but slower, method.  We also end here if
+    // the --harmony-regexp flag is set, because the initial map does not have
+    // space for the 'sticky' flag, since it is from the snapshot, but must work
+    // both with and without --harmony-regexp.  When sticky comes out from under
+    // the flag, we will be able to use the fast initial map.
+    PropertyAttributes final =
+        static_cast<PropertyAttributes>(READ_ONLY | DONT_ENUM | DONT_DELETE);
+    PropertyAttributes writable =
+        static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE);
+    Handle<Object> zero(Smi::FromInt(0), isolate);
+    JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->source_string(),
+                                             source, final).Check();
+    JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->global_string(),
+                                             global, final).Check();
+    JSObject::SetOwnPropertyIgnoreAttributes(
+        regexp, factory->ignore_case_string(), ignore_case, final).Check();
+    JSObject::SetOwnPropertyIgnoreAttributes(
+        regexp, factory->multiline_string(), multiline, final).Check();
+    if (FLAG_harmony_regexps) {
+      JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->sticky_string(),
+                                               sticky, final).Check();
+    }
+    JSObject::SetOwnPropertyIgnoreAttributes(
+        regexp, factory->last_index_string(), zero, writable).Check();
   }
 
-  // Map has changed, so use generic, but slower, method.  We also end here if
-  // the --harmony-regexp flag is set, because the initial map does not have
-  // space for the 'sticky' flag, since it is from the snapshot, but must work
-  // both with and without --harmony-regexp.  When sticky comes out from under
-  // the flag, we will be able to use the fast initial map.
-  PropertyAttributes final =
-      static_cast<PropertyAttributes>(READ_ONLY | DONT_ENUM | DONT_DELETE);
-  PropertyAttributes writable =
-      static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE);
-  Handle<Object> zero(Smi::FromInt(0), isolate);
-  Factory* factory = isolate->factory();
-  JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->source_string(),
-                                           source, final).Check();
-  JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->global_string(),
-                                           global, final).Check();
-  JSObject::SetOwnPropertyIgnoreAttributes(
-      regexp, factory->ignore_case_string(), ignoreCase, final).Check();
-  JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->multiline_string(),
-                                           multiline, final).Check();
-  if (FLAG_harmony_regexps) {
-    JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->sticky_string(),
-                                             sticky, final).Check();
-  }
-  JSObject::SetOwnPropertyIgnoreAttributes(regexp, factory->last_index_string(),
-                                           zero, writable).Check();
-  return *regexp;
+  Handle<Object> result;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result, RegExpImpl::Compile(regexp, source, flags));
+  return *result;
 }
 
 

src/runtime/runtime.h

@@ -155,9 +155,8 @@ namespace internal {
   F(RemPiO2, 1, 1)                                         \
                                                            \
   /* Regular expressions */                                \
-  F(RegExpCompile, 3, 1)                                   \
+  F(RegExpInitializeAndCompile, 3, 1)                      \
   F(RegExpExecMultiple, 4, 1)                              \
-  F(RegExpInitializeObject, 6, 1)                          \
                                                            \
   /* JSON */                                               \
   F(ParseJson, 1, 1)                                       \