Port and integrate the irregexp engine from V8

runtime/lib/regexp.cc

Index: runtime/lib/regexp.cc
diff --git a/runtime/lib/regexp.cc b/runtime/lib/regexp.cc
index 4152923f5419aaf7af939e01c04d2a5b97810fdf..30471fc41726afb4debeafc5fe144877465d465f 100644
--- a/runtime/lib/regexp.cc
+++ b/runtime/lib/regexp.cc
@@ -4,14 +4,19 @@
 
 #include "platform/assert.h"
 #include "vm/bootstrap_natives.h"
+#include "vm/compiler.h"

[Ivan Posva, 2014/10/09 09:54:15]

Why is compiler.h needed here?

[jgruber1, 2014/10/09 13:36:16]

Left over from previous efforts, removed now.

 #include "vm/exceptions.h"
 #include "vm/native_entry.h"
 #include "vm/object.h"
+#include "vm/regexp_parser.h"
 
 #include "lib/regexp_jsc.h"
 
 namespace dart {
 
+DECLARE_FLAG(bool, trace_irregexp);
+
+
 DEFINE_NATIVE_ENTRY(JSSyntaxRegExp_factory, 4) {
   ASSERT(TypeArguments::CheckedHandle(arguments->NativeArgAt(0)).IsNull());
   GET_NON_NULL_NATIVE_ARGUMENT(String, pattern, arguments->NativeArgAt(1));
@@ -21,7 +26,24 @@ DEFINE_NATIVE_ENTRY(JSSyntaxRegExp_factory, 4) {
       Instance, handle_case_sensitive, arguments->NativeArgAt(3));
   bool ignore_case = handle_case_sensitive.raw() != Bool::True().raw();
   bool multi_line = handle_multi_line.raw() == Bool::True().raw();
+
+#if defined(USE_JSCRE)
   return Jscre::Compile(pattern, multi_line, ignore_case);
+#else
+  // Parse the pattern once in order to throw any format exceptions within
+  // the factory constructor. It is parsed again upon compilation.
+  RegExpCompileData compileData;
+  if (!RegExpParser::ParseRegExp(pattern, multi_line, &compileData)) {

[Ivan Posva, 2014/10/09 09:54:15]

Is there any particular reason why the code below cannot throw the exception
while parsing. Going through the regular expression twice seems unnecessary.

[jgruber1, 2014/10/09 13:36:16]

I'm not sure if we're on the same page here; regexps are parsed only once within
the factory constructor, and this needs to be done to catch invalid pattern
formats. They are then parsed again during each compilation run. We could keep
the parsed RegExpTree around in order to avoid reparsing, but it takes such a
miniscule amount of time that this seems like the better tradeoff.

[Ivan Posva, 2014/10/10 06:35:53]

The comment made it sound like the compilation happens within this native call,
which apparently is not the case.

[jgruber1, 2014/10/10 09:29:44]

Acknowledged.

+    // Parsing failures throw an exception.
+    UNREACHABLE();
+  }
+
+  // Create a JSRegExp object containing only the initial parameters.
+  JSRegExp& regexp = JSRegExp::Handle(
+        isolate, RegExpEngine::New(isolate, pattern, multi_line, ignore_case));

[Ivan Posva, 2014/10/09 09:54:15]

RegExpEngine::New should not go into a JSRegExp::Handle. The first implies
allocating an object RegExpEngine.

[jgruber1, 2014/10/09 13:36:16]

Renamed RegExpEngine::New to RegExpEngine::CreateJSRegExp, and removed the
Handle construction.

+
+  return regexp.raw();
+#endif
 }
 
 
@@ -68,7 +90,20 @@ DEFINE_NATIVE_ENTRY(JSSyntaxRegExp_ExecuteMatch, 3) {
   ASSERT(!regexp.IsNull());
   GET_NON_NULL_NATIVE_ARGUMENT(String, str, arguments->NativeArgAt(1));
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, start_index, arguments->NativeArgAt(2));
+
+#if defined(USE_JSCRE)
   return Jscre::Execute(regexp, str, start_index.Value());
+#else
+  // This function is intrinsified. See Intrinsifier::JSRegExp_ExecuteMatch.
+  const intptr_t cid = str.GetClassId();
+
+  // Retrieve the cached function.
+  const Function& fn = Function::Handle(regexp.function(cid));

[Ivan Posva, 2014/10/09 09:54:15]

An API based on class ids seems not the best thing to me.

[jgruber1, 2014/10/09 13:36:16]

What would you like to see instead? The regexp functions are specialized based
on the class ids, so we need to retrieve them by class id.

+  ASSERT(!fn.IsNull());
+
+  // And finally call the generated code.
+  return IRRegExpMacroAssembler::Execute(fn, str, start_index, isolate);
+#endif
 }
 
 }  // namespace dart