[llvm-branch-commits] [llvm] ebafcb8 - [CMake] Respect variables for specifying host tools even without LLVM_USE_HOST_TOOLS set

[Tobias Hieta via llvm-branch-commits]

Author: Martin Storsjö
Date: 2023-03-28T08:59:43+02:00
New Revision: ebafcb86c35c00e9ef7c0af59401e395f89b2097

URL: https://github.com/llvm/llvm-project/commit/ebafcb86c35c00e9ef7c0af59401e395f89b2097
DIFF: https://github.com/llvm/llvm-project/commit/ebafcb86c35c00e9ef7c0af59401e395f89b2097.diff

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

When LLVM_NATIVE_TOOL_DIR was introduced in
d3da9067d143f3d4ce59b6d9ab4606a8ef1dc937 / 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.

The fact that CMAKE_CROSSCOMPILING wasn't set stems from a
non-obvious behaviour of CMake; CMAKE_CROSSCOMPILING isn't supposed
to be set by the user (and if it was, it gets overridden), but one
has to set CMAKE_SYSTEM_NAME to indicate that one is cross compiling,
even if the target OS is the same as the current host.

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. This makes the behaviour
consistent with that of the tablegen executables, which also respect
the externally set variables regardless of LLVM_USE_HOST_TOOLS.

This fixes
https://github.com/llvm/llvm-project/issues/60784.

Differential Revision: https://reviews.llvm.org/D146666

(cherry picked from commit 4a5bc791f38a5156bdba87a0572642b1bf3521e9)

Added: 
    

Modified: 
    llvm/cmake/modules/AddLLVM.cmake

Removed: 
    


################################################################################
diff  --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 9eef4eb7e35d2..94fc83db93445 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -2400,7 +2400,7 @@ endfunction()
 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()
@@ -2409,14 +2409,12 @@ function(setup_host_tool tool_name setting_name exe_var_name target_var_name)
   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})