[clang] [compiler-rt] [libcxx] [libunwind] [llvm] [openmp] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)

2024-06-20 Thread Martin Storsjö via cfe-commits 

mstorsjo wrote:

> Hm, this seems to affect some feature detection (and constraints) later on, 
> though I don't see why/how removing the specification of `--unwindlib=none` 
> to the driver (leaving it only to the linker) would cause that:
> 
> https://github.com/llvm/llvm-project/blob/1c046ca3f3254944483251bdc9c843e72d7f7796/libunwind/src/CMakeLists.txt#L99-L106

I actually tested doing this cleanup back around when I wrote this - sorry I 
never got around to amending the comment here explaining what has to be done. 
The case with libunwind is a bit non-obvious.

I rebased my test change from 2022, see 
https://github.com/mstorsjo/llvm-project/commit/runtimes-link-options-unwindlib-none.
 Can you try this out and see if it fixes the issue you're seeing?

https://github.com/llvm/llvm-project/pull/93429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [compiler-rt] [libcxx] [libunwind] [llvm] [openmp] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)

2024-06-20 Thread via cfe-commits 

h-vetinari wrote:

I think the riddle may be explained by @mstorsjo's excellent commit message 
from 
https://github.com/llvm/llvm-project/commit/4169b5251f58cc0eb2ac6c3b4b9990a51728aac6
 (which I just found):
