Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

atomic.fence validation and binary encoding #140

Open
conrad-watt opened this issue Jun 16, 2019 · 10 comments
Open

atomic.fence validation and binary encoding #140

conrad-watt opened this issue Jun 16, 2019 · 10 comments

Comments

@conrad-watt
Copy link
Collaborator

After the last CG, we agreed to add atomic.fence.

Validation-wise, the instruction has type ([] -> []). Unlike other atomic.* instructions, it is not a validation error for the instruction to occur in a module with a non-shared memory. This is because the instruction fences all memories in the store, not just that of the current module.

Encoding-wise, I hope someone with better intuition about the binary format can suggest something.

@tlively
Copy link
Member

tlively commented Jun 16, 2019

I don't quite understand why fence is allowed to be used with non-shared memories but other atomic instructions are not. Can you elaborate on that a bit?

@sunfishcode
Copy link
Member

The C, C++ and Rust fence constructs have an "order" parameter. The current wasm threads proposal only supports SC, but other orderings may be added in the future. Would it make sense to include an immediate field in the fence encoding, to allow for other orderings to be specified in the future?

@rossberg
Copy link
Member

@tlively, a fence is not tied to any memory, but commits all accesses (which may involve multiple memories or even other state than memories in the future) that have occurred in the thread (which may include accesses perfomed in other modules on the same thread). So it isn't meaningful to require a specific memory to be present locally.

@conrad-watt
Copy link
Collaborator Author

The C, C++ and Rust fence constructs have an "order" parameter. The current wasm threads proposal only supports SC, but other orderings may be added in the future. Would it make sense to include an immediate field in the fence encoding, to allow for other orderings to be specified in the future?

