Skip to content

[embedded] Default Embedded Concurrency to SWIFT_THREADING_PACKAGE=none #72769

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

Merged
merged 2 commits into from
Apr 5, 2024
Merged

[embedded] Default Embedded Concurrency to SWIFT_THREADING_PACKAGE=none #72769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a79e434
[embedded] Default Embedded Concurrency to single-threaded
kubamracek Apr 2, 2024
dcb60ab
[embedded] Build even -macos Embedded Concurrency as THREADING=none, …
kubamracek Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions stdlib/public/Concurrency/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ if(SWIFT_SHOULD_BUILD_EMBEDDED_STDLIB AND SWIFT_SHOULD_BUILD_EMBEDDED_CONCURRENC
set(SWIFT_CONCURRENCY_USES_DISPATCH FALSE)
set(SWIFT_STDLIB_SINGLE_THREADED_CONCURRENCY TRUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this change? We should want to eventually get to being able to specify SWIFT_CONCURRENCY_GLOBAL_EXECUTOR_default = hooked so that SWIFT_CONCURRENCY_GLOBAL_EXECUTOR is set to hooked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do it as a follow-up. If I just naively remove this line, I get...

Undefined symbols for architecture arm64:
  "_swift_reportError", referenced from:
      _swift_task_enqueueGlobal in libswift_Concurrency.a[7](GlobalExecutor.cpp.o)
      swift_task_enqueueGlobalImpl(swift::Job*) in libswift_Concurrency.a[7](GlobalExecutor.cpp.o)
      swift_task_asyncMainDrainQueueImpl() in libswift_Concurrency.a[11](Task.cpp.o)
ld: symbol(s) not found for architecture arm64

...because the hooked version (SWIFT_CONCURRENCY_GLOBAL_EXECUTOR_default = hooked) of Concurrency still brings in the stub implementations from NonDispatchGlobalExecutor.inc, and we should instead really just have these symbols undefined in the runtime, and expect the user to provide them and link them.

Copy link
Contributor

@rokhinip rokhinip Apr 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

and we should instead really just have these symbols undefined in the runtime, and expect the user to provide them and link them.

Don't we just want to funnel this to swift_fatal_error which is already hooked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems like the wrong solution -- it would certainly work, but we are going to need to keep cutting down dependencies and code in the Embedded Swift Concurrency runtime, and IIUC, in hooked mode, we expect the user/platform to provide swift_task_enqueueGlobal, and therefore the two symbols from the listing above (_swift_task_enqueueGlobal, swift_task_enqueueGlobalImpl ) should really just not be part of the hooked build of Concurrency. Doing that sounds like a double win (besides resolving this undefined symbol problem): (1) less code = better codesize, (2) turns a runtime failure into a link time error.

set(SWIFT_STDLIB_CONCURRENCY_TRACING FALSE)
set(SWIFT_STDLIB_HAS_ENVIRON FALSE)
set(SWIFT_STDLIB_HAS_ASL FALSE)

foreach(entry ${EMBEDDED_STDLIB_TARGET_TRIPLES})
string(REGEX REPLACE "[ \t]+" ";" list "${entry}")
Expand All @@ -220,9 +222,11 @@ if(SWIFT_SHOULD_BUILD_EMBEDDED_STDLIB AND SWIFT_SHOULD_BUILD_EMBEDDED_CONCURRENC
set(extra_swift_compile_flags -Xcc -D__MACH__ -Xcc -D__APPLE__ -Xcc -ffreestanding)
endif()

set(SWIFT_SDK_embedded_THREADING_PACKAGE none)
set(SWIFT_SDK_embedded_ARCH_${mod}_MODULE "${mod}")
set(SWIFT_SDK_embedded_LIB_SUBDIR "embedded")
set(SWIFT_SDK_embedded_ARCH_${mod}_TRIPLE "${triple}")

add_swift_target_library_single(
embedded-concurrency-${mod}
swift_Concurrency
Expand Down
11 changes: 2 additions & 9 deletions test/embedded/dependencies-concurrency.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// Fail if there is any entry in actual-dependencies.txt that's not in allowed-dependencies.txt
// RUN: test -z "`comm -13 %t/allowed-dependencies.txt %t/actual-dependencies.txt`"

// DEP: __ZNSt3__111this_thread9sleep_forERKNS_6chrono8durationIxNS_5ratioILl1ELl1000000000EEEEE
// DEP: __ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEE6appendEPKc
// DEP: __ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEE6appendEPKcm
// DEP: __ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEE6insertEmPKc
Expand All @@ -20,12 +19,12 @@
// DEP: __ZdlPv
// DEP: __Znwm
// DEP: ___assert_rtn
// DEP: ___error
// DEP: ___stack_chk_fail
// DEP: ___stack_chk_guard
// DEP: ___stderrp
// DEP: __availability_version_check
// DEP: _abort
// DEP: _asl_log
// DEP: _dispatch_once_f
// DEP: _dlsym
// DEP: _exit
Expand All @@ -37,15 +36,9 @@
// DEP: _fseek
// DEP: _ftell
// DEP: _malloc
// DEP: _memcpy
// DEP: _memset_s
// DEP: _os_unfair_lock_lock
// DEP: _os_unfair_lock_unlock
// DEP: _nanosleep
// DEP: _posix_memalign
// DEP: _pthread_equal
// DEP: _pthread_getspecific
// DEP: _pthread_self
// DEP: _pthread_setspecific
// DEP: _putchar
// DEP: _rewind
// DEP: _sscanf
Expand Down