Fix an MSVC build issue. #111557
Fix an MSVC build issue. #111557
Conversation
Disable the LSE files if COMPILER_RT_HAS_ASM_LSE failed as MSVC cannot seem to handle .S input files. ``` [258/401] Building ASM object CMakeFiles\clang_rt.builtins-aarch64.dir\outline_atomic_helpers.dir\outline_atomic_cas1_3.S.obj Microsoft (R) C/C++ Optimizing Compiler Version 19.38.33135 for ARM64 Copyright (C) Microsoft Corporation. All rights reserved. cl : Command line warning D9035 : option 'o' has been deprecated and will be removed in a future release cl : Command line warning D9024 : unrecognized source file type 'S:\b\5\runtimes\builtins-aarch64-unknown-windows-msvc-bins\outline_atomic_helpers.dir\outline_atomic_cas1_3.S', object file assumed cl : Command line warning D9027 : source file 'S:\b\5\runtimes\builtins-aarch64-unknown-windows-msvc-bins\outline_atomic_helpers.dir\outline_atomic_cas1_3.S' ignored cl : Command line warning D9021 : no action performed ```
|
@compnerd This should fix the arm64 rebranch issue. swiftlang#9394 |
|
If the compiler can't handle I think the change looks reasonable, but I'm not entirely familiar with the context/scope here. |
There was a problem hiding this comment.
Silently skipping compiling bits of compiler-rt seems bad... it seems likely to lead to a situation where a toolchain is broken in a very subtle way. These particular bits of compiler-rt probably aren't that important in most cases, but still, it seems problematic. Could you explain why you're building compiler-rt with MSVC, instead of doing the usual LLVM_ENABLE_RUNTIMES build?
| endforeach(model) | ||
| endforeach(size) | ||
| endforeach(pat) | ||
| if(COMPILER_RT_HAS_ASM_LSE) |
There was a problem hiding this comment.
COMPILER_RT_HAS_ASM_LSE is not the right flag for this; it will turn off these files for other builds where they should be included. If we really want to explicitly exclude MSVC, if (NOT MSVC) is the right check.
There was a problem hiding this comment.
if (NOT MSVC) also covers builds in clang-cl mode (as the cmake MSVC flag primarily indicates the general command line style of the compiler - I think it also is set for the intel compiler for windows, that also uses a cl-style interface).
To properly single out MSVC as opposed to clang-cl, we'd need to check CMAKE_C_COMPILER_ID.
I would presume that they're building compiler-rt with MSVC, for use with MSVC - primarily the sanitizers, but they're getting the builtins at the same time, even if they don't really need/use them. (AFAIK they've been shipping that in some form for years already, but probably with a number of downstream changes to get it to build in their setup - and they're now trying to get rid of the difference to upstream.) |
|
@mstorsjo @efriedma-quic Thank you. I'm hitting this issue in the downstream Swift toolchain arm64 build for Windows cross-compiled on x64 windows by MSVC :) I presume that there's no LLVM buildbot building arm64 compiler-rt with MSVC (maybe is compiler-rt only meant to be built with Clang/LLVM?) Anyhow this issue was mitigated downstream. So I will retract this PR. I appreciate your comments! |
Disable the LSE files if COMPILER_RT_HAS_ASM_LSE failed as MSVC cannot seem to handle .S input files.