Separate native and interpreted regexp by compile time flag, not runtime.

Clean-up of RegExp code.

Review URL: http://codereview.chromium.org/155085


git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2366 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

SConstruct

@@ -95,7 +95,12 @@ ANDROID_LINKFLAGS = ['-nostdlib',
 LIBRARY_FLAGS = {
   'all': {
     'CPPDEFINES':   ['ENABLE_LOGGING_AND_PROFILING'],
-    'CPPPATH': [join(root_dir, 'src')]
+    'CPPPATH': [join(root_dir, 'src')],
+    'regexp:native': {
+      'arch:ia32' : {
+        'CPPDEFINES': ['V8_NATIVE_REGEXP']
+      }
+    }
   },
   'gcc': {
     'all': {
@@ -546,6 +551,11 @@ SIMPLE_OPTIONS = {
     'default': ARCH_GUESS,
     'help': 'the architecture to build for (' + ARCH_GUESS + ')'
   },
+  'regexp': {
+    'values': ['native', 'interpreted'],
+    'default': 'native',
+    'help': 'Whether to use native or interpreted regexp implementation'
+  },
   'snapshot': {
     'values': ['on', 'off', 'nobuild'],
     'default': 'off',
@@ -677,6 +687,8 @@ def VerifyOptions(env):
     return False
   if not IsLegal(env, 'sample', ["shell", "process"]):
     return False
+  if not IsLegal(env, 'regexp', ["native", "interpreted"]):
+    return False
   if env['os'] == 'win32' and env['library'] == 'shared' and env['prof'] == 'on':
     Abort("Profiling on windows only supported for static library.")
   if env['prof'] == 'oprofile' and env['os'] != 'linux':

src/flag-definitions.h

@@ -208,8 +208,6 @@ DEFINE_bool(preemption, false,
 
 // Regexp
 DEFINE_bool(trace_regexps, false, "trace regexp execution")
-DEFINE_bool(regexp_native, true,
-            "use native code regexp implementation (IA32 only)")
 DEFINE_bool(regexp_optimization, true, "generate optimized regexp code")
 
 // Testing flags test/cctest/test-{flags,api,serialization}.cc

src/jsregexp.cc

@@ -263,7 +263,6 @@ Handle<Object> RegExpImpl::AtomExec(Handle<JSRegExp> re,
 
 // Irregexp implementation.
 
-
 // Ensures that the regexp object contains a compiled version of the
 // source for either ASCII or non-ASCII strings.
 // If the compiled version doesn't already exist, it is compiled
@@ -271,25 +270,26 @@ Handle<Object> RegExpImpl::AtomExec(Handle<JSRegExp> re,
 // If compilation fails, an exception is thrown and this function
 // returns false.
 bool RegExpImpl::EnsureCompiledIrregexp(Handle<JSRegExp> re, bool is_ascii) {
-  int index;
-  if (is_ascii) {
-    index = JSRegExp::kIrregexpASCIICodeIndex;
-  } else {
-    index = JSRegExp::kIrregexpUC16CodeIndex;
-  }
-  Object* entry = re->DataAt(index);
-  if (!entry->IsTheHole()) {
-    // A value has already been compiled.
+#ifdef V8_NATIVE_REGEXP
+  if (re->DataAt(JSRegExp::code_index(is_ascii))->IsCode()) return true;
+#else  // ! V8_NATIVE_REGEXP (RegExp interpreter code)
+  if (re->DataAt(JSRegExp::code_index(is_ascii))->IsByteArray()) return true;
+#endif
+  return CompileIrregexp(re, is_ascii);
+}
+
+
+bool RegExpImpl::CompileIrregexp(Handle<JSRegExp> re, bool is_ascii) {
+  // Compile the RegExp.
+  CompilationZoneScope zone_scope(DELETE_ON_EXIT);
+  Object* entry = re->DataAt(JSRegExp::code_index(is_ascii));
   if (entry->IsJSObject()) {
-      // If it's a JS value, it's an error.
+    // If it's a JSObject, a previous compilation failed and threw this object.
+    // Re-throw the object without trying again.
     Top::Throw(entry);
     return false;
   }
-    return true;
-  }
-
-  // Compile the RegExp.
-  CompilationZoneScope zone_scope(DELETE_ON_EXIT);
+  ASSERT(entry->IsTheHole());
 
   JSRegExp::Flags flags = re->GetFlags();
 
@@ -302,7 +302,7 @@ bool RegExpImpl::EnsureCompiledIrregexp(Handle<JSRegExp> re, bool is_ascii) {
   FlatStringReader reader(pattern);
   if (!ParseRegExp(&reader, flags.is_multiline(), &compile_data)) {
     // Throw an exception if we fail to parse the pattern.
-    // THIS SHOULD NOT HAPPEN. We already parsed it successfully once.
+    // THIS SHOULD NOT HAPPEN. We already pre-parsed it successfully once.
     ThrowRegExpException(re,
                          pattern,
                          compile_data.error,
@@ -325,17 +325,15 @@ bool RegExpImpl::EnsureCompiledIrregexp(Handle<JSRegExp> re, bool is_ascii) {
     Handle<Object> regexp_err =
         Factory::NewSyntaxError("malformed_regexp", array);
     Top::Throw(*regexp_err);
-    re->SetDataAt(index, *regexp_err);
+    re->SetDataAt(JSRegExp::code_index(is_ascii), *regexp_err);
     return false;
   }
 
-  NoHandleAllocation no_handles;
-
-  FixedArray* data = FixedArray::cast(re->data());
-  data->set(index, result.code);
-  int register_max = IrregexpMaxRegisterCount(data);
+  Handle<FixedArray> data = Handle<FixedArray>(FixedArray::cast(re->data()));
+  data->set(JSRegExp::code_index(is_ascii), result.code);
+  int register_max = IrregexpMaxRegisterCount(*data);
   if (result.num_registers > register_max) {
-    SetIrregexpMaxRegisterCount(data, result.num_registers);
+    SetIrregexpMaxRegisterCount(*data, result.num_registers);
   }
 
   return true;
@@ -364,24 +362,12 @@ int RegExpImpl::IrregexpNumberOfRegisters(FixedArray* re) {
 
 
 ByteArray* RegExpImpl::IrregexpByteCode(FixedArray* re, bool is_ascii) {
-  int index;
-  if (is_ascii) {
-    index = JSRegExp::kIrregexpASCIICodeIndex;
-  } else {
-    index = JSRegExp::kIrregexpUC16CodeIndex;
-  }
-  return ByteArray::cast(re->get(index));
+  return ByteArray::cast(re->get(JSRegExp::code_index(is_ascii)));
 }
 
 
 Code* RegExpImpl::IrregexpNativeCode(FixedArray* re, bool is_ascii) {
-  int index;
-  if (is_ascii) {
-    index = JSRegExp::kIrregexpASCIICodeIndex;
-  } else {
-    index = JSRegExp::kIrregexpUC16CodeIndex;
-  }
-  return Code::cast(re->get(index));
+  return Code::cast(re->get(JSRegExp::code_index(is_ascii)));
 }
 
 
@@ -408,12 +394,14 @@ Handle<Object> RegExpImpl::IrregexpExec(Handle<JSRegExp> jsregexp,
   int number_of_capture_registers =
       (IrregexpNumberOfCaptures(FixedArray::cast(jsregexp->data())) + 1) * 2;
 
+#ifndef V8_NATIVE_REGEXP
 #ifdef DEBUG
   if (FLAG_trace_regexp_bytecodes) {
     String* pattern = jsregexp->Pattern();
     PrintF("\n\nRegexp match:   /%s/\n\n", *(pattern->ToCString()));
     PrintF("\n\nSubject string: '%s'\n\n", *(subject->ToCString()));
   }
+#endif
 #endif
 
   if (!subject->IsFlat()) {
@@ -422,16 +410,11 @@ Handle<Object> RegExpImpl::IrregexpExec(Handle<JSRegExp> jsregexp,
 
   last_match_info->EnsureSize(number_of_capture_registers + kLastMatchOverhead);
 
-  bool rc;
-  // We have to initialize this with something to make gcc happy but we can't
-  // initialize it with its real value until after the GC-causing things are
-  // over.
-  FixedArray* array = NULL;
+  Handle<FixedArray> array;
 
   // Dispatch to the correct RegExp implementation.
-  Handle<String> original_subject = subject;
   Handle<FixedArray> regexp(FixedArray::cast(jsregexp->data()));
-  if (UseNativeRegexp()) {
+#ifdef V8_NATIVE_REGEXP
 #if V8_TARGET_ARCH_IA32
   OffsetsVector captures(number_of_capture_registers);
   int* captures_vector = captures.vector();
@@ -457,20 +440,19 @@ Handle<Object> RegExpImpl::IrregexpExec(Handle<JSRegExp> jsregexp,
   ASSERT(res == RegExpMacroAssemblerIA32::SUCCESS
       || res == RegExpMacroAssemblerIA32::FAILURE);
 
-    rc = (res == RegExpMacroAssemblerIA32::SUCCESS);
-    if (!rc) return Factory::null_value();
+  if (res != RegExpMacroAssemblerIA32::SUCCESS) return Factory::null_value();
 
-    array = last_match_info->elements();
+  array = Handle<FixedArray>(last_match_info->elements());
   ASSERT(array->length() >= number_of_capture_registers + kLastMatchOverhead);
   // The captures come in (start, end+1) pairs.
   for (int i = 0; i < number_of_capture_registers; i += 2) {
-      SetCapture(array, i, captures_vector[i]);
-      SetCapture(array, i + 1, captures_vector[i + 1]);
+    SetCapture(*array, i, captures_vector[i]);
+    SetCapture(*array, i + 1, captures_vector[i + 1]);
   }
 #else  // !V8_TARGET_ARCH_IA32
     UNREACHABLE();
-#endif
-  } else {
+#endif  // V8_TARGET_ARCH_IA32
+#else  // !V8_NATIVE_REGEXP
   bool is_ascii = subject->IsAsciiRepresentation();
   if (!EnsureCompiledIrregexp(jsregexp, is_ascii)) {
     return Handle<Object>::null();
@@ -486,24 +468,25 @@ Handle<Object> RegExpImpl::IrregexpExec(Handle<JSRegExp> jsregexp,
   }
   Handle<ByteArray> byte_codes(IrregexpByteCode(*regexp, is_ascii));
 
-    rc = IrregexpInterpreter::Match(byte_codes,
+  if (!IrregexpInterpreter::Match(byte_codes,
                                   subject,
                                   register_vector,
-                                    previous_index);
-    if (!rc) return Factory::null_value();
+                                  previous_index)) {
+    return Factory::null_value();
+  }
 
-    array = last_match_info->elements();
+  array = Handle<FixedArray>(last_match_info->elements());
   ASSERT(array->length() >= number_of_capture_registers + kLastMatchOverhead);
   // The captures come in (start, end+1) pairs.
   for (int i = 0; i < number_of_capture_registers; i += 2) {
-      SetCapture(array, i, register_vector[i]);
-      SetCapture(array, i + 1, register_vector[i + 1]);
-    }
+    SetCapture(*array, i, register_vector[i]);
+    SetCapture(*array, i + 1, register_vector[i + 1]);
   }
+#endif  // V8_NATIVE_REGEXP
 
-  SetLastCaptureCount(array, number_of_capture_registers);
-  SetLastSubject(array, *original_subject);
-  SetLastInput(array, *original_subject);
+  SetLastCaptureCount(*array, number_of_capture_registers);
+  SetLastSubject(*array, *subject);
+  SetLastInput(*array, *subject);
 
   return last_match_info;
 }
@@ -4474,11 +4457,13 @@ RegExpEngine::CompilationResult RegExpEngine::Compile(RegExpCompileData* data,
 
   NodeInfo info = *node->info();
 
-  if (RegExpImpl::UseNativeRegexp()) {
+#ifdef V8_NATIVE_REGEXP
 #ifdef V8_TARGET_ARCH_ARM
+  // ARM native regexp not implemented yet.
   UNREACHABLE();
 #endif
 #ifdef V8_TARGET_ARCH_X64
+  // X64 native regexp not implemented yet.
   UNREACHABLE();
 #endif
 #ifdef V8_TARGET_ARCH_IA32
@@ -4495,14 +4480,15 @@ RegExpEngine::CompilationResult RegExpEngine::Compile(RegExpCompileData* data,
                            data->capture_count,
                            pattern);
 #endif
-  }
+#else  // ! V8_NATIVE_REGEXP
+  // Interpreted regexp.
   EmbeddedVector<byte, 1024> codes;
   RegExpMacroAssemblerIrregexp macro_assembler(codes);
   return compiler.Assemble(&macro_assembler,
                            node,
                            data->capture_count,
                            pattern);
+#endif  // V8_NATIVE_REGEXP
 }
 
-
 }}  // namespace v8::internal

src/jsregexp.h

@@ -37,13 +37,15 @@ class RegExpMacroAssembler;
 
 class RegExpImpl {
  public:
-  static inline bool UseNativeRegexp() {
-#ifdef V8_TARGET_ARCH_IA32
-    return FLAG_regexp_native;
+  // Whether V8 is compiled with native regexp support or not.
+  static bool UsesNativeRegExp() {
+#ifdef V8_NATIVE_REGEXP
+    return true;
 #else
     return false;
 #endif
   }
+
   // Creates a regular expression literal in the old space.
   // This function calls the garbage collector if necessary.
   static Handle<Object> CreateRegExpLiteral(Handle<JSFunction> constructor,
@@ -148,7 +150,8 @@ class RegExpImpl {
   static String* last_ascii_string_;
   static String* two_byte_cached_string_;
 
-  static bool EnsureCompiledIrregexp(Handle<JSRegExp> re, bool is_ascii);
+  static bool CompileIrregexp(Handle<JSRegExp> re, bool is_ascii);
+  static inline bool EnsureCompiledIrregexp(Handle<JSRegExp> re, bool is_ascii);
 
 
   // Set the subject cache.  The previous string buffer is not deleted, so the

src/objects-debug.cc

@@ -714,7 +714,7 @@ void JSRegExp::JSRegExpVerify() {
       break;
     }
     case JSRegExp::IRREGEXP: {
-      bool is_native = RegExpImpl::UseNativeRegexp();
+      bool is_native = RegExpImpl::UsesNativeRegExp();
 
       FixedArray* arr = FixedArray::cast(data());
       Object* ascii_data = arr->get(JSRegExp::kIrregexpASCIICodeIndex);

src/objects.h

@@ -3270,6 +3270,9 @@ class JSRegExp: public JSObject {
   inline Object* DataAt(int index);
   // Set implementation data after the object has been prepared.
   inline void SetDataAt(int index, Object* value);
+  static int code_index(bool is_ascii) {
+    return is_ascii ? kIrregexpASCIICodeIndex : kIrregexpUC16CodeIndex;
+  }
 
   static inline JSRegExp* cast(Object* obj);
 

src/runtime.cc

@@ -1016,16 +1016,16 @@ static Object* Runtime_RegExpExec(Arguments args) {
   ASSERT(args.length() == 4);
   CONVERT_ARG_CHECKED(JSRegExp, regexp, 0);
   CONVERT_ARG_CHECKED(String, subject, 1);
-  // Due to the way the JS files are constructed this must be less than the
+  // Due to the way the JS calls are constructed this must be less than the
   // length of a string, i.e. it is always a Smi.  We check anyway for security.
-  CONVERT_CHECKED(Smi, index, args[2]);
+  CONVERT_SMI_CHECKED(index, args[2]);
   CONVERT_ARG_CHECKED(JSArray, last_match_info, 3);
   RUNTIME_ASSERT(last_match_info->HasFastElements());
-  RUNTIME_ASSERT(index->value() >= 0);
-  RUNTIME_ASSERT(index->value() <= subject->length());
+  RUNTIME_ASSERT(index >= 0);
+  RUNTIME_ASSERT(index <= subject->length());
   Handle<Object> result = RegExpImpl::Exec(regexp,
                                            subject,
-                                           index->value(),
+                                           index,
                                            last_match_info);
   if (result.is_null()) return Failure::Exception();
   return *result;

test/cctest/test-debug.cc

@@ -4892,6 +4892,7 @@ TEST(DebugBreakInMessageHandler) {
 }
 
 
+#ifdef V8_NATIVE_REGEXP
 // Debug event handler which gets the function on the top frame and schedules a
 // break a number of times.
 static void DebugEventDebugBreak(
@@ -4928,11 +4929,10 @@ static void DebugEventDebugBreak(
 
 
 TEST(RegExpDebugBreak) {
+  // This test only applies to native regexps.
   v8::HandleScope scope;
   DebugLocalContext env;
 
-  i::FLAG_regexp_native = true;
-
   // Create a function for checking the function when hitting a break point.
   frame_function_name = CompileFunction(&env,
                                         frame_function_name_source,
@@ -4957,6 +4957,7 @@ TEST(RegExpDebugBreak) {
   CHECK_EQ(20, break_point_hit_count);
   CHECK_EQ("exec", last_function_hit);
 }
+#endif  // V8_NATIVE_REGEXP
 
 
 // Common part of EvalContextData and NestedBreakEventContextData tests.

test/cctest/test-regexp.cc

@@ -597,6 +597,8 @@ TEST(DispatchTableConstruction) {
 }
 
 
+// Tests of interpreter.
+
 TEST(MacroAssembler) {
   V8::Initialize(NULL);
   byte codes[1024];
@@ -660,8 +662,8 @@ TEST(MacroAssembler) {
   CHECK_EQ(42, captures[0]);
 }
 
-
-#ifdef V8_TARGET_ARCH_IA32  // IA32 only tests.
+#ifdef V8_TARGET_ARCH_IA32  // IA32 Native Regexp only tests.
+#ifdef V8_NATIVE_REGEXP
 
 class ContextInitializer {
  public:
@@ -1284,10 +1286,10 @@ TEST(MacroAssemblerIA32LotsOfRegisters) {
   Top::clear_pending_exception();
 }
 
+#endif  // V8_REGEXP_NATIVE
+#endif  // V8_TARGET_ARCH_IA32
 
 
-#endif  // !defined ARM
-
 TEST(AddInverseToTable) {
   static const int kLimit = 1000;
   static const int kRangeCount = 16;

tools/gyp/v8.gyp

@@ -419,7 +419,8 @@
       'target_name': 'v8_base',
       'type': '<(library)',
       'defines': [
-        'V8_TARGET_ARCH_IA32'
+        'V8_TARGET_ARCH_IA32',
+        'V8_NATIVE_REGEXP'
       ],
       'include_dirs+': [
         '../../src',
@@ -477,7 +478,8 @@
       'target_name': 'v8_nosnapshot',
       'type': '<(library)',
       'defines': [
-        'V8_TARGET_ARCH_IA32'
+        'V8_TARGET_ARCH_IA32',
+        'V8_NATIVE_REGEXP'
       ],
       'dependencies': [
         'js2c',
@@ -509,7 +511,8 @@
       'target_name': 'v8',
       'type': '<(library)',
       'defines': [
-        'V8_TARGET_ARCH_IA32'
+        'V8_TARGET_ARCH_IA32',
+        'V8_NATIVE_REGEXP'
       ],
       'dependencies': [
         'js2c',
@@ -549,7 +552,8 @@
       'target_name': 'v8_shell',
       'type': 'executable',
       'defines': [
-        'V8_TARGET_ARCH_IA32'
+        'V8_TARGET_ARCH_IA32',
+        'V8_NATIVE_REGEXP'
       ],
       'dependencies': [
         'v8',
@@ -579,7 +583,8 @@
         'v8',
       ],
       'defines': [
-        'V8_TARGET_ARCH_IA32'
+        'V8_TARGET_ARCH_IA32',
+        'V8_NATIVE_REGEXP'
       ],
       'include_dirs': [
         '../../src',

tools/v8.xcodeproj/project.pbxproj

@@ -1449,6 +1449,7 @@
 				GCC_PREPROCESSOR_DEFINITIONS = (
 					"$(GCC_PREPROCESSOR_DEFINITIONS)",
 					V8_TARGET_ARCH_IA32,
+					V8_NATIVE_REGEXP,
 					DEBUG,
 				);
 				HEADER_SEARCH_PATHS = ../src;
@@ -1462,6 +1463,7 @@
 				GCC_PREPROCESSOR_DEFINITIONS = (
 					"$(GCC_PREPROCESSOR_DEFINITIONS)",
 					V8_TARGET_ARCH_IA32,
+					V8_NATIVE_REGEXP,
 					NDEBUG,
 				);
 				HEADER_SEARCH_PATHS = ../src;
@@ -1477,6 +1479,7 @@
 					"$(GCC_PREPROCESSOR_DEFINITIONS)",
 					ENABLE_DISASSEMBLER,
 					V8_TARGET_ARCH_IA32,
+					V8_NATIVE_REGEXP,
 					ENABLE_LOGGING_AND_PROFILING,
 				);
 				HEADER_SEARCH_PATHS = ../src;
@@ -1492,6 +1495,7 @@
 				GCC_PREPROCESSOR_DEFINITIONS = (
 					"$(GCC_PREPROCESSOR_DEFINITIONS)",
 					V8_TARGET_ARCH_IA32,
+					V8_NATIVE_REGEXP,
 					NDEBUG,
 				);
 				HEADER_SEARCH_PATHS = ../src;

tools/visual_studio/ia32.vsprops

@@ -6,6 +6,6 @@
 	>
 	<Tool
 		Name="VCCLCompilerTool"
-		PreprocessorDefinitions="V8_TARGET_ARCH_IA32"
+		PreprocessorDefinitions="V8_TARGET_ARCH_IA32;V8_NATIVE_REGEXP"
 	/>
 </VisualStudioPropertySheet>