From daf2eca1f164e05d6949f949c23c99e37c38dde3 Mon Sep 17 00:00:00 2001 From: James Newling Date: Thu, 28 Nov 2024 06:54:16 -0800 Subject: [PATCH] Improve directory structure for test generated files (#942) Before, certain files were overwritten between tests, if the dispatch operation was the same. Now, no files are overwritten. Each test now has a subdirectory containing all its files (and no further subdirectories). --- build_tools/ci/cpu_comparison/run.py | 116 +++++++++++------- .../AMD-AIE/iree-amd-aie/Target/AIETarget.cpp | 5 - 2 files changed, 73 insertions(+), 48 deletions(-) diff --git a/build_tools/ci/cpu_comparison/run.py b/build_tools/ci/cpu_comparison/run.py index 41eb7621e..7651f3b93 100755 --- a/build_tools/ci/cpu_comparison/run.py +++ b/build_tools/ci/cpu_comparison/run.py @@ -120,7 +120,13 @@ def run(self, config): @abstractmethod def _execute(self, config): - raise RuntimeError("Derived class must implement this method") + raise NotImplementedError("Derived class must implement this method") + + def get_dir(self, config): + return config.get_test_dir(self.name) + + def get_filename(self, config): + return self.get_dir(config) / f"{self.name}.mlir" class ConvolutionFromTemplate(BaseTest): @@ -139,8 +145,7 @@ def __init__(self, params): def _execute(self, config): # Generate MLIR file: - output_dir = config.output_dir - filename = output_dir / f"{self.name}.mlir" + filename = self.get_filename(config) self.generator.write_to_file(filename) # Perform numerical comparison between AIE and CPU: return run_conv_test(config, self.aie_compilation_flags, filename, n_repeats=2) @@ -231,7 +236,10 @@ def __init__( self.labels.append("UKernel") self.function_name = function_name - def vs_cpu(self, config, filename): + def vs_cpu(self, config): + + filename = self.get_filename(config) + if self.use_ukernel and not config.vitis_dir: return False @@ -247,9 +255,13 @@ def vs_cpu(self, config, filename): return True - def benchmark(self, config, filename): + def benchmark(self, config): + + filename = self.get_filename(config) + if self.use_ukernel and not config.vitis_dir: return False + benchmark_aie( config=config, aie_compilation_flags=self.aie_compilation_flags, @@ -261,11 +273,13 @@ def benchmark(self, config, filename): n_repeats=self.n_repeats, n_kernel_runs=self.n_kernel_runs, ) + return True - def generate(self, filename, template_name): + def generate(self, config, template_name): + generate_matmul_test( - filename, + self.get_filename(config), template_name, self.M, self.N, @@ -295,11 +309,10 @@ def __init__(self, M, N, K, input_type, acc_type, run_on_target=["npu1_4col"]): self.name = f"matmul_full_bias_{M}_{N}_{K}_{input_type}_{acc_type}" def _execute(self, config): - filename = config.output_dir / f"{self.name}.mlir" matmul_template_dir = config.file_dir / "matmul_template" template_name = matmul_template_dir / "matmul_bias_MxK_KxN_MxN.mlir" - self.generate(filename, template_name) - self.vs_cpu(config, filename) + self.generate(config, template_name) + self.vs_cpu(config) return True @@ -351,11 +364,10 @@ def __init__( self.use_ukernel = use_ukernel def _execute(self, config): - self.filename = config.output_dir / f"{self.name}.mlir" matmul_template_dir = config.file_dir / "matmul_template" template_name = matmul_template_dir / "matmul_MxK_KxN.mlir" - self.generate(self.filename, template_name) - self.vs_cpu(config, self.filename) + self.generate(config, template_name) + self.vs_cpu(config) return True @@ -413,11 +425,10 @@ def __init__( self.use_ukernel = use_ukernel def _execute(self, config): - self.filename = config.output_dir / f"{self.name}.mlir" matmul_template_dir = config.file_dir / "matmul_template" template_name = matmul_template_dir / "matmul_MxK_KxN.mlir" - self.generate(self.filename, template_name) - return self.benchmark(config, self.filename) + self.generate(config, template_name) + return self.benchmark(config) class MatmulThinBias(BaseMatmul): @@ -453,11 +464,10 @@ def __init__( self.name += "_ukernel" def _execute(self, config): - self.filename = config.output_dir / f"{self.name}.mlir" matmul_template_dir = config.file_dir / "matmul_template" template_name = matmul_template_dir / "matmul_bias_MxK_KxN_N.mlir" - self.generate(self.filename, template_name) - return self.vs_cpu(config, self.filename) + self.generate(config, template_name) + return self.vs_cpu(config) class BatchMatmul(BaseMatmul): @@ -483,11 +493,10 @@ def __init__(self, B, M, N, K, input_type, acc_type, run_on_target=["npu1_4col"] self.B = B def _execute(self, config): - self.filename = config.output_dir / f"{self.name}.mlir" matmul_template_dir = config.file_dir / "matmul_template" template_name = matmul_template_dir / "batch_matmul_BxMxK_BxKxN.mlir" generate_matmul_test( - self.filename, + self.get_filename(config), template_name, k=self.K, b=self.B, @@ -496,7 +505,7 @@ def _execute(self, config): lhs_rhs_type=self.input_type, acc_type=self.acc_type, ) - return self.vs_cpu(config, self.filename) + return self.vs_cpu(config) class MatmulTruncf(BaseMatmul): @@ -539,13 +548,12 @@ def __init__( self.expected_out = expected_out def _execute(self, config): - self.filename = config.output_dir / f"{self.name}.mlir" matmul_template_dir = config.file_dir / "matmul_template" template_name = matmul_template_dir / "matmul_truncf_MxK_KxN.mlir" - self.generate(self.filename, template_name) - + self.generate(config, template_name) + filename = self.get_filename(config) input_args = generate_inputs( - self.filename, config.output_dir, 1, {1: self.lhs, 2: self.rhs} + filename, self.get_dir(config), 1, {1: self.lhs, 2: self.rhs} ) """ currently without enabling loop coalescing and unit dimension collapsing @@ -562,7 +570,7 @@ def _execute(self, config): aie_vs_baseline( config=config, aie_compilation_flags=self.aie_compilation_flags, - test_file=self.filename, + test_file=self.get_filename(config), input_args=input_args, baseline_value=self.expected_out, use_ukernel=self.use_ukernel, @@ -573,7 +581,7 @@ def _execute(self, config): atol=0, lower_to_aie_pipeline=self.lower_to_aie_pipeline, n_repeats=self.n_repeats, - output_type=get_output_type(self.filename), + output_type=get_output_type(self.get_filename(config)), ) return True @@ -670,6 +678,8 @@ def generate_aie_vmfb( additional_flags = aie_compilation_flags + test_dir = config.get_test_dir(name) + aie_compilation_flags = [ config.iree_compile_exe, test_file, @@ -681,7 +691,7 @@ def generate_aie_vmfb( f"--iree-amd-aie-peano-install-dir={config.peano_dir}", f"--iree-amd-aie-install-dir={config.iree_dir}", f"--iree-amd-aie-vitis-install-dir={config.vitis_dir}", - f"--iree-hal-dump-executable-files-to={config.output_dir}", + f"--iree-hal-dump-executable-files-to={test_dir}", f"--iree-amdaie-device-hal={config.device_hal}", "--iree-scheduling-optimize-bindings=false", "--iree-hal-memoization=false", @@ -702,16 +712,16 @@ def generate_aie_vmfb( aie_compilation_flags += [ "-o", - config.output_dir / f"{name}_aie.vmfb", + test_dir / f"{name}_aie.vmfb", ] start = time.monotonic_ns() - shell_out(aie_compilation_flags, config.output_dir, config.verbose) + shell_out(aie_compilation_flags, test_dir, config.verbose) compile_time = time.monotonic_ns() - start if config.verbose: print(f"Time spent in compilation: {compile_time // 1e6} [ms]") - aie_vmfb = config.output_dir / f"{name}_aie.vmfb" + aie_vmfb = test_dir / f"{name}_aie.vmfb" if not aie_vmfb.exists(): raise RuntimeError(f"Failed to compile {test_file} to {aie_vmfb}") @@ -723,7 +733,8 @@ def generate_aie_output(config, aie_vmfb, input_args, function_name, name, outpu Run a compiled AIE module (aie_vmfb), returning a numpy array of the output. """ - aie_bin = config.output_dir / f"{name}_aie.bin" + test_dir = config.get_test_dir(name) + aie_bin = test_dir / f"{name}_aie.bin" run_args = [ config.iree_run_exe, f"--module={aie_vmfb}", @@ -742,7 +753,7 @@ def generate_aie_output(config, aie_vmfb, input_args, function_name, name, outpu shell_out(config.reset_npu_script, verbose=config.verbose) start = time.monotonic_ns() - shell_out(run_args, config.output_dir, config.verbose) + shell_out(run_args, test_dir, config.verbose) run_time = time.monotonic_ns() - start if config.verbose: @@ -757,7 +768,8 @@ def benchmark_aie_kernel_time( """ Benchmark a compiled AIE module's (aie_vmfb) kernel time, average over the specified number of runs. """ - aie_bin = config.output_dir / f"{name}_aie.bin" + test_dir = config.get_test_dir(name) + aie_bin = test_dir / f"{name}_aie.bin" run_args = [ config.iree_benchmark_exe, f"--module={aie_vmfb}", @@ -778,7 +790,7 @@ def benchmark_aie_kernel_time( shell_out(config.reset_npu_script, verbose=config.verbose) start = time.monotonic_ns() - shell_out(run_args, config.output_dir, config.verbose) + shell_out(run_args, test_dir, config.verbose) run_time = time.monotonic_ns() - start if config.verbose: @@ -799,7 +811,8 @@ def generate_llvm_cpu_output( of the output. """ - cpu_vmfb = config.output_dir / f"{name}_cpu.vmfb" + test_dir = config.get_test_dir(name) + cpu_vmfb = test_dir / f"{name}_cpu.vmfb" aie_compilation_flags = [ config.iree_compile_exe, test_file, @@ -808,9 +821,9 @@ def generate_llvm_cpu_output( "-o", f"{cpu_vmfb}", ] - shell_out(aie_compilation_flags, workdir=config.output_dir, verbose=config.verbose) + shell_out(aie_compilation_flags, workdir=test_dir, verbose=config.verbose) - cpu_bin = config.output_dir / f"{name}_cpu.bin" + cpu_bin = test_dir / f"{name}_cpu.bin" run_args = [ config.iree_run_exe, f"--module={cpu_vmfb}", @@ -819,7 +832,7 @@ def generate_llvm_cpu_output( ] if function_name: run_args += [f"--function={function_name}"] - shell_out(run_args, workdir=config.output_dir, verbose=config.verbose) + shell_out(run_args, workdir=test_dir, verbose=config.verbose) return np_from_binfile(cpu_bin, output_type) @@ -828,6 +841,18 @@ class TestConfig: Global state used for all tests. Stores paths to executables used. """ + def get_test_dir(self, test_name): + """ + Return the subdirectory `test_name` of `output_dir`. + (1) Assert that `test_name` is a 'good' name (no '.' no whitespace, etc) + (2) Assert that `output_dir` / `test_name` exists and is a directory (create if not) + """ + if not test_name.isidentifier(): + raise ValueError(f"test_name '{test_name}' is not a valid identifier") + test_dir = self.output_dir / test_name + test_dir.mkdir(parents=True, exist_ok=True) + return test_dir + def __init__( self, output_dir, @@ -1154,7 +1179,8 @@ def benchmark_aie( ) name = name_from_mlir_filename(test_file) - input_args = generate_inputs(test_file, config.output_dir, seed) + test_dir = config.get_test_dir(name) + input_args = generate_inputs(test_file, test_dir, seed) aie_vmfb = generate_aie_vmfb( config, @@ -1210,7 +1236,9 @@ def aie_vs_llvm_cpu( if config.verbose: print(f"Running {name} test") - input_args = generate_inputs(test_file, config.output_dir, seed) + test_dir = config.get_test_dir(name) + + input_args = generate_inputs(test_file, test_dir, seed) output_type = get_output_type(test_file) cpu_output = generate_llvm_cpu_output( @@ -1610,7 +1638,9 @@ def all_tests( abs_path = lambda x: Path(x).absolute() parser.add_argument( - "output_dir", type=abs_path, help="The directory where artifacts are saved." + "output_dir", + type=abs_path, + help="The directory where artifacts are saved. Each test will have a subdirectory in this directory based on its (unique) name, containing all test specific files.", ) parser.add_argument( diff --git a/compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/AIETarget.cpp b/compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/AIETarget.cpp index a3717a48e..dfb8bb00f 100644 --- a/compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/AIETarget.cpp +++ b/compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/AIETarget.cpp @@ -279,10 +279,6 @@ LogicalResult AIETargetBackend::serializeExecutable( IREE::HAL::ExecutableVariantOp variantOp, OpBuilder &executableBuilder) { ModuleOp moduleOp = variantOp.getInnerModule(); - std::string basename = - llvm::join_items("_", serOptions.dumpBaseName, variantOp.getName()); - sanitizeForBootgen(basename); - FailureOr> maybeWorkDir; // If a path for intermediates has been specified, assume it is common for // all executables compiling in parallel, and so create an @@ -290,7 +286,6 @@ LogicalResult AIETargetBackend::serializeExecutable( // separate. if (!serOptions.dumpIntermediatesPath.empty()) { SmallString<128> workDir{serOptions.dumpIntermediatesPath}; - llvm::sys::path::append(workDir, basename); if (auto ecode = llvm::sys::fs::create_directories(workDir)) { return moduleOp.emitError() << "failed to create working directory " << workDir