Skip to content

[build] Make it possible to actually build the stdlib with a prebuilt clang #34628

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
swift-ci merged 3 commits into from
Jan 6, 2021
Merged

[build] Make it possible to actually build the stdlib with a prebuilt clang #34628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd81fb8
Revert "Revert "[build-script] Move --native-{clang,llvm,swift}-tools…
finagolfin Oct 26, 2020
c74884c
[build] Make it possible to actually build the stdlib with a prebuilt…
finagolfin Nov 8, 2020
934823a
[build] Replace SWIFT_CROSS_COMPILING with SWIFT_PREBUILT_CLANG
finagolfin Nov 11, 2020
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
8 changes: 5 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,10 +481,12 @@ if(SWIFT_PATH_TO_CMARK_BUILD)
endif()
message(STATUS "")

if("${SWIFT_NATIVE_LLVM_TOOLS_PATH}" STREQUAL "")
set(SWIFT_CROSS_COMPILING FALSE)
Copy link
Member Author

@finagolfin finagolfin Nov 11, 2020

Choose a reason for hiding this comment

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

@shahmishal, I believe the only reason this was set was because SWIFT_NATIVE_LLVM_TOOLS_PATH was used to find clang in stdlib/CMakeLists.txt below. However, since that was a mistake that I tried to fix in #34437, I now use this clang path check and SWIFT_PREBUILT_CLANG here and in cmake/modules/SwiftSharedCMakeConfig.cmake instead. I believe this SWIFT_PREBUILT_CLANG is a better variable name as the clang might not just be passed in for cross-compiling, but also for natively building the standalone stdlib, as this pull was originally meant for.

# Check if a prebuilt clang path was passed in, as this variable will be
# assigned if not, in SwiftSharedCMakeConfig.
if("${SWIFT_NATIVE_CLANG_TOOLS_PATH}" STREQUAL "")
set(SWIFT_PREBUILT_CLANG FALSE)
else()
set(SWIFT_CROSS_COMPILING TRUE)
set(SWIFT_PREBUILT_CLANG TRUE)
endif()
Copy link
Member

@compnerd compnerd Nov 11, 2020

Choose a reason for hiding this comment

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

Is there a reason to prefer this over just using standard CMAKE_C{,XX}_COMPILER?

Copy link
Member Author

@finagolfin finagolfin Nov 11, 2020

Choose a reason for hiding this comment

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

Yes, because the compiler and stdlib are built using potentially three clang compilers: the --host-cc is used to build a native Swift compiler, while the stdlib can also be built with either a freshly built Swift-forked clang that is usually built alongside this repo or a prebuilt clang that is passed in through --native-clang-tools-path. This new variable and SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER determine which of those three possible clangs to use when building the C++ portions of the stdlib.


include(SwiftSharedCMakeConfig)
Expand Down
4 changes: 2 additions & 2 deletions cmake/modules/SwiftSharedCMakeConfig.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ macro(swift_common_standalone_build_config_llvm product)
fix_imported_targets_for_xcode("${LLVM_EXPORTED_TARGETS}")
endif()

if(NOT CMAKE_CROSSCOMPILING AND NOT SWIFT_CROSS_COMPILING)
if(NOT CMAKE_CROSSCOMPILING)
set(${product}_NATIVE_LLVM_TOOLS_PATH "${LLVM_TOOLS_BINARY_DIR}")
endif()

Expand Down Expand Up @@ -159,7 +159,7 @@ endmacro()
macro(swift_common_standalone_build_config_clang product)
find_package(Clang CONFIG REQUIRED NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)

if (NOT CMAKE_CROSSCOMPILING)
if (NOT CMAKE_CROSSCOMPILING AND NOT SWIFT_PREBUILT_CLANG)
set(${product}_NATIVE_CLANG_TOOLS_PATH "${LLVM_TOOLS_BINARY_DIR}")
endif()

Expand Down
8 changes: 4 additions & 4 deletions stdlib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,11 @@ else()
# If we use Clang-cl or MSVC, CMake provides default compiler and linker flags that are incompatible
# with the frontend of Clang or Clang++.
if(SWIFT_COMPILER_IS_MSVC_LIKE)
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang-cl")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang-cl")
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang-cl")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang-cl")
else()
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang++")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang")
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang++")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang")
endif()