I think this makes sense. The same point holds for other atomic operations - any order encoding for fence should try to be consistent with what they are/will be doing. I can't see anything explicit, but is the memarg part of the existing atomics' binary encoding intended to be used this way in the future? (https://github.com/WebAssembly/threads/blob/master/proposals/threads/Overview.md#spec-changes)

@binji
Copy link
Member

binji commented Jun 17, 2019

I can't see anything explicit, but is the memarg part of the existing atomics' binary encoding intended to be used this way in the future?

Yes, I assumed we would use the memarg immediate for other orderings in the future. We may want to use a single 0 byte for atomic.fence instead of memarg, since the alignment/offset doesn't make sense here, though

@tlively
Copy link
Member

tlively commented Jun 17, 2019

@rossberg, I buy that a fence commits all accesses, even on a single thread, and I think that is a good argument for allowing it in modules without shared memories. But by extension of the same reasoning shouldn't all other atomic operations be allowed in these modules as well? They could still have their ordering guarantees on a single thread, they just wouldn't be as useful.

Also, I still don't quite see how a fence on a single thread would be useful. Can you give me an example where a single-threaded program could be observably different if it included a fence?

@rossberg
Copy link
Member

rossberg commented Jun 17, 2019

@tlively, all other atomic operators operate on a specific memory -- via the memory index 0 that is currently implicit in the instruction but may be explicit once we allow multiple memories. So memory 0 needs to exist in the module.

A fence doesn't have any effect in a single-threaded program. However, my point was that it still has an effect on everything in the same thread in a multi-threaded program. Consider:

(module $A
  (memory (import "" "mem") 1 shared)
  (func (export "rd") (param $i i32) (result i32)
    (i32.load8_u (local.get $i))
  )
  (func (export "wr") (param $i i32)
    (i32.store8 (i32.const 0xff) (local.get $i))
  )
)

(module $B
  (func $rd (import "A" "rd") (param i32))
  (func $wr (import "A" "wr") (param i32))
  (func $run (param $i i32) (result i32)
    (call $wr (local.get $i))
    (atomic.fence)
    (call $rd (i32.eqz (local.get $i)))
  )
)

Imagine you instantiate both $A and $B twice in different threads, but sharing the same zeroed memory. And in one you invoke (call $run (i32.const 0)) and in the other (call $run (i32.const 1)). Despite not seeing any memory in scope, the fence guarantees that they cannot both return 0 (cf. the running example in Conrad's slides).

@conrad-watt
Copy link
Collaborator Author

conrad-watt commented Jun 17, 2019

Just to make @rossberg's example completely tight, the load and store in module $A should be release/acquire atomics.

I think the disconnect here is that not declaring a shared memory doesn't imply that the module is only going to be used in a single-threaded context - that just happens to be (kind of) true for the instructions we've previously defined.

@conrad-watt
Copy link
Collaborator Author

conrad-watt commented Jun 17, 2019

I can't see anything explicit, but is the memarg part of the existing atomics' binary encoding intended to be used this way in the future?

Yes, I assumed we would use the memarg immediate for other orderings in the future. We may want to use a single 0 byte for atomic.fence instead of memarg, since the alignment/offset doesn't make sense here, though

@binji would a reasonable encoding be something like

memargf ::= 0x00

atomic.fence ::= 0xFE 0x0F m:memargf

to keep the convention that (only) atomic memory accesses have a non-zero first nibble, but thematically positioning fence as close as possible to them (and leaving no gap)?

@binji
Copy link
Member

binji commented Jun 17, 2019

@conrad-watt that looks right to me, though I think we'd just inline the memargf (as with call_indirect in this definition). As for the opcode to choose, I don't have a strong opinion. I was thinking it would be 0x03, but not for any good reason, really.

conrad-watt added a commit that referenced this issue Jun 19, 2019
conrad-watt added a commit that referenced this issue Jun 19, 2019
binji pushed a commit that referenced this issue Jul 8, 2019
pull bot pushed a commit to Richienb/v8 that referenced this issue Jul 19, 2019
This adds decoding and compilation of the "atomic.fence" operator, which
is intended to preserve the synchronization guarantees of higher-level
languages.

Unlike other atomic operators, it does not target a particular linear
memory. It may occur in modules which declare no memory, or a non-shared
memory, without causing a validation error.

See proposal: WebAssembly/threads#141
See discussion: WebAssembly/threads#140

R=clemensh@chromium.org
TEST=cctest/test-run-wasm-atomics/RunWasmXXX_AtomicFence
BUG=v8:9452

Change-Id: Ibf7e46227f7edfe5c81c097cfc15924c59614067
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1701856
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62821}
pull bot pushed a commit to Richienb/v8 that referenced this issue Jul 22, 2019
port 4ca8b4d https://crrev.com/c/1701856

Original Commit Message:
    This adds decoding and compilation of the "atomic.fence" operator, which
    is intended to preserve the synchronization guarantees of higher-level
    languages.

    Unlike other atomic operators, it does not target a particular linear
    memory. It may occur in modules which declare no memory, or a non-shared
    memory, without causing a validation error.

    See proposal: WebAssembly/threads#141
    See discussion: WebAssembly/threads#140

Change-Id: Ia60d58a6bf58e8236591d515d30184418cee47c5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1710337
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Yu Yin <xwafish@gmail.com>
Cr-Commit-Position: refs/heads/master@{#62843}
pull bot pushed a commit to Richienb/v8 that referenced this issue Jul 22, 2019
Port 4ca8b4d

Original Commit Message:

    This adds decoding and compilation of the "atomic.fence" operator, which
    is intended to preserve the synchronization guarantees of higher-level
    languages.

    Unlike other atomic operators, it does not target a particular linear
    memory. It may occur in modules which declare no memory, or a non-shared
    memory, without causing a validation error.

    See proposal: WebAssembly/threads#141
    See discussion: WebAssembly/threads#140

R=mstarzinger@chromium.org, joransiu@ca.ibm.com, jyan@ca.ibm.com, michael_dawson@ca.ibm.com
BUG=v8:9452
LOG=N

Change-Id: Ib8ad24e65154d7555a47e537f81110be47f4d4de
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1710620
Commit-Queue: Milad Farazmand <miladfar@ca.ibm.com>
Reviewed-by: Junliang Yan <jyan@ca.ibm.com>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62850}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 29, 2019
This commit implements the 'atomic.fence' Wasm instruction.

Issue: WebAssembly/threads#140
Overview: WebAssembly/threads#141

The instruction is encoded as, 0xFE 0x03, with a reserved byte trailing for a future
memory order immediate. The instruction is implemented by emitting a memoryBarrier
through the macro assembler.

Differential Revision: https://phabricator.services.mozilla.com/D39264

--HG--
extra : moz-landing-system : lando
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jul 30, 2019
This commit implements the 'atomic.fence' Wasm instruction.

Issue: WebAssembly/threads#140
Overview: WebAssembly/threads#141

The instruction is encoded as, 0xFE 0x03, with a reserved byte trailing for a future
memory order immediate. The instruction is implemented by emitting a memoryBarrier
through the macro assembler.

Differential Revision: https://phabricator.services.mozilla.com/D39264
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
This commit implements the 'atomic.fence' Wasm instruction.

Issue: WebAssembly/threads#140
Overview: WebAssembly/threads#141

The instruction is encoded as, 0xFE 0x03, with a reserved byte trailing for a future
memory order immediate. The instruction is implemented by emitting a memoryBarrier
through the macro assembler.

Differential Revision: https://phabricator.services.mozilla.com/D39264

UltraBlame original commit: d32c27f13f21b0927cbfb7ce6f8509c0dea6bf09
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
This commit implements the 'atomic.fence' Wasm instruction.

Issue: WebAssembly/threads#140
Overview: WebAssembly/threads#141

The instruction is encoded as, 0xFE 0x03, with a reserved byte trailing for a future
memory order immediate. The instruction is implemented by emitting a memoryBarrier
through the macro assembler.

Differential Revision: https://phabricator.services.mozilla.com/D39264

UltraBlame original commit: d32c27f13f21b0927cbfb7ce6f8509c0dea6bf09
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
This commit implements the 'atomic.fence' Wasm instruction.

Issue: WebAssembly/threads#140
Overview: WebAssembly/threads#141

The instruction is encoded as, 0xFE 0x03, with a reserved byte trailing for a future
memory order immediate. The instruction is implemented by emitting a memoryBarrier
through the macro assembler.

Differential Revision: https://phabricator.services.mozilla.com/D39264

UltraBlame original commit: d32c27f13f21b0927cbfb7ce6f8509c0dea6bf09
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jun 1, 2024
This commit implements the 'atomic.fence' Wasm instruction.

Issue: WebAssembly/threads#140
Overview: WebAssembly/threads#141

The instruction is encoded as, 0xFE 0x03, with a reserved byte trailing for a future
memory order immediate. The instruction is implemented by emitting a memoryBarrier
through the macro assembler.

Differential Revision: https://phabricator.services.mozilla.com/D39264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants