Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add experimenal wasm32-wasi-threads support #274

Merged
merged 4 commits into from
Jan 20, 2023
Merged

add experimenal wasm32-wasi-threads support #274

merged 4 commits into from
Jan 20, 2023

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Dec 26, 2022 •
edited
Loading

@yamt
Copy link
Contributor Author

yamt commented Dec 26, 2022

#272

@yamt
Copy link
Contributor Author

yamt commented Dec 29, 2022

with this and #275,
i was able to build ffmpeg with pthread.
https://github.com/yamt/FFmpeg-WASI
https://github.com/yamt/FFmpeg-WASI/releases/tag/yamt-20221229

i was able to use more than 1 cpus with the above binary.
eg.

~/git/toywasm/b/toywasm --wasi --wasi-dir videos -- ffmpeg.wasm -i videos/video-15s.avi -c:v libx264 -threads 16 videos/out.mp4

@yamt yamt mentioned this pull request Dec 29, 2022
Open
@yamt
Copy link
Contributor Author

yamt commented Jan 11, 2023

depending on WebAssembly/wasi-threads#22 we might need to wait for a llvm release with https://reviews.llvm.org/D135898

@yamt yamt mentioned this pull request Jan 12, 2023
Closed
@yamt yamt changed the title wasm32-wasi-pthread wasm32-wasi-threads Jan 13, 2023
@abrown
Copy link
Collaborator

abrown commented Jan 13, 2023

I wouldn't worry too much about using your own fork here; even if wasi-libc doesn't have the TLS PR merged yet this would still be a step in the right direction. We just would want to make sure that any missing pieces like that PR get updated in the submodule before @sunfishcode publishes a new wasi-sdk release, which I hear might be in the next few weeks (?).

@yamt
Copy link
Contributor Author

yamt commented Jan 13, 2023 •
edited
Loading

I wouldn't worry too much about using your own fork here; even if wasi-libc doesn't have the TLS PR merged yet this would still be a step in the right direction. We just would want to make sure that any missing pieces like that PR get updated in the submodule before @sunfishcode publishes a new wasi-sdk release, which I hear might be in the next few weeks (?).

ok. i will drop the "use my fork" commit. instead, i mentioned known issues in the description of this PR.

@yamt yamt marked this pull request as ready for review January 13, 2023 07:50
@yamt yamt changed the title wasm32-wasi-threads add experimenal wasm32-wasi-threads support Jan 13, 2023
This was referenced Jan 19, 2023
Closed
Closed
sunfishcode
sunfishcode approved these changes Jan 19, 2023
View reviewed changes
Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@abrown
Copy link
Collaborator

abrown commented Jan 20, 2023

@sunfishcode, you'll have to merge this (I don't have permissions here but I'm a +1 for this PR). @yamt, do we still need to update the wasi-libc commit in the submodule? If so, that can probably be its own PR.

@yamt
Copy link
Contributor Author

yamt commented Jan 20, 2023

@sunfishcode, you'll have to merge this (I don't have permissions here but I'm a +1 for this PR). @yamt, do we still need to update the wasi-libc commit in the submodule? If so, that can probably be its own PR.

the latest wasi-libc should be ok.
i haven't checked if the submodule is latest enough.

my concern at this point is the coming ABI change from: WebAssembly/wasi-threads#26
cf. WebAssembly/wasi-libc#387

@loganek
Copy link

loganek commented Jan 20, 2023

How stable we consider wasi-sdk? Are we ok at that stage to introduce backward incompatible changes (that can be detected at compile time)?

@sbc100
Copy link
Member

sbc100 commented Jan 20, 2023

How stable we consider wasi-sdk? Are we ok at that stage to introduce backward incompatible changes (that can be detected at compile time)?

Are you asking in general, or specifically regarding this PR? This PR doesn't break any existing thing right, it add a completely new set of target libraries while leaving the rest of the SDK untouched?

In the general case, WASI is not yet stable or standardized, and neither is wasi-sdk. Of course, we can and should always do our best to avoid breaking existing code, but we are not at the stage in development of WASI where we rule out such things. Even the core components on which we build wasi-sdk (llvm and musl) make breaking changes from time to time so it not practical to rule out all breaking changes.

@loganek
Copy link

loganek commented Jan 20, 2023

Are you asking in general, or specifically regarding this PR?

Mainly asked in the context of @yamt 's concerns about ABI changes. Looks like wam32-wasi-threads target is even more experimental than the rest of the wasi-sdk, so I don't think we have to wait for the ABI to be finalized?

@abrown
Copy link
Collaborator

abrown commented Jan 20, 2023

Right, to echo @sbc100, we cannot make too strong guarantees about about the wasi-threads ABI. We have an ABI that is good enough for users to experiment with and we can accompany the prerelease of all of this with some warnings in the documentation.

loganek
loganek approved these changes Jan 20, 2023
View reviewed changes
Copy link

@loganek loganek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sunfishcode sunfishcode merged commit f420453 into WebAssembly:main Jan 20, 2023
@TerrorJack
Copy link
Contributor

There doesn't seem to be libcxx/libcxxabi builds for wasm32-wasi-threads yet, is it an oversight?

@abrown
Copy link
Collaborator

abrown commented Feb 8, 2023

No, just not enough time to do everything. Are you thinking of contributing something like this in WebAssembly/wasi-libc#392?

@whitequark
Copy link
Contributor

There doesn't seem to be libcxx/libcxxabi builds for wasm32-wasi-threads yet, is it an oversight?

#301

@abrown abrown mentioned this pull request Mar 13, 2023
Closed
@theoparis theoparis mentioned this pull request Apr 27, 2023
Closed
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Jul 5, 2023
@sternenseemann
wasilibc: 19 -> 20
bf5423c 
pkgsCross.wasi32.llvmPackages: 12 -> 16

* It appears BULK_MEMORY_SOURCES no longer needs to be set to be empty
  to compile firefox. Without it, the build of wasilibc would fail if
  enableThreads is true.

* Include preliminary support for the experimental threads support in
  wasilibc which provides pthreads API. If wasilibc is built with
  support, is exposed via passthru and libcxx / libcxxabi are built with
  threads support accordingly.

  See also:
  - WebAssembly/wasi-sdk#274
  - WebAssembly/wasi-sdk#301
  - WebAssembly/wasi-sdk#314

  wasi-sdk ships it by default, but as a separate variant of libc which
  would be a hassle for us. Let's just make it optional for now.

  You can try it out using the following overlay:

      self: super: {
        wasilibc = super.wasilibc.override { enableThreads = true; };
      }

  Flags for libc++abi are copied from wasi-sdk.
@denis-belov
Copy link

Hello guys! Does it mean that malloc() will be thread-safe from now?

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

@sunfishcode sunfishcode sunfishcode approved these changes

@loganek loganek loganek approved these changes

@abrown abrown abrown approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@yamt @abrown @loganek @sbc100 @TerrorJack @whitequark @denis-belov @sunfishcode