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

@@ -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(

compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/AIETarget.cpp

@@ -279,18 +279,13 @@ 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<SmallString<128>> maybeWorkDir;
   // If a path for intermediates has been specified, assume it is common for
   // all executables compiling in parallel, and so create an
   // executable-specific subdir to keep this executable's intermediates
   // 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