LGTM. Does it run the Mozilla tests too?
http://codereview.chromium.org/12807/diff/1/5 File src/regexp-macro-assembler-ia32.cc (right): http://codereview.chromium.org/12807/diff/1/5#newcode141 Line 141: ReadCurrentChar(eax); The intention of this operation is that it operates on an already loaded current character, so it is wrong to load it here. http://codereview.chromium.org/12807/diff/1/5#newcode169 Line 169: __ test(Operand(ebp, kAtStart), Immediate(0xffffffff)); I think a cmp against zero is less confusing here. http://codereview.chromium.org/12807/diff/1/5#newcode299 Line 299: ReadCurrentChar(eax); This operation should also use the current character already loaded. http://codereview.chromium.org/12807/diff/1/5#newcode322 Line 322: ReadCurrentChar(eax); And again. http://codereview.chromium.org/12807/diff/1/5#newcode361 Line 361: ReadCurrentChar(eax); Ditto. http://codereview.chromium.org/12807/diff/1/5#newcode380 Line 380: ReadCurrentChar(eax); Ditto http://codereview.chromium.org/12807/diff/1/8 File test/cctest/test-regexp.cc (right): http://codereview.chromium.org/12807/diff/1/8#newcode606 Line 606: *code, seq_input.location(), start_offset, end_offset, captures, true); Although the style guide is not strict on this point, V8 style has been to either have the entire call on one line or to have a line per argument, but not this compromise format. Please reformat. Also several other places. http://codereview.chromium.org/12807 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
