-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix CMake dependencies on mlir-linalg-ods-yaml-gen #111973
Conversation
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(); - 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/pr-subscribers-mlir-linalg @llvm/pr-subscribers-mlir Author: Thomas Preud'homme (RoboTux) ChangesFix a number of dependencies issue to build mlir-linalg-ods-yaml-gen
Note that we keep using ${MLIR_LINALG_ODS_YAML_GEN_EXE} in the COMMAND Testing-wise, all three codepaths in get_host_tool_path() were tested
Full diff: https://github.com/llvm/llvm-project/pull/111973.diff 2 Files Affected:
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index c62b5649facae1..abffa3ec20386f 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -2618,7 +2618,7 @@ 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})
@@ -2632,8 +2632,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})
|
Changes in |
Apparently these changes look fine to me. But someone aware with Dialects/Linalg project should approve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks ok to me. The way it was never looked right to me.
(We're trying to remove this piece entirely but it will take some time)
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(); - 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.
This commit causes these errors when I configure: -- Clang version: 20.0.0git
CMake Error at cmake/modules/AddLLVM.cmake:2635 (add_custom_target):
add_custom_target called with invalid target name
"/home/tbaeder/code/llvm-project/build/NATIVE/bin/clang". Target names may
not contain a slash. Use ADD_CUSTOM_COMMAND to generate files.
Call Stack (most recent call first):
/home/tbaeder/code/llvm-project/clang/tools/driver/CMakeLists.txt:41 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2637 (set_target_properties):
set_target_properties Can not find target to add properties to:
/home/tbaeder/code/llvm-project/build/NATIVE/bin/clang
Call Stack (most recent call first):
/home/tbaeder/code/llvm-project/clang/tools/driver/CMakeLists.txt:41 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2635 (add_custom_target):
add_custom_target called with invalid target name
"/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-as". Target names
may not contain a slash. Use ADD_CUSTOM_COMMAND to generate files.
Call Stack (most recent call first):
tools/llvm-as/CMakeLists.txt:15 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2637 (set_target_properties):
set_target_properties Can not find target to add properties to:
/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-as
Call Stack (most recent call first):
tools/llvm-as/CMakeLists.txt:15 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2635 (add_custom_target):
add_custom_target called with invalid target name
"/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-link". Target names
may not contain a slash. Use ADD_CUSTOM_COMMAND to generate files.
Call Stack (most recent call first):
tools/llvm-link/CMakeLists.txt:21 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2637 (set_target_properties):
set_target_properties Can not find target to add properties to:
/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-link
Call Stack (most recent call first):
tools/llvm-link/CMakeLists.txt:21 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2635 (add_custom_target):
add_custom_target called with invalid target name
"/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-nm". Target names
may not contain a slash. Use ADD_CUSTOM_COMMAND to generate files.
Call Stack (most recent call first):
tools/llvm-nm/CMakeLists.txt:29 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2637 (set_target_properties):
set_target_properties Can not find target to add properties to:
/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-nm
Call Stack (most recent call first):
tools/llvm-nm/CMakeLists.txt:29 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2635 (add_custom_target):
add_custom_target called with invalid target name
"/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-readobj". Target
names may not contain a slash. Use ADD_CUSTOM_COMMAND to generate files.
Call Stack (most recent call first):
tools/llvm-readobj/CMakeLists.txt:33 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2637 (set_target_properties):
set_target_properties Can not find target to add properties to:
/home/tbaeder/code/llvm-project/build/NATIVE/bin/llvm-readobj
Call Stack (most recent call first):
tools/llvm-readobj/CMakeLists.txt:33 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2635 (add_custom_target):
add_custom_target called with invalid target name
"/home/tbaeder/code/llvm-project/build/NATIVE/bin/opt". Target names may
not contain a slash. Use ADD_CUSTOM_COMMAND to generate files.
Call Stack (most recent call first):
tools/opt/CMakeLists.txt:53 (setup_host_tool)
CMake Error at cmake/modules/AddLLVM.cmake:2637 (set_target_properties):
set_target_properties Can not find target to add properties to:
/home/tbaeder/code/llvm-project/build/NATIVE/bin/opt
Call Stack (most recent call first):
tools/opt/CMakeLists.txt:53 (setup_host_tool)
-- Configuring incomplete, errors occurred! Reverting the patch makes things work again. |
Ack, I'll revert until I figure out what's the problem. |
This reverts commit d6827f6 due to the following CMake configure failure being observed by some: add_custom_target called with invalid target name
…)" This reverts commit d6827f6.
@tbaederr Was it an incremental build? With this patch the target name should be ${tool_name}, i.e. clang. However it is set in that way: |
Yes, it was my local build. A |
I'm guessing it does not. I could change the variable to not be a cache variable. @mstorsjo Is there a reason why get_host_tool_path() sets the executable and target variables as cache? It seems to me to be build system internals. |
Earlier, if users wanted to use a prebuilt e.g. |
Are you talking about LLVM_TABLEGEN? That seems like a different thing because if I try to trace all calls to get_host_tool_path it seems the cache variable it looks for are named:
I'm tempted to make it a non cache variable because the only alternative I can think of is to tell people to nuke their existing build if they encounter issues. |
Yes, that's the one I meant. Sorry, I didn't have time to dig in closer what this really was touching.
Indeed, I don't see any reason to keep those variables as CACHE variables. |
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(); - 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.
I've respinned the PR to make cache variable regular variable instead in #112224 |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1376 Here is the relevant piece of the build log for the reference
|
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(); - 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.
…)" This reverts commit d6827f6 due to the following CMake configure failure being observed by some: add_custom_target called with invalid target name
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(); - 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.
…)" This reverts commit d6827f6 due to the following CMake configure failure being observed by some: add_custom_target called with invalid target name
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(); - 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.
…)" This reverts commit d6827f6 due to the following CMake configure failure being observed by some: add_custom_target called with invalid target name
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:
LLVM_USE_HOST_TOOLS is true;
for add_custom_target and set_target_properties in setup_host_tool();
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";
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: