From 5cc8cf69820876074416d951a594948747e99ab6 Mon Sep 17 00:00:00 2001 From: Scott Todd Date: Thu, 3 Nov 2022 13:08:55 -0700 Subject: [PATCH 1/2] Fix several tests on Windows. Most of these were comparing paths using string comparison, which breaks when comparing Unix paths with Windows paths or comparing symlinks with realpaths. --- .../collect_compilation_statistics.py | 6 ++- .../common/benchmark_config_test.py | 40 ++++++++++--------- .../benchmarks/common/common_arguments.py | 4 +- .../common/common_arguments_test.py | 14 ++++--- .../cmake_generator/iree_rule_generator.py | 6 ++- .../iree_rule_generator_test.py | 3 +- .../iree/base/internal/wait_handle_win32.c | 3 ++ 7 files changed, 46 insertions(+), 30 deletions(-) diff --git a/build_tools/benchmarks/collect_compilation_statistics.py b/build_tools/benchmarks/collect_compilation_statistics.py index a6ab616b85f1..0e5a1c6ba565 100755 --- a/build_tools/benchmarks/collect_compilation_statistics.py +++ b/build_tools/benchmarks/collect_compilation_statistics.py @@ -52,12 +52,14 @@ def match_module_cmake_target(module_path: str) -> Optional[str]: return None if os.path.splitext(path_parts[3])[1] != MODULE_FILE_EXTENSION: return None - return os.path.join(*path_parts) + # Join to get the CMake target name. This is *not* a filesystem path, so we + # don't want \ separators on Windows that we would get with os.path.join(). + return '/'.join(path_parts) def parse_compilation_time_from_ninja_log(log: TextIO) -> Dict[str, int]: """Retrieve the compilation time (ms) from the Ninja build log. - + Returns: Map of target name and compilation time in ms. """ diff --git a/build_tools/benchmarks/common/benchmark_config_test.py b/build_tools/benchmarks/common/benchmark_config_test.py index 9926b45c9d6a..b4a284c088ac 100644 --- a/build_tools/benchmarks/common/benchmark_config_test.py +++ b/build_tools/benchmarks/common/benchmark_config_test.py @@ -19,14 +19,17 @@ class BenchmarkConfigTest(unittest.TestCase): def setUp(self): self.build_dir = tempfile.TemporaryDirectory() self.tmp_dir = tempfile.TemporaryDirectory() - self.normal_tool_dir = os.path.join(self.build_dir.name, "normal_tool") + self.normal_tool_dir = os.path.realpath( + os.path.join(self.build_dir.name, "normal_tool")) os.mkdir(self.normal_tool_dir) - self.traced_tool_dir = os.path.join(self.build_dir.name, "traced_tool") + self.traced_tool_dir = os.path.realpath( + os.path.join(self.build_dir.name, "traced_tool")) os.mkdir(self.traced_tool_dir) self.trace_capture_tool = tempfile.NamedTemporaryFile() os.chmod(self.trace_capture_tool.name, stat.S_IEXEC) def tearDown(self): + self.trace_capture_tool.close() self.tmp_dir.cleanup() self.build_dir.cleanup() @@ -43,26 +46,27 @@ def test_build_from_args(self): config = BenchmarkConfig.build_from_args(args=args, git_commit_hash="abcd") - per_commit_tmp_dir = os.path.join(self.tmp_dir.name, "abcd") + per_commit_tmp_dir = os.path.realpath( + os.path.join(self.tmp_dir.name, "abcd")) expected_trace_capture_config = TraceCaptureConfig( traced_benchmark_tool_dir=self.traced_tool_dir, - trace_capture_tool=self.trace_capture_tool.name, + trace_capture_tool=os.path.realpath(self.trace_capture_tool.name), capture_tarball=os.path.realpath("capture.tar"), capture_tmp_dir=os.path.join(per_commit_tmp_dir, "captures")) - self.assertEqual( - config, - BenchmarkConfig(root_benchmark_dir=os.path.join(self.build_dir.name, - "benchmark_suites"), - benchmark_results_dir=os.path.join( - per_commit_tmp_dir, "benchmark-results"), - git_commit_hash="abcd", - normal_benchmark_tool_dir=self.normal_tool_dir, - trace_capture_config=expected_trace_capture_config, - driver_filter="a", - model_name_filter="b", - mode_filter="c", - keep_going=True, - benchmark_min_time=10)) + expected_config = BenchmarkConfig( + root_benchmark_dir=os.path.realpath( + os.path.join(self.build_dir.name, "benchmark_suites")), + benchmark_results_dir=os.path.realpath( + os.path.join(per_commit_tmp_dir, "benchmark-results")), + git_commit_hash="abcd", + normal_benchmark_tool_dir=self.normal_tool_dir, + trace_capture_config=expected_trace_capture_config, + driver_filter="a", + model_name_filter="b", + mode_filter="c", + keep_going=True, + benchmark_min_time=10) + self.assertEqual(config, expected_config) def test_build_from_args_benchmark_only(self): args = build_common_argument_parser().parse_args([ diff --git a/build_tools/benchmarks/common/common_arguments.py b/build_tools/benchmarks/common/common_arguments.py index c31cc0baafde..4630b19e6ef2 100644 --- a/build_tools/benchmarks/common/common_arguments.py +++ b/build_tools/benchmarks/common/common_arguments.py @@ -35,12 +35,12 @@ def check_exe_path(path): parser.add_argument( "--normal_benchmark_tool_dir", "--normal-benchmark-tool-dir", - type=check_exe_path, + type=check_dir_path, default=None, help="Path to the normal (non-tracing) iree tool directory") parser.add_argument("--traced_benchmark_tool_dir", "--traced-benchmark-tool-dir", - type=check_exe_path, + type=check_dir_path, default=None, help="Path to the tracing-enabled iree tool directory") parser.add_argument("--trace_capture_tool", diff --git a/build_tools/benchmarks/common/common_arguments_test.py b/build_tools/benchmarks/common/common_arguments_test.py index 8660c84c27ce..520996d3cc88 100644 --- a/build_tools/benchmarks/common/common_arguments_test.py +++ b/build_tools/benchmarks/common/common_arguments_test.py @@ -6,6 +6,8 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import unittest +import shutil +import tempfile from common.common_arguments import build_common_argument_parser @@ -13,11 +15,13 @@ class CommonArgumentsTest(unittest.TestCase): def test_build_common_argument_parser(self): - arg_parser = build_common_argument_parser() - arg_parser.parse_args([ - "--normal_benchmark_tool_dir=/tmp", "--traced_benchmark_tool_dir=/tmp", - "--trace_capture_tool=/bin/ls", "." - ]) + with tempfile.TemporaryDirectory() as tempdir: + arg_parser = build_common_argument_parser() + arg_parser.parse_args([ + "--normal_benchmark_tool_dir=" + tempdir, + "--traced_benchmark_tool_dir=" + tempdir, + "--trace_capture_tool=" + shutil.which("ls"), "." + ]) def test_build_common_argument_parser_check_build_dir(self): arg_parser = build_common_argument_parser() diff --git a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py index feb7b91e5364..2b5d0ff44777 100644 --- a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py +++ b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py @@ -44,8 +44,10 @@ def build_model_import_rule( model = imported_model.model if model.source_type == common_definitions.ModelSourceType.EXPORTED_LINALG_MLIR: - if source_model_rule.file_path != str(output_file_path): - raise ValueError("Separate path for Linalg model isn't supported.") + if pathlib.Path(source_model_rule.file_path) != output_file_path: + raise ValueError("Separate path for Linalg model isn't supported ('" + + source_model_rule.file_path + "' != '" + + str(output_file_path) + "')") return IreeModelImportRule(target_name=source_model_rule.target_name, output_file_path=str(output_file_path), cmake_rules=[]) diff --git a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py index 9709acbe63a0..8c5746e35e80 100644 --- a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py +++ b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py @@ -47,7 +47,8 @@ def test_build_model_import_rule_linalg(self): output_file_path=pathlib.PurePath(model_rule.file_path)) self.assertEqual(rule.target_name, model_rule.target_name) - self.assertEqual(rule.output_file_path, model_rule.file_path) + self.assertEqual(pathlib.Path(rule.output_file_path), + pathlib.Path(model_rule.file_path)) def test_build_module_compile_rule(self): model_import_rule = iree_rule_generator.IreeModelImportRule( diff --git a/runtime/src/iree/base/internal/wait_handle_win32.c b/runtime/src/iree/base/internal/wait_handle_win32.c index b583a36e2268..9822f8a8818b 100644 --- a/runtime/src/iree/base/internal/wait_handle_win32.c +++ b/runtime/src/iree/base/internal/wait_handle_win32.c @@ -383,6 +383,9 @@ iree_status_t iree_wait_any(iree_wait_set_t* set, iree_time_t deadline_ns, iree_status_t iree_wait_one(iree_wait_handle_t* handle, iree_time_t deadline_ns) { + if (handle->type == IREE_WAIT_PRIMITIVE_TYPE_NONE) { + return iree_ok_status(); + } IREE_TRACE_ZONE_BEGIN(z0); // Remap absolute timeout to relative timeout, handling special values as From 7f68f601a8c747c467565260aca813a6ce45eb80 Mon Sep 17 00:00:00 2001 From: Scott Todd Date: Fri, 4 Nov 2022 14:35:01 -0700 Subject: [PATCH 2/2] Use PurePath instead of Path in a few places. --- .../e2e_test_artifacts/cmake_generator/iree_rule_generator.py | 2 +- .../cmake_generator/iree_rule_generator_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py index 2b5d0ff44777..e13139d4af7e 100644 --- a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py +++ b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py @@ -44,7 +44,7 @@ def build_model_import_rule( model = imported_model.model if model.source_type == common_definitions.ModelSourceType.EXPORTED_LINALG_MLIR: - if pathlib.Path(source_model_rule.file_path) != output_file_path: + if pathlib.PurePath(source_model_rule.file_path) != output_file_path: raise ValueError("Separate path for Linalg model isn't supported ('" + source_model_rule.file_path + "' != '" + str(output_file_path) + "')") diff --git a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py index 8c5746e35e80..401d8ce2916b 100644 --- a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py +++ b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator_test.py @@ -47,8 +47,8 @@ def test_build_model_import_rule_linalg(self): output_file_path=pathlib.PurePath(model_rule.file_path)) self.assertEqual(rule.target_name, model_rule.target_name) - self.assertEqual(pathlib.Path(rule.output_file_path), - pathlib.Path(model_rule.file_path)) + self.assertEqual(pathlib.PurePath(rule.output_file_path), + pathlib.PurePath(model_rule.file_path)) def test_build_module_compile_rule(self): model_import_rule = iree_rule_generator.IreeModelImportRule(