LGTM

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

http://codereview.chromium.org/2813046/diff/1/2#newcode1804
src/arm/assembler-arm.cc:1804: static uint64_t DoubleAsInt(double d) {
Please add a comment here:

// Only works for little endian floating point formats.
// We don't support VFP on the mixed endian floating point platform.


Also, both places where you use this you immediately separate out the
low and high bits.  Perhaps it should take two uint32_t*s and put the
answer there.

http://codereview.chromium.org/2813046/diff/1/2#newcode1847
src/arm/assembler-arm.cc:1847: if (((hi ^ (hi << 1)) & (1 << 30)) == 0)
{
It would be more consistent to use 0x40000000 here instead of 1 << 30.

http://codereview.chromium.org/2813046/diff/1/2#newcode1886
src/arm/assembler-arm.cc:1886: // registers of D register dst.
Would it be faster to take a scratch register as an argument and use
that with ip to do the move in one?

http://codereview.chromium.org/2813046/diff/1/2#newcode1894
src/arm/assembler-arm.cc:1894: stemp.code_ = (dst.code_ / 2) + 1;
Please introduce a new variable here instead of updating the one you
have.  That's confusing.  Even better would be to add a couple of
methods to DwVfpRegister that return the low and high half equivalent
registers.

http://codereview.chromium.org/2813046/diff/1/5
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/2813046/diff/1/5#newcode440
src/arm/disasm-arm.cc:440: "#%d", imm);
This isn't very helpful.  Can we output the floating point number
instead of its encoding in the instruction or would that violate some
ARM disassembly standard?

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

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

Reply via email to