1389471 - Preliminaries for Wasm threads

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Lars T Hansen [:lth]]

Changes to moz.build to define a suitable preprocessor macro (ENABLE_WASM_THREAD_OPS) and a TestingFunction to test for presence of threads.

This is small potatoes but I'm breaking this out as a separate bug since several of the other changes depend on it, and it can land independently really.

Comment 1

[Lars T Hansen [:lth]]

Attachment: bug1389471-wasm-thread-preliminaries.patch

See comment 0.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8896246 [details] [diff] [review]
bug1389471-wasm-thread-preliminaries.patch

Review of attachment 8896246 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, LGTM.

::: js/src/builtin/TestingFunctions.cpp
@@ +533,5 @@
> +WasmThreadsSupported(JSContext* cx, unsigned argc, Value* vp)
> +{
> +    CallArgs args = CallArgsFromVp(argc, vp);
> +#ifdef ENABLE_WASM_THREAD_OPS
> +    bool isSupported = true;

This is temporary that it's always true, right? e.g. for mips/arm64/none codegen, there won't be any atomics instructions any time soon, right?

(And I assume that probably other preconditions will apply here, other than just the ifdef)

::: js/src/moz.build
@@ +643,5 @@
> +# In-flight WebAssembly atomic operations, sign extension operations,
> +# and shared memory objects proposal:
> +# https://github.com/WebAssembly/threads
> +if CONFIG['NIGHTLY_BUILD']:
> +    DEFINES['ENABLE_WASM_THREAD_OPS'] = True

(might be ok to group with the above group of if[NIGHTLY], since it seems to be the common case in this file; although wasm thread ops are muuuuuch more likely to land in release sooner than binary data/simd, so it makes sense to start a new `if` group; I'll leave it to your preference)

Comment 3

[Lars T Hansen [:lth]]

Excellent point about the always-trueness of the return value...  Hm.  The right fix is perhaps to engage in some ifdeffery on the moz.build level (or similar) so that for ARM64, MIPS, et al we don't risk anything... on the other hand, we MOZ_CRASH for JS atomics on ARM64 and MIPS and could probably do that here too...  Would certainly be cleaner.  Certainly the wasm atomics implementations will crash on those platforms.

At a minimum this should not return true if wasm support is disabled, oy!  Will fix.

Re the new CONFIG block, it was mainly to have a nice point for the comment.  And as you say, this stuff is likely to land soon :)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3777a6f18c8a
Build config + testing predicate for wasm thread functionality, v2. r=bbouvier

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3777a6f18c8a