1561747 - Planning to use bulk memory in pthreads toolchain

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, task, P2)

Description

[Thomas Lively]

I'm planning to update LLVM soon to make the atomics feature imply that bulk memory is enabled. This will allow LLD to emit passive data segments when producing multithreaded modules, removing the current requirement for data to be initialized from an external file.

This change will affect both the Emscripten and Rust toolchains, and will mean that toggling FireFox's javascript.options.shared_memory option will be insufficient for testing threads on FireFox, since that does not enable bulk memory support. Please let me know if you'd like me to delay making this change for any reason.

Comment 1

[Lars T Hansen [:lth]]

(In reply to Thomas Lively from comment #0)

I'm planning to update LLVM soon to make the atomics feature imply that bulk memory is enabled. This will allow LLD to emit passive data segments when producing multithreaded modules, removing the current requirement for data to be initialized from an external file.

This change will affect both the Emscripten and Rust toolchains, and will mean that toggling FireFox's javascript.options.shared_memory option will be insufficient for testing threads on FireFox, since that does not enable bulk memory support. Please let me know if you'd like me to delay making this change for any reason.

Thanks for the warning. This seems like a sensible dependency. We can plausibly make a change to implement that in FF69 (beta July 8, ships September 3). What time frame did you have in mind for the toolchain changes?

Comment 2

[Thomas Lively]

I could get the changes out as early as next week, but it's not urgent so I'm happy to delay. Would the disruption be manageable if I landed it soon after July 8? There will be no disruption to the Rust toolchain since they actively update their LLVM tools.

Comment 3

[Lars T Hansen [:lth]]

(In reply to Thomas Lively from comment #2)

I could get the changes out as early as next week, but it's not urgent so I'm happy to delay. Would the disruption be manageable if I landed it soon after July 8?

Yes, that would be fine. I would want to land a patch for this before we branch for beta.

Comment 4

[Lars T Hansen [:lth]]

Attachment: Bug 1561747 - Enable bulk memory on non-Nightly if shared memory is enabled. r=jseward

Toolchains that support wasm threads will soon start relying on bulk
memory being available (as Chrome more or less ships both proposals
already). Currently we have bulk memory on Nightly under an ifdef,
but shared memory in all channels under a flag; as a result, we are
not compatible on non-Nightly with code that will soon come out of
these toolchains.

This patch keeps bulk memory enabled on Nightly but additionally
enables it on other channels if shared memory has been enabled by the
flag.

Mostly this is straightforward: we always enable decoding of bulk
memory operations, but will subsequently fail compilation for these
ops if bulk memory has not been enabled by the ifdef and shared memory
has not been enabled by the flag.

The only tricky part is really that the bulk memory proposal changes
the bounds checking for memory.init and table.init from eager to lazy,
so here the combinations of flags selects the bounds checking
behavior.

Once we ship bulk memory all of this complexity disappears.

Comment 5

[Lars T Hansen [:lth]]

Thomas, do you know anything wrt toolchains using the new atomic.fence instruction? For this it would be good to hold off longer, the earliest we can hope to have this in Firefox is FF70.

Comment 6

[Thomas Lively]

We will probably start emitting the fence instruction behind a flag as soon as they are specified and implemented in V8. It will be a while before we want to emit them by default though, since we will have to wait for the V8 implementation to ride the release train all the way to stable to avoid breaking our current users. If fences were implemented today, they would not hit stable until mid september (Chrome 77).

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ad86a1ac335
Enable bulk memory on non-Nightly if shared memory is enabled. r=jseward

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/7ad86a1ac335