[cmake] Remove custom linker flag check function #86602
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
cmake
|
@llvm/pr-subscribers-clang Author: Keith Smiley (keith) ChangesSince LLVM now requires a minimum cmake version of 3.20.0, we no longer need this abstraction and can use the built in version directly as we do for the other feature check functions. Full diff: https://github.com/llvm/llvm-project/pull/86602.diff 7 Files Affected:
diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt
index d3dec1984b78d2..69634f78aeafe8 100644
--- a/clang/tools/clang-repl/CMakeLists.txt
+++ b/clang/tools/clang-repl/CMakeLists.txt
@@ -29,7 +29,7 @@ endif()
# gold. This flag tells the linker to build a PLT for the full address range.
# Linkers without this flag are assumed to support proper PLTs by default.
set(flag_long_plt "-Wl,--long-plt")
-llvm_check_linker_flag(CXX ${flag_long_plt} HAVE_LINKER_FLAG_LONG_PLT)
+check_linker_flag(CXX ${flag_long_plt} HAVE_LINKER_FLAG_LONG_PLT)
if(HAVE_LINKER_FLAG_LONG_PLT)
target_link_options(clang-repl PRIVATE ${flag_long_plt})
endif()
diff --git a/clang/tools/driver/CMakeLists.txt b/clang/tools/driver/CMakeLists.txt
index d70b92b0984e52..ca825d9b3e036e 100644
--- a/clang/tools/driver/CMakeLists.txt
+++ b/clang/tools/driver/CMakeLists.txt
@@ -104,7 +104,7 @@ endif()
if(CLANG_ORDER_FILE AND
(LLVM_LINKER_IS_APPLE OR LLVM_LINKER_IS_GOLD OR LLVM_LINKER_IS_LLD))
- include(LLVMCheckLinkerFlag)
+ include(CheckLinkerFlag)
if (LLVM_LINKER_IS_APPLE OR (LLVM_LINKER_IS_LLD AND APPLE))
set(LINKER_ORDER_FILE_OPTION "-Wl,-order_file,${CLANG_ORDER_FILE}")
@@ -115,7 +115,7 @@ if(CLANG_ORDER_FILE AND
endif()
# This is a test to ensure the actual order file works with the linker.
- llvm_check_linker_flag(CXX ${LINKER_ORDER_FILE_OPTION} LINKER_ORDER_FILE_WORKS)
+ check_linker_flag(CXX ${LINKER_ORDER_FILE_OPTION} LINKER_ORDER_FILE_WORKS)
# Passing an empty order file disables some linker layout optimizations.
# To work around this and enable workflows for re-linking when the order file
diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
index b5b6d2807d714c..32abedf59478b7 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -190,8 +190,7 @@ if(ENABLE_SHARED)
include(CheckLinkerFlag)
# The Solaris 11.4 linker supports a subset of GNU ld version scripts,
# but requires a special option to enable it.
- llvm_check_linker_flag(CXX "-Wl,-z,gnu-version-script-compat"
- LINKER_SUPPORTS_Z_GNU_VERSION_SCRIPT_COMPAT)
+ check_linker_flag(CXX "-Wl,-z,gnu-version-script-compat" LINKER_SUPPORTS_Z_GNU_VERSION_SCRIPT_COMPAT)
# Older Solaris (and illumos) linker does not support GNU ld version scripts
# and does not support GNU version script compat.
if (LINKER_SUPPORTS_Z_GNU_VERSION_SCRIPT_COMPAT)
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 745935f1405170..32693bdd3d3197 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -302,8 +302,8 @@ function(add_link_opts target_name)
elseif(${CMAKE_SYSTEM_NAME} MATCHES "SunOS" AND LLVM_LINKER_IS_SOLARISLD)
# Support for ld -z discard-unused=sections was only added in
# Solaris 11.4. GNU ld ignores it, but warns every time.
- include(LLVMCheckLinkerFlag)
- llvm_check_linker_flag(CXX "-Wl,-z,discard-unused=sections" LINKER_SUPPORTS_Z_DISCARD_UNUSED)
+ include(CheckLinkerFlag)
+ check_linker_flag(CXX "-Wl,-z,discard-unused=sections" LINKER_SUPPORTS_Z_DISCARD_UNUSED)
if (LINKER_SUPPORTS_Z_DISCARD_UNUSED)
set_property(TARGET ${target_name} APPEND_STRING PROPERTY
LINK_FLAGS " -Wl,-z,discard-unused=sections")
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 185266c0861e86..a4688f069f7210 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -1077,8 +1077,8 @@ if (LLVM_USE_SPLIT_DWARF AND
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:-gsplit-dwarf>)
- include(LLVMCheckLinkerFlag)
- llvm_check_linker_flag(CXX "-Wl,--gdb-index" LINKER_SUPPORTS_GDB_INDEX)
+ include(CheckLinkerFlag)
+ check_linker_flag(CXX "-Wl,--gdb-index" LINKER_SUPPORTS_GDB_INDEX)
append_if(LINKER_SUPPORTS_GDB_INDEX "-Wl,--gdb-index"
CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
endif()
@@ -1099,8 +1099,8 @@ endif()
# lld doesn't print colored diagnostics when invoked from Ninja
if (UNIX AND CMAKE_GENERATOR MATCHES "Ninja")
- include(LLVMCheckLinkerFlag)
- llvm_check_linker_flag(CXX "-Wl,--color-diagnostics" LINKER_SUPPORTS_COLOR_DIAGNOSTICS)
+ include(CheckLinkerFlag)
+ check_linker_flag(CXX "-Wl,--color-diagnostics" LINKER_SUPPORTS_COLOR_DIAGNOSTICS)
append_if(LINKER_SUPPORTS_COLOR_DIAGNOSTICS "-Wl,--color-diagnostics"
CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
endif()
diff --git a/llvm/cmake/modules/HandleLLVMStdlib.cmake b/llvm/cmake/modules/HandleLLVMStdlib.cmake
index 7afc10cff74ff0..a7e138aa0789b7 100644
--- a/llvm/cmake/modules/HandleLLVMStdlib.cmake
+++ b/llvm/cmake/modules/HandleLLVMStdlib.cmake
@@ -13,12 +13,12 @@ if(NOT DEFINED LLVM_STDLIB_HANDLED)
endfunction()
include(CheckCXXCompilerFlag)
- include(LLVMCheckLinkerFlag)
+ include(CheckLinkerFlag)
set(LLVM_LIBCXX_USED 0)
if(LLVM_ENABLE_LIBCXX)
if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
check_cxx_compiler_flag("-stdlib=libc++" CXX_COMPILER_SUPPORTS_STDLIB)
- llvm_check_linker_flag(CXX "-stdlib=libc++" CXX_LINKER_SUPPORTS_STDLIB)
+ check_linker_flag(CXX "-stdlib=libc++" CXX_LINKER_SUPPORTS_STDLIB)
if(CXX_COMPILER_SUPPORTS_STDLIB AND CXX_LINKER_SUPPORTS_STDLIB)
append("-stdlib=libc++"
CMAKE_CXX_FLAGS CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS
@@ -36,7 +36,7 @@ if(NOT DEFINED LLVM_STDLIB_HANDLED)
if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
check_cxx_compiler_flag("-static-libstdc++"
CXX_COMPILER_SUPPORTS_STATIC_STDLIB)
- llvm_check_linker_flag(CXX "-static-libstdc++" CXX_LINKER_SUPPORTS_STATIC_STDLIB)
+ check_linker_flag(CXX "-static-libstdc++" CXX_LINKER_SUPPORTS_STATIC_STDLIB)
if(CXX_COMPILER_SUPPORTS_STATIC_STDLIB AND
CXX_LINKER_SUPPORTS_STATIC_STDLIB)
append("-static-libstdc++"
diff --git a/llvm/cmake/modules/LLVMCheckLinkerFlag.cmake b/llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
deleted file mode 100644
index e09bbc66f2d26e..00000000000000
--- a/llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
+++ /dev/null
@@ -1,28 +0,0 @@
-include(CheckLinkerFlag OPTIONAL)
-
-if (COMMAND check_linker_flag)
- macro(llvm_check_linker_flag)
- check_linker_flag(${ARGN})
- endmacro()
-else()
- # Until the minimum CMAKE version is 3.18
-
- include(CheckCXXCompilerFlag)
-
- # cmake builtin compatible, except we assume lang is C or CXX
- function(llvm_check_linker_flag lang flag out_var)
- cmake_policy(PUSH)
- cmake_policy(SET CMP0056 NEW)
- set(_CMAKE_EXE_LINKER_FLAGS_SAVE ${CMAKE_EXE_LINKER_FLAGS})
- set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
- 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()
- set(CMAKE_EXE_LINKER_FLAGS ${_CMAKE_EXE_LINKER_FLAGS_SAVE})
- cmake_policy(POP)
- endfunction()
-endif()
|
keith
force-pushed
the
ks/cmake-remove-custom-linker-flag-check-function
branch
from
612027f
to
7bd6708
Compare
Since LLVM now requires a minimum cmake version of 3.20.0, we no longer need this abstraction and can use the built in version directly as we do for the other feature check functions.
keith
force-pushed
the
ks/cmake-remove-custom-linker-flag-check-function
branch
from
7bd6708
to
36886d2
Compare
|
I'd actually prefer this not be merged as is. I have already made a similar change locally but during testing I discovered a major issue with |
|
@petrhosek Would you be able to add a comment to the llvm_check_linker_flag function explaining this so someone else doesn't try to do the same thing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since LLVM now requires a minimum cmake version of 3.20.0, we no longer need this abstraction and can use the built in version directly as we do for the other feature check functions.