Add support for two byte strings in direct call to RegExp...

Add support for two byte strings in direct call to RegExp

The stub for calling RegExp directly now also handles two byte strings.

Support for flat cons strings added for both ascii and two byte.

Some code code simplifications and added a few constants.

Committed: http://code.google.com/p/v8/source/detail?r=3678

[Lasse Reichstein, 2010-01-21 13:20:56]

LGTM

http://codereview.chromium.org/545151/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/545151/diff/1/2#newcode8223
src/ia32/codegen-ia32.cc:8223: // Check for flat ascii cons string.
Instead of testing for flat ASCII and Two-Byte strings independently, could you
just test for a flat string (const string with second part being The Empty
String), and then loop back to checking the representation and encoding with the
first part?

http://codereview.chromium.org/545151/diff/1/2#newcode8224
src/ia32/codegen-ia32.cc:8224: __ cmp(ebx, kStringTag | kConsStringTag |
kAsciiStringTag);
I.e., ignore the representation tag here, and just see if it's a cons ...

http://codereview.chromium.org/545151/diff/1/2#newcode8229
src/ia32/codegen-ia32.cc:8229: __ mov(eax, FieldOperand(eax,
ConsString::kFirstOffset));
... and jump back to line 8209 here.

http://codereview.chromium.org/545151/diff/1/2#newcode8241
src/ia32/codegen-ia32.cc:8241: __ cmp(ebx, kStringTag | kConsStringTag |
kStringTag);
Two occurrences of kStringTag and non of kTwoByteStringTag?

http://codereview.chromium.org/545151/diff/1/2#newcode8261
src/ia32/codegen-ia32.cc:8261: __ mov(edx, FieldOperand(ecx,
JSRegExp::kDataAsciiCodeOffset));
If we had the string's instance type here (I believe it's in ebc), we could do
compute both the offset and the 0/1 for being two-byte from the type &
kStringEncodingMask (ASCII being 4 and UC16 being 0). It's probably not a lot
shorter than having both branches of code, though.
But we could completely avoid checking the encoding above.

http://codereview.chromium.org/545151/diff/1/2#newcode8275
src/ia32/codegen-ia32.cc:8275: __ xor_(edi, Operand(edi));  // Type is two byte.
You can use __ Set(edi, Immediate(0)); to make this more readable. The Set macro
uses xor if the argument is zero and mov if it's not.

http://codereview.chromium.org/545151/diff/1/2#newcode8276
src/ia32/codegen-ia32.cc:8276: __ jmp(&invoke_regexp);
Should just fall through.

http://codereview.chromium.org/545151/diff/1/3
File src/regexp-macro-assembler.cc (right):

http://codereview.chromium.org/545151/diff/1/3#newcode128
src/regexp-macro-assembler.cc:128: ASSERT_EQ(0,
ConsString::cast(subject_ptr)->second()->length());
The thing you check against in the assembler is being equal to
Heap::empty_string(), not just having length zero.
Maybe make a comment that there can only be one zero-length string.

[Søren Thygesen Gjesse, 2010-01-22 08:31:03]

http://codereview.chromium.org/545151/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/545151/diff/1/2#newcode8223
src/ia32/codegen-ia32.cc:8223: // Check for flat ascii cons string.
On 2010/01/21 13:20:56, Lasse Reichstein wrote:
> Instead of testing for flat ASCII and Two-Byte strings independently, could
you
> just test for a flat string (const string with second part being The Empty
> String), and then loop back to checking the representation and encoding with
the
> first part? 

Good point. Restructured the code to first check for seq string using

test(Operand(ebx),
          Immediate(kIsNotStringMask | kStringRepresentationMask))

As kStringTag and kSeqStringTag are both zero and ump forward to seq string
code. Fall through to check for flat cons string and fall through to seq string
code if it is.

Seq string code handles both encodings.

http://codereview.chromium.org/545151/diff/1/2#newcode8224
src/ia32/codegen-ia32.cc:8224: __ cmp(ebx, kStringTag | kConsStringTag |
kAsciiStringTag);
On 2010/01/21 13:20:56, Lasse Reichstein wrote:
> I.e., ignore the representation tag here, and just see if it's a cons ...

See above.

http://codereview.chromium.org/545151/diff/1/2#newcode8229
src/ia32/codegen-ia32.cc:8229: __ mov(eax, FieldOperand(eax,
ConsString::kFirstOffset));
On 2010/01/21 13:20:56, Lasse Reichstein wrote:
> ... and jump back to line 8209 here.

See above.

http://codereview.chromium.org/545151/diff/1/2#newcode8241
src/ia32/codegen-ia32.cc:8241: __ cmp(ebx, kStringTag | kConsStringTag |
kStringTag);
On 2010/01/21 13:20:56, Lasse Reichstein wrote:
> Two occurrences of kStringTag and non of kTwoByteStringTag?

Fixed. Only worked becahse kTwoByteStringTag and kStringTag are both zero.

http://codereview.chromium.org/545151/diff/1/2#newcode8261
src/ia32/codegen-ia32.cc:8261: __ mov(edx, FieldOperand(ecx,
JSRegExp::kDataAsciiCodeOffset));
On 2010/01/21 13:20:56, Lasse Reichstein wrote:
> If we had the string's instance type here (I believe it's in ebc), we could do
> compute both the offset and the 0/1 for being two-byte from the type &
> kStringEncodingMask (ASCII being 4 and UC16 being 0). It's probably not a lot
> shorter than having both branches of code, though.
> But we could completely avoid checking the encoding above.

Decided not to try to compute the offset, that is did not want to do
ASSERT_EQ(kAsciiStringTag == (JSRegExp::kDataUC16CodeOffset -
JSRegExp::kDataAsciiCodeOffset)).

http://codereview.chromium.org/545151/diff/1/2#newcode8275
src/ia32/codegen-ia32.cc:8275: __ xor_(edi, Operand(edi));  // Type is two byte.
On 2010/01/21 13:20:56, Lasse Reichstein wrote:
> You can use __ Set(edi, Immediate(0)); to make this more readable. The Set
macro
> uses xor if the argument is zero and mov if it's not.

Done.

http://codereview.chromium.org/545151/diff/1/2#newcode8276
src/ia32/codegen-ia32.cc:8276: __ jmp(&invoke_regexp);
On 2010/01/21 13:20:56, Lasse Reichstein wrote:
> Should just fall through.

Done.