This is an archive of the discontinued LLVM Phabricator instance.

Differential D117176

[bazel] Separate capi_deps from deps in mlir_c_api_cc_library.
ClosedPublic

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

Details

Reviewers
GMNGeoffrey
phawkins
Commits
rGb93eded7b945: [bazel] Separate capi_deps from deps in mlir_c_api_cc_library.
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 created this revision.Jan 12 2022, 5:37 PM
Herald added subscribers: sdasgup3, wenzhicui, wrengr and 20 others. · View Herald TranscriptJan 12 2022, 5:37 PM
stellaraccident requested review of this revision.Jan 12 2022, 5:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2022, 5:37 PM
Herald added subscribers: llvm-commits, stephenneuendorffer, nicolasvasilache. · View Herald Transcript
stellaraccident added reviewers: GMNGeoffrey, phawkins.Jan 12 2022, 5:38 PM
Harbormaster completed remote builds in B143053: Diff 399520.Jan 12 2022, 5:42 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 updated this revision to Diff 399521.Jan 12 2022, 5:51 PM

Comment.

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?

Harbormaster completed remote builds in B143054: Diff 399521.Jan 12 2022, 5:54 PM
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:

phawkins added a comment.Jan 13 2022, 6:25 AM

I verified this works in the JAX build.

Closed by commit rGb93eded7b945: [bazel] Separate capi_deps from deps in mlir_c_api_cc_library. (authored by stellaraccident). · Explain WhyJan 13 2022, 10:08 PM
This revision was automatically updated to reflect the committed changes.
stellaraccident marked an inline comment as done.
stellaraccident added a commit: rGb93eded7b945: [bazel] Separate capi_deps from deps in mlir_c_api_cc_library..

Revision Contents

PathSize
utils/
bazel/
llvm-project-overlay/
mlir/
59 lines
16 lines

Diff 399895

utils/bazel/llvm-project-overlay/mlir/BUILD.bazel

