Skip to content

[stdlib] Implement Mutex in Synchronization #71383

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
Merged
merged 21 commits into from
Jun 5, 2024
Merged

[stdlib] Implement Mutex in Synchronization #71383

merged 21 commits into from
Jun 5, 2024

Conversation

Azoy
Copy link
Contributor

@Azoy Azoy commented Feb 5, 2024

This patch adds a new primitive in the Synchronization module, Mutex, which is a wrapper over a platform's implementation.

@Azoy Azoy requested review from compnerd, ktoso and al45tair February 5, 2024 08:02
al45tair
al45tair reviewed Feb 5, 2024
View reviewed changes
Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

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

I think the Linux futex code is slightly wrong.

harlanhaskins
harlanhaskins reviewed Feb 5, 2024
View reviewed changes
@Azoy Azoy marked this pull request as ready for review February 21, 2024 02:18
@Azoy Azoy requested a review from a team as a code owner February 21, 2024 02:18
al45tair
al45tair approved these changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

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

LGTM, but I think we do want to cache the thread ID on Linux if we can. (The Linux pthreads implementation caches it too, FWIW.)

@Azoy
Copy link
Contributor Author

Azoy commented Mar 19, 2024

@swift-ci please smoke test

@Azoy
Copy link
Contributor Author

Azoy commented Mar 19, 2024

@swift-ci test webassembly

@Azoy
Copy link
Contributor Author

Azoy commented Mar 19, 2024

@swift-ci please build toolchain macOS

@Azoy
Copy link
Contributor Author

Azoy commented Mar 19, 2024

@swift-ci please smoke test

@Azoy
Copy link
Contributor Author

Azoy commented Mar 19, 2024

@swift-ci please smoke test webassembly

@Azoy
Copy link
Contributor Author

Azoy commented Mar 19, 2024

@swift-ci please test webassembly

@Azoy
Copy link
Contributor Author

Azoy commented Mar 29, 2024

@swift-ci please smoke test

@Azoy
Copy link
Contributor Author

Azoy commented Mar 29, 2024

@swift-ci please test webassembly

@Azoy
Copy link
Contributor Author

Azoy commented Mar 29, 2024

@swift-ci please build toolchain macOS

@Azoy
Copy link
Contributor Author

Azoy commented Mar 29, 2024

@swift-ci please test webassembly

@Azoy
Copy link
Contributor Author

Azoy commented Apr 1, 2024

@swift-ci please test

@Azoy
Copy link
Contributor Author

Azoy commented Apr 1, 2024

@swift-ci please build toolchain

@Azoy
Copy link
Contributor Author

Azoy commented Apr 1, 2024

@swift-ci please test

@Azoy
Copy link
Contributor Author

Azoy commented Apr 1, 2024

@swift-ci please test webassembly

@Azoy Azoy force-pushed the mutex branch 2 times, most recently from dd27d6a to fa5248b Compare April 5, 2024 22:44
@Azoy
Copy link
Contributor Author

Azoy commented Apr 5, 2024

@swift-ci please test

@Azoy
Copy link
Contributor Author

Azoy commented Apr 5, 2024

@swift-ci please build toolchain

@Azoy
Copy link
Contributor Author

Azoy commented Apr 5, 2024

@swift-ci please test webassembly

@Azoy
Copy link
Contributor Author

Azoy commented Apr 25, 2024

@swift-ci please test

Azoy added 21 commits June 4, 2024 09:06
@Azoy
Implement Mutex in Synchronization
2a60c88
@Azoy
Attempt to release the lock and wake when we can't
1f206ec
@Azoy
Directly call gettid syscall instead of library function
882568c
@Azoy
Add the underscored unsafe locking primitives
1146b41
@Azoy
Add a WasmMutex
5b7e3d8
@Azoy
Cache TID in TLS on Linux
92e6a98
@Azoy
Actually implement the API now
89748d4
@Azoy
Move atomic sources into a folder
ba1787c
@Azoy
Adopt transferring for Mutex init
97072e5
@Azoy
Move atomics tests under a synchronization folder
ce89c75
@Azoy
Add a basic mutex test
726609a
@Azoy
It is WASI, not WASM
6d6e518
@Azoy
Use Builtin.addressOfRawLayout
e992688
@Azoy
Fix tests and toolchain build
9b389a3
@Azoy
Update x86_64 abi test
b2cffb6
@Azoy
Properly handle the syscalls on Linux and do some spins
3a115fc 
fix some stuff
@Azoy
Update LockSingleConsumerStack.swift
6bce596
@Azoy
Update LinuxImpl.swift
fbfb218
@Azoy
Underscore some things, add explicit internal
66ec7ab 
Fix some compile issues
@Azoy
Support visionOS for Mutex
6b29e77
@Azoy
Adopt sending and movesAsLike
cb3fc4b
@Azoy
Copy link
Contributor Author

Azoy commented Jun 4, 2024

@swift-ci please test

@Azoy Azoy merged commit 643c277 into swiftlang:main Jun 5, 2024
5 checks passed
@Azoy Azoy deleted the mutex branch June 5, 2024 01:38
Azoy added a commit to Azoy/swift that referenced this pull request Jun 6, 2024
@Azoy
Merge pull request swiftlang#71383 from Azoy/mutex
b3a98d6 
[stdlib] Implement Mutex in Synchronization
@Azoy Azoy mentioned this pull request Jun 6, 2024
Merged
@finagolfin
Copy link
Member

@Azoy, is this supposed to work on 32-bit ARM? This pull crashes the trunk compiler when building for Android armv7 on my daily CI, though it builds fine for AArch64 and x86_64:

<unknown>:0: error: fatal error encountered during compilation; please submit a bug report (https://swift.org/contributing/#reporting-bugs)
<unknown>:0: note: Cannot select: intrinsic <<INTERNAL ERROR: unparseable argument index in diagnostic text>>.aarch64.hint
--- snip compiler command from stack dump ---
1.	Swift version 6.0-dev (LLVM 6acfe4c343918fc, Swift 2175abd4e9efe4a)
2.	Compiling with effective version 5.10
3.	Running pass 'Function Pass Manager' on module '/home/runner/work/swift-android-sdk/swift-android-sdk/sdk-config/build/Ninja-Release/swift-android-armv7/stdlib/public/Synchronization/ANDROID/armv7/Synchronization.o'.
4.	Running pass 'ARM Instruction Selection' on function '@"$s15Synchronization12_MutexHandleV9_lockSlowyys6UInt32VF"'

@finagolfin finagolfin mentioned this pull request Jun 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harlanhaskins harlanhaskins harlanhaskins left review comments

@al45tair al45tair al45tair approved these changes

@compnerd compnerd Awaiting requested review from compnerd

@ktoso ktoso Awaiting requested review from ktoso

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

Successfully merging this pull request may close these issues.
4 participants
@Azoy @finagolfin @harlanhaskins @al45tair