Skip to content

Commit 7017e6c

Browse files
committed
[cmake] Partially deduplicate {llvm,compiler_rt}_check_linker_flag for runtime libs and llvm
We previously had a few varied definitions of this floating around. I had tried to make the one installed with LLVM handle all the cases, and then made the others use it, but this ran into issues with `HandleOutOfTreeLLVM` not working for compiler-rt, and also `CMAKE_EXE_LINKER_FLAGS` not working right without `CMP0056` set to the new behavior. My compromise solution is this: - No not completely deduplicate: the runtime libs will instead use a version that still exists as part of the internal and not installed common shared CMake utilities. This avoids `HandleOutOfTreeLLVM` or a workaround for compiler-rt. - Continue to use `CMAKE_REQUIRED_FLAGS`, which effects compilation and linking. Maybe this is unnecessary, but it's safer to leave that as a future change. Also means we can avoid `CMP0056` for now, to try out later, which is good incrementality too. - Call it `llvm_check_compiler_linker_flag` since it, in fact is about both per its implementation (before and after this patch), so there is no name collision. In the future, we might still enable CMP0056 and make compiler-rt work with HandleOutOfTreeLLVM, which case we delete `llvm_check_compiler_flag` and go back to the old way (as these are, in fact, linking related flags), but that I leave for someone else as future work. The original issue was reported to me in https://reviews.llvm.org/D116521#3248117 as D116521 made clang and LLVM use the common cmake utils. Reviewed By: sebastian-ne, phosek, #libunwind, #libc, #libc_abi, ldionne Differential Revision: https://reviews.llvm.org/D117537
1 parent 184f94a commit 7017e6c

File tree

7 files changed

+61
-41
lines changed

7 files changed

+61
-41
lines changed

cmake/Modules/CheckLinkerFlag.cmake

Lines changed: 0 additions & 17 deletions
This file was deleted.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
include(CMakePushCheckState)
2+
3+
include(CheckCompilerFlag OPTIONAL)
4+
5+
if(NOT COMMAND check_compiler_flag)
6+
include(CheckCCompilerFlag)
7+
include(CheckCXXCompilerFlag)
8+
endif()
9+
10+
function(llvm_check_compiler_linker_flag lang flag out_var)
11+
# If testing a flag with check_c_compiler_flag, it gets added to the compile
12+
# command only, but not to the linker command in that test. If the flag
13+
# is vital for linking to succeed, the test would fail even if it would
14+
# have succeeded if it was included on both commands.
15+
#
16+
# Therefore, try adding the flag to CMAKE_REQUIRED_FLAGS, which gets
17+
# added to both compiling and linking commands in the tests.
18+
19+
cmake_push_check_state()
20+
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${flag}")
21+
if(COMMAND check_compiler_flag)
22+
check_compiler_flag("${lang}" "" ${out_var})
23+
else()
24+
# Until the minimum CMAKE version is 3.19
25+
# cmake builtin compatible, except we assume lang is C or CXX
26+
if("${lang}" STREQUAL "C")
27+
check_c_compiler_flag("" ${out_var})
28+
elseif("${lang}" STREQUAL "CXX")
29+
check_cxx_compiler_flag("" ${out_var})
30+
else()
31+
message(FATAL_ERROR "\"${lang}\" is not C or CXX")
32+
endif()
33+
endif()
34+
cmake_pop_check_state()
35+
endfunction()

compiler-rt/cmake/config-ix.cmake

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,12 @@
11
include(CMakePushCheckState)
2+
include(LLVMCheckCompilerLinkerFlag)
23
include(CheckCCompilerFlag)
34
include(CheckCXXCompilerFlag)
45
include(CheckIncludeFiles)
56
include(CheckLibraryExists)
67
include(CheckSymbolExists)
78
include(TestBigEndian)
89