if(CMAKE_C_COMPILER_LAUNCHER MATCHES ".*distcc")
Expand Down
3 changes: 2 additions & 1 deletion stdlib/cmake/modules/AddSwiftStdlib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,8 @@ function(add_swift_target_library name)
list(APPEND SWIFTLIB_SWIFT_COMPILE_FLAGS "-warn-implicit-overrides")
endif()

if(NOT SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER AND NOT BUILD_STANDALONE)
if(NOT SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER AND NOT BUILD_STANDALONE AND
NOT SWIFT_PREBUILT_CLANG)
Copy link
Member Author

@finagolfin finagolfin Nov 8, 2020

Choose a reason for hiding this comment

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

@gottesmm, I'm guessing you passed in --build-runtime-with-host-compiler=1 to the standalone stdlib preset only to keep this from erroring out? This and the next change make that flag no longer necessary.

list(APPEND SWIFTLIB_DEPENDS clang)
endif()

Expand Down
3 changes: 2 additions & 1 deletion stdlib/public/SwiftShims/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ endif()
# First extract the "version" used for Clang's resource directory.
string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
"${LLVM_PACKAGE_VERSION}")
if(NOT SWIFT_INCLUDE_TOOLS AND SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER)
if(NOT SWIFT_INCLUDE_TOOLS AND
(SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER OR SWIFT_PREBUILT_CLANG))
Copy link
Member Author

@finagolfin finagolfin Nov 11, 2020

Choose a reason for hiding this comment

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

@gottesmm and @shahmishal, should this be using the passed-in prebuilt compiler's resource directory even when SWIFT_INCLUDE_TOOLS is true, ie should this be if(SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER OR SWIFT_PREBUILT_CLANG) instead? This only applies when building the standalone stdlib now, but since you guys have started cross-compiling the full Mac toolchain with SWIFT_NATIVE_CLANG_TOOLS_PATH set (as do I when cross-compiling the Termux toolchain for Android), I'm not sure which resource directory should be used, the freshly built one that's now used or the one that comes with the prebuilt clang, or if it matters.

Copy link
Member

@compnerd compnerd Nov 11, 2020

Choose a reason for hiding this comment

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

This code path uses CMAKE_C_COMPILER which no longer is setup by build-script-impl now. How is this intended to work? It seems a bit confusing.

Copy link
Member Author

@finagolfin finagolfin Nov 11, 2020

Choose a reason for hiding this comment

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

It is still using it, it may help if you look at all three commits in this pull together.

I agree that all the CMAKE_C_COMPILER invocations are confusing, as I explained in #33675 a couple weeks ago (see that original comment for the links too):

The build-script requires a host_cc to be set,
which it looks up in the path or can be passed
in with --host-cc and then it sets CMAKE_{C,CXX}_COMPILER
for all CMake invocations with that host clang/clang++.
Then, several build products override that with
another CMAKE_{C,CXX}_COMPILER invocation,
either set to the freshly built Swift clang or native_clang_tools_path, #32922.

On top of this, you've now added a third CMAKE_{C,CXX}_COMPILER
invocation for all build-script-impl CMake invocations,
including when compiling CMark, LLVM, and Swift.

I'm only removing that third invocation he added in this pull, CMAKE_C_COMPILER is still set for the stdlib using SWIFT_NATIVE_CLANG_TOOLS_PATH in stdlib/CMakeLists.txt in this pull.

