MIPS: Fix unaligned address of double.

MIPS: Fix unaligned address of double.

TEST=mjsunit/debug-evaluate-locals-optimized-double

BUG=

Committed: https://code.google.com/p/v8/source/detail?r=14828

[palfia, 2013-05-08 20:32:00]

Hi Danno, Jakob,

Please take a look.

[Paul Lind, 2013-05-08 22:09:46]

lgtm, but we need approval from higher authorities. Danno or Jakob, PTAL.

[Jakob Kummerow, 2013-05-10 10:06:28]

LGTM with nits.

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h
File src/deoptimizer.h (right):

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h#newcode40
src/deoptimizer.h:40: 
nit: two empty newlines between top-level things, please.

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h#newcode51
src/deoptimizer.h:51: c.u[0] = (*reinterpret_cast<uint32_t*>(p));
nit: don't need the outer parentheses.

[danno, 2013-05-10 10:15:55]

not quite l g t m to me yet....

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h
File src/deoptimizer.h (right):

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h#newcode44
src/deoptimizer.h:44: #ifndef V8_TARGET_ARCH_MIPS
I think it's better to use the platform independent V8_HOST_CAN_READ_UNALIGNED
rather than make this MIPS-specific

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h#newcode50
src/deoptimizer.h:50: } c;
And if you do that, you will need to make sure that endianess works correctly,
something like:
uint64_t data = *reinterpret_cast<uint64_t*>(p);
return BitCast<double>(data);

[palfia, 2013-05-17 17:24:32]

Hi Danno and Jakob,

Thanks for your comments!

Danno, we tried your solution with uint64 load + bitcast, but unfortunately it
also generates ldc1 instruction, which needs 8-byte aligned pointer on MIPS.

However, we're sure, that the union-based version will work on both big and
little endian archs.

I've uploaded a new patchset, that address all other comments. PTAL.

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h
File src/deoptimizer.h (right):

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h#newcode40
src/deoptimizer.h:40: 
On 2013/05/10 10:06:28, Jakob wrote:
> nit: two empty newlines between top-level things, please.

Done.

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h#newcode44
src/deoptimizer.h:44: #ifndef V8_TARGET_ARCH_MIPS
On 2013/05/10 10:15:55, danno wrote:
> I think it's better to use the platform independent V8_HOST_CAN_READ_UNALIGNED
> rather than make this MIPS-specific

Done.

https://codereview.chromium.org/14631016/diff/1/src/deoptimizer.h#newcode51
src/deoptimizer.h:51: c.u[0] = (*reinterpret_cast<uint32_t*>(p));
On 2013/05/10 10:06:28, Jakob wrote:
> nit: don't need the outer parentheses.

Done.

[kisg, 2013-05-27 13:07:31]

Hi Danno and Jakob,

Could you please take a look at the revised patch? MIPS is failing a lot of
tests on hardware without this patch.

Thanks,
Gergely

[Jakob Kummerow, 2013-05-27 14:24:14]

I've already given my approval ;-)

Danno's on vacation until the end of this week. Since you have addressed his
comment about using V8_HOST_CAN_READ_UNALIGNED instead of V8_TARGET_ARCH_MIPS,
and since I agree that the bit cast is endian agnostic (it's essentially a
memcpy with 32bit chunks), I'll just go ahead and give you the LGTM on his
behalf as well.

[palfia, 2013-05-27 17:12:57]

Committed manually as r14828 (presubmit successful).