9-
function(compiler_rt_check_linker_flag flag out_var)
10-
cmake_push_check_state()
11-
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${flag}")
12-
check_cxx_compiler_flag("" ${out_var})
13-
cmake_pop_check_state()
14-
endfunction()
15-
1610
check_library_exists(c fopen "" COMPILER_RT_HAS_LIBC)
1711
if (COMPILER_RT_USE_BUILTINS_LIBRARY)
1812
include(HandleCompilerRT)
@@ -171,12 +165,12 @@ check_library_exists(c++ __cxa_throw "" COMPILER_RT_HAS_LIBCXX)
171165
check_library_exists(stdc++ __cxa_throw "" COMPILER_RT_HAS_LIBSTDCXX)
172166

173167
# Linker flags.
174-
compiler_rt_check_linker_flag("-Wl,-z,text" COMPILER_RT_HAS_Z_TEXT)
175-
compiler_rt_check_linker_flag("-fuse-ld=lld" COMPILER_RT_HAS_FUSE_LD_LLD_FLAG)
168+
llvm_check_compiler_linker_flag(CXX "-Wl,-z,text" COMPILER_RT_HAS_Z_TEXT)
169+
llvm_check_compiler_linker_flag(CXX "-fuse-ld=lld" COMPILER_RT_HAS_FUSE_LD_LLD_FLAG)
176170

