This is an archive of the discontinued LLVM Phabricator instance.

[bazel] Separate capi_deps from deps in mlir_c_api_cc_library.
ClosedPublic

Authored by stellaraccident on Jan 12 2022, 5:37 PM.

Details

Summary

This is important because *Objects targets need to only depend on other *Objects targets, not on the unsuffixed CAPI rules. Depending on how it is linked in the current setup, it can cause duplicate symbols.

Diff Detail

Event Timeline

stellaraccident requested review of this revision.Jan 12 2022, 5:37 PM
GMNGeoffrey accepted this revision.Jan 12 2022, 5:46 PM

This LGTM, but I would wait for Peter to weigh in to make sure.

utils/bazel/llvm-project-overlay/mlir/build_defs.bzl
34

Can you document this argument? I think we're getting in to the territory of non-obvious on this one. I would probably just bite the bullet and do the standard "Args:" block, but mostly because I'm used to linters complaining if I don't do exactly that. I would say that header_deps and capi_deps are worth documenting here, at least.

This revision is now accepted and ready to land.Jan 12 2022, 5:46 PM
stellaraccident marked an inline comment as done.Jan 12 2022, 5:51 PM
stellaraccident added inline comments.
utils/bazel/llvm-project-overlay/mlir/build_defs.bzl
34

How's that?

GMNGeoffrey added inline comments.Jan 12 2022, 5:57 PM
utils/bazel/llvm-project-overlay/mlir/build_defs.bzl
34

πŸ‘

GMNGeoffrey added inline comments.Jan 12 2022, 6:01 PM
utils/bazel/llvm-project-overlay/mlir/build_defs.bzl
34

Macro dont-cross-the-streams:

I verified this works in the JAX build.

This revision was automatically updated to reflect the committed changes.
stellaraccident marked an inline comment as done.