-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Don't enable shared memory by default with Wasm atomics #147225
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
Conversation
Some changes occurred in compiler/rustc_codegen_ssa |
This comment has been minimized.
This comment has been minimized.
6f67cc5
to
5b809b3
Compare
Should the |
AFAIK it already does: rust/compiler/rustc_target/src/spec/targets/wasm32_wasip1_threads.rs Lines 20 to 32 in 1e1a394
|
That is missing the |
These commits modify compiler targets. |
Its interesting that so many flags were overlapping then. |
@bors: r+ |
… r=alexcrichton Don't enable shared memory by default with Wasm atomics This prepares us for a future where LLVM eventually stabilizes the atomics target feature, in which case we don't want to inflate atomics with threads. Otherwise users would be stuck with shared memory even when they don't want it/need it. ### Context Currently the atomics target features is unstable and can't be used without re-building Std with it (`-Zbuild-std`). Enabling the atomics target feature automatically enables shared memory. Shared memory is required to actually allow multi-threading. However, shared memory comes with a performance overhead when atomic instructions aren't able to be lowered to regular memory access instructions or when interacting with certain Web APIs. So it is very undesirable to enable shared memory by default for the majority of users. While it is possible to use atomics without shared memory, the question remains what use-case this scenario has. The only one I can think of would involve multiple memories, where the main memory remains un-shared but a second shared memory exists. While Rust doesn't support multiple memories, it might be possible with inline assembly (rust-lang#136382). So alternatively, we might consider *not* enabling atomics by default even when LLVM does. In which case everything would remain the same. --- This will break current Web multi-threading users. To address this they can add the following `RUSTFLAGS`: ``` -Clink-args=--shared-memory,--max-memory=1073741824,--import-memory,--export=__wasm_init_tls,--export=__tls_size,--export=__tls_align,--export=__tls_base ``` We could add a new experimental flag that enables the right linker arguments for users, but I feel that's not in Rusts scope. Or like suggested before: a Rust-only `threads` target feature. Addresses rust-lang#77839. r? `@alexcrichton`
Rollup of 10 pull requests Successful merges: - #146281 (Support `#[rustc_align_static]` inside `thread_local!`) - #146535 (mbe: Implement `unsafe` attribute rules) - #146585 (indexing: reword help) - #147004 (Tweak handling of "struct like start" where a struct isn't supported) - #147221 (Forbid `//@ compile-flags: -Cincremental=` in tests) - #147225 (Don't enable shared memory by default with Wasm atomics) - #147227 (implement `Box::take`) - #147231 (Extending `#[rustc_force_inline]` to be applicable to inherent methods) - #147233 (Initialize llvm submodule if not already the case to run citool) - #147236 (Update books) r? `@ghost` `@rustbot` modify labels: rollup
… r=alexcrichton Don't enable shared memory by default with Wasm atomics This prepares us for a future where LLVM eventually stabilizes the atomics target feature, in which case we don't want to inflate atomics with threads. Otherwise users would be stuck with shared memory even when they don't want it/need it. ### Context Currently the atomics target features is unstable and can't be used without re-building Std with it (`-Zbuild-std`). Enabling the atomics target feature automatically enables shared memory. Shared memory is required to actually allow multi-threading. However, shared memory comes with a performance overhead when atomic instructions aren't able to be lowered to regular memory access instructions or when interacting with certain Web APIs. So it is very undesirable to enable shared memory by default for the majority of users. While it is possible to use atomics without shared memory, the question remains what use-case this scenario has. The only one I can think of would involve multiple memories, where the main memory remains un-shared but a second shared memory exists. While Rust doesn't support multiple memories, it might be possible with inline assembly (rust-lang#136382). So alternatively, we might consider *not* enabling atomics by default even when LLVM does. In which case everything would remain the same. --- This will break current Web multi-threading users. To address this they can add the following `RUSTFLAGS`: ``` -Clink-args=--shared-memory,--max-memory=1073741824,--import-memory,--export=__wasm_init_tls,--export=__tls_size,--export=__tls_align,--export=__tls_base ``` We could add a new experimental flag that enables the right linker arguments for users, but I feel that's not in Rusts scope. Or like suggested before: a Rust-only `threads` target feature. Addresses rust-lang#77839. r? ``@alexcrichton``
… r=alexcrichton Don't enable shared memory by default with Wasm atomics This prepares us for a future where LLVM eventually stabilizes the atomics target feature, in which case we don't want to inflate atomics with threads. Otherwise users would be stuck with shared memory even when they don't want it/need it. ### Context Currently the atomics target features is unstable and can't be used without re-building Std with it (`-Zbuild-std`). Enabling the atomics target feature automatically enables shared memory. Shared memory is required to actually allow multi-threading. However, shared memory comes with a performance overhead when atomic instructions aren't able to be lowered to regular memory access instructions or when interacting with certain Web APIs. So it is very undesirable to enable shared memory by default for the majority of users. While it is possible to use atomics without shared memory, the question remains what use-case this scenario has. The only one I can think of would involve multiple memories, where the main memory remains un-shared but a second shared memory exists. While Rust doesn't support multiple memories, it might be possible with inline assembly (rust-lang#136382). So alternatively, we might consider *not* enabling atomics by default even when LLVM does. In which case everything would remain the same. --- This will break current Web multi-threading users. To address this they can add the following `RUSTFLAGS`: ``` -Clink-args=--shared-memory,--max-memory=1073741824,--import-memory,--export=__wasm_init_tls,--export=__tls_size,--export=__tls_align,--export=__tls_base ``` We could add a new experimental flag that enables the right linker arguments for users, but I feel that's not in Rusts scope. Or like suggested before: a Rust-only `threads` target feature. Addresses rust-lang#77839. r? `@alexcrichton`
Rollup of 9 pull requests Successful merges: - #146281 (Support `#[rustc_align_static]` inside `thread_local!`) - #146535 (mbe: Implement `unsafe` attribute rules) - #146585 (indexing: reword help) - #147004 (Tweak handling of "struct like start" where a struct isn't supported) - #147221 (Forbid `//@ compile-flags: -Cincremental=` in tests) - #147225 (Don't enable shared memory by default with Wasm atomics) - #147227 (implement `Box::take`) - #147233 (Initialize llvm submodule if not already the case to run citool) - #147236 (Update books) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #147225 - daxpedda:wasm-u-u-atomics-threads, r=alexcrichton Don't enable shared memory by default with Wasm atomics This prepares us for a future where LLVM eventually stabilizes the atomics target feature, in which case we don't want to inflate atomics with threads. Otherwise users would be stuck with shared memory even when they don't want it/need it. ### Context Currently the atomics target features is unstable and can't be used without re-building Std with it (`-Zbuild-std`). Enabling the atomics target feature automatically enables shared memory. Shared memory is required to actually allow multi-threading. However, shared memory comes with a performance overhead when atomic instructions aren't able to be lowered to regular memory access instructions or when interacting with certain Web APIs. So it is very undesirable to enable shared memory by default for the majority of users. While it is possible to use atomics without shared memory, the question remains what use-case this scenario has. The only one I can think of would involve multiple memories, where the main memory remains un-shared but a second shared memory exists. While Rust doesn't support multiple memories, it might be possible with inline assembly (#136382). So alternatively, we might consider *not* enabling atomics by default even when LLVM does. In which case everything would remain the same. --- This will break current Web multi-threading users. To address this they can add the following `RUSTFLAGS`: ``` -Clink-args=--shared-memory,--max-memory=1073741824,--import-memory,--export=__wasm_init_tls,--export=__tls_size,--export=__tls_align,--export=__tls_base ``` We could add a new experimental flag that enables the right linker arguments for users, but I feel that's not in Rusts scope. Or like suggested before: a Rust-only `threads` target feature. Addresses #77839. r? ``@alexcrichton``
Rollup of 9 pull requests Successful merges: - rust-lang/rust#146281 (Support `#[rustc_align_static]` inside `thread_local!`) - rust-lang/rust#146535 (mbe: Implement `unsafe` attribute rules) - rust-lang/rust#146585 (indexing: reword help) - rust-lang/rust#147004 (Tweak handling of "struct like start" where a struct isn't supported) - rust-lang/rust#147221 (Forbid `//@ compile-flags: -Cincremental=` in tests) - rust-lang/rust#147225 (Don't enable shared memory by default with Wasm atomics) - rust-lang/rust#147227 (implement `Box::take`) - rust-lang/rust#147233 (Initialize llvm submodule if not already the case to run citool) - rust-lang/rust#147236 (Update books) r? `@ghost` `@rustbot` modify labels: rollup
I think we should do this, yes - it's too many arguments for users to copy-paste in each project. It should be purely additive though - if top-level crate enables it, that should be enough to pass these arguments to the linker even if standard library was built with just |
This prepares us for a future where LLVM eventually stabilizes the atomics target feature, in which case we don't want to inflate atomics with threads. Otherwise users would be stuck with shared memory even when they don't want it/need it.
Context
Currently the atomics target features is unstable and can't be used without re-building Std with it (
-Zbuild-std
).Enabling the atomics target feature automatically enables shared memory.
Shared memory is required to actually allow multi-threading.
However, shared memory comes with a performance overhead when atomic instructions aren't able to be lowered to regular memory access instructions or when interacting with certain Web APIs.
So it is very undesirable to enable shared memory by default for the majority of users.
While it is possible to use atomics without shared memory, the question remains what use-case this scenario has.
The only one I can think of would involve multiple memories, where the main memory remains un-shared but a second shared memory exists. While Rust doesn't support multiple memories, it might be possible with inline assembly (#136382).
So alternatively, we might consider not enabling atomics by default even when LLVM does. In which case everything would remain the same.
This will break current Web multi-threading users. To address this they can add the following
RUSTFLAGS
:We could add a new experimental flag that enables the right linker arguments for users, but I feel that's not in Rusts scope. Or like suggested before: a Rust-only
threads
target feature.Addresses #77839.
r? @alexcrichton