From 9d82c51323c4486e1d46c4e5cafc7058edf61c5d Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 9 May 2023 11:17:41 +0100 Subject: [PATCH 1/5] CMake: fix missing `SWIFT_CONCURRENCY_GLOBAL_EXECUTOR` value `SWIFT_CONCURRENCY_GLOBAL_EXECUTOR` is defined in `stdlib/cmake/modules/StdlibOptions.cmake`, which is not included during the first pass of evaluation of the root `CMakeLists.txt`. It is available on subsequent evaluations after the value is stored in CMake cache. This led to subtle bugs, where `usr/lib/swift_static/linux/static-stdlib-args.lnk` didn't contain certain flags on clean toolchain builds, but did contain them in incremental builds. Not having these autolinking flags in toolchain builds leads to errors when statically linking executables on Linux. --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index d4778f4d251b5..ea5e23dbb7709 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -202,6 +202,10 @@ set(SWIFT_HOST_VARIANT_ARCH "${SWIFT_HOST_VARIANT_ARCH_default}" CACHE STRING # This is primarily to support building smaller or faster project files. # +# Subsequent options may refer to `StdlibOptions`, which have to be defined first. +list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/stdlib/cmake/modules) +include(StdlibOptions) + option(SWIFT_APPEND_VC_REV "Embed the version control system revision in Swift" TRUE) From 7d1e9a7b74245f8022309b24704c2876dec303d1 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 10 May 2023 14:05:25 +0100 Subject: [PATCH 2/5] Fix `-DSWIFT_PATH_TO_LIBDISPATCH_STATIC_BUILD` value --- test/Driver/static-stdlib-autolink-linux.swift | 3 +-- test/Driver/static-stdlib-linux.swift | 2 +- test/lit.cfg | 18 ++++++++---------- utils/build-script-impl | 2 +- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/test/Driver/static-stdlib-autolink-linux.swift b/test/Driver/static-stdlib-autolink-linux.swift index 2877c48d5c192..7d24e424c0b28 100644 --- a/test/Driver/static-stdlib-autolink-linux.swift +++ b/test/Driver/static-stdlib-autolink-linux.swift @@ -7,8 +7,7 @@ // RUN: echo 'public func asyncFunc() async { print("Hello") }' > %t/asyncModule.swift // RUN: %target-swiftc_driver -emit-library -emit-module -module-name asyncModule -module-link-name asyncModule %t/asyncModule.swift -static -static-stdlib -o %t/libasyncModule.a -// TODO: "-ldispatch -lBlocksRuntime" should be told by asyncModule.swiftmodule transitively -// RUN: %target-swiftc_driver -parse-as-library -static -static-stdlib -module-name main %s %import-static-libdispatch -I%t -L%t -ldispatch -lBlocksRuntime -o %t/main +// RUN: %target-swiftc_driver -parse-as-library -static -static-stdlib -module-name main %s %import-static-libdispatch -I%t -L%t -o %t/main // RUN: %t/main | %FileCheck %s // CHECK: Hello diff --git a/test/Driver/static-stdlib-linux.swift b/test/Driver/static-stdlib-linux.swift index 59d5e290efe32..e9cad2053867d 100644 --- a/test/Driver/static-stdlib-linux.swift +++ b/test/Driver/static-stdlib-linux.swift @@ -3,7 +3,7 @@ // REQUIRES: static_stdlib print("hello world!") // RUN: %empty-directory(%t) -// RUN: %target-swiftc_driver -static-stdlib -o %t/static-stdlib %s +// RUN: %target-swiftc_driver %import-static-libdispatch -static-stdlib -o %t/static-stdlib %s // RUN: %t/static-stdlib | %FileCheck %s // RUN: ldd %t/static-stdlib | %FileCheck %s --check-prefix=LDD // CHECK: hello world! diff --git a/test/lit.cfg b/test/lit.cfg index 407f17ec0e2ec..bffd7014a3e73 100644 --- a/test/lit.cfg +++ b/test/lit.cfg @@ -1591,19 +1591,13 @@ elif (run_os in ['linux-gnu', 'linux-gnueabihf', 'freebsd', 'openbsd', 'windows- config.import_libdispatch = ('-I %s -I %s -L %s' % (libdispatch_source_dir, libdispatch_swift_module_dir, libdispatch_artifact_dir)) - libdispatch_static_artifact_dir = config.libdispatch_static_build_path - libdispatch_swift_static_module_dir = make_path(libdispatch_static_artifact_dir, 'src', 'swift', 'swift') + libdispatch_static_artifact_dir = os.path.join(config.libdispatch_static_build_path, 'lib') libdispatch_static_artifacts = [ - make_path(libdispatch_static_artifact_dir, 'src', 'libdispatch.a'), - make_path(libdispatch_static_artifact_dir, 'src', 'swift', 'libswiftDispatch.a'), - make_path(libdispatch_swift_static_module_dir, 'Dispatch.swiftmodule')] + make_path(libdispatch_static_artifact_dir, 'libdispatch.a'), + make_path(libdispatch_static_artifact_dir, 'libBlocksRuntime.a')] if (all(os.path.exists(p) for p in libdispatch_static_artifacts)): config.available_features.add('libdispatch_static') - config.import_libdispatch_static = ('-I %s -I %s -L %s -L %s -L %s' - % (libdispatch_source_dir, libdispatch_swift_static_module_dir, - make_path(libdispatch_static_artifact_dir, 'src'), - make_path(libdispatch_static_artifact_dir, 'src', 'BlocksRuntime'), - make_path(libdispatch_static_artifact_dir, 'src', 'swift'))) + config.import_libdispatch_static = '-L %s' % libdispatch_static_artifact_dir config.target_build_swift = ( '%s -target %s -toolchain-stdlib-rpath %s %s %s %s %s' @@ -2644,6 +2638,10 @@ run_filecheck = '%s %s --allow-unused-prefixes --sanitize BUILD_DIR=%s --sanitiz config.substitutions.append(('%FileCheck', run_filecheck)) config.substitutions.append(('%raw-FileCheck', shell_quote(config.filecheck))) config.substitutions.append(('%import-libdispatch', getattr(config, 'import_libdispatch', ''))) +# WARNING: the order of components in a substitution name has to be different from the previous one, as lit does +# a pure string substitution without understanding that these components are grouped together. That is, the following +# subsitution name can't be `%import-libdispatch-static`, otherwise the first two components will be substituted with +# the value of `%import-libdispatch` substitution with `-static` string appended to it. config.substitutions.append(('%import-static-libdispatch', getattr(config, 'import_libdispatch_static', ''))) # Disable COW sanity checks in the swift runtime by default. diff --git a/utils/build-script-impl b/utils/build-script-impl index 9721000de0eed..a5f8e7ca5fd8b 100755 --- a/utils/build-script-impl +++ b/utils/build-script-impl @@ -1901,7 +1901,7 @@ for host in "${ALL_HOSTS[@]}"; do -DSWIFT_PATH_TO_CMARK_BUILD:PATH="$(build_directory ${host} cmark)" -DSWIFT_PATH_TO_LIBDISPATCH_SOURCE:PATH="${LIBDISPATCH_SOURCE_DIR}" -DSWIFT_PATH_TO_LIBDISPATCH_BUILD:PATH="$(build_directory ${host} libdispatch)" - -DSWIFT_PATH_TO_LIBDISPATCH_STATIC_BUILD:PATH="$(build_directory ${host} libdispatch_static)" + -DSWIFT_PATH_TO_LIBDISPATCH_STATIC_BUILD:PATH="$(build_directory ${host} swift)/$(basename $(build_directory ${host} libdispatch))-static-prefix" ) if [[ "${SWIFT_SDKS}" ]] ; then From 88da2d7e0cee388e4e2efe734e518be8912e62f1 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 10 May 2023 19:36:27 +0100 Subject: [PATCH 3/5] Move `SWIFT_CONCURRENCY_USES_DISPATCH` to `StdlibOptions.cmake` It is only used in the stdlib build, so really has no business being set in the root `CMakeLists.txt`. --- CMakeLists.txt | 11 ++--------- stdlib/cmake/modules/StdlibOptions.cmake | 13 +++++++++++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ea5e23dbb7709..e46a7ab473cb2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -700,13 +700,6 @@ if(NOT EXISTS "${SWIFT_PATH_TO_SWIFT_SYNTAX_SOURCE}") message(SEND_ERROR "swift-syntax is required to build the Swift compiler. Please run update-checkout or specify SWIFT_PATH_TO_SWIFT_SYNTAX_SOURCE") endif() -# Use dispatch as the system scheduler by default. -# For convenience, we set this to false when concurrency is disabled. -set(SWIFT_CONCURRENCY_USES_DISPATCH FALSE) -if(SWIFT_ENABLE_EXPERIMENTAL_CONCURRENCY AND "${SWIFT_CONCURRENCY_GLOBAL_EXECUTOR}" STREQUAL "dispatch") - set(SWIFT_CONCURRENCY_USES_DISPATCH TRUE) -endif() - set(SWIFT_BUILD_HOST_DISPATCH FALSE) if(SWIFT_ENABLE_DISPATCH AND NOT CMAKE_SYSTEM_NAME STREQUAL Darwin) # Only build libdispatch for the host if the host tools are being built and @@ -715,9 +708,9 @@ if(SWIFT_ENABLE_DISPATCH AND NOT CMAKE_SYSTEM_NAME STREQUAL Darwin) set(SWIFT_BUILD_HOST_DISPATCH TRUE) endif() - if(SWIFT_BUILD_HOST_DISPATCH OR SWIFT_CONCURRENCY_USES_DISPATCH) + if(SWIFT_BUILD_HOST_DISPATCH) if(NOT EXISTS "${SWIFT_PATH_TO_LIBDISPATCH_SOURCE}") - message(SEND_ERROR "SourceKit and concurrency require libdispatch on non-Darwin hosts. Please specify SWIFT_PATH_TO_LIBDISPATCH_SOURCE") + message(SEND_ERROR "SourceKit requires libdispatch on non-Darwin hosts. Please specify SWIFT_PATH_TO_LIBDISPATCH_SOURCE") endif() endif() endif() diff --git a/stdlib/cmake/modules/StdlibOptions.cmake b/stdlib/cmake/modules/StdlibOptions.cmake index 880d45be93d72..3fc9813675167 100644 --- a/stdlib/cmake/modules/StdlibOptions.cmake +++ b/stdlib/cmake/modules/StdlibOptions.cmake @@ -239,3 +239,16 @@ set(SWIFT_RUNTIME_FIXED_BACKTRACER_PATH "" CACHE STRING "If set, provides a fixed path to the swift-backtrace binary. This will disable dynamic determination of the path and will also disable the setting in SWIFT_BACKTRACE.") + +# Use dispatch as the system scheduler by default. +# For convenience, we set this to false when concurrency is disabled. +set(SWIFT_CONCURRENCY_USES_DISPATCH FALSE) +if(SWIFT_ENABLE_EXPERIMENTAL_CONCURRENCY AND "${SWIFT_CONCURRENCY_GLOBAL_EXECUTOR}" STREQUAL "dispatch") + set(SWIFT_CONCURRENCY_USES_DISPATCH TRUE) +endif() + +if(SWIFT_CONCURRENCY_USES_DISPATCH) + if(NOT EXISTS "${SWIFT_PATH_TO_LIBDISPATCH_SOURCE}") + message(SEND_ERROR "Concurrency require libdispatch on non-Darwin hosts. Please specify SWIFT_PATH_TO_LIBDISPATCH_SOURCE") + endif() +endif() From 98d02b891840ba3736db9efebe68502d85a15e0c Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 10 May 2023 19:37:51 +0100 Subject: [PATCH 4/5] Remove redundant `include(StdlibOptions)` --- CMakeLists.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e46a7ab473cb2..3c5626cbe50b4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -202,10 +202,6 @@ set(SWIFT_HOST_VARIANT_ARCH "${SWIFT_HOST_VARIANT_ARCH_default}" CACHE STRING # This is primarily to support building smaller or faster project files. # -# Subsequent options may refer to `StdlibOptions`, which have to be defined first. -list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/stdlib/cmake/modules) -include(StdlibOptions) - option(SWIFT_APPEND_VC_REV "Embed the version control system revision in Swift" TRUE) From be35d465bf75b2eed958927f06b49a97f8887d1a Mon Sep 17 00:00:00 2001 From: Dario Rexin Date: Tue, 23 May 2023 13:17:14 -0700 Subject: [PATCH 5/5] [Build] Only require libdispatch source on non-Darwin (#66081) --- stdlib/cmake/modules/StdlibOptions.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/cmake/modules/StdlibOptions.cmake b/stdlib/cmake/modules/StdlibOptions.cmake index 3fc9813675167..1116df016a8cb 100644 --- a/stdlib/cmake/modules/StdlibOptions.cmake +++ b/stdlib/cmake/modules/StdlibOptions.cmake @@ -247,7 +247,7 @@ if(SWIFT_ENABLE_EXPERIMENTAL_CONCURRENCY AND "${SWIFT_CONCURRENCY_GLOBAL_EXECUTO set(SWIFT_CONCURRENCY_USES_DISPATCH TRUE) endif() -if(SWIFT_CONCURRENCY_USES_DISPATCH) +if(SWIFT_CONCURRENCY_USES_DISPATCH AND NOT CMAKE_SYSTEM_NAME STREQUAL Darwin) if(NOT EXISTS "${SWIFT_PATH_TO_LIBDISPATCH_SOURCE}") message(SEND_ERROR "Concurrency require libdispatch on non-Darwin hosts. Please specify SWIFT_PATH_TO_LIBDISPATCH_SOURCE") endif()