[PATCH] D113565: Split headers from implementations in MLIR C API Bazel build.

[Peter Hawkins via Phabricator via llvm-commits]

phawkins added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:774
+    ],
+    alwayslink = True,
 )
----------------
phawkins wrote:
> GMNGeoffrey wrote:
> > phawkins wrote:
> > > mehdi_amini wrote:
> > > > GMNGeoffrey wrote:
> > > > > 😢 alwayslink is a scourge. Is it really the only way?
> > > > Same concern here: why is this needed?
> > > I removed it from this target, where I don't think it's needed. In general it *is* needed on all the libraries that implement the exported C API, because otherwise Bazel will strip the C API out.
> > Why wasn't it needed before? We're using this internally without issue IIUC. We've worked hard in many places to scour this from the codebase, so I don't love seeing it return
> > 
> > @stellaraccident can you weigh in on these build rules?
> It wasn't needed before, because internally we build monolithic binaries.
> 
> Here I'm doing something different: I'm building one shared library that vends the C API (capi.so) and another shared library (a Python extension) that consumes it. Without `alwayslink`, the C API `.so` file will not contain the C API: it ends up completely empty because there are no *users* of the API present and everything ends up stripped out. I did not put this here idly.
> 
> I note this is exactly how IREE deploys things, although IREE does it with CMake not Bazel.
> 
To add to that, from the Bazel docs, the definition of `alwayslink` is:
"If 1, any binary that depends (directly or indirectly) on this C++ precompiled library will link in all the object files archived in the static library, even if some contain no symbols referenced by the binary. This is useful if your code isn't explicitly called by code in the binary, e.g., if your code registers to receive some callback provided by some service."

That's precisely what we want here, for use in a rule like this:
```
cc_binary(
    name = "mlir_capi.so",
    deps = [
        "@llvm-project//mlir:MLIRBindingsPythonCAPI",
    ],
    linkshared = 1,
    linkopts = ["-Wl,-soname=mlir_capi.so", "-Wl,-rpath='$$ORIGIN'"],
)
```

The only alternative I can think of would be to have two copies of the implementation rules, one with `alwayslink` and one without, or a macro that does that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113565/new/

https://reviews.llvm.org/D113565