Show First 20 Lines • Show All 376 Lines • ▼ Show 20 Linesmlir_c_api_cc_library(
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIInterfaces", name = "CAPIInterfaces",
srcs = [ srcs = [
"lib/CAPI/Interfaces/Interfaces.cpp", "lib/CAPI/Interfaces/Interfaces.cpp",
], ],
capi_deps = [
":CAPIIR",
],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":IR", ":IR",
":InferTypeOpInterface", ":InferTypeOpInterface",
"//llvm:Support", "//llvm:Support",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIAsync", name = "CAPIAsync",
srcs = [ srcs = [
"lib/CAPI/Dialect/Async.cpp", "lib/CAPI/Dialect/Async.cpp",
"lib/CAPI/Dialect/AsyncPasses.cpp", "lib/CAPI/Dialect/AsyncPasses.cpp",
], ],
hdrs = ["include/mlir-c/Dialect/Async.h"], hdrs = ["include/mlir-c/Dialect/Async.h"],
capi_deps = [
":CAPIIR",
],
header_deps = [ header_deps = [
":AsyncPassIncGen", ":AsyncPassIncGen",
":CAPIIRHeaders",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":Async", ":Async",
":AsyncTransforms", ":AsyncTransforms",
":CAPIIR",
":Pass", ":Pass",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPILinalg", name = "CAPILinalg",
srcs = [ srcs = [
"lib/CAPI/Dialect/Linalg.cpp", "lib/CAPI/Dialect/Linalg.cpp",
"lib/CAPI/Dialect/LinalgPasses.cpp", "lib/CAPI/Dialect/LinalgPasses.cpp",
], ],
hdrs = [ hdrs = [
"include/mlir-c/Dialect/Linalg.h", "include/mlir-c/Dialect/Linalg.h",
], ],
capi_deps = [
":CAPIIR",
],
header_deps = [ header_deps = [
":CAPIIRHeaders",
":LinalgPassIncGen", ":LinalgPassIncGen",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":LinalgOps", ":LinalgOps",
":LinalgTransforms", ":LinalgTransforms",
":Pass", ":Pass",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPILLVM", name = "CAPILLVM",
srcs = [ srcs = [
"lib/CAPI/Dialect/LLVM.cpp", "lib/CAPI/Dialect/LLVM.cpp",
], ],
hdrs = [ hdrs = [
"include/mlir-c/Dialect/LLVM.h", "include/mlir-c/Dialect/LLVM.h",
], ],
header_deps = [ capi_deps = [
":CAPIIRHeaders", ":CAPIIR",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":LLVMDialect", ":LLVMDialect",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIGPU", name = "CAPIGPU",
srcs = [ srcs = [
"lib/CAPI/Dialect/GPU.cpp", "lib/CAPI/Dialect/GPU.cpp",
"lib/CAPI/Dialect/GPUPasses.cpp", "lib/CAPI/Dialect/GPUPasses.cpp",
], ],
hdrs = [ hdrs = [
"include/mlir-c/Dialect/GPU.h", "include/mlir-c/Dialect/GPU.h",
], ],
capi_deps = [
":CAPIIR",
],
header_deps = [ header_deps = [
":CAPIIRHeaders",
":GPUPassIncGen", ":GPUPassIncGen",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":GPUDialect", ":GPUDialect",
":GPUTransforms", ":GPUTransforms",
":Pass", ":Pass",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPISparseTensor", name = "CAPISparseTensor",
srcs = [ srcs = [
"lib/CAPI/Dialect/SparseTensor.cpp", "lib/CAPI/Dialect/SparseTensor.cpp",
"lib/CAPI/Dialect/SparseTensorPasses.cpp", "lib/CAPI/Dialect/SparseTensorPasses.cpp",
], ],
hdrs = [ hdrs = [
"include/mlir-c/Dialect/SparseTensor.h", "include/mlir-c/Dialect/SparseTensor.h",
], ],
capi_deps = [
":CAPIIR",
],
header_deps = [ header_deps = [
":CAPIIRHeaders",
":SparseTensorPassIncGen", ":SparseTensorPassIncGen",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":Pass", ":Pass",
":SparseTensor", ":SparseTensor",
":SparseTensorTransforms", ":SparseTensorTransforms",
":Support", ":Support",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIQuant", name = "CAPIQuant",
srcs = [ srcs = [
"lib/CAPI/Dialect/Quant.cpp", "lib/CAPI/Dialect/Quant.cpp",
], ],
hdrs = [ hdrs = [
"include/mlir-c/Dialect/Quant.h", "include/mlir-c/Dialect/Quant.h",
], ],
header_deps = [ capi_deps = [
":CAPIIRHeaders", ":CAPIIR",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":QuantOps", ":QuantOps",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIPDL", name = "CAPIPDL",
srcs = [ srcs = [
"lib/CAPI/Dialect/PDL.cpp", "lib/CAPI/Dialect/PDL.cpp",
Show All 12 Lines deps = [
":PDLTypesIncGen", ":PDLTypesIncGen",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIConversion", name = "CAPIConversion",
srcs = ["lib/CAPI/Conversion/Passes.cpp"], srcs = ["lib/CAPI/Conversion/Passes.cpp"],
hdrs = ["include/mlir-c/Conversion.h"], hdrs = ["include/mlir-c/Conversion.h"],
capi_deps = [
":CAPIIR",
],
header_deps = [ header_deps = [
":CAPIIRHeaders",
":ConversionPassIncGen", ":ConversionPassIncGen",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":ConversionPasses", ":ConversionPasses",
":Pass", ":Pass",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIDebug", name = "CAPIDebug",
srcs = ["lib/CAPI/Debug/Debug.cpp"], srcs = ["lib/CAPI/Debug/Debug.cpp"],
hdrs = ["include/mlir-c/Debug.h"], hdrs = ["include/mlir-c/Debug.h"],
header_deps = [ capi_deps = [
":CAPIIRHeaders", ":CAPIIR",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":Support", ":Support",
"//llvm:Support", "//llvm:Support",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIExecutionEngine", name = "CAPIExecutionEngine",
srcs = ["lib/CAPI/ExecutionEngine/ExecutionEngine.cpp"], srcs = ["lib/CAPI/ExecutionEngine/ExecutionEngine.cpp"],
hdrs = [ hdrs = [
"include/mlir-c/ExecutionEngine.h", "include/mlir-c/ExecutionEngine.h",
"include/mlir/CAPI/ExecutionEngine.h", "include/mlir/CAPI/ExecutionEngine.h",
], ],
capi_deps = [
":CAPIIR",
],
header_deps = [ header_deps = [
":CAPIIRHeaders",
":ExecutionEngine", ":ExecutionEngine",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":ExecutionEngineUtils", ":ExecutionEngineUtils",
":LLVMToLLVMIRTranslation", ":LLVMToLLVMIRTranslation",
"//llvm:OrcJIT", "//llvm:OrcJIT",
"//llvm:Support", "//llvm:Support",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPITransforms", name = "CAPITransforms",
srcs = ["lib/CAPI/Transforms/Passes.cpp"], srcs = ["lib/CAPI/Transforms/Passes.cpp"],
hdrs = ["include/mlir-c/Transforms.h"], hdrs = ["include/mlir-c/Transforms.h"],
capi_deps = [
":CAPIIR",
],
header_deps = [ header_deps = [
":CAPIIRHeaders",
":TransformsPassIncGen", ":TransformsPassIncGen",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":CAPIIR",
":Pass", ":Pass",
":Transforms", ":Transforms",
], ],
) )
mlir_c_api_cc_library( mlir_c_api_cc_library(
name = "CAPIRegistration", name = "CAPIRegistration",
srcs = ["lib/CAPI/Registration/Registration.cpp"], srcs = ["lib/CAPI/Registration/Registration.cpp"],
hdrs = ["include/mlir-c/Registration.h"], hdrs = ["include/mlir-c/Registration.h"],
header_deps = [ capi_deps = [
":CAPIIRHeaders", ":CAPIIR",
], ],
includes = ["include"], includes = ["include"],
deps = [ deps = [
":AllPassesAndDialects", ":AllPassesAndDialects",
":CAPIIR",
":LLVMToLLVMIRTranslation", ":LLVMToLLVMIRTranslation",
], ],
) )
##---------------------------------------------------------------------------## ##---------------------------------------------------------------------------##
# Sources of Python bindings. # Sources of Python bindings.
#----------------------------------------------------------------------------## #----------------------------------------------------------------------------##
▲ Show 20 Lines • Show All 7,458 Lines • Show Last 20 Lines

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

Show All 25 Lines
) )
def mlir_c_api_cc_library( def mlir_c_api_cc_library(
name, name,
srcs = [], srcs = [],
hdrs = [], hdrs = [],
deps = [], deps = [],
header_deps = [], header_deps = [],
capi_deps = [],
GMNGeoffreyUnsubmitted
Done
ReplyInline Actions

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.

GMNGeoffrey: Can you document this argument? I think we're getting in to the territory of non-obvious on…
stellaraccidentAuthorUnsubmitted
Done
ReplyInline Actions

How's that?

stellaraccident: How's that?
GMNGeoffreyUnsubmitted
Done
ReplyInline Actions

👍

GMNGeoffrey: 👍
GMNGeoffreyUnsubmitted
Done
ReplyInline Actions

Macro dont-cross-the-streams:

GMNGeoffrey: {meme, src="dont-cross-the-streams"}
**kwargs): **kwargs):
"""Macro that generates three targets for MLIR C API libraries. """Macro that generates three targets for MLIR C API libraries.
* A standard cc_library target ("Name"), * A standard cc_library target ("Name"),
* A header-only cc_library target ("NameHeaders") * A header-only cc_library target ("NameHeaders")
* An implementation cc_library target tagged `alwayslink` suitable for * An implementation cc_library target tagged `alwayslink` suitable for
inclusion in a shared library built with cc_binary() ("NameObjects"). inclusion in a shared library built with cc_binary() ("NameObjects").
"""
In order to avoid duplicate symbols, it is important that
mlir_c_api_cc_library targets only depend on other mlir_c_api_cc_library
targets via the "capi_deps" parameter. This makes it so that "FooObjects"
depend on "BarObjects" targets and "Foo" targets depend on "Bar" targets.
Don't cross the streams.
"""
capi_header_deps = ["%sHeaders" % d for d in capi_deps]
capi_object_deps = ["%sObjects" % d for d in capi_deps]
native.cc_library( native.cc_library(
name = name, name = name,
srcs = srcs, srcs = srcs,
hdrs = hdrs, hdrs = hdrs,
deps = deps + header_deps, deps = deps + capi_deps + header_deps,
**kwargs **kwargs
) )
native.cc_library( native.cc_library(
name = name + "Headers", name = name + "Headers",
hdrs = hdrs, hdrs = hdrs,
deps = header_deps, deps = header_deps + capi_header_deps,
**kwargs **kwargs
) )
native.cc_library( native.cc_library(
name = name + "Objects", name = name + "Objects",
srcs = srcs, srcs = srcs,
hdrs = hdrs, hdrs = hdrs,
deps = deps + header_deps, deps = deps + capi_object_deps + capi_header_deps + header_deps,
alwayslink = True, alwayslink = True,
**kwargs **kwargs
) )