Reland "[windows][toolchain] Enable builtins and sanitizers" #78630
Conversation
weliveindetail
requested review from
slavapestov,
rjmccall,
artemcm and
compnerd
as code owners
| armv7-unknown-linux-androideabi) | ||
| endif() | ||
|
|
||
| set(LLVM_BUILTIN_TARGETS ${default_targets} CACHE STRING "") |
There was a problem hiding this comment.
Generally speaking, mutating lists is generally considered bad form with CMake. I think that it might be better to just have the duplication here which would also make it easier to follow.
There was a problem hiding this comment.
Done in Windows-x86_64. If it works in CI, I will replicate it here.
| set(BUILTINS_${target}_CMAKE_C_COMPILER_TARGET "${target}21" CACHE STRING "") | ||
| set(BUILTINS_${target}_CMAKE_CXX_COMPILER_TARGET "${target}21" CACHE STRING "") |
There was a problem hiding this comment.
How about re-using ${BUILTINS_${target}_CMAKE_ANDROID_API} instead of replicating 21 here?
There was a problem hiding this comment.
Added explicit CMAKE_ANDROID_API with a note to keep it in sync with build.ps1 (which is unfortunate, but I don't see how to do it better)
test/Driver/sanitize_coverage.swift
Outdated
| @@ -1,3 +1,5 @@ | |||
| // UNSUPPORTED: OS=windows-msvc | |||
There was a problem hiding this comment.
Is this due to the missing functionality in the driver?
There was a problem hiding this comment.
Yes, I think that was the case back in November. I had found a similar case when trying to build the toolchain with sanitizers enabled here: weliveindetail@5551c86
There was a problem hiding this comment.
Can you file an issue to track re-enabling the set of tests that have been disabled here? The UNSUPPORTED: OS=windows-msvc means that we cannot identify when the test starts to work.
There was a problem hiding this comment.
Right, let's see if they can be XFAIL as well
|
@swift-ci please test Windows platform |
|
The build failed while testing compiler-rt changes can have a wide range of side-effects, but this really seems unrelated: |
|
That looks like a nondeterministic failure I’m seeing for the first time. I’m starting to understand the problem and am working on a solution. Re-running CI should resolve the failure for now. |
|
@swift-ci Please test Windows |
…oss-compilation LLVM Runtimes contain various compiler-rt components like builtins, crt, profile and sanitizer libraries, etc. The LLVM project spawns a nested build-tree for each configured target architecture. For Windows we aim to build builtins, profile and sanitizer for x86_64, aarch64 and i686. In regular builds LLVM selects the just-built Clang compiler for cross-compilation in the nested build-trees. In cross-builds, it falls back to the default host compiler. On Windows this resolves to MSVC, which doesn't work. We have to override the compiler in each nested build-tree manually, to select the stage-1 Clang compiler, that we use for the LLVM Project build itself.
weliveindetail
force-pushed
the
reland-windows-toolchain-sanitizers
branch
from
8cc55b6 to
16d8eff
Compare
|
@swift-ci Please test Windows |
|
That worked. Let's make sure we can turn UNSUPPORTED into XFAIL and check the other platforms as well. @swift-ci Please test |
|
The Linux bot failed during checkout. Previous PR builds are running fine, so it might be a temporary problem. |
|
It's too hard to build Android SDKs that way. Let's do for standalone builds everywhere: #78861 |
Profile and builtin libraries from compiler-rt are static and do not require a link step. We can enable them in our CMake caches and we can build and install them in the unified LLVM build step.
Sanitizers contain dynamic libraries. We need the target-specific Visual Studio shell to perform the link step. This patch adds extra build steps in build.ps1 that reconfigure, build and install the target-specific nested build-trees in the LLVM runtimes directory through their respective Visual Studio shells.
My previous attempt in #77770 failed, because it didn't handle cross-compile cases properly. Since the just-built compiler cannot be executed in a cross-build, the LLVM build step was falling back to the host compiler. On Windows this resolves to MSVC, which doesn't work.
This PR aims to fix that and reland the feature. We have to override the compiler in each nested build-tree manually, to select the stage-1 Clang compiler that we use for the LLVM build step itself.