From 5897ab557e285b93af6e43b4a7adbb65d75d8aa2 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 9 May 2023 11:17:41 +0100 Subject: [PATCH 1/4] 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 f2ab512a9f20f..a7bf97d3cf3c3 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 2db62cade8a2d77f644eaa1aff8de62f7610e2c3 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 10 May 2023 14:05:25 +0100 Subject: [PATCH 2/4] 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 0a24c74c02b9d..9564c4ec6c9db 100644 --- a/test/lit.cfg +++ b/test/lit.cfg @@ -1593,19 +1593,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' @@ -2649,6 +2643,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 f832dcb0f85a8..04ed848c1c5e4 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 5275255d58fc3d63d554e52cd835006f7a583bcc Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 10 May 2023 19:36:27 +0100 Subject: [PATCH 3/4] 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 a7bf97d3cf3c3..31d9c2265dc66 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -691,13 +691,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 @@ -706,9 +699,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 a25432dada25a0d5dfd30561e6ee92c61cd6f174 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 10 May 2023 19:37:51 +0100 Subject: [PATCH 4/4] Remove redundant `include(StdlibOptions)` --- CMakeLists.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 31d9c2265dc66..c7350335bb84d 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)