Skip to content

[compiler-rt][MSVC] Update check to include clang-cl #150108

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

Jump to bottom
Merged
merged 1 commit into from
Jul 23, 2025
Merged

[compiler-rt][MSVC] Update check to include clang-cl #150108

merged 1 commit into from
Jul 23, 2025

Conversation

zacklj89
Copy link
Contributor

Follow up to #149823 to include clang-cl

@zacklj89 zacklj89 requested review from nico and pcc July 22, 2025 20:48
@zacklj89 zacklj89 mentioned this pull request Jul 22, 2025
Merged
mstorsjo
mstorsjo reviewed Jul 22, 2025
View reviewed changes
compiler-rt/lib/builtins/CMakeLists.txt
# Append sources specific to AArch64 targets that aren't supported by MSVC.
if(NOT MSVC)
# Append sources specific to AArch64 targets that aren't supported by cl.exe
if(CLANG_CL OR NOT MSVC)
Copy link
Member

Choose a reason for hiding this comment

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

I wasn’t aware of the cmake variable CLANG_CL - is that set in all supported cmake versions? Otherwise I think the common way is e.g. to check CMAKE_C_COMPILER_ID MATCHES "Clang" or something like that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's set by llvm/cmake/modules/CheckCompilerVersion.cmake.

But I don't think it's the right thing, in any case. The issue here isn't whether you're using a compiler in cl emulation mode, the issue is that Windows COFF requires different assembler directives. The correct check for that is something like WIN32 OR CYGWIN.

Copy link
Contributor

Choose a reason for hiding this comment

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

The assembly in emupac.cpp uses "portable" assembly directives (portable in the sense that clang supports them in all object formats). The fact that my compile command from #149823 (comment) worked is evidence of that. So I think the condition in the PR is correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I read the condition backwards.

I guess this is okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the reviews 👍
Apologies for rushing the original PR in without testing my assumptions of clang-cl!

efriedma-quic
efriedma-quic approved these changes Jul 23, 2025
View reviewed changes
compiler-rt/lib/builtins/CMakeLists.txt
# Append sources specific to AArch64 targets that aren't supported by MSVC.
if(NOT MSVC)
# Append sources specific to AArch64 targets that aren't supported by cl.exe
if(CLANG_CL OR NOT MSVC)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I read the condition backwards.

I guess this is okay.

@zacklj89 zacklj89 merged commit 8c4fa11 into llvm:main Jul 23, 2025
12 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 23, 2025

LLVM Buildbot has detected a new failure on builder premerge-monolithic-linux running on premerge-linux-1 while building compiler-rt at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/39019

Here is the relevant piece of the build log for the reference 
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
...
PASS: lld :: COFF/common-replacement.s (98774 of 101813)
PASS: lld :: COFF/arm64ec-entry-mangle.test (98775 of 101813)
PASS: lld :: COFF/checksum.test (98776 of 101813)
PASS: lld :: COFF/comdat-gcc-compatibility.s (98777 of 101813)
PASS: lld :: COFF/comdat-jumptable.s (98778 of 101813)
PASS: lld :: COFF/conflict.test (98779 of 101813)
PASS: cfi-standalone-lld-x86_64 :: base-derived-destructor.cpp (98780 of 101813)
PASS: lld :: COFF/comdat-gcc-compatibility-size.test (98781 of 101813)
PASS: lld :: COFF/cygwin-symbols.s (98782 of 101813)
TIMEOUT: MLIR :: Examples/standalone/test.toy (98783 of 101813)
******************** TEST 'MLIR :: Examples/standalone/test.toy' FAILED ********************
Exit Code: 1
Timeout: Reached timeout of 60 seconds

Command Output (stdout):
--
# RUN: at line 1
"/etc/cmake/bin/cmake" "/build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone" -G "Ninja"  -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang  -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir  -DLLVM_USE_LINKER=lld  -DPython3_EXECUTABLE="/usr/bin/python3.10"
# executed command: /etc/cmake/bin/cmake /build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone -G Ninja -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld -DPython3_EXECUTABLE=/usr/bin/python3.10
# .---command stdout------------
# | -- The CXX compiler identification is Clang 16.0.6
# | -- The C compiler identification is Clang 16.0.6
# | -- Detecting CXX compiler ABI info
# | -- Detecting CXX compiler ABI info - done
# | -- Check for working CXX compiler: /usr/bin/clang++ - skipped
# | -- Detecting CXX compile features
# | -- Detecting CXX compile features - done
# | -- Detecting C compiler ABI info
# | -- Detecting C compiler ABI info - done
# | -- Check for working C compiler: /usr/bin/clang - skipped
# | -- Detecting C compile features
# | -- Detecting C compile features - done
# | -- Looking for histedit.h
# | -- Looking for histedit.h - found
# | -- Found LibEdit: /usr/include (found version "2.11") 
# | -- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.11") 
# | -- Found LibXml2: /usr/lib/x86_64-linux-gnu/libxml2.so (found version "2.9.13") 
# | -- Using MLIRConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir
# | -- Using LLVMConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/llvm
# | -- Linker detection: unknown
# | -- Performing Test LLVM_LIBSTDCXX_MIN
# | -- Performing Test LLVM_LIBSTDCXX_MIN - Success
# | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR
# | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR - Success
# | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER
# | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER - Success
# | -- Performing Test C_SUPPORTS_FPIC
# | -- Performing Test C_SUPPORTS_FPIC - Success
# | -- Performing Test CXX_SUPPORTS_FPIC

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
@zacklj89 @mahesh-attarde
[compiler-rt][MSVC] Update check to include clang-cl (llvm#150108)
95aa626 
Follow up to llvm#149823 to include `clang-cl` for AArch64 builtins sources.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mstorsjo mstorsjo mstorsjo left review comments

@efriedma-quic efriedma-quic efriedma-quic approved these changes

@nico nico Awaiting requested review from nico

@pcc pcc Awaiting requested review from pcc

Assignees
No one assigned
Labels
compiler-rt:builtins compiler-rt
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zacklj89 @llvm-ci @mstorsjo @pcc @efriedma-quic @llvmbot