[Mlir-commits] [mlir] [mlir][transform] Support symlinks in module loading. Reorganize tests. (PR #69329)
Ingo Müller
llvmlistbot at llvm.org
Tue Oct 17 06:24:13 PDT 2023
https://github.com/ingomueller-net created https://github.com/llvm/llvm-project/pull/69329
A recent commit (#69190) broke the bazel builds. Turns out that Bazel uses symlinks for providing the test files, which the path expansion of the module loading mechanism did not handle correctly. This PR fixes that.
It also reorganizes the tests better: It puts all `.mlir` files that are included by some other test into a common `include` folder. This greatly simplifies the definition of the dependencies between the different `.mlir` files in Bazel's `BUILD` file. The commit also adds a comment to all included files why these aren't tested themselves direclty and uses the `%{fs-sep}` expansion for paths more consistently. Finally, it uncomments all but one of the tests excluded in Bazel because they seem to run now. (The remaining one includes a file that it itself a test, so it would have to live *in* and *outside* of the `include` folder.)
>From 2d670ffeffce3369c6c1880ba99af0b1ddea9a6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ingo=20M=C3=BCller?= <ingomueller at google.com>
Date: Tue, 17 Oct 2023 12:49:09 +0000
Subject: [PATCH] [mlir][transform] Support symlinks in module loading.
Reorganize tests.
A recent commit (#69190) broke the bazel builds. Turns out that Bazel
uses symlinks for providing the test files, which the path expansion of
the module loading mechanism did not handle correctly. This PR fixes
that.
It also reorganizes the tests better: It puts all `.mlir` files that are
included by some other test into a common `include` folder. This greatly
simplifies the definition of the dependencies between the different
`.mlir` files in Bazel's `BUILD` file. The commit also adds a comment to
all included files why these aren't tested themselves direclty and uses
the `%{fs-sep}` expansion for paths more consistently. Finally, it
uncomments all but one of the tests excluded in Bazel because they seem
to run now. (The remaining one includes a file that it itself a test, so
it would have to live *in* and *outside* of the `include` folder.)
---
.../Transforms/TransformInterpreterUtils.cpp | 3 ++-
.../{ => include}/Library/lower-to-llvm.mlir | 1 +
...st-interpreter-external-concurrent-source.mlir | 0
.../test-interpreter-external-source.mlir | 0
...t-interpreter-external-symbol-def-invalid.mlir | 0
.../definitions-self-contained.mlir | 1 +
.../definitions-with-unresolved.mlir | 1 +
mlir/test/Dialect/Transform/preload-library.mlir | 2 +-
.../test-interpreter-external-concurrent.mlir | 2 +-
...rpreter-external-symbol-decl-and-schedule.mlir | 4 ++--
...test-interpreter-external-symbol-decl-dir.mlir | 6 +++---
...-interpreter-external-symbol-decl-invalid.mlir | 2 +-
.../test-interpreter-external-symbol-decl.mlir | 4 ++--
.../Transform/test-interpreter-external.mlir | 2 +-
.../mlir/test/Dialect/BUILD.bazel | 15 ++-------------
15 files changed, 18 insertions(+), 25 deletions(-)
rename mlir/test/Dialect/Transform/{ => include}/Library/lower-to-llvm.mlir (96%)
rename mlir/test/Dialect/Transform/{ => include}/test-interpreter-external-concurrent-source.mlir (100%)
rename mlir/test/Dialect/Transform/{ => include}/test-interpreter-external-source.mlir (100%)
rename mlir/test/Dialect/Transform/{ => include}/test-interpreter-external-symbol-def-invalid.mlir (100%)
rename mlir/test/Dialect/Transform/{ => include}/test-interpreter-library/definitions-self-contained.mlir (96%)
rename mlir/test/Dialect/Transform/{ => include}/test-interpreter-library/definitions-with-unresolved.mlir (78%)
diff --git a/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp b/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
index 41feffffaf97b3f..e6d692072267c1f 100644
--- a/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
+++ b/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
@@ -61,7 +61,8 @@ LogicalResult transform::detail::expandPathsToMLIRFiles(
it != itEnd && !ec; it.increment(ec)) {
const std::string &fileName = it->path();
- if (it->type() != llvm::sys::fs::file_type::regular_file) {
+ if (it->type() != llvm::sys::fs::file_type::regular_file &&
+ it->type() != llvm::sys::fs::file_type::symlink_file) {
LLVM_DEBUG(DBGS() << " Skipping non-regular file '" << fileName
<< "'\n");
continue;
diff --git a/mlir/test/Dialect/Transform/Library/lower-to-llvm.mlir b/mlir/test/Dialect/Transform/include/Library/lower-to-llvm.mlir
similarity index 96%
rename from mlir/test/Dialect/Transform/Library/lower-to-llvm.mlir
rename to mlir/test/Dialect/Transform/include/Library/lower-to-llvm.mlir
index 0ba50bd2362b34d..afd1c89dd2b52f6 100644
--- a/mlir/test/Dialect/Transform/Library/lower-to-llvm.mlir
+++ b/mlir/test/Dialect/Transform/include/Library/lower-to-llvm.mlir
@@ -1,4 +1,5 @@
// RUN: mlir-opt %s
+// No need to check anything else than parsing here, this is being used by another test as data.
/// Schedule to lower to LLVM.
module @lower_module_to_llvm attributes { transform.with_named_sequence } {
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-concurrent-source.mlir b/mlir/test/Dialect/Transform/include/test-interpreter-external-concurrent-source.mlir
similarity index 100%
rename from mlir/test/Dialect/Transform/test-interpreter-external-concurrent-source.mlir
rename to mlir/test/Dialect/Transform/include/test-interpreter-external-concurrent-source.mlir
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-source.mlir b/mlir/test/Dialect/Transform/include/test-interpreter-external-source.mlir
similarity index 100%
rename from mlir/test/Dialect/Transform/test-interpreter-external-source.mlir
rename to mlir/test/Dialect/Transform/include/test-interpreter-external-source.mlir
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-def-invalid.mlir b/mlir/test/Dialect/Transform/include/test-interpreter-external-symbol-def-invalid.mlir
similarity index 100%
rename from mlir/test/Dialect/Transform/test-interpreter-external-symbol-def-invalid.mlir
rename to mlir/test/Dialect/Transform/include/test-interpreter-external-symbol-def-invalid.mlir
diff --git a/mlir/test/Dialect/Transform/test-interpreter-library/definitions-self-contained.mlir b/mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-self-contained.mlir
similarity index 96%
rename from mlir/test/Dialect/Transform/test-interpreter-library/definitions-self-contained.mlir
rename to mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-self-contained.mlir
index 66f0f1f62683b7e..58a8f76c5791a27 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-library/definitions-self-contained.mlir
+++ b/mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-self-contained.mlir
@@ -1,4 +1,5 @@
// RUN: mlir-opt %s
+// No need to check anything else than parsing here, this is being used by another test as data.
module attributes {transform.with_named_sequence} {
transform.named_sequence private @private_helper(%arg0: !transform.any_op {transform.readonly}) {
diff --git a/mlir/test/Dialect/Transform/test-interpreter-library/definitions-with-unresolved.mlir b/mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-with-unresolved.mlir
similarity index 78%
rename from mlir/test/Dialect/Transform/test-interpreter-library/definitions-with-unresolved.mlir
rename to mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-with-unresolved.mlir
index b3d076f4698495f..a3b315952b30970 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-library/definitions-with-unresolved.mlir
+++ b/mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-with-unresolved.mlir
@@ -1,4 +1,5 @@
// RUN: mlir-opt %s
+// No need to check anything else than parsing here, this is being used by another test as data.
module attributes {transform.with_named_sequence} {
transform.named_sequence @print_message(%arg0: !transform.any_op {transform.readonly})
diff --git a/mlir/test/Dialect/Transform/preload-library.mlir b/mlir/test/Dialect/Transform/preload-library.mlir
index 61d22252dc61dfd..9beefa44d673d9f 100644
--- a/mlir/test/Dialect/Transform/preload-library.mlir
+++ b/mlir/test/Dialect/Transform/preload-library.mlir
@@ -1,5 +1,5 @@
// RUN: mlir-opt %s \
-// RUN: -transform-preload-library=transform-library-paths=%p%{fs-sep}test-interpreter-library \
+// RUN: -transform-preload-library=transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library \
// RUN: -transform-interpreter=entry-point=private_helper \
// RUN: -split-input-file -verify-diagnostics
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-concurrent.mlir b/mlir/test/Dialect/Transform/test-interpreter-external-concurrent.mlir
index 46a1a130d9bcb37..59c2b672a6e6b10 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-external-concurrent.mlir
+++ b/mlir/test/Dialect/Transform/test-interpreter-external-concurrent.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(func.func(test-transform-dialect-interpreter{transform-file-name=%p/test-interpreter-external-concurrent-source.mlir}))" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(func.func(test-transform-dialect-interpreter{transform-file-name=%p%{fs-sep}include%{fs-sep}test-interpreter-external-concurrent-source.mlir}))" \
// RUN: --verify-diagnostics
// Exercising the pass on multiple functions of different lengths that may be
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-and-schedule.mlir b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-and-schedule.mlir
index 2c4812bf32b0f03..9e50ec1efac946c 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-and-schedule.mlir
+++ b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-and-schedule.mlir
@@ -1,7 +1,7 @@
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-file-name=%p/test-interpreter-external-symbol-decl.mlir transform-library-paths=%p/test-interpreter-library/definitions-self-contained.mlir})" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-file-name=%p%{fs-sep}test-interpreter-external-symbol-decl.mlir transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library/definitions-self-contained.mlir})" \
// RUN: --verify-diagnostics
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-file-name=%p/test-interpreter-external-symbol-decl.mlir transform-library-paths=%p/test-interpreter-library/definitions-self-contained.mlir}, test-transform-dialect-interpreter{transform-file-name=%p/test-interpreter-external-symbol-decl.mlir transform-library-paths=%p/test-interpreter-library/definitions-self-contained.mlir})" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-file-name=%p%{fs-sep}test-interpreter-external-symbol-decl.mlir transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library/definitions-self-contained.mlir}, test-transform-dialect-interpreter{transform-file-name=%p%{fs-sep}test-interpreter-external-symbol-decl.mlir transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library/definitions-self-contained.mlir})" \
// RUN: --verify-diagnostics
// The external transform script has a declaration to the named sequence @foo,
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-dir.mlir b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-dir.mlir
index 8b8254976e9aeec..3681b913dc5b974 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-dir.mlir
+++ b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-dir.mlir
@@ -1,10 +1,10 @@
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}test-interpreter-library})" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library})" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}test-interpreter-library/definitions-self-contained.mlir,%p%{fs-sep}test-interpreter-library/definitions-with-unresolved.mlir})" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library/definitions-self-contained.mlir,%p%{fs-sep}include%{fs-sep}test-interpreter-library/definitions-with-unresolved.mlir})" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}test-interpreter-library}, test-transform-dialect-interpreter)" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library}, test-transform-dialect-interpreter)" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s
// The definition of the @foo named sequence is provided in another file. It
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-invalid.mlir b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-invalid.mlir
index c1bd071dc138d56..060dab334ed4388 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-invalid.mlir
+++ b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-invalid.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p/test-interpreter-external-symbol-def-invalid.mlir}, test-transform-dialect-interpreter)" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-external-symbol-def-invalid.mlir}, test-transform-dialect-interpreter)" \
// RUN: --verify-diagnostics --split-input-file
// The definition of the @print_message named sequence is provided in another file. It
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl.mlir b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl.mlir
index 339e62072cd5510..8a35e981bd48b76 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl.mlir
+++ b/mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl.mlir
@@ -1,7 +1,7 @@
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p/test-interpreter-library/definitions-self-contained.mlir})" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library/definitions-self-contained.mlir})" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p/test-interpreter-library/definitions-self-contained.mlir}, test-transform-dialect-interpreter)" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library/definitions-self-contained.mlir}, test-transform-dialect-interpreter)" \
// RUN: --verify-diagnostics --split-input-file | FileCheck %s
// The definition of the @print_message named sequence is provided in another
diff --git a/mlir/test/Dialect/Transform/test-interpreter-external.mlir b/mlir/test/Dialect/Transform/test-interpreter-external.mlir
index 5ac6b66c817afef..ba8e0c6870dbf8a 100644
--- a/mlir/test/Dialect/Transform/test-interpreter-external.mlir
+++ b/mlir/test/Dialect/Transform/test-interpreter-external.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-file-name=%p/test-interpreter-external-source.mlir})" \
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(test-transform-dialect-interpreter{transform-file-name=%p%{fs-sep}include%{fs-sep}test-interpreter-external-source.mlir})" \
// RUN: --verify-diagnostics
// The schedule in the separate file emits remarks at the payload root.
diff --git a/utils/bazel/llvm-project-overlay/mlir/test/Dialect/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/test/Dialect/BUILD.bazel
index e5b877a48d5e848..1fd6885db8bca93 100644
--- a/utils/bazel/llvm-project-overlay/mlir/test/Dialect/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/test/Dialect/BUILD.bazel
@@ -18,11 +18,7 @@ package(default_visibility = ["//visibility:public"])
] + glob([
"IRDL/*.irdl.mlir",
"LLVM/*-symbol-def.mlir",
- "Transform/*-source.mlir",
- "Transform/*-symbol-def.mlir",
- "Transform/*-symbol-decl-and-schedule.mlir",
- "Transform/Library/*.mlir",
- "Transform/test-interpreter-library/*.mlir",
+ "Transform/include/**/*.mlir",
]),
)
for src in glob(
@@ -30,15 +26,8 @@ package(default_visibility = ["//visibility:public"])
exclude = [
"IRDL/*.irdl.mlir",
"LLVM/*-symbol-def.mlir",
- "Transform/*-source.mlir",
- "Transform/*-symbol-def.mlir",
"Transform/*-symbol-decl-and-schedule.mlir",
- "Transform/*-symbol-decl-dir.mlir",
- "Transform/*-symbol-decl-invalid.mlir",
- "Transform/Library/*.mlir",
- "Transform/preload-library.mlir",
- "Transform/test-interpreter-library/*.mlir",
- "Transform/test-repro-dump.mlir",
+ "Transform/include/**/*.mlir",
],
)
]
More information about the Mlir-commits
mailing list