From 2d670ffeffce3369c6c1880ba99af0b1ddea9a6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20M=C3=BCller?= 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 41feffffaf97b..e6d692072267c 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 0ba50bd2362b3..afd1c89dd2b52 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 66f0f1f62683b..58a8f76c5791a 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 b3d076f469849..a3b315952b309 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 61d22252dc61d..9beefa44d673d 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 46a1a130d9bcb..59c2b672a6e6b 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 2c4812bf32b0f..9e50ec1efac94 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 8b8254976e9ae..3681b913dc5b9 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 c1bd071dc138d..060dab334ed43 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 339e62072cd55..8a35e981bd48b 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 5ac6b66c817af..ba8e0c6870dbf 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 e5b877a48d5e8..1fd6885db8bca 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", ], ) ]