-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Build][Runtime] Add a new threading library. #59287
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
Merged
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
0cf687a
[Build][Runtime] Replace SWIFT_STDLIB_SINGLE_THREADED_RUNTIME.
al45tair 66b9d21
[Runtime] Remove all use of read/write locks.
al45tair f5bdb85
[Threading] Create new threading library and use it.
al45tair 2ba80e1
[Unittests] Make the Threading unit tests work.
al45tair 785f4a2
[Unittests] Add more threading library tests.
al45tair e934cbc
[Threading] Fix case sensitivity bug.
al45tair a699633
[Threading] Auto-detect threading in the preprocessor.
al45tair e0d23ed
[Threading] A couple of header file fixes.
al45tair 6d1b6db
[Threading] Fix the Linux build.
al45tair 84083af
[Build] Move initialization of SWIFT_HOST_VARIANT_SDK further up.
al45tair 8615983
[Build] Fix static linking.
al45tair 02b3431
[Build] Fix the name of the threading package setting.
al45tair 4eca62a
[Threading][Windows] Fix missing close brace.
al45tair 9d5736e
[Threading][Windows] Fix another Windows niggle.
al45tair d76f80d
[Threading][Windows] More header file fixes.
al45tair a7e42d0
[Threading][Windows] Undefine Yield and ERROR.
al45tair 05cccf9
[Runtime][Windows] Need to link with Synchronization.lib.
al45tair 2e28ac4
[Threading][Windows] Don't include <windows.h>
al45tair 66f6eb6
[Runtime][Windows] A couple of files need to include <windows.h>
al45tair 14a4bd4
[Concurrency][Threading] Remove use of platform thread functions.
al45tair d6cff94
[Threading][Windows] Try to avoid declaring _RTL_SRWLOCK
al45tair e305a7d
[Concurrency][Windows] Add a couple of includes for Windows.
al45tair ef2e40a
[Concurrency][Windows] Remove unnecessary includes.
al45tair 3bfc7e0
[Threading][Windows] Link the unit test against Synchronization.lib
al45tair 210b772
[UnitTests][Windows] Link with Synchronization.lib
al45tair f3a412d
[Threading] Remove thread_get_main().
al45tair 0e9318c
[Threading] Put everything through git clang-format.
al45tair c7c1c1b
[Threading] Fix some problems with the C11 threading code.
al45tair eb4c81d
[Threading] Don't use TLS keys as template arguments.
al45tair b5c8b79
[Threading] Move stack bounds fetching into the threading library.
al45tair 4825578
[Concurrency] Remove redundant include.
al45tair dc809f8
[Threading] A few include tweaks.
al45tair 6962758
[Threading] Add the ability to use any C++ callable in swift::once().
al45tair 668bcbc
[Threading] Fix a few things following rebase.
al45tair c401cc3
[Threading] Use llvm::Optional<> rather than making a zero lower boun…
al45tair 33f103f
[Build] Fix some issues with the standalone library build.
al45tair 124581a
[Build][Concurrency] Fix Concurrency build to work for Linux.
al45tair aa0265b
[Threading] Use raw gettid syscall for older glibc compatibility
kateinoigakukun c46d4af
[Build] Explicitly include threading library when not building stdlib.
al45tair ede4010
[BackDeployConcurrency] Fix to use the new threading library.
al45tair File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# Get the default threading package for the platform | ||
function(threading_package_default sdk out_var) | ||
if(sdk IN_LIST SWIFT_DARWIN_PLATFORMS) | ||
set("${out_var}" "darwin" PARENT_SCOPE) | ||
elseif(sdk STREQUAL "LINUX") | ||
set("${out_var}" "linux" PARENT_SCOPE) | ||
elseif(sdk STREQUAL "WINDOWS") | ||
set("${out_var}" "win32" PARENT_SCOPE) | ||
elseif(sdk STREQUAL "WASI") | ||
set("${out_var}" "none" PARENT_SCOPE) | ||
else() | ||
set("${out_var}" "pthreads" PARENT_SCOPE) | ||
endif() | ||
endfunction() | ||
|
||
# Given the threading package, find the name for the preprocessor | ||
# define that we need to make. Also deals with the default platform | ||
# setting. | ||
function(threading_package_name sdk out_var) | ||
precondition(SWIFT_HOST_VARIANT_SDK) | ||
precondition(SWIFT_DARWIN_PLATFORMS) | ||
|
||
string(TOUPPER "${SWIFT_THREADING_PACKAGE}" package) | ||
if(package STREQUAL "") | ||
threading_package_default(package) | ||
string(TOUPPER "${package}" package) | ||
endif() | ||
set("${out_var}" "${package}" PARENT_SCOPE) | ||
endfunction() |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think setting this to
SWIFT_THREADING_PACKAGE
by default is a mistake, as it is set to the host platform by default. Since it's possible to cross-compile multiple non-host stdlibs, that means it will build all of them with the host threading library name.I'm not sure how the couple lines just below are supposed to work either, how are you supposed to pass an empty string to
build-script
? I tried various things like--swift-threading-package=""
and nothing worked. Even if it did, you only havethreading_package_default()
below taking one variable, when it needs two.This just hit me because I cross-compile the Android armv7 SDK and it failed because it used pthreads, which doesn't support 32-bit here.
I was confused though because it built fine on the community Android armv7 CI, then I dug into it and it's because the host SDK there is
LINUX
so it applies that threading library to Android armv7 too, based on the default logic above. My own Android CI doesn't build for the linux host though, only for Android armv7 with the flags--cross-compile-hosts=android-armv7 --skip-local-build
, so the host default logic chooses pthreads for the Android host and fails for armv7 only.That suggests two fixes:
Let me know what you think, @al45tair, you may want to get pthreads working for 32-bit too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making pthreads work for 32-bit is a fairly trivial fix, I think, and probably worth doing (possibly the right thing is to use
uintptr_t
instead ofuint64_t
for the type of theswift::once_t
), though maybe we should be picking thelinux
setting on Android?The
swift-threading-package
setting isn't presently supposed to be explicitly set to empty, which is why that doesn't work. It's there to let you override it and explicitly specify what you want, not the opposite (not sure why you'd need to do that). If you're looking at the headers and have seen the bit in the header file where it tries to guess, that has to be there because of lldb, rather than because I thought that was a good idea.As for the cross compilation problem, could you file a separate issue for that? The threading code gets used both for the host and the target, so I think you're right that the automatic detection might indeed need to be tweaked for the target platform.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure: both appear to work on 64-bit platforms, so I will look into it and let you know.
I thought you explicitly included the empty string in this list of threading options to allow people to set it, but I think the bash check a couple lines above wouldn't allow it through anyway.
OK, this check that I mentioned will never be true then? I thought the intent was for it to signify "auto-detect" for each stdlib, but it currently doesn't do that either.
Will doDone, #59568.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR to fix the remaining 32-bit issues (@compnerd fixed it already for Windows): #59586