[arm64] Use exclusive instructions in exchange

[arm64] Use exclusive instructions in exchange

Implement load/store exclusive instructions and use in place of acquire/release
exclusive.

This is a correctness fix, as exchange operations are currently implemented
using acquire/release, and these can't be safely mixed with other exclusive
access-based atomic operations.

BUG=v8:6097

[martyn.capewell, 2017-03-15 18:09:37]

martyn.capewell@arm.com changed reviewers:
+ aseemgarg@chromium.org, bmeurer@chromium.org

[Benedikt Meurer, 2017-03-15 18:10:25]

bmeurer@chromium.org changed reviewers:
+ jarin@chromium.org

[Benedikt Meurer, 2017-03-15 18:10:26]

Adding jarin@.

[aseemgarg, 2017-03-15 21:43:32]

aseemgarg@chromium.org changed reviewers:
+ binji@chromium.org

[aseemgarg, 2017-03-15 21:43:33]

+binji
l-g-t-m. I am not owner for any of these files.

[binji, 2017-03-15 22:17:35]

lgtm

https://codereview.chromium.org/2754543006/diff/1/src/arm64/decoder-arm64-inl.h
File src/arm64/decoder-arm64-inl.h (right):

https://codereview.chromium.org/2754543006/diff/1/src/arm64/decoder-arm64-inl...
src/arm64/decoder-arm64-inl.h:216: if ((instr->Bit(28) == 0) && (instr->Bit(29)
== 0) && (instr->Bit(26) == 1)) {
Not necessary to check for instr->Bit(24) here?

[aseemgarg, 2017-03-16 00:14:54]

Couldn't we replace load and store to use ldaxr and stlxr instead? Seems like
acquire/release semantics are preferred method anyways..

[martyn.capewell, 2017-03-16 11:15:41]

Description was changed from

==========
[arm64] Use exclusive instructions in exchange

Implement load/store exclusive instructions and use in place of acquire/release
exclusive.

This is a correctness fix, as exchange operations are currently implemented
using acquire/release, and these can't be safely mixed with other exclusive
access-based atomic operations.

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

to

==========
[arm64] Use exclusive instructions in exchange

Implement load/store exclusive instructions and use in place of acquire/release
exclusive.

This is a correctness fix, as exchange operations are currently implemented
using acquire/release, and these can't be safely mixed with other exclusive
access-based atomic operations.

BUG=v8:6097
==========

[martyn.capewell, 2017-03-16 11:29:36]

Yes, I'd like to replace all of them with acquire/release, as that's preferred.

However, this was meant to be a quick correctness fix to avoid a subtle bug, as
the acquire/release replacements required more thought. Though it became less
quick when I found the exclusive instructions were missing.

[binji, 2017-03-16 22:06:17]

On 2017/03/16 11:29:36, martyn.capewell wrote:
> Yes, I'd like to replace all of them with acquire/release, as that's
preferred.
> 
> However, this was meant to be a quick correctness fix to avoid a subtle bug,
as
> the acquire/release replacements required more thought. Though it became less
> quick when I found the exclusive instructions were missing.

As Aseem mentioned, since the only uses of arm64 dmb are in atomic load/store,
perhaps it would be easier to omit these instructions and change those to use
ldaxr/stlxr instead? Or do you think we'll have a need for these instructions
elsewhere?

[martyn.capewell, 2017-03-17 14:07:48]

OK, I'll work on a patch to fix ATOMIC_LOAD/STORE instead - it'll just require a
bit more study.

[binji, 2017-03-17 18:58:12]

On 2017/03/17 14:07:48, martyn.capewell wrote:
> OK, I'll work on a patch to fix ATOMIC_LOAD/STORE instead - it'll just require
a
> bit more study.

Thanks, Martyn!

[martyn.capewell, 2017-03-20 17:19:07]

https://codereview.chromium.org/2760963002/