[mlir][spirv] Fix cyclical dependency in bazel

[basioli-k]

No description provided.

[llvmbot]

@llvm/pr-subscribers-mlir-spirv

@llvm/pr-subscribers-mlir

Author: Karlo Basioli (basioli-k)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/132785.diff

3 Files Affected:

• (modified) mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h (+13)
• (modified) mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp (+1-1)
• (modified) utils/bazel/llvm-project-overlay/mlir/BUILD.bazel (+48-26)

diff --git a/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h b/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h
index ca5a1dcd88171..4ba5d096a1b27 100644
--- a/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h
+++ b/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h
@@ -9,6 +9,19 @@
 #ifndef MLIR_DIALECT_SPIRV_IMAGE_INTERFACES_H_
 #define MLIR_DIALECT_SPIRV_IMAGE_INTERFACES_H_
 
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/BuiltinTypes.h"
+#include "mlir/IR/OpDefinition.h"
+#include "mlir/IR/OpImplementation.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/Value.h"
+#include "mlir/Interfaces/CallInterfaces.h"
+#include "mlir/Interfaces/ControlFlowInterfaces.h"
+#include "mlir/Interfaces/FunctionInterfaces.h"
+#include "mlir/Interfaces/InferTypeOpInterface.h"
+#include "mlir/Interfaces/SideEffectInterfaces.h"
+#include "llvm/Support/PointerLikeTypeTraits.h"
+
 #include "mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h.inc"
 
 #endif // MLIR_DIALECT_SPIRV_IMAGE_INTERFACES_H_
diff --git a/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp b/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp
index e10da60184eeb..fce341d3a5d72 100644
--- a/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp
+++ b/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "mlir/Dialect/SPIRV/IR/SPIRVOps.h"
+#include "mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h"
 
 using namespace mlir;
 
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index 64e8fba9168c0..bfea17fd18625 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -6486,6 +6486,7 @@ cc_library(
         ":Pass",
         ":SPIRVCommonAttrToLLVMConversion",
         ":SPIRVDialect",
+        ":SPIRVImageInterfaces",
         ":SPIRVUtils",
         ":Support",
         ":TransformUtils",
@@ -7389,14 +7390,16 @@ cc_library(
     ],
 )
 
