[Mlir-commits] [llvm] [mlir] Fix CMake dependencies on mlir-linalg-ods-yaml-gen (PR #113565)

[Thomas Preud'homme]

https://github.com/RoboTux created https://github.com/llvm/llvm-project/pull/113565

Fix a number of dependencies issue to build mlir-linalg-ods-yaml-gen
host binary which make a cross-build using the Make generator fail.
Namely:

- do not use binary path for the custom target created when
  LLVM_USE_HOST_TOOLS is true;
- use target name instead of name of variable holding the target name
  for add_custom_target and set_target_properties in setup_host_tool();
- force setting of executable and target cache variable which are only
  used as global variables;
- remove dependency on target defined in different directory in
  add_linalg_ods_yaml_gen() since add_custom_target DEPENDS can only be
  used on "files and outputs of custom commands created with
  add_custom_command() command calls in the same directory";
- remove unneeded dependency on ${MLIR_LINALG_ODS_YAML_GEN_EXE}, the
  target dependency will ensure the binary will be built.

Note that we keep using ${MLIR_LINALG_ODS_YAML_GEN_EXE} in the COMMAND
rather than use ${MLIR_LINALG_ODS_YAML_GEN_TARGET} because when
LLVM_NATIVE_TOOL_DIR is used the latter is an empty string.

Testing-wise, all three codepaths in get_host_tool_path() were tested
with both GNU Make and Ninja generators:
- cross-compiling with LLVM_NATIVE_TOOL_DIR checks the if path;
- cross-compiling without LLVM_NATIVE_TOOL_DIR checks the elseif path;
- native build without LLVM_NATIVE_TOOL_DIR checks the else path.


>From 0776e83034cd7f0cfc50ff3c9c5f5e136780d350 Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme <thomas.preudhomme at arm.com>
Date: Mon, 21 Oct 2024 23:19:16 +0100
Subject: [PATCH] Fix CMake dependencies on mlir-linalg-ods-yaml-gen

Fix a number of dependencies issue to build mlir-linalg-ods-yaml-gen
host binary which make a cross-build using the Make generator fail.
Namely:

- do not use binary path for the custom target created when
  LLVM_USE_HOST_TOOLS is true;
- use target name instead of name of variable holding the target name
  for add_custom_target and set_target_properties in setup_host_tool();
- force setting of executable and target cache variable which are only
  used as global variables;
- remove dependency on target defined in different directory in
  add_linalg_ods_yaml_gen() since add_custom_target DEPENDS can only be
  used on "files and outputs of custom commands created with
  add_custom_command() command calls in the same directory";
- remove unneeded dependency on ${MLIR_LINALG_ODS_YAML_GEN_EXE}, the
  target dependency will ensure the binary will be built.

Note that we keep using ${MLIR_LINALG_ODS_YAML_GEN_EXE} in the COMMAND
rather than use ${MLIR_LINALG_ODS_YAML_GEN_TARGET} because when
LLVM_NATIVE_TOOL_DIR is used the latter is an empty string.

Testing-wise, all three codepaths in get_host_tool_path() were tested
with both GNU Make and Ninja generators:
- cross-compiling with LLVM_NATIVE_TOOL_DIR checks the if path;
- cross-compiling without LLVM_NATIVE_TOOL_DIR checks the elseif path;
- native build without LLVM_NATIVE_TOOL_DIR checks the else path.
---
 llvm/cmake/modules/AddLLVM.cmake                   | 13 ++++++++-----
 mlir/include/mlir/Dialect/Linalg/IR/CMakeLists.txt |  3 ---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 9f5e4344b898a7..006dfb6de3a199 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -2618,13 +2618,16 @@ function(get_host_tool_path tool_name setting_name exe_var_name target_var_name)
     set(target_name "")
   elseif(LLVM_USE_HOST_TOOLS)
     get_native_tool_path(${tool_name} exe_name)
-    set(target_name ${exe_name})
+    set(target_name host_${tool_name})
   else()
     set(exe_name $<TARGET_FILE:${tool_name}>)
     set(target_name ${tool_name})
   endif()
-  set(${exe_var_name} "${exe_name}" CACHE STRING "")
-  set(${target_var_name} "${target_name}" CACHE STRING "")
+  # Force setting the cache variable because they are only used for being
+  # global in scope. Using regular variables would require careful audit of the
+  # code with several parent scope set commands.
+  set(${exe_var_name} "${exe_name}" CACHE STRING "" FORCE)
+  set(${target_var_name} "${target_name}" CACHE STRING "" FORCE)
 endfunction()
 
 function(setup_host_tool tool_name setting_name exe_var_name target_var_name)
@@ -2632,8 +2635,8 @@ function(setup_host_tool tool_name setting_name exe_var_name target_var_name)
   # Set up a native tool build if necessary
   if(LLVM_USE_HOST_TOOLS AND NOT ${setting_name})
     build_native_tool(${tool_name} exe_name DEPENDS ${tool_name})
-    add_custom_target(${target_var_name} DEPENDS ${exe_name})
+    add_custom_target(${${target_var_name}} DEPENDS ${exe_name})
     get_subproject_title(subproject_title)
-    set_target_properties(${target_var_name} PROPERTIES FOLDER "${subproject_title}/Native")
+    set_target_properties(${${target_var_name}} PROPERTIES FOLDER "${subproject_title}/Native")
   endif()
 endfunction()
diff --git a/mlir/include/mlir/Dialect/Linalg/IR/CMakeLists.txt b/mlir/include/mlir/Dialect/Linalg/IR/CMakeLists.txt
index 289c0e4bbdaf68..71214b4404c550 100644
--- a/mlir/include/mlir/Dialect/Linalg/IR/CMakeLists.txt
+++ b/mlir/include/mlir/Dialect/Linalg/IR/CMakeLists.txt
@@ -15,13 +15,10 @@ function(add_linalg_ods_yaml_gen yaml_ast_file output_file)
     MAIN_DEPENDENCY
     ${YAML_AST_SOURCE}
     DEPENDS
-    ${MLIR_LINALG_ODS_YAML_GEN_EXE}
     ${MLIR_LINALG_ODS_YAML_GEN_TARGET})
   add_custom_target(
     MLIR${output_file}YamlIncGen
     DEPENDS
-    ${MLIR_LINALG_ODS_YAML_GEN_EXE}
-    ${MLIR_LINALG_ODS_YAML_GEN_TARGET}
     ${GEN_ODS_FILE} ${GEN_CPP_FILE})
   set_target_properties(MLIR${output_file}YamlIncGen PROPERTIES FOLDER "MLIR/Tablegenning")
   list(APPEND LLVM_TARGET_DEPENDS ${GEN_ODS_FILE})