Fix CMake dependencies on mlir-linalg-ods-yaml-gen #112224
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
RoboTux
requested review from
dcaballe,
nicolasvasilache and
rengolin
as code owners
llvmbot
added
cmake
|
@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/112224.diff 2 Files Affected:
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index c62b5649facae1..eba27c267702d5 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -2618,13 +2618,13 @@ 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 "")
+ set(${exe_var_name} "${exe_name}")
+ set(${target_var_name} "${target_name}")
endfunction()
function(setup_host_tool tool_name setting_name exe_var_name target_var_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})
|
|
@llvm/pr-subscribers-mlir-linalg 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/112224.diff 2 Files Affected:
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index c62b5649facae1..eba27c267702d5 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -2618,13 +2618,13 @@ 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 "")
+ set(${exe_var_name} "${exe_name}")
+ set(${target_var_name} "${target_name}")
endfunction()
function(setup_host_tool tool_name setting_name exe_var_name target_var_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})
|
|
Just a small update over #111973 with cache variable becoming normal variable. |
|
CI failure appears unrelated:
|
|
Ping? |
joker-eph
approved these changes
Oct 21, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/134/builds/7267 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/7076 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/53/builds/6580 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/10757 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/9213 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/5037 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/6245 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/77/builds/4684 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/116/builds/5281 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/9838 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/6478 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/841 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/8013 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/29/builds/5392 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/9338 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/80/builds/5214 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/172/builds/5015 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/4045 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/5264 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/20/builds/4619 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/4641 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/7155 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/8/builds/6042 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/5272 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/54/builds/3268 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/8009 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/155/builds/3399 Here is the relevant piece of the build log for the reference |
joker-eph
added a commit
that referenced
this pull request
Oct 21, 2024
This reverts commit a26bc43.
joker-eph
added a commit
that referenced
this pull request
Oct 21, 2024
Reverts #112224 Many bots are broken
|
I just reverted, seeing how many bots are broken here this seemed like straightforward. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/7/builds/6070 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/12456 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/6720 Here is the relevant piece of the build log for the reference |
Indeed, thanks. That's very odd, previous version didn't break bots and the only difference is the variable no longer being a cache variable but the variable being cache meant incremental builds across the commit was not possible. Sorry about that, I'll figure out what went wrong. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/10387 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/10782 Here is the relevant piece of the build log for the reference |
|
New version in #113565 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: