* Fixes and tweaks to regexp-ia32.

Fixes (last?) bugs in regexp-ia32 core functionality. All tests run!

[Lasse Reichstein, 2008-11-28 16:14:19]

Small code review.

[Erik Corry, 2008-11-30 19:39:26]

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.

[Lasse Reichstein, 2008-12-04 15:18:37]

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 ReadCurrentChar function merely moves the already loaded  value into a
register. It's a simple abstraction on top of the "edx" register, but probably
one that has outlived its usefullness. I have reworded it to "mov(eax,
current_character())", where "current_character()" is just a more readable way
of saying "edx".

http://codereview.chromium.org/12807/diff/1/5#newcode169
Line 169: __ test(Operand(ebp, kAtStart), Immediate(0xffffffff));
On 2008/11/30 19:39:27, Erik Corry wrote:
> I think a cmp against zero is less confusing here.

Done.

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);
On 2008/11/30 19:39:27, Erik Corry wrote:
> 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.

Done.