+td_library(
+    name = "SPIRVImageInterfacesTdFiles",
+    srcs = ["include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.td"],
+    includes = ["include"],
+    deps = [":OpBaseTdFiles"],
+)
+
 td_library(
     name = "SPIRVOpsTdFiles",
-    srcs = glob(["include/mlir/Dialect/SPIRV/IR/*.td"]) + [
-        # TODO: resolve circular dep, e.g.
-        # * SPIRVImageInterfaces.td uses SPIRVBase.td
-        # * SPIRVOps.h uses SPIRVImageInterfaces.h
-        "include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.td",
-    ],
+    srcs = glob(["include/mlir/Dialect/SPIRV/IR/*.td"]),
     includes = ["include"],
     deps = [
         ":BuiltinDialectTdFiles",
@@ -7405,10 +7408,48 @@ td_library(
         ":FunctionInterfacesTdFiles",
         ":InferTypeOpInterfaceTdFiles",
         ":OpBaseTdFiles",
+        ":SPIRVImageInterfacesTdFiles",
         ":SideEffectInterfacesTdFiles",
     ],
 )
 
+gentbl_cc_library(
+    name = "SPIRVImageInterfacesIncGen",
+    tbl_outs = [
+        (
+            ["-gen-op-interface-decls"],
+            "include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h.inc",
+        ),
+        (
+            ["-gen-op-interface-defs"],
+            "include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp.inc",
+        ),
+    ],
+    tblgen = ":mlir-tblgen",
+    td_file = "include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.td",
+    deps = [
+        ":SPIRVImageInterfacesTdFiles",
+        ":SPIRVOpsTdFiles",
+    ],
+)
+
+cc_library(
+    name = "SPIRVImageInterfaces",
+    srcs = ["lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp"],
+    hdrs = ["include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h"],
+    includes = ["include"],
+    deps = [
+        ":CallOpInterfaces",
+        ":ControlFlowInterfaces",
+        ":FunctionInterfaces",
+        ":IR",
+        ":InferTypeOpInterface",
+        ":SPIRVImageInterfacesIncGen",
+        ":SideEffectInterfaces",
+        "//llvm:Support",
+    ],
+)
+
 gentbl_cc_library(
     name = "SPIRVOpsIncGen",
     tbl_outs = [
@@ -7536,33 +7577,14 @@ gentbl_cc_library(
     deps = [":SPIRVOpsTdFiles"],
 )
 
-gentbl_cc_library(
-    name = "SPIRVImageInterfacesIncGen",
-    tbl_outs = [
-        (
-            ["-gen-op-interface-decls"],
-            "include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h.inc",
-        ),
-        (
-            ["-gen-op-interface-defs"],
-            "include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp.inc",
-        ),
-    ],
-    tblgen = ":mlir-tblgen",
-    td_file = "include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.td",
-    deps = [":SPIRVOpsTdFiles"],
-)
-
 cc_library(
     name = "SPIRVDialect",
     srcs = glob([
         "lib/Dialect/SPIRV/IR/*.cpp",
         "lib/Dialect/SPIRV/IR/*.h",
-        "lib/Dialect/SPIRV/Interfaces/*.cpp",
     ]),
     hdrs = glob([
         "include/mlir/Dialect/SPIRV/IR/*.h",
-        "include/mlir/Dialect/SPIRV/Interfaces/*.h",
     ]),
     includes = ["include"],
     deps = [
@@ -7580,7 +7602,7 @@ cc_library(
         ":SPIRVAttributesIncGen",
         ":SPIRVAvailabilityIncGen",
         ":SPIRVCanonicalizationIncGen",
-        ":SPIRVImageInterfacesIncGen",
+        ":SPIRVImageInterfaces",
         ":SPIRVOpsIncGen",
         ":SideEffectInterfaces",
         ":Support",

[kuhar]

Thanks for fixing this. Just one question.

[kuhar]

Do we actually need all of these?

[IgWod-IMG]

I'm not familiar with Bazel but I managed to remove the dependency of the interface on the IR:

diff --git a/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.td b/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInte
rfaces.td
index 2b681d9367a4..fbb77a7ed2d6 100644
--- a/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.td
+++ b/mlir/include/mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.td
@@ -13,7 +13,7 @@
 #ifndef MLIR_DIALECT_SPIRV_IMAGE_INTERFACES
 #define MLIR_DIALECT_SPIRV_IMAGE_INTERFACES

-include "mlir/Dialect/SPIRV/IR/SPIRVBase.td"
+include "mlir/IR/OpBase.td"

 // -----

diff --git a/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp b/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp
index e10da60184ee..eabcfcbfba93 100644
--- a/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp
+++ b/mlir/lib/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.cpp
@@ -6,7 +6,9 @@
 //
 //===----------------------------------------------------------------------===//

-#include "mlir/Dialect/SPIRV/IR/SPIRVOps.h"
+#include "mlir/IR/OpDefinition.h"
+
+#include "mlir/Dialect/SPIRV/Interfaces/SPIRVImageInterfaces.h"

 using namespace mlir;

This builds and passes tests with: cmake --build build-linux/ --target check-mlir. I don't have an ability to test it with Bazel, but I think it should resolve the cyclical dependency.

[IgWod-IMG]

Also, thanks for fixing the issue, and apologies for causing it. Let me know if my fix works and whether you would like me to submit it as a separate PR.

[basioli-k]

Also, thanks for fixing the issue, and apologies for causing it. Let me know if my fix works and whether you would like me to submit it as a separate PR.

Thank you for the proposed solution. It does work with bazel as well. I will add it with this PR so we don't have to commit a solution to the same problem twice.
I think it should be fine now, but let me know if anything gets broken!