Skip to content

Commit

Permalink
Add missing dependency of check-onnx-lit on onnx-mlir (onnx#597)
Browse files Browse the repository at this point in the history
All of the CI builds (and in our normal workflows) generally build onnx-mlir first and then proceed to build the tests, so onnx-mlir usually exists which is why this hadn't been caught before. The Windows CI build was not running the tests correctly because of python issues (see onnx#590), so it thought they always passed marking the Windows CI build as succeeded because it is the last step to run. However, that was masking the fact that the build of onnx-mlir is failing!

This command:

```call cmake --build . --config Release --target onnx-mlir -- /m```

would run and fail. Then this command:

```call cmake --build . --config Release --target check-onnx-lit```

would run but not execute the tests and mark the build as succeeded. Since check-onnx-lit doesn't have a dependency on onnx-mlir, it does not build onnx-mlir and the earlier failure of the onnx-mlir build gets lost.

Long story short, check-onnx-lit has a dependency on onnx-mlir which this change adds. After the addition of the dependency, the Windows CI build is now correctly reporting that the Windows build of onnx-mlir in master is broken.

Signed-off-by: Stella Stamenova <[email protected]>

Co-authored-by: Kevin O'Brien <[email protected]>
  • Loading branch information
sstamenova and caoimhinuibrian authored Apr 9, 2021
1 parent fe6b58a commit 137cdc1
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 3 deletions.
8 changes: 6 additions & 2 deletions test/mlir/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ configure_lit_site_cfg(${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
MAIN_CONFIG
${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py)

set(ONNX_MLIR_TEST_DEPENDS onnx-mlir-opt binary-decoder)
set(ONNX_MLIR_TEST_DEPENDS
binary-decoder
onnx-mlir
onnx-mlir-opt
)

add_lit_testsuite(check-onnx-lit
"Running the ONNX MLIR regression tests"
Expand All @@ -19,4 +23,4 @@ set_target_properties(check-onnx-lit PROPERTIES FOLDER "Tests")
add_lit_testsuites(ONNX_MLIR
${CMAKE_CURRENT_SOURCE_DIR}
DEPENDS
${ONNX_MLIR_TEST_DEPS})
${ONNX_MLIR_TEST_DEPENDS})
2 changes: 1 addition & 1 deletion test/mlir/lit.cfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
config.onnx_mlir_tools_dir, config.mlir_tools_dir, config.llvm_tools_dir
]
tool_names = [
'onnx-mlir', 'onnx-mlir-opt', 'mlir-opt', 'mlir-translate', "binary-decoder"
'onnx-mlir', 'onnx-mlir-opt', 'mlir-opt', 'mlir-translate', 'binary-decoder'
]
tools = [ToolSubst(s, unresolved='ignore') for s in tool_names]
llvm_config.add_tool_substitutions(tools, tool_dirs)

0 comments on commit 137cdc1

Please sign in to comment.