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

std: Start implementing wasm32 atomics #54017

Merged
merged 1 commit into from Oct 5, 2018

Conversation

Projects
None yet
7 participants
@alexcrichton
Member

alexcrichton commented Sep 7, 2018

This commit is an initial start at implementing the standard library for
wasm32-unknown-unknown with the experimental atomics feature enabled. None of
these changes will be visible to users of the wasm32-unknown-unknown target
because they all require recompiling the standard library. The hope with this is
that we can get this support into the standard library and start iterating on it
in-tree to enable experimentation.

Currently there's a few components in this PR:

  • Atomic fences are disabled on wasm as there's no corresponding atomic op and
    it's not clear yet what the convention should be, but this will change in the
    future!
  • Implementations of Mutex, Condvar, and RwLock were all added based on
    the atomic intrinsics that wasm has.
  • The ReentrantMutex and thread-local-storage implementations panic currently
    as there's no great way to get a handle on the current thread's "id" yet.

Right now the wasm32 target with atomics is unfortunately pretty unusable,
requiring a lot of manual things here and there to actually get it operational.
This will likely continue to evolve as the story for atomics and wasm unfolds,
but we also need more LLVM support for some operations like custom global
directives for this to work best.

@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Sep 7, 2018

Collaborator

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

Collaborator

rust-highfive commented Sep 7, 2018

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Sep 7, 2018

Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.
Collaborator

rust-highfive commented Sep 7, 2018

⚠️ Warning ⚠️

  • These commits modify submodules.
@TimNN

This comment has been minimized.

Show comment
Hide comment
@TimNN

TimNN Sep 11, 2018

Contributor

Ping from triage @shepmaster: This PR requires your review.

Contributor

TimNN commented Sep 11, 2018

Ping from triage @shepmaster: This PR requires your review.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Sep 14, 2018

@rust-highfive rust-highfive assigned sfackler and unassigned shepmaster Sep 14, 2018

pub fn fence(order: Ordering) {
// On wasm32 it looks like fences aren't implemented in LLVM yet in that

This comment has been minimized.

@mzji

mzji Sep 14, 2018

Should we add a FIXME here?

@mzji

mzji Sep 14, 2018

Should we add a FIXME here?

This comment has been minimized.

@sfackler

sfackler Sep 24, 2018

Member

Can we have this panic or fail to compile on wasm rather than being a nop?

@sfackler

sfackler Sep 24, 2018

Member

Can we have this panic or fail to compile on wasm rather than being a nop?

This comment has been minimized.

@alexcrichton

alexcrichton Sep 24, 2018

Member

@sfackler unfortunately this is used by Arc which means that apps quickly stop working if it fails to compile or panics :(

Alternatively we could remove libstd's usage of fences on wasm, but this seemed like a smaller local change for now

@alexcrichton

alexcrichton Sep 24, 2018

Member

@sfackler unfortunately this is used by Arc which means that apps quickly stop working if it fails to compile or panics :(

Alternatively we could remove libstd's usage of fences on wasm, but this seemed like a smaller local change for now

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Sep 22, 2018

ping r? @sfackler

pub fn fence(order: Ordering) {
// On wasm32 it looks like fences aren't implemented in LLVM yet in that

This comment has been minimized.

@sfackler

sfackler Sep 24, 2018

Member

Can we have this panic or fail to compile on wasm rather than being a nop?

@sfackler

sfackler Sep 24, 2018

Member

Can we have this panic or fail to compile on wasm rather than being a nop?

Show outdated Hide outdated src/libstd/sys/wasm/condvar_atomics.rs Outdated
Show outdated Hide outdated src/libstd/sys/wasm/rwlock_atomics.rs Outdated
// unlocking the lock will notify *all* waiters rather than just readers or just
// writers. This can cause lots of "thundering stampede" problems. While
// hopefully correct this implementation is very likely to want to be changed in
// the future.

This comment has been minimized.

@sfackler

sfackler Sep 24, 2018

Member

This implementation additionally doesn't provide any fairness guarantees, right?

@sfackler

sfackler Sep 24, 2018

Member

This implementation additionally doesn't provide any fairness guarantees, right?

This comment has been minimized.

@alexcrichton

alexcrichton Sep 24, 2018

Member

Correct yeah, it's not really that great of an rwlock implementation but I figure it's hopefully at least correct and we can continue to iterate in the future.

@alexcrichton

alexcrichton Sep 24, 2018

Member

Correct yeah, it's not really that great of an rwlock implementation but I figure it's hopefully at least correct and we can continue to iterate in the future.

Show outdated Hide outdated src/libstd/sys/wasm/condvar_atomics.rs Outdated
std: Start implementing wasm32 atomics
This commit is an initial start at implementing the standard library for
wasm32-unknown-unknown with the experimental `atomics` feature enabled. None of
these changes will be visible to users of the wasm32-unknown-unknown target
because they all require recompiling the standard library. The hope with this is
that we can get this support into the standard library and start iterating on it
in-tree to enable experimentation.

Currently there's a few components in this PR:

* Atomic fences are disabled on wasm as there's no corresponding atomic op and
  it's not clear yet what the convention should be, but this will change in the
  future!
* Implementations of `Mutex`, `Condvar`, and `RwLock` were all added based on
  the atomic intrinsics that wasm has.
* The `ReentrantMutex` and thread-local-storage implementations panic currently
  as there's no great way to get a handle on the current thread's "id" yet.

Right now the wasm32 target with atomics is unfortunately pretty unusable,
requiring a lot of manual things here and there to actually get it operational.
This will likely continue to evolve as the story for atomics and wasm unfolds,
but we also need more LLVM support for some operations like custom `global`
directives for this to work best.
@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Oct 4, 2018

ping r? @sfackler

@sfackler

This comment has been minimized.

Show comment
Hide comment
@sfackler
Member

sfackler commented Oct 5, 2018

@bors r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 5, 2018

Contributor

📌 Commit b4877ed has been approved by sfackler

Contributor

bors commented Oct 5, 2018

📌 Commit b4877ed has been approved by sfackler

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 5, 2018

Contributor

⌛️ Testing commit b4877ed with merge b8bea5a...

Contributor

bors commented Oct 5, 2018

⌛️ Testing commit b4877ed with merge b8bea5a...

bors added a commit that referenced this pull request Oct 5, 2018

Auto merge of #54017 - alexcrichton:wasm-atomics2, r=sfackler
std: Start implementing wasm32 atomics

This commit is an initial start at implementing the standard library for
wasm32-unknown-unknown with the experimental `atomics` feature enabled. None of
these changes will be visible to users of the wasm32-unknown-unknown target
because they all require recompiling the standard library. The hope with this is
that we can get this support into the standard library and start iterating on it
in-tree to enable experimentation.

Currently there's a few components in this PR:

* Atomic fences are disabled on wasm as there's no corresponding atomic op and
  it's not clear yet what the convention should be, but this will change in the
  future!
* Implementations of `Mutex`, `Condvar`, and `RwLock` were all added based on
  the atomic intrinsics that wasm has.
* The `ReentrantMutex` and thread-local-storage implementations panic currently
  as there's no great way to get a handle on the current thread's "id" yet.

Right now the wasm32 target with atomics is unfortunately pretty unusable,
requiring a lot of manual things here and there to actually get it operational.
This will likely continue to evolve as the story for atomics and wasm unfolds,
but we also need more LLVM support for some operations like custom `global`
directives for this to work best.
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 5, 2018

Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: sfackler
Pushing b8bea5a to master...

Contributor

bors commented Oct 5, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: sfackler
Pushing b8bea5a to master...

@bors bors merged commit b4877ed into rust-lang:master Oct 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@alexcrichton alexcrichton deleted the alexcrichton:wasm-atomics2 branch Oct 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment