Skip to content

[Synchronization] Fix wasm atomic intrinsic declarations #74171

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

Merged

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Jun 6, 2024

  • 0536c9a Fix wasm atomic intrinsic declarations
    Otherwise, isel will not be able to select the desired atomic instructions.
  • 2dbbbcf Skip atomic operations in single-threaded mode on WebAssembly
    Use of atomics instructions requires the explicit support of threads proposal and it's not widely supported yet. So we should enable actual atomic operations only when targeting wasm32-uknown-wasip1-threads.

Those changes repair the current test failure:

Command Output (stderr):
--
wasm-ld: error: /home/build-user/build/buildbot_linux/wasmstdlib-linux-x86_64/./lib/swift_static/wasi/libswiftSynchronization.a(Synchronization.o): undefined symbol: llvm.wasm32.memory.atomic.wait32
wasm-ld: error: /home/build-user/build/buildbot_linux/wasmstdlib-linux-x86_64/./lib/swift_static/wasi/libswiftSynchronization.a(Synchronization.o): undefined symbol: llvm.wasm32.memory.atomic.notify
clang: error: linker command failed with exit code 1 (use -v to see invocation)
<unknown>:0: error: link command failed with exit code 1 (use -v to see invocation)

https://ci.swift.org/job/swift-PR-Linux-crosscompile-wasm/106/console

Otherwise, isel will not be able to select the desired atomic
instructions.
…ebAssembly

Use of atomics instructions requires the support of threads proposal and
it's not widely supported yet. So we should enable actual atomic
operations only when targeting wasm32-uknown-wasip1-threads.
@kateinoigakukun
Copy link
Member Author

@swift-ci test WebAssembly

@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

@kateinoigakukun kateinoigakukun marked this pull request as ready for review June 6, 2024 07:58
@kateinoigakukun kateinoigakukun requested a review from a team as a code owner June 6, 2024 07:58
@kateinoigakukun kateinoigakukun merged commit da6f015 into swiftlang:main Jun 6, 2024
4 checks passed
@kateinoigakukun kateinoigakukun deleted the yt/fix-mutex-wasm-test branch June 6, 2024 07:58
@kateinoigakukun kateinoigakukun added the WebAssembly Platform: WebAssembly label Jun 6, 2024

extension Atomic where Value == _MutexHandle.State {
internal borrowing func _wait(expected: _MutexHandle.State) {
_swift_stdlib_wait(
#if _runtime(_multithreaded)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should generally make Mutex unavailable when the runtime isn't multithreaded

Copy link
Contributor

Choose a reason for hiding this comment

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

although I guess it doesn't really matter because Mutex can always acquire in the single threaded context and needing to call wait or notify should ever occur so this is probably fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, it might be better to have a separate noop implementations for single threaded targets instead of just skipping wait/notify operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebAssembly Platform: WebAssembly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants