Allow atomic operations on unshared memories #147
Conversation
As discussed in the CG on November 12, 2019 and in WebAssembly#144. Specifically, all atomic accesses are now allowed to validate and execute normally on unshared memories and wait operations trap when used with unshared memories. The PR updates Overview.md and makes a best-effort attempt at updating the spec, making no changes when there is already a TODO for updating spec text. It also adds new TODO comments to the reference interpreter where changes will have to be made.
There was a problem hiding this comment.
Technically, the changes look good to me, modulo comment. Personally, I'm fine with these semantics, but did the CG actually declare consensus on it? The Nov meeting was a bit inconclusive. Now that you re-clarified the options on the issue and everyone had enough time to dwell on it, it may be appropriate to bring it back to the CG to get official sign-off via a poll.
| @@ -440,8 +440,6 @@ Atomic Memory Instructions | |||
|
|
|||
| * Let :math:`\limits~\share` be the :ref:`memory type <syntax-memtype>` :math:`C.\CMEMS[0]`. | |||
|
|
|||
| * The sharedness :math:`\share` must be |MSHARED|. | |||
There was a problem hiding this comment.
You'll also need to adjust the respective premise in the formal rule below to keep it in sync, e.g., by changing C.\CMEMS[0] = \limits~\MSHARED to C.\CMEMS[0] = \memtype (here and elsewhere).
There was a problem hiding this comment.
From previous discussions, my conclusions were
- Allowing plain atomic operations on non-shared memory was uncontroversial
- There are still open questions about Atomics.wait semantics
- It's unclear to me from the notes/discussions whether Atomics.notify always returning 0 was already agreed upon
The semantics as laid out in this PR lgtm, +1 to @rossberg's suggestion of adding explicit polls in an upcoming meeting.
|
Sounds good, I'll add a vote to the agenda for the next meeting 👍 |
The proposed semantics can be found in WebAssembly/threads#147
The proposed semantics can be found in WebAssembly/threads#147
|
@rossberg I just want to confirm that the latest changes are what you had in mind. |
|
@rossberg ping! |
|
I'll merge this now, but of course I am happy to fix any mistakes or holes in follow-up PRs. |
Adjust atomics.wait, atomics.notify semantics for when they are used with non-shared Wasm memory to mirror the spec change introduced in: WebAssembly/threads#147. This does not need to be gated by the flag here, as this will only decode if the flag is enabled. Bug: v8:9921 Change-Id: I7f2e018fed6bd131ad4c386def1e838626c28a4d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2256863 Reviewed-by: Ben Smith <binji@chromium.org> Commit-Queue: Deepti Gandluri <gdeepti@chromium.org> Cr-Commit-Position: refs/heads/master@{#68468}
As discussed in the CG on November 12, 2019 and in #144. Specifically,
all atomic accesses are now allowed to validate and execute normally
on unshared memories and wait operations trap when used with unshared
memories.
The PR updates Overview.md and makes a best-effort
attempt at updating the spec, making no changes when there is
already a TODO for updating spec text. It also adds new TODO comments
to the reference interpreter where changes will have to be made.