LGTM - nice use of ARM conditional instructions.

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

http://codereview.chromium.org/571005/diff/1/2#newcode7228
src/arm/codegen-arm.cc:7228: //  sp[4]: left string
Woops. Good catch.

http://codereview.chromium.org/571005/diff/1/2#newcode7271
src/arm/codegen-arm.cc:7271: ASSERT_EQ(0, kSmiTag);
Add JumpIfNotBothSmi to macro assembler?

http://codereview.chromium.org/571005/diff/1/2#newcode7403
src/arm/codegen-arm.cc:7403: __ add(r6, r7,
Operand(SeqAsciiString::kHeaderSize - kHeapObjectTag));
Use MemOperand to avoid "- kHeapObjectTag"?

http://codereview.chromium.org/571005/diff/1/2#newcode7405
src/arm/codegen-arm.cc:7405: __ add(r0, r0,
Operand(SeqAsciiString::kHeaderSize - kHeapObjectTag));
Ditto.

http://codereview.chromium.org/571005/diff/1/2#newcode7415
src/arm/codegen-arm.cc:7415: __ add(r1, r1,
Operand(SeqAsciiString::kHeaderSize - kHeapObjectTag));
Ditto.

http://codereview.chromium.org/571005/diff/1/2#newcode7441
src/arm/codegen-arm.cc:7441: __ add(r6, r7,
Operand(SeqTwoByteString::kHeaderSize - kHeapObjectTag));
Ditto.

http://codereview.chromium.org/571005/diff/1/2#newcode7443
src/arm/codegen-arm.cc:7443: __ add(r0, r0,
Operand(SeqTwoByteString::kHeaderSize - kHeapObjectTag));
Ditto.

http://codereview.chromium.org/571005/diff/1/2#newcode7454
src/arm/codegen-arm.cc:7454: __ add(r1, r1,
Operand(SeqTwoByteString::kHeaderSize - kHeapObjectTag));
Ditto.

http://codereview.chromium.org/571005/diff/1/3
File src/arm/codegen-arm.h (right):

http://codereview.chromium.org/571005/diff/1/3#newcode572
src/arm/codegen-arm.h:572: class StringAddStub: public StringStubBase {
Now this exists and is (I think) identical on all platforms. Would it be
 a good idea to move it to codegen.h?

http://codereview.chromium.org/571005

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

Reply via email to