Add a SWIFT_STDLIB_LLVM_LTO CMake option to enable building stdlib with LTO #34972
Conversation
|
@swift-ci please test |
| @@ -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 | |||
There was a problem hiding this comment.
What do you think of SWIFT_STDLIB_LTO_TYPE and having the options of none, thin, and full?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I think that llvm-thin and llvm-lto for the values is reasonable. The less we have to munge the values, the better.
| 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}") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Michael added this via #36570. |
No description provided.