[PATCH] D113565: Split headers from implementations in MLIR C API Bazel build.
Geoffrey Martin-Noble via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 10 14:05:21 PST 2021
GMNGeoffrey added a subscriber: stellaraccident.
GMNGeoffrey added inline comments.
================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:315
+
+CAPI_IR_HEADERS = [
+ "include/mlir-c/AffineExpr.h",
----------------
phawkins wrote:
> GMNGeoffrey wrote:
> > Could this be a glob with an exclude for each of the things in a separate library? I think that would be easier to maintain
> It could be, but note that's not something this PR changes: here we just move the list around. If we want to do that, can we do it in another PR?
Separate patch SGTM. I only mentioned it because you were already moving this list around.
================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:774
+ ],
+ alwayslink = True,
)
----------------
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?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113565/new/
https://reviews.llvm.org/D113565
More information about the llvm-commits
mailing list