http://codereview.chromium.org/993002/diff/1/7 File src/arm/assembler-arm.cc (right):
http://codereview.chromium.org/993002/diff/1/7#newcode1330 src/arm/assembler-arm.cc:1330: ASSERT(offset % 4 == 0); Here and in the function above we should assert that the offset is not out of range. http://codereview.chromium.org/993002/diff/1/7#newcode1407 src/arm/assembler-arm.cc:1407: There should be 2 blank lines here. http://codereview.chromium.org/993002/diff/1/7#newcode1411 src/arm/assembler-arm.cc:1411: And here. http://codereview.chromium.org/993002/diff/1/7#newcode1412 src/arm/assembler-arm.cc:1412: static bool is_signed(VFPType type) { Function names are CapitalCamelCase. This goes for all of them. http://codereview.chromium.org/993002/diff/1/7#newcode1423 src/arm/assembler-arm.cc:1423: Two lines here. http://codereview.chromium.org/993002/diff/1/7#newcode1454 src/arm/assembler-arm.cc:1454: int *Vm, Wrong placement of * and wrong capitalization of parameter name. http://codereview.chromium.org/993002/diff/1/7#newcode1468 src/arm/assembler-arm.cc:1468: const int dst_code, Wrong indentation. http://codereview.chromium.org/993002/diff/1/6 File src/arm/constants-arm.cc (right): http://codereview.chromium.org/993002/diff/1/6#newcode88 src/arm/constants-arm.cc:88: 2 blank lines please. http://codereview.chromium.org/993002/diff/1/6#newcode89 src/arm/constants-arm.cc:89: int VFPRegisters::Number(const char* name, bool *is_double) { Asterisk placement. http://codereview.chromium.org/993002/diff/1/10 File src/arm/disasm-arm.cc (right): http://codereview.chromium.org/993002/diff/1/10#newcode934 src/arm/disasm-arm.cc:934: if (instr->Bit(23) == 1) { This is getting very messy (not your fault) and I would like it cleaned up. See Table A7-24. http://codereview.chromium.org/993002/diff/1/11 File src/arm/ic-arm.cc (right): http://codereview.chromium.org/993002/diff/1/11#newcode644 src/arm/ic-arm.cc:644: // This functions would not work correctly for 0. functions would -> function does http://codereview.chromium.org/993002/diff/1/11#newcode650 src/arm/ic-arm.cc:650: const int meaningfull_bits meaningfull -> meaningful Please don't split over two lines if it fits on one. http://codereview.chromium.org/993002/diff/1/11#newcode654 src/arm/ic-arm.cc:654: = meaningfull_bits - HeapNumber::kMantissaBitsInTopWord; = should go on the line before (also 2 other places). http://codereview.chromium.org/993002/diff/1/11#newcode722 src/arm/ic-arm.cc:722: __ mov(r0, Operand(r0, ASR, kSmiTagSize)); // Untag key. You don't need to untag this. Instead you should assert that the kSmiTag is 0 and that the kSmiTagSize is 1, then you should just leave r0 tagged. http://codereview.chromium.org/993002/diff/1/11#newcode789 src/arm/ic-arm.cc:789: __ vcvt_f64_s32(d0, s0); We can do vstr here. http://codereview.chromium.org/993002/diff/1/11#newcode866 src/arm/ic-arm.cc:866: // VFP is not available do manual single to double conversion. available do -> available, do http://codereview.chromium.org/993002/diff/1/11#newcode1058 src/arm/ic-arm.cc:1058: __ BranchOnNotSmi(r0, &slow); You might use GetInt32 to do this without going into the runtime system (a different change probably). http://codereview.chromium.org/993002/diff/1/11#newcode1144 src/arm/ic-arm.cc:1144: static void ConvertIntToFloat(MacroAssembler* masm, I ran out of time here. http://codereview.chromium.org/993002 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