if(SWIFT_COMPILER_IS_MSVC_LIKE)
execute_process(COMMAND ${CMAKE_C_COMPILER} /clang:-print-resource-dir
OUTPUT_VARIABLE clang_headers_location
Expand Down
8 changes: 4 additions & 4 deletions unittests/runtime/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ if(("${SWIFT_HOST_VARIANT_SDK}" STREQUAL "${SWIFT_PRIMARY_VARIANT_SDK}") AND
# If we use Clang-cl or MSVC, CMake provides default compiler and linker flags that are incompatible
# with the frontend of Clang or Clang++.
if(SWIFT_COMPILER_IS_MSVC_LIKE)
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang-cl")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang-cl")
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang-cl")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang-cl")
else()
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang++")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_LLVM_TOOLS_PATH}/clang")
set(CMAKE_CXX_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang++")
set(CMAKE_C_COMPILER "${SWIFT_NATIVE_CLANG_TOOLS_PATH}/clang")
endif()

if(CMAKE_C_COMPILER_LAUNCHER MATCHES ".*distcc")
Expand Down
5 changes: 2 additions & 3 deletions utils/build-presets.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2283,10 +2283,9 @@ skip-build-cmark
skip-build-benchmarks
skip-test-cmark

# This triggers the stdlib standalone build: Don't build tools (the compiler),
# assume we are working with the host compiler.
# This triggers the stdlib standalone build: don't build the native tools from
# scratch, ie the compiler.
build-swift-tools=0
build-runtime-with-host-compiler=1

# Then set the paths to our native tools. If compiling against a toolchain,
# these should all be the ./usr/bin directory.
Expand Down
12 changes: 12 additions & 0 deletions utils/build-script
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,18 @@ class BuildScriptInvocation(object):
impl_args += [
"--host-libtool", toolchain.libtool,
]
if args.native_clang_tools_path is not None:
impl_args += [
"--native-clang-tools-path=%s" % args.native_clang_tools_path
]
if args.native_llvm_tools_path is not None:
impl_args += [
"--native-llvm-tools-path=%s" % args.native_llvm_tools_path
]
if args.native_swift_tools_path is not None:
impl_args += [
"--native-swift-tools-path=%s" % args.native_swift_tools_path
]

# If we have extra_swift_args, combine all of them together and then
# add them as one command.
Expand Down
7 changes: 0 additions & 7 deletions utils/build-script-impl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,13 +1485,6 @@ for host in "${ALL_HOSTS[@]}"; do
fi
fi

if [[ "${NATIVE_CLANG_TOOLS_PATH}" ]] ; then
common_cmake_options_host+=(
-DCMAKE_C_COMPILER="${NATIVE_CLANG_TOOLS_PATH}/clang"
-DCMAKE_CXX_COMPILER="${NATIVE_CLANG_TOOLS_PATH}/clang++"
)
fi

llvm_cmake_options=(
"${llvm_cmake_options[@]}"
-DCMAKE_INSTALL_PREFIX:PATH="$(get_host_install_prefix ${host})"
Expand Down
9 changes: 9 additions & 0 deletions utils/build_swift/build_swift/driver_arguments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,15 @@ def create_argument_parser():
option('--host-cxx', store_path(executable=True),
help='the absolute path to CXX, the "clang++" compiler for the '
'host platform. Default is auto detected.')
option('--native-swift-tools-path', store_path,
help='the path to a directory that contains prebuilt Swift tools '
'that are executable on the host platform')
option('--native-clang-tools-path', store_path,
help='the path to a directory that contains prebuilt Clang tools '
'that are executable on the host platform')
option('--native-llvm-tools-path', store_path,
help='the path to a directory that contains prebuilt LLVM tools '
'that are executable on the host platform')
option('--cmake-c-launcher', store_path(executable=True),
default=os.environ.get('C_COMPILER_LAUNCHER', None),
help='the absolute path to set CMAKE_C_COMPILER_LAUNCHER')
Expand Down
6 changes: 6 additions & 0 deletions utils/build_swift/tests/expected_options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@
'lto_type': None,
'maccatalyst': False,
'maccatalyst_ios_tests': False,
'native_clang_tools_path': None,
'native_llvm_tools_path': None,
'native_swift_tools_path': None,
'dump_config': False,
'show_sdks': False,
'skip_build': False,
Expand Down Expand Up @@ -653,6 +656,9 @@ class BuildScriptImplOption(_BaseOption):
PathOption('--install-symroot'),
PathOption('--install-destdir'),
EnableOption('--install-all'),
PathOption('--native-clang-tools-path'),
PathOption('--native-llvm-tools-path'),
PathOption('--native-swift-tools-path'),
PathOption('--symbols-package'),
PathOption('--cmake-c-launcher'),
PathOption('--cmake-cxx-launcher'),
Expand Down
5 changes: 4 additions & 1 deletion utils/swift_build_support/swift_build_support/products/product.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,11 @@ def install_toolchain_path(self, host_target):
"""toolchain_path() -> string

Returns the path to the toolchain that is being created as part of this
build.
build, or to a native prebuilt toolchain that was passed in.
Copy link
Member Author

@finagolfin finagolfin Dec 11, 2020

Choose a reason for hiding this comment

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

Rebased and updated this comment.

"""
if self.args.native_swift_tools_path is not None:
return os.path.split(self.args.native_swift_tools_path)[0]

install_destdir = self.args.install_destdir
if self.args.cross_compile_hosts:
build_root = os.path.dirname(self.build_dir)
Expand Down