From 5074a20dec70ef2afef650f770fb45eb0247a4f7 Mon Sep 17 00:00:00 2001
From: Peter Hawkins <phawkins@google.com>
Date: Fri, 12 Nov 2021 12:02:18 -0800
Subject: Don't define //mlir:MLIRBindingsPythonCore in terms of the NoCAPI and
 CAPIDeps targets.

We noticed that the library structure causes link ordering problems in Google's internal build. However, we don't think the problem is specific to Google's build, it probably can be reproduced anywhere with the right library structure.

In general splitting the Python bindings from their dependencies (the C API targets) creates the possibility that the two libraries might end up in the wrong order on the linker command line. We can avoid this problem happening by reverting the structure of the MLIRBindingsPythonCore to represent its dependencies in the usual way, rather than composing an incomplete `MLIRBindingsPythonCoreNoCAPI` target and their CAPI dependencies. It was probably a mistake to rewrite this particular `cc_library()` rule in terms of the two, since nothing guarantees that the two will be correctly ordered by the linker when both are being linked into the same binary, and it was only an incidental "cleanup" done in passing.

Otherwise the previous PR (D113565) is fine, since that was about the case where both are being built into two separate shared libraries. It just shouldn't have made this (unrelated) change.

Reviewed By: GMNGeoffrey

Differential Revision: https://reviews.llvm.org/D113773
---
 utils/bazel/llvm-project-overlay/mlir/BUILD.bazel | 90 ++++++++++++-----------
 1 file changed, 48 insertions(+), 42 deletions(-)

(limited to 'utils')

diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index a498a78..4c876b1 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -620,20 +620,22 @@ cc_library(
     ],
 )
 
+MLIR_PYTHON_BINDINGS_SOURCES = [
+    "lib/Bindings/Python/DialectLinalg.cpp",
+    "lib/Bindings/Python/DialectSparseTensor.cpp",
+    "lib/Bindings/Python/IRAffine.cpp",
+    "lib/Bindings/Python/IRAttributes.cpp",
+    "lib/Bindings/Python/IRCore.cpp",
+    "lib/Bindings/Python/IRInterfaces.cpp",
+    "lib/Bindings/Python/IRModule.cpp",
+    "lib/Bindings/Python/IRTypes.cpp",
+    "lib/Bindings/Python/Pass.cpp",
+    "lib/Bindings/Python/PybindUtils.cpp",
+]
+
 cc_library(
-    name = "MLIRBindingsPythonCoreNoCAPI",
-    srcs = [
-        "lib/Bindings/Python/DialectLinalg.cpp",
-        "lib/Bindings/Python/DialectSparseTensor.cpp",
-        "lib/Bindings/Python/IRAffine.cpp",
-        "lib/Bindings/Python/IRAttributes.cpp",
-        "lib/Bindings/Python/IRCore.cpp",
-        "lib/Bindings/Python/IRInterfaces.cpp",
-        "lib/Bindings/Python/IRModule.cpp",
-        "lib/Bindings/Python/IRTypes.cpp",
-        "lib/Bindings/Python/Pass.cpp",
-        "lib/Bindings/Python/PybindUtils.cpp",
-    ],
+    name = "MLIRBindingsPythonCore",
+    srcs = MLIR_PYTHON_BINDINGS_SOURCES,
     # These flags are needed for pybind11 to work.
     copts = [
         "-fexceptions",
@@ -648,14 +650,15 @@ cc_library(
         "nobuildkite",  # TODO(gcmn): Add support for this target
     ],
     deps = [
-        ":CAPIAsyncHeaders",
-        ":CAPIDebugHeaders",
-        ":CAPIGPUHeaders",
-        ":CAPIIRHeaders",
-        ":CAPILinalgHeaders",
-        ":CAPIRegistrationHeaders",
-        ":CAPISparseTensorHeaders",
-        ":MLIRBindingsPythonHeaders",
+        ":CAPIAsync",
+        ":CAPIDebug",
+        ":CAPIGPU",
+        ":CAPIIR",
+        ":CAPIInterfaces",
+        ":CAPILinalg",
+        ":CAPIRegistration",
+        ":CAPISparseTensor",
+        ":MLIRBindingsPythonHeadersAndDeps",
         "//llvm:Support",
         "@pybind11",
         "@python_runtime//:headers",
@@ -663,23 +666,38 @@ cc_library(
 )
 
 cc_library(
-    name = "MLIRBindingsPythonCAPIDeps",
+    name = "MLIRBindingsPythonCoreNoCAPI",
+    srcs = MLIR_PYTHON_BINDINGS_SOURCES,
+    # These flags are needed for pybind11 to work.
+    copts = [
+        "-fexceptions",
+        "-frtti",
+    ],
+    features = [
+        # Cannot use header_modules (parse_headers feature fails).
+        "-use_header_modules",
+    ],
     tags = [
         "manual",  # External dependency
         "nobuildkite",  # TODO(gcmn): Add support for this target
     ],
     deps = [
-        ":CAPIAsync",
-        ":CAPIDebug",
-        ":CAPIGPU",
-        ":CAPIIR",
-        ":CAPIInterfaces",
-        ":CAPILinalg",
-        ":CAPIRegistration",
-        ":CAPISparseTensor",
+        ":CAPIAsyncHeaders",
+        ":CAPIDebugHeaders",
+        ":CAPIGPUHeaders",
+        ":CAPIIRHeaders",
+        ":CAPILinalgHeaders",
+        ":CAPIRegistrationHeaders",
+        ":CAPISparseTensorHeaders",
+        ":MLIRBindingsPythonHeaders",
+        "//llvm:Support",
+        "@pybind11",
+        "@python_runtime//:headers",
     ],
 )
 
+# Target that bundles together the CAPI objects needed for
+# MLIRBindingsPythonCoreNoCAPI.
 cc_library(
     name = "MLIRBindingsPythonCAPIObjects",
     tags = [
@@ -698,18 +716,6 @@ cc_library(
     ],
 )
 
-cc_library(
-    name = "MLIRBindingsPythonCore",
-    tags = [
-        "manual",  # External dependency
-        "nobuildkite",  # TODO(gcmn): Add support for this target
-    ],
-    deps = [
-        ":MLIRBindingsPythonCAPIDeps",
-        ":MLIRBindingsPythonCoreNoCAPI",
-    ],
-)
-
 PYBIND11_COPTS = [
     "-fexceptions",
     "-frtti",
-- 
cgit v1.1