> If the CMake requirement is bumped to 3.14, we could use
`CMAKE_REQUIRED_LINK_OPTIONS` instead, removing the need for the
`--{start,end}-no-unused-arguments` options. (However, do note that
`CMAKE_REQUIRED_LINK_OPTIONS` is problematic in combination with
`CMAKE_TRY_COMPILE_TARGET_TYPE` set to `STATIC_LIBRARY`; see
https://gitlab.kitware.com/cmake/cmake/-/issues/23454.)

... which is indeed what libunwind specifies:
https://github.com/llvm/llvm-project/blob/6859685a87ad093d60c8bed60b116143c0a684c7/libunwind/CMakeLists.txt#L224

https://github.com/llvm/llvm-project/pull/93429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [compiler-rt] [libcxx] [libunwind] [llvm] [openmp] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)

2024-06-20 Thread via cfe-commits 

h-vetinari wrote:

Found some more CMake cruft... Unfortunately, it doesn't improve the situation 
with libunwind, but at least that means I should be able to break this out into 
a separate PR.

https://github.com/llvm/llvm-project/pull/93429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [compiler-rt] [libcxx] [libunwind] [llvm] [openmp] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)

2024-06-20 Thread via cfe-commits 

https://github.com/h-vetinari updated 
https://github.com/llvm/llvm-project/pull/93429

>From c8a0b127ade4330151951144f6d4841beaf66d23 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" 
Date: Sun, 26 May 2024 13:21:46 +1100
Subject: [PATCH 1/3] [runtimes] remove workaround for old CMake when setting
 `--unwindlib=none`

---
 runtimes/CMakeLists.txt | 22 +-
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 24f4851169591..15f9334776e05 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -116,27 +116,7 @@ 
filter_prefixed("${CMAKE_ASM_IMPLICIT_INCLUDE_DIRECTORIES}" ${LLVM_BINARY_DIR} C
 # brittle. We should ideally move this to runtimes/CMakeLists.txt.
 llvm_check_compiler_linker_flag(C "--unwindlib=none" 
CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
 if (CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
-  set(ORIG_CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}")
-  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
-  # TODO: When we can require CMake 3.14, we should use
-  # CMAKE_REQUIRED_LINK_OPTIONS here. Until then, we need a workaround:
-  # When using CMAKE_REQUIRED_FLAGS, this option gets added both to
-  # compilation and linking commands. That causes warnings in the
-  # compilation commands during cmake tests. This is normally benign, but
-  # when testing whether -Werror works, that test fails (due to the
-  # preexisting warning).
-  #
-  # Therefore, before we can use CMAKE_REQUIRED_LINK_OPTIONS, check if we
-  # can use --start-no-unused-arguments to silence the warnings about
-  # --unwindlib=none during compilation.
-  #
-  # We must first add --unwindlib=none to CMAKE_REQUIRED_FLAGS above, to
-  # allow this subsequent test to succeed, then rewrite CMAKE_REQUIRED_FLAGS
-  # below.
-  check_c_compiler_flag("--start-no-unused-arguments" 
C_SUPPORTS_START_NO_UNUSED_ARGUMENTS)
-  if (C_SUPPORTS_START_NO_UNUSED_ARGUMENTS)
-set(CMAKE_REQUIRED_FLAGS "${ORIG_CMAKE_REQUIRED_FLAGS} 
--start-no-unused-arguments --unwindlib=none --end-no-unused-arguments")
-  endif()
+  set(CMAKE_REQUIRED_LINK_OPTIONS "${CMAKE_REQUIRED_LINK_OPTIONS} 
--unwindlib=none")
 endif()
 
 # Disable use of the installed C++ standard library when building runtimes.

>From b8dad4f4ff7a505af5f8d9a1a3a6b9c6e7601522 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" 
Date: Thu, 20 Jun 2024 21:43:31 +1100
Subject: [PATCH 2/3] [cmake] switch to CMake's native check_linker_flag,
 delete LLVMCheckLinkerFlag.cmake

now that CMake baseline has moved past 3.18; see
https://cmake.org/cmake/help/latest/module/CheckLinkerFlag.html
---
 clang/tools/driver/CMakeLists.txt|  4 +--
 llvm/cmake/modules/AddLLVM.cmake |  4 +--
 llvm/cmake/modules/HandleLLVMOptions.cmake   |  8 +++---
 llvm/cmake/modules/HandleLLVMStdlib.cmake|  6 ++---
 llvm/cmake/modules/LLVMCheckLinkerFlag.cmake | 28 
 5 files changed, 11 insertions(+), 39 deletions(-)
 delete mode 100644 llvm/cmake/modules/LLVMCheckLinkerFlag.cmake

diff --git a/clang/tools/driver/CMakeLists.txt 
b/clang/tools/driver/CMakeLists.txt
index 290bf2a42536d..018605c2fd4f2 100644
--- a/clang/tools/driver/CMakeLists.txt
+++ b/clang/tools/driver/CMakeLists.txt
@@ -107,7 +107,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}")
@@ -118,7 +118,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/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 03f4e1f190fd9..cac5470435d91 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -327,8 +327,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

[clang] [compiler-rt] [libcxx] [libunwind] [llvm] [openmp] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)

2024-06-20 Thread via cfe-commits 

https://github.com/h-vetinari updated 
https://github.com/llvm/llvm-project/pull/93429

>From c8a0b127ade4330151951144f6d4841beaf66d23 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" 
Date: Sun, 26 May 2024 13:21:46 +1100
Subject: [PATCH 1/3] [runtimes] remove workaround for old CMake when setting
 `--unwindlib=none`

---
 runtimes/CMakeLists.txt | 22 +-
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 24f4851169591..15f9334776e05 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -116,27 +116,7 @@ 
filter_prefixed("${CMAKE_ASM_IMPLICIT_INCLUDE_DIRECTORIES}" ${LLVM_BINARY_DIR} C
 # brittle. We should ideally move this to runtimes/CMakeLists.txt.
 llvm_check_compiler_linker_flag(C "--unwindlib=none" 
CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
 if (CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
-  set(ORIG_CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}")
-  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
-  # TODO: When we can require CMake 3.14, we should use
-  # CMAKE_REQUIRED_LINK_OPTIONS here. Until then, we need a workaround:
-  # When using CMAKE_REQUIRED_FLAGS, this option gets added both to
-  # compilation and linking commands. That causes warnings in the
-  # compilation commands during cmake tests. This is normally benign, but
-  # when testing whether -Werror works, that test fails (due to the
-  # preexisting warning).
-  #
-  # Therefore, before we can use CMAKE_REQUIRED_LINK_OPTIONS, check if we
-  # can use --start-no-unused-arguments to silence the warnings about
-  # --unwindlib=none during compilation.
-  #
-  # We must first add --unwindlib=none to CMAKE_REQUIRED_FLAGS above, to
-  # allow this subsequent test to succeed, then rewrite CMAKE_REQUIRED_FLAGS
-  # below.
-  check_c_compiler_flag("--start-no-unused-arguments" 
C_SUPPORTS_START_NO_UNUSED_ARGUMENTS)
-  if (C_SUPPORTS_START_NO_UNUSED_ARGUMENTS)
-set(CMAKE_REQUIRED_FLAGS "${ORIG_CMAKE_REQUIRED_FLAGS} 
--start-no-unused-arguments --unwindlib=none --end-no-unused-arguments")
-  endif()
+  set(CMAKE_REQUIRED_LINK_OPTIONS "${CMAKE_REQUIRED_LINK_OPTIONS} 
--unwindlib=none")
 endif()
 
 # Disable use of the installed C++ standard library when building runtimes.

>From 3c6b98a32ec9c1d4b0e8edac6da8689b5892d0f6 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" 
Date: Thu, 20 Jun 2024 21:43:31 +1100
Subject: [PATCH 2/3] [cmake] switch to CMake's native check_linker_flag,
 delete LLVMCheckLinkerFlag.cmake

now that CMake baseline has moved past 3.18; see
https://cmake.org/cmake/help/latest/module/CheckLinkerFlag.html
---
 clang/tools/driver/CMakeLists.txt|  4 +--
 llvm/cmake/modules/AddLLVM.cmake |  4 +--
 llvm/cmake/modules/HandleLLVMOptions.cmake   |  6 ++---
 llvm/cmake/modules/HandleLLVMStdlib.cmake|  6 ++---
 llvm/cmake/modules/LLVMCheckLinkerFlag.cmake | 28 
 5 files changed, 10 insertions(+), 38 deletions(-)
 delete mode 100644 llvm/cmake/modules/LLVMCheckLinkerFlag.cmake

diff --git a/clang/tools/driver/CMakeLists.txt 
b/clang/tools/driver/CMakeLists.txt
index 290bf2a42536d..018605c2fd4f2 100644
--- a/clang/tools/driver/CMakeLists.txt
+++ b/clang/tools/driver/CMakeLists.txt
@@ -107,7 +107,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}")
@@ -118,7 +118,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/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 03f4e1f190fd9..cac5470435d91 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -327,8 +327,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