[PATCH] D143052: [CMake] Unify llvm_check_linker_flag and llvm_check_compiler_linker_flag

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 1 02:00:04 PST 2023


mstorsjo accepted this revision.
mstorsjo added a comment.

LGTM, this looks great!

I wonder why we didn't think of that before (in particular, using `CMAKE_EXE_LINKER_FLAGS` instead of `CMAKE_REQUIRED_FLAGS` before) - I wonder if there's some pitfalls we don't come to think of right now? I can put this through a full bootstrap build of mine too. And I guess we'll see if the libcxx CI says anything.

What are the practical differences between `CMAKE_EXE_LINKER_FLAGS` and `CMAKE_REQUIRED_FLAGS` here - both get used in all linking tests, while `CMAKE_REQUIRED_FLAGS` also gets used in pure compilation tests (which is suboptimal)? In a large comment in `runtimes/CMakeLists.txt`, I've noted that we should move to using `CMAKE_REQUIRED_LINK_OPTIONS` when we can require cmake 3.14 - what would be the differences between `CMAKE_REQUIRED_LINK_OPTIONS` and `CMAKE_EXE_LINKER_FLAGS` here?

(I've noted one such difference when testing `CMAKE_REQUIRED_LINK_OPTIONS`; when testing with `CMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY`, the flags from `CMAKE_REQUIRED_LINK_OPTIONS` end up passed to the static library tool, which fails - and that wouldn't be the issue with `CMAKE_EXE_LINKER_FLAGS`.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143052/new/

https://reviews.llvm.org/D143052



More information about the llvm-commits mailing list