-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mlir][bazel] Port https://github.com/llvm/llvm-project/commit/517800e37e8d3a4ee84214bef65e227612c2a98b #127544
Conversation
Introduces a `LinalgInterfaces` target so that `TensorDialect` doesn't need to depend on `LinalgDialect`, which would introduce a circular dependency.
@llvm/pr-subscribers-mlir-tensor @llvm/pr-subscribers-mlir-linalg Author: Christian Sigg (chsigg) ChangesIntroduces a Full diff: https://github.com/llvm/llvm-project/pull/127544.diff 4 Files Affected:
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgInterfaces.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgInterfaces.cpp
index 466a9799295f9..c16c38ea22a5d 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgInterfaces.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgInterfaces.cpp
@@ -13,8 +13,6 @@
#include "mlir/Dialect/Arith/Utils/Utils.h"
#include "mlir/Dialect/Complex/IR/Complex.h"
#include "mlir/Dialect/Linalg/IR/Linalg.h"
-#include "mlir/Dialect/MemRef/IR/MemRef.h"
-#include "mlir/Dialect/Tensor/IR/Tensor.h"
#include "mlir/IR/AffineExpr.h"
#include "mlir/IR/AffineExprVisitor.h"
#include "mlir/IR/AffineMap.h"
diff --git a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
index e741144647043..72e9d4d9c64d9 100644
--- a/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
+++ b/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
@@ -10,7 +10,7 @@
#include "mlir/Dialect/Arith/IR/Arith.h"
#include "mlir/Dialect/Arith/Utils/Utils.h"
#include "mlir/Dialect/Complex/IR/Complex.h"
-#include "mlir/Dialect/Linalg/IR/Linalg.h"
+#include "mlir/Dialect/Linalg/IR/LinalgInterfaces.h"
#include "mlir/Dialect/Tensor/IR/Tensor.h"
#include "mlir/Dialect/Tensor/Utils/Utils.h"
#include "mlir/Dialect/Utils/IndexingUtils.h"
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index 92aedac837197..60f66b154e313 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -7763,7 +7763,6 @@ td_library(
name = "TensorOpsTdFiles",
srcs = [
"include/mlir/Dialect/Tensor/IR/TensorBase.td",
- "include/mlir/Dialect/Tensor/IR/TensorInterfaces.td",
"include/mlir/Dialect/Tensor/IR/TensorOps.td",
],
includes = ["include"],
@@ -7813,23 +7812,6 @@ gentbl_cc_library(
deps = [":TensorOpsTdFiles"],
)
-gentbl_cc_library(
- name = "TensorInterfacesIncGen",
- tbl_outs = [
- (
- ["--gen-op-interface-decls"],
- "include/mlir/Dialect/Tensor/IR/TensorInterfaces.h.inc",
- ),
- (
- ["--gen-op-interface-defs"],
- "include/mlir/Dialect/Tensor/IR/TensorInterfaces.cpp.inc",
- ),
- ],
- tblgen = ":mlir-tblgen",
- td_file = "include/mlir/Dialect/Tensor/IR/TensorInterfaces.td",
- deps = [":TensorOpsTdFiles"],
-)
-
cc_library(
name = "TensorDialect",
srcs = [
@@ -7859,13 +7841,13 @@ cc_library(
":InferIntRangeInterface",
":InferTypeOpInterface",
":InliningUtils",
+ ":LinalgInterfaces",
":LoopLikeInterface",
":ParallelCombiningOpInterface",
":ShapedOpInterfaces",
":SideEffectInterfaces",
":SubsetOpInterface",
":Support",
- ":TensorInterfacesIncGen",
":TensorOpsIncGen",
":TilingInterface",
":TransformDialectInterfaces",
@@ -11206,6 +11188,23 @@ gentbl_cc_library(
deps = [":LinalgOpsTdFiles"],
)
+gentbl_cc_library(
+ name = "LinalgRelayoutOpsIncGen",
+ tbl_outs = [
+ (
+ ["-gen-op-decls"],
+ "include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.h.inc",
+ ),
+ (
+ ["-gen-op-defs"],
+ "include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.cpp.inc",
+ ),
+ ],
+ tblgen = ":mlir-tblgen",
+ td_file = "include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td",
+ deps = [":LinalgOpsTdFiles"],
+)
+
gentbl_cc_library(
name = "LinalgEnumsIncGen",
tbl_outs = [
@@ -11532,10 +11531,50 @@ cc_library(
],
)
+cc_library(
+ name = "LinalgInterfaces",
+ srcs = [
+ "include/mlir/Dialect/Linalg/IR/Linalg.h",
+ "lib/Dialect/Linalg/IR/LinalgInterfaces.cpp",
+ ],
+ hdrs = ["include/mlir/Dialect/Linalg/IR/LinalgInterfaces.h"],
+ includes = ["include"],
+ deps = [
+ ":AffineDialect",
+ ":ArithDialect",
+ ":ArithUtils",
+ ":BytecodeOpInterface",
+ ":ComplexDialect",
+ ":ControlFlowInterfaces",
+ ":CopyOpInterface",
+ ":DestinationStyleOpInterface",
+ ":DialectUtils",
+ ":IR",
+ ":InferTypeOpInterface",
+ ":LinalgEnumsIncGen",
+ ":LinalgInterfacesIncGen",
+ ":LinalgOpsIncGen",
+ ":LinalgRelayoutOpsIncGen",
+ ":LinalgStructuredOpsIncGen",
+ ":SideEffectInterfaces",
+ ":Support",
+ ":TilingInterface",
+ ":ViewLikeInterface",
+ "//third_party/llvm/llvm-project/llvm:Support",
+ ],
+)
+
cc_library(
name = "LinalgDialect",
- srcs = glob(["lib/Dialect/Linalg/IR/*.cpp"]),
- hdrs = glob(["include/mlir/Dialect/Linalg/IR/*.h"]),
+ srcs = [
+ "lib/Dialect/Linalg/IR/LinalgDialect.cpp",
+ "lib/Dialect/Linalg/IR/LinalgOps.cpp",
+ "lib/Dialect/Linalg/IR/ValueBoundsOpInterfaceImpl.cpp",
+ ],
+ hdrs = [
+ "include/mlir/Dialect/Linalg/IR/Linalg.h",
+ "include/mlir/Dialect/Linalg/IR/ValueBoundsOpInterfaceImpl.h",
+ ],
includes = ["include"],
deps = [
":AffineDialect",
@@ -11554,9 +11593,10 @@ cc_library(
":InferTypeOpInterface",
":InliningUtils",
":LinalgEnumsIncGen",
- ":LinalgInterfacesIncGen",
+ ":LinalgInterfaces",
":LinalgNamedStructuredOpsYamlIncGen",
":LinalgOpsIncGen",
+ ":LinalgRelayoutOpsIncGen",
":LinalgStructuredOpsIncGen",
":MathDialect",
":MemRefDialect",
@@ -11568,6 +11608,7 @@ cc_library(
":SubsetOpInterface",
":Support",
":TensorDialect",
+ ":TensorUtils",
":TilingInterface",
":ValueBoundsOpInterface",
":ViewLikeInterface",
@@ -11599,6 +11640,7 @@ cc_library(
":IR",
":IndexDialect",
":LinalgDialect",
+ ":LinalgInterfaces",
":LinalgMatchOpsIncGen",
":LinalgTransformEnumsIncGen",
":LinalgTransformOpsIncGen",
@@ -11710,6 +11752,7 @@ cc_library(
":IR",
":IndexDialect",
":LinalgDialect",
+ ":LinalgInterfaces",
":LinalgPassIncGen",
":LinalgStructuredOpsIncGen",
":LinalgUtils",
diff --git a/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel
index 3e6114abfc078..9b005b206a101 100644
--- a/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel
@@ -425,6 +425,7 @@ cc_library(
"//mlir:LLVMDialect",
"//mlir:LLVMIRToLLVMTranslation",
"//mlir:LinalgDialect",
+ "//mlir:LinalgInterfaces",
"//mlir:LoopLikeInterface",
"//mlir:MemorySlotInterfaces",
"//mlir:Pass",
|
Replaces #127533 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about BAZEL but the changes look good to me, thank you!
I'm assuming the extra headers were unnecessary, since the CMake pre-commit buildbot is green, too. But keep an eye on the PR for potential issues with post-commit bots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the quick fix!
Let me just note that we should probably revisit the underlying layering issue at some point. This is addressing Bazel build issue specifically, but ideally Tensor should not require anything from Linalg. But that's something that will apply to other dialects as well.
The BUILD file changes in #127544 adds `LinalgInterfaces` which is incomplete without `LinalgDialect`. For now, just add the `LinalgDialect` as dependency to tests which do not otherwise depend on it (but depend on `LinalgInterfaces` through e.g. `TensorDialect`). This is a temporary solution until the dependency of `TensorDialect` is trimmed to just the `linalg::RelayoutOpInterface`, but not the other linalg interfaces. See #127544 (review).
The BUILD file changes in llvm/llvm-project#127544 adds `LinalgInterfaces` which is incomplete without `LinalgDialect`. For now, just add the `LinalgDialect` as dependency to tests which do not otherwise depend on it (but depend on `LinalgInterfaces` through e.g. `TensorDialect`). This is a temporary solution until the dependency of `TensorDialect` is trimmed to just the `linalg::RelayoutOpInterface`, but not the other linalg interfaces. See llvm/llvm-project#127544 (review).
+1, do we have a tracking issue for this? (Removing of linalg deps tensor side) |
Now we do #127668 😄 |
Introduces a `LinalgInterfaces` target so that `TensorDialect` doesn't need to depend on `LinalgDialect`, which would introduce a circular dependency.
The BUILD file changes in llvm#127544 adds `LinalgInterfaces` which is incomplete without `LinalgDialect`. For now, just add the `LinalgDialect` as dependency to tests which do not otherwise depend on it (but depend on `LinalgInterfaces` through e.g. `TensorDialect`). This is a temporary solution until the dependency of `TensorDialect` is trimmed to just the `linalg::RelayoutOpInterface`, but not the other linalg interfaces. See llvm#127544 (review).
Introduces a
LinalgInterfaces
target so thatTensorDialect
doesn't need to depend onLinalgDialect
, which would introduce a circular dependency.