LGTM
http://codereview.chromium.org/552186/diff/9001/10003 File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/552186/diff/9001/10003#newcode6858 src/arm/codegen-arm.cc:6858: __ ldrb(scratch, MemOperand(src, count), pl); Maybe add a comment on why ldrb is before b. Maybe before bind(&loop). http://codereview.chromium.org/552186/diff/9001/10003#newcode6870 src/arm/codegen-arm.cc:6870: Add an empty line. http://codereview.chromium.org/552186/diff/9001/10003#newcode6882 src/arm/codegen-arm.cc:6882: bool dest_always_aligned = (flags & DEST_ALWAYS_ALIGNED) != 0; Perhaps generated code assert that dest is actually aligned when it is suposed to be? http://codereview.chromium.org/552186/diff/9001/10003#newcode6884 src/arm/codegen-arm.cc:6884: // Check that reading an entire aligned word containing the last character Check -> Ensure. http://codereview.chromium.org/552186/diff/9001/10003#newcode6908 src/arm/codegen-arm.cc:6908: __ and_(scratch4, dest, Operand(0x03), SetCC); 0x03 -> kPointerAlignmentMask? http://codereview.chromium.org/552186/diff/9001/10003#newcode6924 src/arm/codegen-arm.cc:6924: __ and_(scratch4, scratch4, Operand(0x03), SetCC); 0x03 -> kPointerAlignmentMask? http://codereview.chromium.org/552186/diff/9001/10003#newcode6956 src/arm/codegen-arm.cc:6956: // scratch (eight times that number in scratch4). We may have read past scratch -> scratch1 http://codereview.chromium.org/552186/diff/9001/10003#newcode6986 src/arm/codegen-arm.cc:6986: __ cmp(scratch3, Operand(8)); I guess that this need to be 8 and not 4. http://codereview.chromium.org/552186/diff/9001/10003#newcode7106 src/arm/codegen-arm.cc:7106: __ add(r1, r0, Operand(SeqAsciiString::kHeaderSize - kHeapObjectTag)); // Locate first character of "from". http://codereview.chromium.org/552186/diff/9001/10003#newcode7134 src/arm/codegen-arm.cc:7134: __ add(r1, r0, Operand(SeqTwoByteString::kHeaderSize - kHeapObjectTag)); // Locate first character of "from". http://codereview.chromium.org/552186/diff/9001/10010 File test/mjsunit/substr.js (right): http://codereview.chromium.org/552186/diff/9001/10010#newcode47 test/mjsunit/substr.js:47: //for (var i = 0; i < s.length; i++) Code in comments. http://codereview.chromium.org/552186 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
