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 ')')

http://codereview.chromium.org/1575035/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

To unsubscribe, reply using "remove me" as the subject.

Reply via email to