[PATCH] D146666: [CMake] Respect variables for specifying host tools even without LLVM_USE_HOST_TOOLS set

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added reviewers: smeenai, tstellar, beanz.
Herald added subscribers: bzcheeseman, rriddle.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added subscribers: limo1996, stephenneuendorffer.
Herald added a project: LLVM.

When LLVM_NATIVE_TOOL_DIR was introduced in
d3da9067d143f3d4ce59b6d9ab4606a8ef1dc937 <https://reviews.llvm.org/rGd3da9067d143f3d4ce59b6d9ab4606a8ef1dc937> / D131052 <https://reviews.llvm.org/D131052>, it consisted
of refactoring a couple cases of manual logic for tools in
clang-tools-extra/clang-tidy, clang-tools-extra/pseudo/include
and mlir/tools/mlir-linalg-ods-gen. The former two had the same
consistent behaviour while the latter was slightly different, so
the refactoring would end up slightly adjusting one or the other.

The difference was that the clang-tools-extra tools respected the
external variable for setting the tool name, regardless of the
LLVM_USE_HOST_TOOLS variable, while mlir-linalg-ods-gen tool
only checked its external variable if LLVM_USE_HOST_TOOLS was set.

LLVM_USE_HOST_TOOLS is supposed to be enabled automatically whenever
cross compiling, so this shouldn't have been an issue.

In https://github.com/llvm/llvm-project/issues/60784, it seems like
some users do cross compile LLVM, without CMake knowing about it
(without CMAKE_CROSSCOMPILING being set). In these cases, their
build broke, as the variables for pointing to external host tools
no longer were being respected.

Skip the checks for LLVM_USE_HOST_TOOLS and always respect the
variables for pointing to external tools (both the old tool specific
variables, and the new LLVM_NATIVE_TOOL_DIR), if they're set. This
makes the logic within setup_host_tool more exactly match the
logic for the clang-tools-extra tools from before the refactoring
in d3da9067d143f3d4ce59b6d9ab4606a8ef1dc937 <https://reviews.llvm.org/rGd3da9067d143f3d4ce59b6d9ab4606a8ef1dc937>.

This should hopefully fix
https://github.com/llvm/llvm-project/issues/60784 (and should
probably be backported to the 16.x release branch in the end).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146666

Files:
  llvm/cmake/modules/AddLLVM.cmake


Index: llvm/cmake/modules/AddLLVM.cmake
===================================================================
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -2418,7 +2418,7 @@
 function(setup_host_tool tool_name setting_name exe_var_name target_var_name)
   set(${setting_name}_DEFAULT "${tool_name}")
 
-  if(LLVM_USE_HOST_TOOLS AND LLVM_NATIVE_TOOL_DIR)
+  if(LLVM_NATIVE_TOOL_DIR)
     if(EXISTS "${LLVM_NATIVE_TOOL_DIR}/${tool_name}${LLVM_HOST_EXECUTABLE_SUFFIX}")
       set(${setting_name}_DEFAULT "${LLVM_NATIVE_TOOL_DIR}/${tool_name}${LLVM_HOST_EXECUTABLE_SUFFIX}")
     endif()
@@ -2427,14 +2427,12 @@
   set(${setting_name} "${${setting_name}_DEFAULT}" CACHE
     STRING "Host ${tool_name} executable. Saves building if cross-compiling.")
 
-  if(LLVM_USE_HOST_TOOLS)
-    if(NOT ${setting_name} STREQUAL "${tool_name}")
-      set(exe_name ${${setting_name}})
-      set(target_name ${${setting_name}})
-    else()
-      build_native_tool(${tool_name} exe_name DEPENDS ${tool_name})
-      set(target_name ${exe_name})
-    endif()
+  if(NOT ${setting_name} STREQUAL "${tool_name}")
+    set(exe_name ${${setting_name}})
+    set(target_name ${${setting_name}})
+  elseif(LLVM_USE_HOST_TOOLS)
+    build_native_tool(${tool_name} exe_name DEPENDS ${tool_name})
+    set(target_name ${exe_name})
   else()
     set(exe_name $<TARGET_FILE:${tool_name}>)
     set(target_name ${tool_name})


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146666.507531.patch
Type: text/x-patch
Size: 1426 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230322/891f6d07/attachment.bin>