MIPS: Fix unaligned read of bytecodes in interpreter.

MIPS: Fix unaligned read/write of bytecodes in interpreter.

On MIPS arch, all memory accesses (including halfword)
must be aligned to their native size or an alignment exception occurs.
The kernel will fix this up, but with performance penalty.

TEST=test-bytecode-generator/CallRuntime
BUG=

Committed: https://crrev.com/53c46f87dacaac7721cbc81e810b4606cb639ab5
Cr-Commit-Position: refs/heads/master@{#31845}

[paul.l..., 2015-10-30 17:40:32]

paul.lind@imgtec.com changed reviewers:
+ paul.lind@imgtec.com

[paul.l..., 2015-10-30 17:40:32]

In commit message you should change read to read/write.

And add a second line: On MIPS arch, all memory accesses (including halfword)
must be aligned to their native size or an alignment exception occurs. The
kernel will fix this up, but with performance penalty.

https://codereview.chromium.org/1423373004/diff/1/src/utils.h
File src/utils.h (right):

https://codereview.chromium.org/1423373004/diff/1/src/utils.h#newcode1739
src/utils.h:1739: #ifndef V8_TARGET_ARCH_MIPS
This needs to be done for MIPS64 also (unlike the ReadDoubleValue() which is
needed only on 32-bit MIPS.)

https://codereview.chromium.org/1423373004/diff/1/src/utils.h#newcode1746
src/utils.h:1746: uint8_t u[2];
the field names should be h and b (for half-word and byte). Here and below.

[akos.palfi.imgtec, 2015-10-30 21:24:05]

Description was changed from

==========
MIPS: Fix unaligned read of bytecodes in interpreter.

TEST=test-bytecode-generator/CallRuntime
BUG=
==========

to

==========
MIPS: Fix unaligned read/write of bytecodes in interpreter.

On MIPS arch, all memory accesses (including halfword)
must be aligned to their native size or an alignment exception occurs. The
kernel will fix this up, but with performance penalty.

TEST=test-bytecode-generator/CallRuntime
BUG=
==========

[akos.palfi.imgtec, 2015-10-30 21:24:20]

Description was changed from

==========
MIPS: Fix unaligned read/write of bytecodes in interpreter.

On MIPS arch, all memory accesses (including halfword)
must be aligned to their native size or an alignment exception occurs. The
kernel will fix this up, but with performance penalty.

TEST=test-bytecode-generator/CallRuntime
BUG=
==========

to

==========
MIPS: Fix unaligned read/write of bytecodes in interpreter.

On MIPS arch, all memory accesses (including halfword)
must be aligned to their native size or an alignment exception occurs.
The kernel will fix this up, but with performance penalty.

TEST=test-bytecode-generator/CallRuntime
BUG=
==========

[akos.palfi.imgtec, 2015-10-30 22:26:32]

Thanks, done!

https://codereview.chromium.org/1423373004/diff/1/src/utils.h
File src/utils.h (right):

https://codereview.chromium.org/1423373004/diff/1/src/utils.h#newcode1739
src/utils.h:1739: #ifndef V8_TARGET_ARCH_MIPS
On 2015/10/30 17:40:32, paul.l... wrote:
> This needs to be done for MIPS64 also (unlike the ReadDoubleValue() which is
> needed only on 32-bit MIPS.)

Done.

https://codereview.chromium.org/1423373004/diff/1/src/utils.h#newcode1746
src/utils.h:1746: uint8_t u[2];
On 2015/10/30 17:40:32, paul.l... wrote:
> the field names should be h and b (for half-word and byte). Here and below.

Done.

[paul.l..., 2015-11-05 17:36:51]

paul.lind@imgtec.com changed reviewers:
+ rmcilroy@chromium.org

[paul.l..., 2015-11-05 17:36:51]

LGTM. Ross, could you please take a look at this?

[rmcilroy, 2015-11-05 18:12:01]

https://codereview.chromium.org/1423373004/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecodes.cc (right):

https://codereview.chromium.org/1423373004/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecodes.cc:186: return ReadShortValue(bytes);
Could you call this ReadUnalignedUInt16 and get rid of ShortOperandFromBytes and
just replace all calls with the util method (and same for Write below).

[akos.palfi.imgtec, 2015-11-05 23:18:01]

I've addressed comments, PTAL.

https://codereview.chromium.org/1423373004/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecodes.cc (right):

https://codereview.chromium.org/1423373004/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecodes.cc:186: return ReadShortValue(bytes);
On 2015/11/05 18:12:01, rmcilroy wrote:
> Could you call this ReadUnalignedUInt16 and get rid of ShortOperandFromBytes
and
> just replace all calls with the util method (and same for Write below).

Done.

[akos.palfi.imgtec, 2015-11-05 23:18:18]

The CQ bit was checked by akos.palfi@imgtec.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-05 23:18:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1423373004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1423373004/40001

[rmcilroy, 2015-11-05 23:33:58]

Lgtm, thanks.

[commit-bot: I haz the power, 2015-11-05 23:37:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-05 23:37:36]

Dry run: This issue passed the CQ dry run.

[akos.palfi.imgtec, 2015-11-05 23:40:25]

The CQ bit was checked by akos.palfi@imgtec.com

[akos.palfi.imgtec, 2015-11-05 23:40:26]

The patchset sent to the CQ was uploaded after l-g-t-m from paul.lind@imgtec.com
Link to the patchset: https://codereview.chromium.org/1423373004/#ps40001
(title: "Addressed comments.")

[commit-bot: I haz the power, 2015-11-05 23:40:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1423373004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1423373004/40001

[commit-bot: I haz the power, 2015-11-05 23:42:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-05 23:43:08]

Patchset 3 (id:??) landed as
https://crrev.com/53c46f87dacaac7721cbc81e810b4606cb639ab5
Cr-Commit-Position: refs/heads/master@{#31845}