[compiler-rt] 7017e6c - [cmake] Partially deduplicate `{llvm,compiler_rt}_check_linker_flag` for runtime libs and llvm
John Ericson via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 28 22:07:29 PST 2022
Author: John Ericson
Date: 2022-01-29T06:07:24Z
New Revision: 7017e6c9cfd2de3122ce9528f338a97d61e96373
URL: https://github.com/llvm/llvm-project/commit/7017e6c9cfd2de3122ce9528f338a97d61e96373
DIFF: https://github.com/llvm/llvm-project/commit/7017e6c9cfd2de3122ce9528f338a97d61e96373.diff
LOG: [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
Added:
cmake/Modules/LLVMCheckCompilerLinkerFlag.cmake
Modified:
compiler-rt/cmake/config-ix.cmake
libcxx/cmake/config-ix.cmake
libunwind/cmake/config-ix.cmake
llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
runtimes/CMakeLists.txt
Removed:
cmake/Modules/CheckLinkerFlag.cmake
################################################################################
diff --git a/cmake/Modules/CheckLinkerFlag.cmake b/cmake/Modules/CheckLinkerFlag.cmake
deleted file mode 100644
index 722fe5b1b8ead..0000000000000
--- a/cmake/Modules/CheckLinkerFlag.cmake
+++ /dev/null
@@ -1,17 +0,0 @@
-include(CMakePushCheckState)
-include(CheckCCompilerFlag)
-
-function(llvm_check_linker_flag flag dest)
- # If testing a flag with check_c_compiler_flag, it gets added to the compile
- # command only, but not to the linker command in that test. If the flag
- # is vital for linking to succeed, the test would fail even if it would
- # have succeeded if it was included on both commands.
- #
- # Therefore, try adding the flag to CMAKE_REQUIRED_FLAGS, which gets
- # added to both compiling and linking commands in the tests.
-
- cmake_push_check_state()
- set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${flag}")
- check_c_compiler_flag("" ${dest})
- cmake_pop_check_state()
-endfunction()
diff --git a/cmake/Modules/LLVMCheckCompilerLinkerFlag.cmake b/cmake/Modules/LLVMCheckCompilerLinkerFlag.cmake
new file mode 100644
index 0000000000000..f61ec0585f9a4
--- /dev/null
+++ b/cmake/Modules/LLVMCheckCompilerLinkerFlag.cmake
@@ -0,0 +1,35 @@
+include(CMakePushCheckState)
+
+include(CheckCompilerFlag OPTIONAL)
+
+if(NOT COMMAND check_compiler_flag)
+ include(CheckCCompilerFlag)
+ include(CheckCXXCompilerFlag)
+endif()
+
+function(llvm_check_compiler_linker_flag lang flag out_var)
+ # If testing a flag with check_c_compiler_flag, it gets added to the compile
+ # command only, but not to the linker command in that test. If the flag
+ # is vital for linking to succeed, the test would fail even if it would
+ # have succeeded if it was included on both commands.
+ #
+ # Therefore, try adding the flag to CMAKE_REQUIRED_FLAGS, which gets
+ # added to both compiling and linking commands in the tests.
+
+ cmake_push_check_state()
+ set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${flag}")
+ if(COMMAND check_compiler_flag)
+ check_compiler_flag("${lang}" "" ${out_var})
+ else()
+ # Until the minimum CMAKE version is 3.19
+ # cmake builtin compatible, except we assume lang is C or CXX
+ if("${lang}" STREQUAL "C")
+ check_c_compiler_flag("" ${out_var})
+ elseif("${lang}" STREQUAL "CXX")
+ check_cxx_compiler_flag("" ${out_var})
+ else()
+ message(FATAL_ERROR "\"${lang}\" is not C or CXX")
+ endif()
+ endif()
+ cmake_pop_check_state()
+endfunction()
diff --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index 33693ce60321d..4299a0589a7b7 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -1,4 +1,5 @@
include(CMakePushCheckState)
+include(LLVMCheckCompilerLinkerFlag)
include(CheckCCompilerFlag)
include(CheckCXXCompilerFlag)
include(CheckIncludeFiles)
@@ -6,13 +7,6 @@ include(CheckLibraryExists)
include(CheckSymbolExists)
include(TestBigEndian)
-function(compiler_rt_check_linker_flag flag out_var)
- cmake_push_check_state()
- set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${flag}")
- check_cxx_compiler_flag("" ${out_var})
- cmake_pop_check_state()
-endfunction()
-
check_library_exists(c fopen "" COMPILER_RT_HAS_LIBC)
if (COMPILER_RT_USE_BUILTINS_LIBRARY)
include(HandleCompilerRT)
@@ -171,12 +165,12 @@ check_library_exists(c++ __cxa_throw "" COMPILER_RT_HAS_LIBCXX)
check_library_exists(stdc++ __cxa_throw "" COMPILER_RT_HAS_LIBSTDCXX)
# Linker flags.
-compiler_rt_check_linker_flag("-Wl,-z,text" COMPILER_RT_HAS_Z_TEXT)
-compiler_rt_check_linker_flag("-fuse-ld=lld" COMPILER_RT_HAS_FUSE_LD_LLD_FLAG)
+llvm_check_compiler_linker_flag(CXX "-Wl,-z,text" COMPILER_RT_HAS_Z_TEXT)
+llvm_check_compiler_linker_flag(CXX "-fuse-ld=lld" COMPILER_RT_HAS_FUSE_LD_LLD_FLAG)
if(${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
set(VERS_COMPAT_OPTION "-Wl,-z,gnu-version-script-compat")
- compiler_rt_check_linker_flag("${VERS_COMPAT_OPTION}" COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
+ llvm_check_compiler_linker_flag(CXX "${VERS_COMPAT_OPTION}" COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
endif()
set(DUMMY_VERS ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/dummy.vers)
@@ -187,10 +181,10 @@ if(COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
# -z gnu-version-script-compat.
string(APPEND VERS_OPTION " ${VERS_COMPAT_OPTION}")
endif()
-compiler_rt_check_linker_flag("${VERS_OPTION}" COMPILER_RT_HAS_VERSION_SCRIPT)
+llvm_check_compiler_linker_flag(CXX "${VERS_OPTION}" COMPILER_RT_HAS_VERSION_SCRIPT)
if(ANDROID)
- compiler_rt_check_linker_flag("-Wl,-z,global" COMPILER_RT_HAS_Z_GLOBAL)
+ llvm_check_compiler_linker_flag(CXX "-Wl,-z,global" COMPILER_RT_HAS_Z_GLOBAL)
check_library_exists(log __android_log_write "" COMPILER_RT_HAS_LIBLOG)
endif()
@@ -436,7 +430,7 @@ if(APPLE)
-lc++
-lc++abi)
- compiler_rt_check_linker_flag("-fapplication-extension" COMPILER_RT_HAS_APP_EXTENSION)
+ llvm_check_compiler_linker_flag(CXX "-fapplication-extension" COMPILER_RT_HAS_APP_EXTENSION)
if(COMPILER_RT_HAS_APP_EXTENSION)
list(APPEND DARWIN_COMMON_LINK_FLAGS "-fapplication-extension")
endif()
diff --git a/libcxx/cmake/config-ix.cmake b/libcxx/cmake/config-ix.cmake
index 689a9d09c0179..868bb89d82c7d 100644
--- a/libcxx/cmake/config-ix.cmake
+++ b/libcxx/cmake/config-ix.cmake
@@ -1,6 +1,6 @@
include(CMakePushCheckState)
include(CheckLibraryExists)
-include(CheckLinkerFlag)
+include(LLVMCheckCompilerLinkerFlag)
include(CheckCCompilerFlag)
include(CheckCXXCompilerFlag)
include(CheckCSourceCompiles)
@@ -12,7 +12,7 @@ include(CheckCSourceCompiles)
# libunwind (and the compiler implicit -lunwind wouldn't succeed as the newly
# built libunwind isn't installed yet). For those cases, it'd be good to
# link with --uwnindlib=none. Check if that option works.
-llvm_check_linker_flag("--unwindlib=none" LIBCXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
+llvm_check_compiler_linker_flag(C "--unwindlib=none" LIBCXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
if (LIBCXX_SUPPORTS_UNWINDLIB_NONE_FLAG)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
endif()
diff --git a/libunwind/cmake/config-ix.cmake b/libunwind/cmake/config-ix.cmake
index c814611d43785..f78a793142f88 100644
--- a/libunwind/cmake/config-ix.cmake
+++ b/libunwind/cmake/config-ix.cmake
@@ -2,14 +2,14 @@ include(CMakePushCheckState)
include(CheckCCompilerFlag)
include(CheckCXXCompilerFlag)
include(CheckLibraryExists)
-include(CheckLinkerFlag)
+include(LLVMCheckCompilerLinkerFlag)
include(CheckSymbolExists)
include(CheckCSourceCompiles)
# The compiler driver may be implicitly trying to link against libunwind, which
# might not work if libunwind doesn't exist yet. Try to check if
# --unwindlib=none is supported, and use that if possible.
-llvm_check_linker_flag("--unwindlib=none" LIBUNWIND_SUPPORTS_UNWINDLIB_NONE_FLAG)
+llvm_check_compiler_linker_flag(C "--unwindlib=none" LIBUNWIND_SUPPORTS_UNWINDLIB_NONE_FLAG)
if (LIBUNWIND_SUPPORTS_UNWINDLIB_NONE_FLAG)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
endif()
@@ -34,11 +34,11 @@ endif()
# required for the link to go through. We remove sanitizers from the
# configuration checks to avoid spurious link errors.
-llvm_check_linker_flag(-nostdlib++ LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
+llvm_check_compiler_linker_flag(C "-nostdlib++" LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
else()
- llvm_check_linker_flag(-nodefaultlibs LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
+ llvm_check_compiler_linker_flag(C "-nodefaultlibs" LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
if (LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
endif()
diff --git a/llvm/cmake/modules/LLVMCheckLinkerFlag.cmake b/llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
index 253dd768654a2..79c4e2cb4c2cd 100644
--- a/llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
+++ b/llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
@@ -5,14 +5,22 @@ if (COMMAND check_linker_flag)
check_linker_flag(${ARGN})
endmacro()
else()
+ # Until the minimum CMAKE version is 3.18
+
include(CheckCXXCompilerFlag)
include(CMakePushCheckState)
- # cmake builtin compatible, except we assume lang is CXX
+ # cmake builtin compatible, except we assume lang is C or CXX
function(llvm_check_linker_flag lang flag out_var)
cmake_push_check_state()
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
- check_cxx_compiler_flag("" ${out_var})
+ if("${lang}" STREQUAL "C")
+ check_c_compiler_flag("" ${out_var})
+ elseif("${lang}" STREQUAL "CXX")
+ check_cxx_compiler_flag("" ${out_var})
+ else()
+ message(FATAL_ERROR "\"${lang}\" is not C or CXX")
+ endif()
cmake_pop_check_state()
endfunction()
endif()
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index cedce7b3541e5..1400233b73f6d 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -88,7 +88,7 @@ set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
set(LLVM_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../llvm)
include(CheckLibraryExists)
-include(CheckLinkerFlag)
+include(LLVMCheckCompilerLinkerFlag)
include(CheckCCompilerFlag)
include(CheckCXXCompilerFlag)
@@ -100,7 +100,7 @@ if (NOT LLVM_RUNTIMES_LINKING_WORKS)
# --unwindlib=none is supported, and use that if possible.
# Don't add this if not necessary to fix linking, as it can break using
# e.g. ASAN/TSAN.
- llvm_check_linker_flag("--unwindlib=none" LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG)
+ llvm_check_compiler_linker_flag(C "--unwindlib=none" LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG)
if (LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
endif()
@@ -110,7 +110,7 @@ endif()
# Check for -nostdlib++ first; if there's no C++ standard library yet,
# all check_cxx_compiler_flag commands will fail until we add -nostdlib++
# (or -nodefaultlibs).
-llvm_check_linker_flag(-nostdlib++ LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG)
+llvm_check_compiler_linker_flag(C "-nostdlib++" LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG)
if (LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
endif()
More information about the llvm-commits
mailing list