-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Description
Awhile back I started a discussion on the Rust Zulip about implementing the wasm threads proposal in Pulley. The conclusion at the time was that (a) this isn't possible in safe Rust, (b) it should be possible with inline assembly, and (c) we weren't going to pursue it for Pulley due to the portability goal of Pulley in the meantime. The consequence of this in Wasmtime is this line of code and a notable lack of anything atomic in Pulley's interpreter.
In the meantime the discussion on the Zulip issue continued on and resulted in publishing a crate named ecmascript_atomics
which is targeted at this exact use case. This crate looks to be drawn from SpiderMonkey's implementation of the same. There are two reasons I think we'll want to integrate this crate:
- The primary one is the resolution of shared memory: audit safety of pointer copies in the presence of shared memoriesย #4203. For example this memcpy, entirely unrelated to Pulley, is almost surely "technically not sound" given Rust's memory model. Functions like
RacySlice::copy_from_slice
, however, look tailor made for this use case. - Integrating this crate into Pulley would enable implementing the wasm threads proposal. Integrating this would require adding atomic instructions to Pulley and additionally updating all reads and writes to use this crate. Ordinary reads/writes would use
Unordered
and atomics would useSeqCst
.
The main barrier to adopting this crate is portability at this time. As of the time of this writing it has support for x86, x86_64, aarch64, and arm. This notably does not include platforms Cranelift supports such as s390x and riscv64 and does not include "theoretically all platforms Rust compiles to" for Pulley. Nevertheless I think this would be worthwhile to integrate into Wasmtime as so:
- Add a wrapper around this crate to
wasmtime-internal-math
in-tree (probably renaming this to maybewasmtime-internal-core
in the process). This would useecmascript_atomics
on supported platforms and some emulated fallback on unsupported platforms (e.g. the unsoundstd::ptr::copy
for a memcpy-based move). - Update Pulley's support of the wasm threads proposal to be architecture-specific. For example it would support the proposal on platforms that
ecmascript_atomics
supports, but it would continue to not have support on other platforms (e.g. Pulley powerpc would not have support for the atomics proposal) - Update Pulley's reads/writes of today to use
Unordered
atomic reads/writes - Add atomic instructions, probably in the "extended" opcode namespace for Pulley, and equivalent lowerings to Cranelift to support generating atomics.
With all that in place we could still benefit from this crate, despite the lack of portability. While it probably isn't too hard to add s390x/riscv64 the lack of portability in this approach is pretty fundamental. I don't think that should block us from integrating it at all, however, but conditionally disabling threads in Pulley would be a reasonable consequence that reflects an accurate depiction of support.