Addressed comments and ported to x64 and arm. Please take another look.
http://codereview.chromium.org/10386090/diff/1/src/ia32/regexp-macro-assembler-ia32.h File src/ia32/regexp-macro-assembler-ia32.h (right): http://codereview.chromium.org/10386090/diff/1/src/ia32/regexp-macro-assembler-ia32.h#newcode139 src/ia32/regexp-macro-assembler-ia32.h:139: // For the case of global regular expression, we have room to store more On 2012/05/11 11:01:00, Erik Corry wrote:
more than one -> at least one ?
Done. http://codereview.chromium.org/10386090/diff/1/src/ia32/regexp-macro-assembler-ia32.h#newcode160 src/ia32/regexp-macro-assembler-ia32.h:160: // The highest bit on the output registers count signals success. This On 2012/05/11 11:01:00, Erik Corry wrote:
lowest bit?
Not even necessary anymore. Removed. http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc File src/jsregexp.cc (right): http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc#newcode904 src/jsregexp.cc:904: RegExpCompiler(int capture_count, On 2012/05/11 11:01:00, Erik Corry wrote:
You don't need this change.
Done. http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc#newcode981 src/jsregexp.cc:981: bool ignore_case, On 2012/05/11 11:01:00, Erik Corry wrote:
You don't need this change.
Done. http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc#newcode5796 src/jsregexp.cc:5796: RegExpCompiler compiler(data->capture_count, On 2012/05/11 11:01:00, Erik Corry wrote:
Or this.
Done. http://codereview.chromium.org/10386090/diff/1/src/regexp-macro-assembler.h File src/regexp-macro-assembler.h (right): http://codereview.chromium.org/10386090/diff/1/src/regexp-macro-assembler.h#newcode186 src/regexp-macro-assembler.h:186: // Set wether the regular expression has the global flag. Failures in a On 2012/05/11 11:01:00, Erik Corry wrote:
A wether is a castrated billy-goat :-)
Done. I never knew that. Learned something new today :) http://codereview.chromium.org/10386090/diff/1/src/regexp-macro-assembler.h#newcode187 src/regexp-macro-assembler.h:187: // global regexp may still mean success overal. On 2012/05/11 11:01:00, Erik Corry wrote:
overall.
Done. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3798 src/runtime.cc:3798: static int SearchRegExpNoCaptureMultiple( On 2012/05/11 11:01:00, Erik Corry wrote:
I wonder if we can unify this and SearchRegExpMultiple without a
performance
hit.
I'll try this in another CL. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3818 src/runtime.cc:3818: int result = On 2012/05/11 11:01:00, Erik Corry wrote:
How about giving this variable a name like number_of_matches, and
getting rid of
RE_SUCCESS and RE_FAILURE. Just comparing == 0 and > 0 makes sense if
the
variable has the right name.
Done. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3940 src/runtime.cc:3940: match = isolate->factory()->NewProperSubString(subject, On 2012/05/11 11:01:00, Erik Corry wrote:
Not sure why we need NewProperSubString here.
For every substring save for the first one we know that it's a real substring, so we can omit the length- and start-check using NewProperSubString. Unfortunately, this does make a performance difference. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3949 src/runtime.cc:3949: for (int i = 1; i <= capture_count; i++) { On 2012/05/11 11:01:00, Erik Corry wrote:
I think you could just start this loop at 0 and remove some code
duplication. After removing the call to NewSubString here (first is false in any case), I think it makes sense to have the first loop stand out: we don't do a check for -1 for undefined there and can be sure not to use NewSubString. http://codereview.chromium.org/10386090/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
