std: Implement TLS for wasm32-unknown-unknown #54951
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @bluss (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @SimonSapin |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
alexcrichton
force-pushed
the
more-wasm-threads
branch
from
4873556 to
7524ad7
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I know some of those words, but I feel very much not qualified to review this and don’t know who would be. |
alexcrichton
force-pushed
the
more-wasm-threads
branch
from
7524ad7 to
651ea5d
Compare
|
@sfackler sorry I don't mean to dump all reviews on you, but you might be most familiar with all this as well, mind taking a look? If you don't have time though just let me know! |
sfackler
reviewed
Oct 10, 2018
| if key >= MAX_KEYS { | ||
| panic!("cannot allocate space for more TLS keys"); | ||
| } | ||
| return key + 1 // offset by 1 so we never hand out 0 |
There was a problem hiding this comment.
Why don't we want to hand out 0?
There was a problem hiding this comment.
Ah this is a bit cryptic (and perhaps backwards), but it's due to this block where out platform-agnostic thread local code can't deal with a TLS descriptor of 0 so it just asks for another one if that happens
alexcrichton
force-pushed
the
more-wasm-threads
branch
from
651ea5d to
08237b2
Compare
sfackler
reviewed
Oct 11, 2018
| panic!("TLS on wasm with atomics not implemented yet"); | ||
| pub unsafe fn create(dtor: Option<unsafe extern fn(*mut u8)>) -> Key { | ||
| drop(dtor); // FIXME: need to figure out how to hook thread exit to run this | ||
| let key = NEXT_KEY.fetch_add(1, SeqCst); |
There was a problem hiding this comment.
This can technically overflow after 4 billion repeated failures but that may not be worth worrying about?
There was a problem hiding this comment.
Heh I'm not too worried about overflow but it's easy enough to add a store beneath this in the failure case back to MAX_KEYS, so done now!
alexcrichton
force-pushed
the
more-wasm-threads
branch
from
08237b2 to
d205e5b
Compare
This adds an implementation of thread local storage for the
`wasm32-unknown-unknown` target when the `atomics` feature is
implemented. This, however, comes with a notable caveat of that it
requires a new feature of the standard library, `wasm-bindgen-threads`,
to be enabled.
Thread local storage for wasm (when `atomics` are enabled and there's
actually more than one thread) is powered by the assumption that an
external entity can fill in some information for us. It's not currently
clear who will fill in this information nor whose responsibility it
should be long-term. In the meantime there's a strategy being gamed out
in the `wasm-bindgen` project specifically, and the hope is that we can
continue to test and iterate on the standard library without committing
to a particular strategy yet.
As to the details of `wasm-bindgen`'s strategy, LLVM doesn't currently
have the ability to emit custom `global` values (thread locals in a
`WebAssembly.Module`) so we leverage the `wasm-bindgen` CLI tool to do
it for us. To that end we have a few intrinsics, assuming two global values:
* `__wbindgen_current_id` - gets the current thread id as a 32-bit
integer. It's `wasm-bindgen`'s responsibility to initialize this
per-thread and then inform libstd of the id. Currently `wasm-bindgen`
performs this initialization as part of the `start` function.
* `__wbindgen_tcb_{get,set}` - in addition to a thread id it's assumed
that there's a global available for simply storing a pointer's worth
of information (a thread control block, which currently only contains
thread local storage). This would ideally be a native `global`
injected by LLVM, but we don't have a great way to support that right
now.
To reiterate, this is all intended to be unstable and purely intended
for testing out Rust on the web with threads. The story is very likely
to change in the future and we want to make sure that we're able to do
that!
alexcrichton
force-pushed
the
more-wasm-threads
branch
from
d205e5b to
cbe9f33
Compare
|
@bors r+ |
|
📌 Commit cbe9f33 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit cbe9f33 with merge 3431fa629099c29a8ade8fc1ad5eb9019e076527... |
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors: retry |
bors
removed
the
S-waiting-on-review
bors
added
the
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Oct 13, 2018
std: Implement TLS for wasm32-unknown-unknown
This adds an implementation of thread local storage for the
`wasm32-unknown-unknown` target when the `atomics` feature is
implemented. This, however, comes with a notable caveat of that it
requires a new feature of the standard library, `wasm-bindgen-threads`,
to be enabled.
Thread local storage for wasm (when `atomics` are enabled and there's
actually more than one thread) is powered by the assumption that an
external entity can fill in some information for us. It's not currently
clear who will fill in this information nor whose responsibility it
should be long-term. In the meantime there's a strategy being gamed out
in the `wasm-bindgen` project specifically, and the hope is that we can
continue to test and iterate on the standard library without committing
to a particular strategy yet.
As to the details of `wasm-bindgen`'s strategy, LLVM doesn't currently
have the ability to emit custom `global` values (thread locals in a
`WebAssembly.Module`) so we leverage the `wasm-bindgen` CLI tool to do
it for us. To that end we have a few intrinsics, assuming two global values:
* `__wbindgen_current_id` - gets the current thread id as a 32-bit
integer. It's `wasm-bindgen`'s responsibility to initialize this
per-thread and then inform libstd of the id. Currently `wasm-bindgen`
performs this initialization as part of the `start` function.
* `__wbindgen_tcb_{get,set}` - in addition to a thread id it's assumed
that there's a global available for simply storing a pointer's worth
of information (a thread control block, which currently only contains
thread local storage). This would ideally be a native `global`
injected by LLVM, but we don't have a great way to support that right
now.
To reiterate, this is all intended to be unstable and purely intended
for testing out Rust on the web with threads. The story is very likely
to change in the future and we want to make sure that we're able to do
that!
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds an implementation of thread local storage for the
wasm32-unknown-unknowntarget when theatomicsfeature isimplemented. This, however, comes with a notable caveat of that it
requires a new feature of the standard library,
wasm-bindgen-threads,to be enabled.
Thread local storage for wasm (when
atomicsare enabled and there'sactually more than one thread) is powered by the assumption that an
external entity can fill in some information for us. It's not currently
clear who will fill in this information nor whose responsibility it
should be long-term. In the meantime there's a strategy being gamed out
in the
wasm-bindgenproject specifically, and the hope is that we cancontinue to test and iterate on the standard library without committing
to a particular strategy yet.
As to the details of
wasm-bindgen's strategy, LLVM doesn't currentlyhave the ability to emit custom
globalvalues (thread locals in aWebAssembly.Module) so we leverage thewasm-bindgenCLI tool to doit for us. To that end we have a few intrinsics, assuming two global values:
__wbindgen_current_id- gets the current thread id as a 32-bitinteger. It's
wasm-bindgen's responsibility to initialize thisper-thread and then inform libstd of the id. Currently
wasm-bindgenperforms this initialization as part of the
startfunction.__wbindgen_tcb_{get,set}- in addition to a thread id it's assumedthat there's a global available for simply storing a pointer's worth
of information (a thread control block, which currently only contains
thread local storage). This would ideally be a native
globalinjected by LLVM, but we don't have a great way to support that right
now.
To reiterate, this is all intended to be unstable and purely intended
for testing out Rust on the web with threads. The story is very likely
to change in the future and we want to make sure that we're able to do
that!