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) {
On 2010/07/06 12:36:17, Erik Corry wrote:
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.

Done.

http://codereview.chromium.org/2813046/diff/1/2#newcode1847
src/arm/assembler-arm.cc:1847: if (((hi ^ (hi << 1)) & (1 << 30)) == 0)
{
On 2010/07/06 12:36:17, Erik Corry wrote:
It would be more consistent to use 0x40000000 here instead of 1 << 30.

Done.

http://codereview.chromium.org/2813046/diff/1/2#newcode1886
src/arm/assembler-arm.cc:1886: // registers of D register dst.
You would save an instruction and potentially a few interlocks. The
current version is probably good enough for now and could be optimised
later if necessary (maybe using literal pool).

http://codereview.chromium.org/2813046/diff/1/2#newcode1894
src/arm/assembler-arm.cc:1894: stemp.code_ = (dst.code_ / 2) + 1;
On 2010/07/06 12:36:17, Erik Corry wrote:
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.

Done.

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 is the way gcc disasemble the instruction but it is not a
requirement. Updated to provide the doube value.

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

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

Reply via email to