Skip to content
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] Export more headers by a single target only. #86637

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

chsigg
Copy link
Contributor

@chsigg chsigg commented Mar 26, 2024

Ideally, header files should be used by only one target, but this is hard because CMake is less strict with headers (no layering check). But even with bazel, headers should only be exported once in the hdrs attribute. Other targets may use them in the srcs attribute to avoid circular dependencies.

Ideally, header files should be used by only one target, but this is hard because CMake is less strict with headers (no layering check). But even with bazel, headers should only be exported once in the `hdrs` attribute. Other targets may use them in the `srcs` attribute to avoid circular dependencies.
@chsigg chsigg requested a review from akuegel March 26, 2024 07:45
@chsigg chsigg requested a review from rupprecht as a code owner March 26, 2024 07:45
@llvmbot llvmbot added mlir:llvm mlir bazel "Peripheral" support tier build system: utils/bazel labels Mar 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 26, 2024

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Christian Sigg (chsigg)

Changes

Ideally, header files should be used by only one target, but this is hard because CMake is less strict with headers (no layering check). But even with bazel, headers should only be exported once in the hdrs attribute. Other targets may use them in the srcs attribute to avoid circular dependencies.


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

2 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h (-1)
  • (modified) utils/bazel/llvm-project-overlay/mlir/BUILD.bazel (+27-24)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h b/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h
index 06df4a601b7a3f..9341a5a11cd629 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h
@@ -30,7 +30,6 @@
 #include "mlir/Interfaces/InferTypeOpInterface.h"
 #include "mlir/Interfaces/SideEffectInterfaces.h"
 #include "mlir/Support/ThreadLocalCache.h"