177171
if(${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
178172
set(VERS_COMPAT_OPTION "-Wl,-z,gnu-version-script-compat")
179-
compiler_rt_check_linker_flag("${VERS_COMPAT_OPTION}" COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
173+
llvm_check_compiler_linker_flag(CXX "${VERS_COMPAT_OPTION}" COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
180174
endif()
181175

182176
set(DUMMY_VERS ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/dummy.vers)
@@ -187,10 +181,10 @@ if(COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
187181
# -z gnu-version-script-compat.
188182
string(APPEND VERS_OPTION " ${VERS_COMPAT_OPTION}")
189183
endif()
190-
compiler_rt_check_linker_flag("${VERS_OPTION}" COMPILER_RT_HAS_VERSION_SCRIPT)
184+
llvm_check_compiler_linker_flag(CXX "${VERS_OPTION}" COMPILER_RT_HAS_VERSION_SCRIPT)
191185

192186
if(ANDROID)
193-
compiler_rt_check_linker_flag("-Wl,-z,global" COMPILER_RT_HAS_Z_GLOBAL)
187+
llvm_check_compiler_linker_flag(CXX "-Wl,-z,global" COMPILER_RT_HAS_Z_GLOBAL)
194188
check_library_exists(log __android_log_write "" COMPILER_RT_HAS_LIBLOG)
195189
endif()
196190

@@ -436,7 +430,7 @@ if(APPLE)
436430
-lc++
437431
-lc++abi)
438432

439-
compiler_rt_check_linker_flag("-fapplication-extension" COMPILER_RT_HAS_APP_EXTENSION)
433+
llvm_check_compiler_linker_flag(CXX "-fapplication-extension" COMPILER_RT_HAS_APP_EXTENSION)
440434
if(COMPILER_RT_HAS_APP_EXTENSION)
441435
list(APPEND DARWIN_COMMON_LINK_FLAGS "-fapplication-extension")
442436
endif()

libcxx/cmake/config-ix.cmake

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
include(CMakePushCheckState)
22
include(CheckLibraryExists)
3-
include(CheckLinkerFlag)
3+
include(LLVMCheckCompilerLinkerFlag)
44
include(CheckCCompilerFlag)
55
include(CheckCXXCompilerFlag)
66
include(CheckCSourceCompiles)
@@ -12,7 +12,7 @@ include(CheckCSourceCompiles)
1212
# libunwind (and the compiler implicit -lunwind wouldn't succeed as the newly
1313
# built libunwind isn't installed yet). For those cases, it'd be good to
1414
# link with --uwnindlib=none. Check if that option works.
15-
llvm_check_linker_flag("--unwindlib=none" LIBCXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
15+
llvm_check_compiler_linker_flag(C "--unwindlib=none" LIBCXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
1616
if (LIBCXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
1717
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
1818
endif()

libunwind/cmake/config-ix.cmake

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ include(CMakePushCheckState)
22
include(CheckCCompilerFlag)
33
include(CheckCXXCompilerFlag)
44
include(CheckLibraryExists)
5-
include(CheckLinkerFlag)
5+
include(LLVMCheckCompilerLinkerFlag)
66
include(CheckSymbolExists)
77
include(CheckCSourceCompiles)
88

99
# The compiler driver may be implicitly trying to link against libunwind, which
1010
# might not work if libunwind doesn't exist yet. Try to check if
1111
# --unwindlib=none is supported, and use that if possible.
12-
llvm_check_linker_flag("--unwindlib=none" LIBUNWIND_SUPPORTS_UNWINDLIB_NONE_FLAG)
12+
llvm_check_compiler_linker_flag(C "--unwindlib=none" LIBUNWIND_SUPPORTS_UNWINDLIB_NONE_FLAG)
1313
if (LIBUNWIND_SUPPORTS_UNWINDLIB_NONE_FLAG)
1414
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
1515
endif()
@@ -34,11 +34,11 @@ endif()
3434
# required for the link to go through. We remove sanitizers from the
3535
# configuration checks to avoid spurious link errors.
3636

37-
llvm_check_linker_flag(-nostdlib++ LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
37+
llvm_check_compiler_linker_flag(C "-nostdlib++" LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
3838
if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
3939
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
4040
else()
41-
llvm_check_linker_flag(-nodefaultlibs LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
41+
llvm_check_compiler_linker_flag(C "-nodefaultlibs" LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
4242
if (LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
4343
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
4444
endif()

llvm/cmake/modules/LLVMCheckLinkerFlag.cmake

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,22 @@ if (COMMAND check_linker_flag)
55
check_linker_flag(${ARGN})
66
endmacro()
77
else()
8+
# Until the minimum CMAKE version is 3.18
9+
810
include(CheckCXXCompilerFlag)
911
include(CMakePushCheckState)
1012

11-
# cmake builtin compatible, except we assume lang is CXX
13+
# cmake builtin compatible, except we assume lang is C or CXX
1214
function(llvm_check_linker_flag lang flag out_var)
1315
cmake_push_check_state()
1416
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
15-
check_cxx_compiler_flag("" ${out_var})
17+
if("${lang}" STREQUAL "C")
18+
check_c_compiler_flag("" ${out_var})
19+
elseif("${lang}" STREQUAL "CXX")
20+
check_cxx_compiler_flag("" ${out_var})
21+
else()
22+
message(FATAL_ERROR "\"${lang}\" is not C or CXX")
23+
endif()
1624
cmake_pop_check_state()
1725
endfunction()
1826
endif()

runtimes/CMakeLists.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
8888
set(LLVM_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../llvm)
8989

9090
include(CheckLibraryExists)
91-
include(CheckLinkerFlag)
91+
include(LLVMCheckCompilerLinkerFlag)
9292
include(CheckCCompilerFlag)
9393
include(CheckCXXCompilerFlag)
9494

@@ -100,7 +100,7 @@ if (NOT LLVM_RUNTIMES_LINKING_WORKS)
100100
# --unwindlib=none is supported, and use that if possible.
101101
# Don't add this if not necessary to fix linking, as it can break using
102102
# e.g. ASAN/TSAN.
103-
llvm_check_linker_flag("--unwindlib=none" LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG)
103+
llvm_check_compiler_linker_flag(C "--unwindlib=none" LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG)
104104
if (LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG)
105105
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
106106
endif()
@@ -110,7 +110,7 @@ endif()
110110
# Check for -nostdlib++ first; if there's no C++ standard library yet,
111111
# all check_cxx_compiler_flag commands will fail until we add -nostdlib++
112112
# (or -nodefaultlibs).
113-
llvm_check_linker_flag(-nostdlib++ LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG)
113+
llvm_check_compiler_linker_flag(C "-nostdlib++" LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG)
114114
if (LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG)
115115
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
116116
endif()

0 commit comments

Comments
 (0)