Implement UnaligedLoad and UnaligedStore turbofan operators.

Implement UnaligedLoad and UnaligedStore turbofan operators.

Implement UnalignedLoad and UnalignedStore optional
turbofan operators and use them in WasmCompiler for unaligned
memory access.

BUG=

Committed: https://crrev.com/580fdf3c05872b1937a136f2f44b39897ecc0972
Cr-Commit-Position: refs/heads/master@{#37988}

[ivica.bogosavljevic, 2016-07-05 09:07:52]

Description was changed from

==========
Implement UnaligedLoad and UnaligedStore turbofan operators

Implement UnalignedLoad and UnalignedStore optional
turbofan operators and use them in WasmCompiler for unaligned
memory access.

BUG=
==========

to

==========
Implement UnaligedLoad and UnaligedStore turbofan operators.

Implement UnalignedLoad and UnalignedStore optional
turbofan operators and use them in WasmCompiler for unaligned
memory access.

BUG=
==========

[ivica.bogosavljevic, 2016-07-05 09:07:52]

ivica.bogosavljevic@imgtec.com changed reviewers:
+ titzer@chromium.org

[ivica.bogosavljevic, 2016-07-05 09:09:06]

PTAL

ARM people, please implement UnalignedLoad/UnaliginedStore operators if needed.

[martyn.capewell, 2016-07-05 13:13:59]

On 2016/07/05 09:09:06, ivica.bogosavljevic wrote:
> PTAL
> 
> ARM people, please implement UnalignedLoad/UnaliginedStore operators if
needed.

OK, I'll take a look at this.

[martyn.capewell, 2016-07-07 14:55:24]

I have an implementation of the ARM support needed here. As "git cl checkout"
isn't working for me, I suggest that I put the patch up for review after this is
committed, so I can rebase on top of master+this patch.

[martyn.capewell, 2016-07-07 16:35:46]

martyn.capewell@arm.com changed reviewers:
+ martyn.capewell@arm.com

[martyn.capewell, 2016-07-07 16:35:46]

https://codereview.chromium.org/2122853002/diff/1/src/compiler/mips/instructi...
File src/compiler/mips/instruction-selector-mips.cc (right):

https://codereview.chromium.org/2122853002/diff/1/src/compiler/mips/instructi...
src/compiler/mips/instruction-selector-mips.cc:1004: case
MachineRepresentation::kWord8:
Can 8-bit accesses ever be unaligned? If not, these should be UNREACHABLE().

[ivica.bogosavljevic, 2016-07-08 13:58:43]

https://codereview.chromium.org/2122853002/diff/1/src/compiler/mips/instructi...
File src/compiler/mips/instruction-selector-mips.cc (right):

https://codereview.chromium.org/2122853002/diff/1/src/compiler/mips/instructi...
src/compiler/mips/instruction-selector-mips.cc:1004: case
MachineRepresentation::kWord8:
On 2016/07/07 16:35:46, martyn.capewell wrote:
> Can 8-bit accesses ever be unaligned? If not, these should be UNREACHABLE().

Acknowledged.

[titzer, 2016-07-11 08:40:00]

https://codereview.chromium.org/2122853002/diff/1/src/compiler/machine-operat...
File src/compiler/machine-operator.h (right):

https://codereview.chromium.org/2122853002/diff/1/src/compiler/machine-operat...
src/compiler/machine-operator.h:133: kUnalignedLoad = 1u << 24,
Isn't this superfluous with the AlignmentRequirements?

https://codereview.chromium.org/2122853002/diff/1/src/compiler/wasm-compiler.cc
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2122853002/diff/1/src/compiler/wasm-compiler....
src/compiler/wasm-compiler.cc:2903: load = BuildUnalignedLoad(type, memtype,
index, offset, alignment);
I thought this routine would go away. Is it the case that the only other
platform that doesn't support unaligned accesses is arm, and the instruction
selector/code generator must implement this?

Otherwise, we end up with the complexity of both solutions :(

[ivica.bogosavljevic, 2016-07-11 09:34:43]

https://codereview.chromium.org/2122853002/diff/1/src/compiler/machine-operat...
File src/compiler/machine-operator.h (right):

https://codereview.chromium.org/2122853002/diff/1/src/compiler/machine-operat...
src/compiler/machine-operator.h:133: kUnalignedLoad = 1u << 24,
On 2016/07/11 08:40:00, titzer wrote:
> Isn't this superfluous with the AlignmentRequirements?

Acknowledged.

https://codereview.chromium.org/2122853002/diff/1/src/compiler/wasm-compiler.cc
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2122853002/diff/1/src/compiler/wasm-compiler....
src/compiler/wasm-compiler.cc:2903: load = BuildUnalignedLoad(type, memtype,
index, offset, alignment);
On 2016/07/11 08:40:00, titzer wrote:
> I thought this routine would go away. Is it the case that the only other
> platform that doesn't support unaligned accesses is arm, and the instruction
> selector/code generator must implement this?
> 
> Otherwise, we end up with the complexity of both solutions :(

Agreed. In that case we'll make UnalignedLoad/UnalignedStore mandatory operator,
but it will never be called for those architectures that support unaligned
access. And the BuildUnalignedLoad goes away.

[ivica.bogosavljevic, 2016-07-12 09:54:56]

PTAL

[titzer, 2016-07-14 15:32:02]

On 2016/07/12 09:54:56, ivica.bogosavljevic wrote:
> PTAL

Looking good.

Can you add some cctests for the new operators in test-run-load-store.cc?

[ivica.bogosavljevic, 2016-07-21 13:41:38]

Implemented tests for UnalignedLoad and UnalignedStore turbofan operators, as
well as int64 lowering tests

PTAL

[titzer, 2016-07-21 13:54:37]

lgtm modulo last naming nit.

https://codereview.chromium.org/2122853002/diff/40001/test/cctest/compiler/te...
File test/cctest/compiler/test-run-load-store.cc (right):

https://codereview.chromium.org/2122853002/diff/40001/test/cctest/compiler/te...
test/cctest/compiler/test-run-load-store.cc:50: enum LoadStoreKind {
Can you simply name this TestAlignment { kAligned, kUnaligned } ?

[ivica.bogosavljevic, 2016-07-22 20:29:28]

The CQ bit was checked by ivica.bogosavljevic@imgtec.com

[ivica.bogosavljevic, 2016-07-22 20:29:28]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/2122853002/#ps60001
(title: "Nits. Fixes some errors")

[commit-bot: I haz the power, 2016-07-22 20:29:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 20:55:06]

Description was changed from

==========
Implement UnaligedLoad and UnaligedStore turbofan operators.

Implement UnalignedLoad and UnalignedStore optional
turbofan operators and use them in WasmCompiler for unaligned
memory access.

BUG=
==========

to

==========
Implement UnaligedLoad and UnaligedStore turbofan operators.

Implement UnalignedLoad and UnalignedStore optional
turbofan operators and use them in WasmCompiler for unaligned
memory access.

BUG=
==========

[commit-bot: I haz the power, 2016-07-22 20:55:08]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-22 20:56:38]

Description was changed from

==========
Implement UnaligedLoad and UnaligedStore turbofan operators.

Implement UnalignedLoad and UnalignedStore optional
turbofan operators and use them in WasmCompiler for unaligned
memory access.

BUG=
==========

to

==========
Implement UnaligedLoad and UnaligedStore turbofan operators.

Implement UnalignedLoad and UnalignedStore optional
turbofan operators and use them in WasmCompiler for unaligned
memory access.

BUG=

Committed: https://crrev.com/580fdf3c05872b1937a136f2f44b39897ecc0972
Cr-Commit-Position: refs/heads/master@{#37988}
==========

[commit-bot: I haz the power, 2016-07-22 20:56:39]

Patchset 4 (id:??) landed as
https://crrev.com/580fdf3c05872b1937a136f2f44b39897ecc0972
Cr-Commit-Position: refs/heads/master@{#37988}

[Michael Achenbach, 2016-07-25 09:09:24]

This crashes on real arm hardware (chrubuntu chromebooks):
https://build.chromium.org/p/client.v8.ports/builders/V8%20Arm/builds/669
https://build.chromium.org/p/client.v8.ports/builders/V8%20Arm%20-%20debug/bu...

Could you skip tests or revert to get the build green?

[ivica.bogosavljevic, 2016-07-25 09:14:15]

On 2016/07/25 09:09:24, Michael Achenbach (slow) wrote:
> This crashes on real arm hardware (chrubuntu chromebooks):
> https://build.chromium.org/p/client.v8.ports/builders/V8%20Arm/builds/669
>
https://build.chromium.org/p/client.v8.ports/builders/V8%20Arm%20-%20debug/bu...
> 
> Could you skip tests or revert to get the build green?

I can add them to the skip list, and when ARM implements unalignedLoad and
UnalignedStore for FLOAT and DOUBLE,  you can remove them.
Please don't revert this change, it was a large effort to get it through.

[ivica.bogosavljevic, 2016-07-25 09:22:07]

ivica.bogosavljevic@imgtec.com changed reviewers:
+ machenbach@chromium.org

[ivica.bogosavljevic, 2016-07-25 09:22:07]

PTAL