-#include "mlir/Transforms/Mem2Reg.h"
 #include "llvm/ADT/PointerEmbeddedInt.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/LLVMContext.h"
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index 5b6e4678a05ef4..c17cc9704c3f7a 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -3823,9 +3823,10 @@ cc_library(
     srcs = glob([
         "lib/Dialect/*.cpp",
     ]),
-    hdrs = glob([
-        "include/mlir/Dialect/*.h",
-    ]),
+    hdrs = glob(
+        include = ["include/mlir/Dialect/*.h"],
+        exclude = ["include/mlir/Dialect/CommonFolders.h"],
+    ),
     includes = ["include"],
     deps = [
         ":IR",
@@ -4534,6 +4535,7 @@ cc_library(
         ":ArithDialect",
         ":BufferizationInterfaces",
         ":CastInterfaces",
+        ":CommonFolders",
         ":ControlFlowInterfaces",
         ":Dialect",
         ":FuncDialect",
@@ -5336,9 +5338,7 @@ cc_library(
             "include/mlir/Dialect/LLVMIR/VCIX*.h",
             "include/mlir/Dialect/LLVMIR/*X86Vector*.h",
         ],
-    ) + [
-        "include/mlir/Transforms/Mem2Reg.h",
-    ],
+    ),
     includes = ["include"],
     deps = [
         ":CallOpInterfaces",
@@ -8635,17 +8635,26 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "ParseUtilities",
+    hdrs = ["include/mlir/Tools/ParseUtilities.h"],
+    includes = ["include"],
+    deps = [
+        ":IR",
+        ":Parser",
+    ],
+)
+
 cc_library(
     name = "TranslateLib",
     srcs = glob([
         "lib/Tools/mlir-translate/*.cpp",
-    ]) + [
-        "include/mlir/Tools/ParseUtilities.h",
-    ],
+    ]),
     hdrs = glob(["include/mlir/Tools/mlir-translate/*.h"]),
     includes = ["include"],
     deps = [
         ":IR",
+        ":ParseUtilities",
         ":Parser",
         ":Support",
         "//llvm:Support",
@@ -8655,6 +8664,7 @@ cc_library(
 cc_library(
     name = "ToLLVMIRTranslation",
     srcs = [
+        "lib/Target/LLVMIR/AttrKindDetail.h",
         "lib/Target/LLVMIR/DebugTranslation.cpp",
         "lib/Target/LLVMIR/DebugTranslation.h",
         "lib/Target/LLVMIR/LoopAnnotationTranslation.cpp",
@@ -8667,7 +8677,6 @@ cc_library(
         "include/mlir/Target/LLVMIR/LLVMTranslationInterface.h",
         "include/mlir/Target/LLVMIR/ModuleTranslation.h",
         "include/mlir/Target/LLVMIR/TypeToLLVM.h",
-        "lib/Target/LLVMIR/AttrKindDetail.h",
     ],
     includes = ["include"],
     deps = [
@@ -8988,9 +8997,11 @@ cc_library(
 cc_library(
     name = "FromLLVMIRTranslation",
     srcs = [
+        "lib/Target/LLVMIR/AttrKindDetail.h",
         "lib/Target/LLVMIR/DataLayoutImporter.cpp",
         "lib/Target/LLVMIR/DataLayoutImporter.h",
         "lib/Target/LLVMIR/DebugImporter.cpp",
+        "lib/Target/LLVMIR/DebugImporter.h",
         "lib/Target/LLVMIR/LoopAnnotationImporter.cpp",
         "lib/Target/LLVMIR/LoopAnnotationImporter.h",
         "lib/Target/LLVMIR/ModuleImport.cpp",
@@ -9001,8 +9012,6 @@ cc_library(
         "include/mlir/Target/LLVMIR/LLVMImportInterface.h",
         "include/mlir/Target/LLVMIR/ModuleImport.h",
         "include/mlir/Target/LLVMIR/TypeFromLLVM.h",
-        "lib/Target/LLVMIR/AttrKindDetail.h",
-        "lib/Target/LLVMIR/DebugImporter.h",
     ],
     includes = ["include"],
     deps = [
@@ -9109,10 +9118,7 @@ cc_library(
 
 cc_library(
     name = "MlirOptLib",
-    srcs = [
-        "include/mlir/Tools/ParseUtilities.h",
-        "lib/Tools/mlir-opt/MlirOptMain.cpp",
-    ],
+    srcs = ["lib/Tools/mlir-opt/MlirOptMain.cpp"],
     hdrs = ["include/mlir/Tools/mlir-opt/MlirOptMain.h"],
     includes = ["include"],
     deps = [
@@ -9121,6 +9127,7 @@ cc_library(
         ":Debug",
         ":IR",
         ":IRDLDialect",
+        ":ParseUtilities",
         ":Parser",
         ":Pass",
         ":PluginsLib",
@@ -9393,10 +9400,7 @@ cc_binary(
 cc_library(
     name = "MlirJitRunner",
     srcs = ["lib/ExecutionEngine/JitRunner.cpp"],
-    hdrs = [
-        "include/mlir/ExecutionEngine/JitRunner.h",
-        "include/mlir/Tools/ParseUtilities.h",
-    ],
+    hdrs = ["include/mlir/ExecutionEngine/JitRunner.h"],
     includes = ["include"],
     deps = [
         ":AllPassesAndDialects",
@@ -9407,6 +9411,7 @@ cc_library(
         ":LLVMToLLVMIRTranslation",
         ":OpenACCToLLVMIRTranslation",
         ":OpenMPToLLVMIRTranslation",
+        ":ParseUtilities",
         ":Parser",
         ":SCFToControlFlow",
         ":Support",
@@ -13746,13 +13751,11 @@ cc_library(
 cc_library(
     name = "MlirReduceLib",
     srcs = ["lib/Tools/mlir-reduce/MlirReduceMain.cpp"],
-    hdrs = [
-        "include/mlir/Tools/ParseUtilities.h",
-        "include/mlir/Tools/mlir-reduce/MlirReduceMain.h",
-    ],
+    hdrs = ["include/mlir/Tools/mlir-reduce/MlirReduceMain.h"],
     includes = ["include"],
     deps = [
         ":IR",
+        ":ParseUtilities",
         ":Parser",
         ":Pass",
         ":Reducer",

@chsigg chsigg requested a review from bchetioui March 26, 2024 08:55
@chsigg chsigg merged commit eb70b48 into llvm:main Mar 26, 2024
8 checks passed
@chsigg chsigg deleted the piper_export_cl_619087346 branch March 26, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel mlir:llvm mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants