Port direct call to native RegExp from JavaScript to ARM...

Port direct call to native RegExp from JavaScript to ARM

The ia32 version was implemented in r3542 and r3543. The x64 was implementeed in r3740, r3741 and r3742.

Minor tweaks to the is32 and x64 code as well.

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

[Erik Corry, 2010-04-13 13:20:15]

LGTM

http://codereview.chromium.org/1575035/diff/1/4
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1575035/diff/1/4#newcode7604
src/arm/codegen-arm.cc:7604: // string length. A negative value will be greater
(usigned comparison).
usigned -> unsigned

http://codereview.chromium.org/1575035/diff/1/4#newcode7658
src/arm/codegen-arm.cc:7658: __ cmp(r0, Operand(Factory::empty_string()));
There's a root for this.  You can do LoadRoot(ip, kEmptyStringRootIndex) and
then do a register register compare.  Like this version it's two instructions,
but unlike this version there's no constant pool impact and you can hoist the
load a little (if you don't use ip).

http://codereview.chromium.org/1575035/diff/1/4#newcode7675
src/arm/codegen-arm.cc:7675: __ cmp(r1, Operand(kSeqTwoByteString));
I guess it's too devious to use the fact that r1 is either 0 or 4 here?

http://codereview.chromium.org/1575035/diff/1/4#newcode7724
src/arm/codegen-arm.cc:7724: // Argument 4 (r3): End of string data
Confusing comment.

http://codereview.chromium.org/1575035/diff/1/4#newcode7727
src/arm/codegen-arm.cc:7727: __ tst(r3, Operand(r3));
Perhaps easier to shift r0 and r1 by r3 here.

http://codereview.chromium.org/1575035/diff/1/4#newcode7730
src/arm/codegen-arm.cc:7730: __ add(r3, r2, Operand(r0), LeaveCC, ne);  // ASCII
Full stops.

http://codereview.chromium.org/1575035/diff/1/4#newcode7736
src/arm/codegen-arm.cc:7736: // Alreay there
Alreay -> Already

http://codereview.chromium.org/1575035/diff/1/4#newcode7764
src/arm/codegen-arm.cc:7764: // TODO(592) Rerunning the RegExp to get the stack
overflow exception.
Lint? (colon after ')')

[Søren Thygesen Gjesse, 2010-04-14 08:34:05]

http://codereview.chromium.org/1575035/diff/1/4
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1575035/diff/1/4#newcode7604
src/arm/codegen-arm.cc:7604: // string length. A negative value will be greater
(usigned comparison).
On 2010/04/13 13:20:15, Erik Corry wrote:
> usigned -> unsigned

Done.

http://codereview.chromium.org/1575035/diff/1/4#newcode7658
src/arm/codegen-arm.cc:7658: __ cmp(r0, Operand(Factory::empty_string()));
On 2010/04/13 13:20:15, Erik Corry wrote:
> There's a root for this.  You can do LoadRoot(ip, kEmptyStringRootIndex) and
> then do a register register compare.  Like this version it's two instructions,
> but unlike this version there's no constant pool impact and you can hoist the
> load a little (if you don't use ip).

Done. I did not hoist the load as then I would have to load it before the
branch.

http://codereview.chromium.org/1575035/diff/1/4#newcode7675
src/arm/codegen-arm.cc:7675: __ cmp(r1, Operand(kSeqTwoByteString));
On 2010/04/13 13:20:15, Erik Corry wrote:
> I guess it's too devious to use the fact that r1 is either 0 or 4 here?

It's not. Other parts of the code already use the fact that kSeqStringTag is
zero. Added the proper asserts.

http://codereview.chromium.org/1575035/diff/1/4#newcode7724
src/arm/codegen-arm.cc:7724: // Argument 4 (r3): End of string data
On 2010/04/13 13:20:15, Erik Corry wrote:
> Confusing comment.

Changed.

http://codereview.chromium.org/1575035/diff/1/4#newcode7727
src/arm/codegen-arm.cc:7727: __ tst(r3, Operand(r3));
On 2010/04/13 13:20:15, Erik Corry wrote:
> Perhaps easier to shift r0 and r1 by r3 here.

Shifted by (r3 ^ 0x1) (r3 is 1 for ASCII and o for two byte).

Also added the assert

  ASSERT_EQ(SeqAsciiString::kHeaderSize, SeqTwoByteString::kHeaderSize);

http://codereview.chromium.org/1575035/diff/1/4#newcode7730
src/arm/codegen-arm.cc:7730: __ add(r3, r2, Operand(r0), LeaveCC, ne);  // ASCII
On 2010/04/13 13:20:15, Erik Corry wrote:
> Full stops.

After using shift by (r3 ^ 0x1) these comments are no longer relevant.

http://codereview.chromium.org/1575035/diff/1/4#newcode7736
src/arm/codegen-arm.cc:7736: // Alreay there
On 2010/04/13 13:20:15, Erik Corry wrote:
> Alreay -> Already

Done.

http://codereview.chromium.org/1575035/diff/1/4#newcode7764
src/arm/codegen-arm.cc:7764: // TODO(592) Rerunning the RegExp to get the stack
overflow exception.
On 2010/04/13 13:20:15, Erik Corry wrote:
> Lint? (colon after ')')

Done (lints without though).