Skip to content

Add a SWIFT_STDLIB_LLVM_LTO CMake option to enable building stdlib with LTO #34972

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

Closed
wants to merge 1 commit into from
Closed

Add a SWIFT_STDLIB_LLVM_LTO CMake option to enable building stdlib with LTO #34972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions stdlib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ option(SWIFT_BUILD_TEST_SUPPORT_MODULES
"Whether to build StdlibUnittest and other test support modules. Defaults to On when SWIFT_BUILD_SDK_OVERLAY is On, or when SWIFT_INCLUDE_TESTS is On."
"${SWIFT_BUILD_TEST_SUPPORT_MODULES_default}")

set(SWIFT_STDLIB_LLVM_LTO OFF CACHE STRING
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of SWIFT_STDLIB_LTO_TYPE and having the options of none, thin, and full?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question: The frontend values for lto are called "llvm-thin" and "llvm-full", I assume to explicitly point out they're enabling LLVM IR LTO, and not any other form of LTO (future SIL LTO, perhaps). Should we keep the "LLVM" name in the variable? Or in the values?

Copy link
Member

Choose a reason for hiding this comment

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

I think that llvm-thin and llvm-lto for the values is reasonable. The less we have to munge the values, the better.

"Build stdlib with LLVM LTO. Valid values are 'OFF' (the default), 'thin' and 'full'.")

#
# End of user-configurable options.
#
Expand Down
4 changes: 3 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 @@ -1161,7 +1161,9 @@ function(_add_swift_target_library_single target name)
set(analyze_code_coverage "${SWIFT_ANALYZE_CODE_COVERAGE}")
endif()

if (NOT SWIFTLIB_SINGLE_TARGET_LIBRARY)
if(SWIFTLIB_SINGLE_TARGET_LIBRARY)
set(lto_type "${SWIFT_STDLIB_LLVM_LTO}")
else()
set(lto_type "${SWIFT_TOOLS_ENABLE_LTO}")
Copy link
Member

Choose a reason for hiding this comment

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

I think that this may be dead code, not sure if you have the time/interest/inclination to check and cleanup. It was here before though.

Copy link
Contributor Author

@kubamracek kubamracek Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Honestly, I fail to understand what does "SWIFTLIB_SINGLE_TARGET_LIBRARY" mean and also what it means when it's set or not set. I just noticed in git history that "being set" is used to mean "we're building the stdlib", so that's how I'm using it here too.

It's used in a whole bunch of places, though, so I'm a bit worried about just eliminating it...

Do you have any more knowledge you could share here?

Copy link
Member

Choose a reason for hiding this comment

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

Its dead code, its from before CMake supported Swift properly and the custom build system implementation in Swift. The SWIFTLIB_SINGLE_TARGET_LIBRARY simply meant it was building a target library (which is always true under the stdlib directory) under a single target (i.e. it is one of the slices of the final MachO binary). The prefix here was for the old name of the function. I think that cleaning this up, which would be appreciated, is not necessarily in scope for the changes you are doing, and would really be a separate commit any way.

endif()

Expand Down
18 changes: 17 additions & 1 deletion stdlib/cmake/modules/SwiftSource.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ function(swift_optimize_flag_for_build_type build_type result_var_name)
endif()
endfunction()

function(swift_lto_flag option out_var)
string(TOLOWER "${option}" lowercase_option)
if (lowercase_option STREQUAL "full")
set(${out_var} "-lto=llvm-full" PARENT_SCOPE)
elseif (lowercase_option STREQUAL "thin")
set(${out_var} "-lto=llvm-thin" PARENT_SCOPE)
endif()
endfunction()

# Process the sources within the given variable, pulling out any Swift
# sources to be compiled with 'swift' directly. This updates
# ${sourcesvar} in place with the resulting list and ${externalvar} with the
Expand Down Expand Up @@ -370,6 +379,7 @@ function(_compile_swift_files

# Compute flags for the Swift compiler.
set(swift_flags)
set(swift_o_flags)
set(swift_module_flags)

_add_target_variant_swift_compile_flags(
Expand Down Expand Up @@ -456,6 +466,12 @@ function(_compile_swift_files
list(APPEND swift_flags "-Xfrontend" "-disable-objc-interop")
endif()

swift_lto_flag("${SWIFT_STDLIB_LLVM_LTO}" _lto_flag_out)
if (_lto_flag_out)
list(APPEND swift_flags ${_lto_flag_out})
list(APPEND swift_o_flags "-emit-bc")
endif()

list(APPEND swift_flags ${SWIFT_EXPERIMENTAL_EXTRA_FLAGS})

if(SWIFTFILE_OPT_FLAGS)
Expand Down Expand Up @@ -753,7 +769,7 @@ function(_compile_swift_files
dependency_target
COMMAND
"$<TARGET_FILE:Python3::Interpreter>" "${line_directive_tool}" "@${file_path}" --
"${swift_compiler_tool}" "${main_command}" ${swift_flags}
"${swift_compiler_tool}" "${main_command}" ${swift_flags} ${swift_o_flags}
${output_option} ${embed_bitcode_option} "@${file_path}"
${command_touch_standard_outputs}
OUTPUT ${standard_outputs}
Expand Down
8 changes: 8 additions & 0 deletions utils/build-script-impl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ KNOWN_SETTINGS=(
swift-stdlib-single-threaded-runtime "0" "whether to build stdlib as a single-threaded runtime only"
swift-stdlib-os-versioning "1" "whether to build stdlib with availability based on OS versions (Darwin only)"
swift-stdlib-stable-abi "" "should stdlib be built with stable ABI, if not set defaults to true on Darwin, false otherwise"
swift-stdlib-llvm-lto "" "should stdlib be built with llvm lto, if set must be either 'thin' or 'full'"

## FREESTANDING Stdlib Options
swift-freestanding-sdk "" "which SDK to use when building the FREESTANDING stdlib"
Expand Down Expand Up @@ -1959,6 +1960,13 @@ for host in "${ALL_HOSTS[@]}"; do
)
fi

if [[ "${SWIFT_STDLIB_LLVM_LTO}" ]] ; then
cmake_options=(
"${cmake_options[@]}"
-DSWIFT_STDLIB_LLVM_LTO:STRING="${SWIFT_STDLIB_LLVM_LTO}"
)
fi

if [ "${SWIFT_INSTALL_COMPONENTS}" ] ; then
cmake_options=(
"${cmake_options[@]}"
Expand Down