LGTM Please remember to file a bug to do the optimization on x64 and ARM too.
We just return a failure if there is a stack overflow exception. Apparently that is what the old code did too :-/ http://codereview.chromium.org/521028/diff/1/3 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/521028/diff/1/3#newcode7955 src/ia32/codegen-ia32.cc:7955: __ j(equal, &runtime, not_taken); This looks superfluous. If it's the undefined object then it isn't a fixed array, so it will go to the runtime anyway. Alternatively it is the fixed array check that is unnecessary. http://codereview.chromium.org/521028/diff/1/3#newcode7963 src/ia32/codegen-ia32.cc:7963: __ j(not_zero, &runtime, not_taken); Superfluous test. http://codereview.chromium.org/521028/diff/1/3#newcode7968 src/ia32/codegen-ia32.cc:7968: // Check that the numbers of captures fit in the static offsets vector buffer. numbers -> number http://codereview.chromium.org/521028/diff/1/3#newcode7970 src/ia32/codegen-ia32.cc:7970: __ test(edx, Immediate(kSmiTagMask)); Can this ever happen? http://codereview.chromium.org/521028/diff/1/3#newcode7975 src/ia32/codegen-ia32.cc:7975: __ cmp(edx, OffsetsVector::kStaticOffsetsVectorSize); Some comment and an assert is needed here as you are using the fact that Smis are 2x untagged values. http://codereview.chromium.org/521028/diff/1/3#newcode7987 src/ia32/codegen-ia32.cc:7987: __ and_(ebx, kStringRepresentationMask); Where are you checking that the string is ASCII? http://codereview.chromium.org/521028/diff/1/3#newcode8004 src/ia32/codegen-ia32.cc:8004: __ j(greater, &runtime); Greater_equal? http://codereview.chromium.org/521028/diff/1/3#newcode8006 src/ia32/codegen-ia32.cc:8006: // ebx: Length of subject string We don't use this. http://codereview.chromium.org/521028/diff/1/3#newcode8015 src/ia32/codegen-ia32.cc:8015: // Check that the JSArray is in fast case. Can we ensure that this is always the case? http://codereview.chromium.org/521028/diff/1/3#newcode8033 src/ia32/codegen-ia32.cc:8033: __ and_(ebx, kStringEncodingMask); You can do this together with the representation above by using the full representation. Of course if you intend to support both encodings you can move the representation test down here instead. http://codereview.chromium.org/521028/diff/1/3#newcode8038 src/ia32/codegen-ia32.cc:8038: // Check that a RegExp stack is allocated. Can we ensure that there is always a small regexp stack available so we can skip this test? http://codereview.chromium.org/521028/diff/1/3#newcode8053 src/ia32/codegen-ia32.cc:8053: __ test(edx, Immediate(kSmiTagMask)); It must be possible to ensure that this is not a Smi. http://codereview.chromium.org/521028/diff/1/3#newcode8144 src/ia32/codegen-ia32.cc:8144: __ shl(edx, 1); // Number of capture registers to smi. I think the macro assembler has a function call for this. http://codereview.chromium.org/521028/diff/1/3#newcode8146 src/ia32/codegen-ia32.cc:8146: __ shr(edx, 1); // Number of capture registers back from smi. And this? http://codereview.chromium.org/521028/diff/1/3#newcode8175 src/ia32/codegen-ia32.cc:8175: __ test(edi, Immediate(0x80000000)); Didn't the shl already set the negative flag? http://codereview.chromium.org/521028/diff/1/2 File src/ia32/regexp-macro-assembler-ia32.cc (right): http://codereview.chromium.org/521028/diff/1/2#newcode62 src/ia32/regexp-macro-assembler-ia32.cc:62: * - at_start (if 1, start at start of string, if 0, don't) start at start of string, if 0, don't-> we are starting at the start of the string, otherwise 0 http://codereview.chromium.org/521028/diff/1/2#newcode66 src/ia32/regexp-macro-assembler-ia32.cc:66: * - start index (character index if start) if -> of http://codereview.chromium.org/521028 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
