[CMake] Respect LLVM instrumentation variables in AddPureSwift.cmake #71135
Conversation
If one is building Swift against an instrumented LLVM build, when the functions in AddPureSwift.cmake tries to link Swift code that also links LLVM instrumented code, the linking will fail because the linker will not use the profiling runtimes. The modifications replicate the existing code in LLVM HandleLLVMOptions.cmake, but adapted to Swift variables where possible. The necessary arguments are forwarded through `-Xclang-linker` because the spelling of the Swift driver does not give access to all the options that Clang provides. This has been tested with an unified build, but a standalone build could still pass the `LLVM_*` variables to the Swift build system to make use of this code.
|
@swift-ci please test |
| # This replicates he code existing in LLVM llvm/cmake/modules/HandleLLVMOptions.cmake | ||
| # The second part of the clause replicates the LINKER_IS_LLD_LINK of the | ||
| # original. | ||
| if(LLVM_BUILD_INSTRUMENTED AND NOT (SWIFT_COMPILER_IS_MSVC_LIKE AND SWIFT_USE_LINKER STREQUAL "lld")) |
There was a problem hiding this comment.
The original code does not explain why those conditions are needed. The original commit that introduced the conditions doesn't explain it either: swiftlang/llvm-project@a84b200
The stable/20230725 code is in https://github.com/apple/llvm-project/blob/stable/20230725/llvm/cmake/modules/HandleLLVMOptions.cmake#L1095-L1133
The linker options are only added for cases that are NOT LINKER_IS_LLD_LINK
The values of LINKER_IS_LLD_LINK comes from https://github.com/apple/llvm-project/blob/stable/20230725/llvm/cmake/modules/HandleLLVMOptions.cmake#L20-L30
If I had read the things correctly, the requirements for LINKER_IS_LLD_LINK is that we are using MSVC (either clang-cl or the actual cl) and using LLD to link, which if I had translated correctly should happen when SWIFT_COMPILER_IS_MSVC_LIKE and when SWIFT_USE_LINKER is lld.
There was a problem hiding this comment.
Am I just failing to apply DeMorgan's Law? NOT (MSVC AND LLD) -> NOT MSVC OR NOT LLD. It would make sense as AND (NOT MSVC OR LLD).
There was a problem hiding this comment.
Let's see if I remember doing these things:
MSVC_LIKE = SWIFT_COMPILER_IS_MSVC_LIKE
LLD = SWIFT_USE_LINKER STREQUAL "lld"
| MSVC_LIKE | LLD | NOT (MSVC AND LLD) | NOT MSVC OR LLD |
| T | T | F | T |
| T | F | T | F |
| F | T | T | T |
| F | F | T | T |
For the version that I think I implemented, the only F is when MSVC and LLD are both T, which I think is the case in the original code. For the proposed version the case when MSVC and LLD is T, and the case of using MSVC but not LLD is F, which I think is the opposite of what we want.
PD: logic is hard, let's go shopping, and all the normal disclosures when doing booleans.
|
Thinking in merging this later today, if nobody else have any objections. |
…tables (#73629) In #71135 I added this code to `add_pure_swift_host_library` but failed to realize that it was also needed in `add_pure_swift_host_tool`. Extract the code into a helper function and invoke it from both functions in order to be able to instrument the `swift-plugin-server` and other pure Swift tools.
If one is building Swift against an instrumented LLVM build, when the functions in AddPureSwift.cmake tries to link Swift code that also links LLVM instrumented code, the linking will fail because the linker will not use the profiling runtimes.
The modifications replicate the existing code in LLVM HandleLLVMOptions.cmake, but adapted to Swift variables where possible.
The necessary arguments are forwarded through
-Xclang-linkerbecause the spelling of the Swift driver does not give access to all the options that Clang provides.This has been tested with an unified build, but a standalone build could still pass the
LLVM_*variables to the Swift build system to make use of this code.