[Mlir-commits] [mlir] f681225 - [mlir][transform] Support symlinks in module loading. Reorganize tests. (#69329)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Thu Oct 19 01:45:36 PDT 2023


Author: Ingo Müller
Date: 2023-10-19T10:45:33+02:00
New Revision: f68122570091445a63a18eb45e4ad3d0015b3070

URL: https://github.com/llvm/llvm-project/commit/f68122570091445a63a18eb45e4ad3d0015b3070
DIFF: https://github.com/llvm/llvm-project/commit/f68122570091445a63a18eb45e4ad3d0015b3070.diff

LOG: [mlir][transform] Support symlinks in module loading. Reorganize tests. (#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.)

Added: 
    mlir/test/Dialect/Transform/include/Library/lower-to-llvm.mlir
    mlir/test/Dialect/Transform/include/test-interpreter-external-concurrent-source.mlir
    mlir/test/Dialect/Transform/include/test-interpreter-external-source.mlir
    mlir/test/Dialect/Transform/include/test-interpreter-external-symbol-def-invalid.mlir
    mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-self-contained.mlir
    mlir/test/Dialect/Transform/include/test-interpreter-library/definitions-with-unresolved.mlir

Modified: 
    mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
    mlir/test/Dialect/Transform/preload-library.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-concurrent.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-and-schedule.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-dir.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl-invalid.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-symbol-decl.mlir
    mlir/test/Dialect/Transform/test-interpreter-external.mlir
    utils/bazel/llvm-project-overlay/mlir/test/Dialect/BUILD.bazel

Removed: 
    mlir/test/Dialect/Transform/Library/lower-to-llvm.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-concurrent-source.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-source.mlir
    mlir/test/Dialect/Transform/test-interpreter-external-symbol-def-invalid.mlir
    mlir/test/Dialect/Transform/test-interpreter-library/definitions-self-contained.mlir
    mlir/test/Dialect/Transform/test-interpreter-library/definitions-with-unresolved.mlir


################################################################################
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 
diff erent 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