From 07b749800c5cd4105d49ab46be5f0a2079dd709a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 11 Jun 2022 06:11:59 +0000 Subject: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore First of all, `LLVM_TOOLS_INSTALL_DIR` put there breaks our NixOS builds, because `LLVM_TOOLS_INSTALL_DIR` defined the same as `CMAKE_INSTALL_BINDIR` becomes an *absolute* path, and then when downstream projects try to install there too this breaks because our builds always install to fresh directories for isolation's sake. Second of all, note that `LLVM_TOOLS_INSTALL_DIR` stands out against the other specially crafted `LLVM_CONFIG_*` variables substituted in `llvm/cmake/modules/LLVMConfig.cmake.in`. @beanz added it in d0e1c2a550ef348aae036d0fe78cab6f038c420c to fix a dangling reference in `AddLLVM`, but I am suspicious of how this variable doesn't follow the pattern. Those other ones are carefully made to be build-time vs install-time variables depending on which `LLVMConfig.cmake` is being generated, are carefully made relative as appropriate, etc. etc. For my NixOS use-case they are also fine because they are never used as downstream install variables, only for reading not writing. To avoid the problems I face, and restore symmetry, I deleted the exported and arranged to have many `${project}_TOOLS_INSTALL_DIR`s. `AddLLVM` now instead expects each project to define its own, and they do so based on `CMAKE_INSTALL_BINDIR`. `LLVMConfig` still exports `LLVM_TOOLS_BINARY_DIR` which is the location for the tools defined in the usual way, matching the other remaining exported variables. For the `AddLLVM` changes, I tried to copy the existing pattern of internal vs non-internal or for LLVM vs for downstream function/macro names, but it would good to confirm I did that correctly. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D117977 --- openmp/libomptarget/tools/CMakeLists.txt | 14 ++++++++++++++ openmp/libomptarget/tools/deviceinfo/CMakeLists.txt | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'openmp') diff --git a/openmp/libomptarget/tools/CMakeLists.txt b/openmp/libomptarget/tools/CMakeLists.txt index ca5e785..9237035 100644 --- a/openmp/libomptarget/tools/CMakeLists.txt +++ b/openmp/libomptarget/tools/CMakeLists.txt @@ -10,4 +10,18 @@ # ##===----------------------------------------------------------------------===## +set(OPENMP_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE PATH + "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')") +mark_as_advanced(OPENMP_TOOLS_INSTALL_DIR) + +# Move these macros to AddOpenMP if such a CMake module is ever created. + +macro(add_openmp_tool name) + llvm_add_tool(OPENMP ${ARGV}) +endmacro() + +macro(add_openmp_tool_symlink name) + llvm_add_tool_symlink(OPENMP ${ARGV}) +endmacro() + add_subdirectory(deviceinfo) diff --git a/openmp/libomptarget/tools/deviceinfo/CMakeLists.txt b/openmp/libomptarget/tools/deviceinfo/CMakeLists.txt index 290fc83..62d36d7 100644 --- a/openmp/libomptarget/tools/deviceinfo/CMakeLists.txt +++ b/openmp/libomptarget/tools/deviceinfo/CMakeLists.txt @@ -12,7 +12,7 @@ libomptarget_say("Building the llvm-omp-device-info tool") -add_llvm_tool(llvm-omp-device-info llvm-omp-device-info.cpp) +add_openmp_tool(llvm-omp-device-info llvm-omp-device-info.cpp) llvm_update_compile_flags(llvm-omp-device-info) -- cgit v1.1