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