[llvm] [libc][bazel] Use Bazel aspects to implement libc_release_library. (PR #134948)
Alexey Samsonov via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 9 15:31:04 PDT 2025
https://github.com/vonosmas updated https://github.com/llvm/llvm-project/pull/134948
>From 25212567e74feeccfa42b2c80a4e8e22bf57afc0 Mon Sep 17 00:00:00 2001
From: Alexey Samsonov <vonosmas at gmail.com>
Date: Tue, 8 Apr 2025 16:16:01 -0700
Subject: [PATCH 1/2] [libc][bazel] Use Bazel aspects to implement
libc_release_library.
Instead of creating hundreds of implicit "filegroup" targets to keep
track of sources and textual headers required to build each libc
function or helper library, use Bazel aspects (see
https://bazel.build/versions/8.0.0/extending/aspects), which enable
transparent collection of transitive sources / textual headers while
walking the dependency DAG, and minimizes the Starlark overhead.
---
.../libc/libc_build_rules.bzl | 165 ++++++++++++------
1 file changed, 112 insertions(+), 53 deletions(-)
diff --git a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
index 4e73170be1e81..5640bf02392fb 100644
--- a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
@@ -10,8 +10,9 @@ load(":libc_configure_options.bzl", "LIBC_CONFIGURE_OPTIONS")
load(":libc_namespace.bzl", "LIBC_NAMESPACE")
load(":platforms.bzl", "PLATFORM_CPU_X86_64")
+# TODO: Remove this helper function once all donwstream users are migrated.
def libc_internal_target(name):
- return name + ".__internal__"
+ return name
def libc_common_copts():
root_label = Label(":libc")
@@ -44,84 +45,134 @@ def libc_release_copts():
})
return copts + platform_copts
-def _libc_library(name, copts = [], deps = [], local_defines = [], **kwargs):
+def _libc_library(name, **kwargs):
"""Internal macro to serve as a base for all other libc library rules.
Args:
name: Target name.
- copts: The special compiler options for the target.
- deps: The list of target dependencies if any.
- local_defines: The list of target local_defines if any.
**kwargs: All other attributes relevant for the cc_library rule.
"""
+ for attr in ["copts", "local_defines"]:
+ if attr in kwargs:
+ fail("disallowed attribute: '{}' in rule: '{}'".format(attr, name))
native.cc_library(
name = name,
- copts = copts + libc_common_copts(),
- local_defines = local_defines + LIBC_CONFIGURE_OPTIONS,
- deps = deps,
+ copts = libc_common_copts(),
+ local_defines = LIBC_CONFIGURE_OPTIONS,
linkstatic = 1,
**kwargs
)
-def _libc_library_filegroups(
- name,
- is_function,
- srcs = [],
- hdrs = [],
- textual_hdrs = [],
- deps = [],
- # We're not using kwargs, but instead explicitly list all possible
- # arguments that can be passed to libc_support_library or
- # libc_function macros. This is done to limit the configurability
- # and ensure the consistent and tightly controlled set of flags
- # (see libc_common_copts and libc_release_copts above) is used to build
- # libc code both for tests and for release configuration.
- target_compatible_with = None): # @unused
- """Internal macro to collect sources and headers required to build a library.
- """
-
- # filegroups created from "libc_function" macro has an extra "_fn" in their
- # name to ensure that no other libc target can depend on libc_function.
- prefix = name + ("_fn" if is_function else "")
- native.filegroup(
- name = prefix + "_srcs",
- srcs = srcs + hdrs + [dep + "_srcs" for dep in deps],
- )
- native.filegroup(
- name = prefix + "_textual_hdrs",
- srcs = textual_hdrs + [dep + "_textual_hdrs" for dep in deps],
- )
-
# A convenience function which should be used to list all libc support libraries.
# Any library which does not define a public function should be listed with
# libc_support_library.
def libc_support_library(name, **kwargs):
_libc_library(name = name, **kwargs)
- _libc_library_filegroups(name = name, is_function = False, **kwargs)
def libc_function(name, **kwargs):
"""Add target for a libc function.
This macro creates an internal cc_library that can be used to test this
- function, and creates filegroups required to include this function into
- a release build of libc.
+ function.
Args:
- name: Target name. It is normally the name of the function this target is
- for.
+ name: Target name. Typically the name of the function this target is for.
**kwargs: Other attributes relevant for a cc_library. For example, deps.
"""
- # Build "internal" library with a function, the target has ".__internal__" suffix and contains
- # C++ functions in the "LIBC_NAMESPACE" namespace. This allows us to test the function in the
+ # Builds "internal" library with a function, exposed as a C++ function in
+ # the "LIBC_NAMESPACE" namespace. This allows us to test the function in the
# presence of another libc.
_libc_library(
name = libc_internal_target(name),
**kwargs
)
- _libc_library_filegroups(name = name, is_function = True, **kwargs)
+# LibcLibraryInfo is used to collect all sources and textual headers required
+# to build a particular libc_function or libc_support_library.
+LibcLibraryInfo = provider(
+ fields = ["srcs", "textual_hdrs"],
+)
+
+def _get_libc_info_aspect_impl(target, ctx):
+ maybe_srcs = getattr(ctx.rule.attr, "srcs", [])
+ maybe_hdrs = getattr(ctx.rule.attr, "hdrs", [])
+ maybe_textual_hdrs = getattr(ctx.rule.attr, "textual_hdrs", [])
+ maybe_deps = getattr(ctx.rule.attr, "deps", [])
+ return LibcLibraryInfo(
+ srcs = depset(
+ [
+ f
+ for src in maybe_srcs + maybe_hdrs
+ for f in src.files.to_list()
+ if f.is_source
+ ],
+ transitive = [
+ dep[LibcLibraryInfo].srcs
+ for dep in maybe_deps
+ if LibcLibraryInfo in dep
+ ],
+ ),
+ textual_hdrs = depset(
+ [
+ f
+ for hdr in maybe_textual_hdrs
+ for f in hdr.files.to_list()
+ if f.is_source
+ ],
+ transitive = [
+ dep[LibcLibraryInfo].textual_hdrs
+ for dep in maybe_deps
+ if LibcLibraryInfo in dep
+ ],
+ ),
+ )
+
+_get_libc_info_aspect = aspect(
+ implementation = _get_libc_info_aspect_impl,
+ attr_aspects = ["deps"],
+)
+
+def _get_libc_srcs_impl(ctx):
+ return DefaultInfo(
+ files = depset(transitive = [
+ fn[LibcLibraryInfo].srcs
+ for fn in ctx.attr.libs
+ ]),
+ )
+
+# get_libc_srcs returns the list of sources required to build all
+# specified libraries.
+get_libc_srcs = rule(
+ implementation = _get_libc_srcs_impl,
+ attrs = {
+ "libs": attr.label_list(
+ mandatory = True,
+ aspects = [_get_libc_info_aspect],
+ ),
+ },
+)
+
+def _get_libc_textual_hdrs_impl(ctx):
+ return DefaultInfo(
+ files = depset(transitive = [
+ fn[LibcLibraryInfo].textual_hdrs
+ for fn in ctx.attr.libs
+ ]),
+ )
+
+# get_libc_textual_hdrs returns the list of textual headers required to compile
+# all specified libraries.
+get_libc_textual_hdrs = rule(
+ implementation = _get_libc_textual_hdrs_impl,
+ attrs = {
+ "libs": attr.label_list(
+ mandatory = True,
+ aspects = [_get_libc_info_aspect],
+ ),
+ },
+)
def libc_release_library(
name,
@@ -138,15 +189,18 @@ def libc_release_library(
**kwargs: Other arguments relevant to cc_library.
"""
- # Combine all sources into a single filegroup to avoid repeated sources error.
- native.filegroup(
+ get_libc_srcs(
name = name + "_srcs",
- srcs = [function + "_fn_srcs" for function in libc_functions],
+ libs = libc_functions,
)
+ get_libc_textual_hdrs(
+ name = name + "_textual_hdrs",
+ libs = libc_functions,
+ )
native.cc_library(
name = name + "_textual_hdr_library",
- textual_hdrs = [function + "_fn_textual_hdrs" for function in libc_functions],
+ textual_hdrs = [":" + name + "_textual_hdrs"],
)
weak_attributes = [
@@ -175,15 +229,20 @@ def libc_header_library(name, hdrs, deps = [], **kwargs):
**kwargs: All other attributes relevant for the cc_library rule.
"""
- # Combine sources from dependencies to create a single cc_library target.
- native.filegroup(
+ get_libc_srcs(
name = name + "_hdr_deps",
- srcs = [dep + "_srcs" for dep in deps],
+ libs = deps,
+ )
+
+ get_libc_textual_hdrs(
+ name = name + "_textual_hdrs",
+ libs = deps,
)
native.cc_library(
name = name + "_textual_hdr_library",
- textual_hdrs = [dep + "_textual_hdrs" for dep in deps],
+ textual_hdrs = [":" + name + "_textual_hdrs"],
)
+
native.cc_library(
name = name,
# Technically speaking, we should put _hdr_deps in srcs, as they are
>From dcd7e67b3b96772d0c2cb6c19950322ca80195c2 Mon Sep 17 00:00:00 2001
From: Alexey Samsonov <vonosmas at gmail.com>
Date: Wed, 9 Apr 2025 14:30:39 -0700
Subject: [PATCH 2/2] Address comments from rupprecht@ - better docs and names.
---
.../libc/libc_build_rules.bzl | 51 ++++++++-----------
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
index 5640bf02392fb..86dfb53a86014 100644
--- a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
@@ -89,42 +89,37 @@ def libc_function(name, **kwargs):
**kwargs
)
-# LibcLibraryInfo is used to collect all sources and textual headers required
-# to build a particular libc_function or libc_support_library.
LibcLibraryInfo = provider(
+ "All source files and textual headers for building a particular library.",
fields = ["srcs", "textual_hdrs"],
)
-def _get_libc_info_aspect_impl(target, ctx):
+def _get_libc_info_aspect_impl(
+ target, # @unused
+ ctx):
maybe_srcs = getattr(ctx.rule.attr, "srcs", [])
maybe_hdrs = getattr(ctx.rule.attr, "hdrs", [])
maybe_textual_hdrs = getattr(ctx.rule.attr, "textual_hdrs", [])
maybe_deps = getattr(ctx.rule.attr, "deps", [])
return LibcLibraryInfo(
srcs = depset(
- [
- f
- for src in maybe_srcs + maybe_hdrs
- for f in src.files.to_list()
- if f.is_source
- ],
transitive = [
dep[LibcLibraryInfo].srcs
for dep in maybe_deps
if LibcLibraryInfo in dep
+ ] + [
+ src.files
+ for src in maybe_srcs + maybe_hdrs
],
),
textual_hdrs = depset(
- [
- f
- for hdr in maybe_textual_hdrs
- for f in hdr.files.to_list()
- if f.is_source
- ],
transitive = [
dep[LibcLibraryInfo].textual_hdrs
for dep in maybe_deps
if LibcLibraryInfo in dep
+ ] + [
+ hdr.files
+ for hdr in maybe_textual_hdrs
],
),
)
@@ -134,7 +129,7 @@ _get_libc_info_aspect = aspect(
attr_aspects = ["deps"],
)
-def _get_libc_srcs_impl(ctx):
+def _libc_srcs_filegroup_impl(ctx):
return DefaultInfo(
files = depset(transitive = [
fn[LibcLibraryInfo].srcs
@@ -142,10 +137,9 @@ def _get_libc_srcs_impl(ctx):
]),
)
-# get_libc_srcs returns the list of sources required to build all
-# specified libraries.
-get_libc_srcs = rule(
- implementation = _get_libc_srcs_impl,
+_libc_srcs_filegroup = rule(
+ doc = "Returns all sources for building the specified libraries.",
+ implementation = _libc_srcs_filegroup_impl,
attrs = {
"libs": attr.label_list(
mandatory = True,
@@ -154,7 +148,7 @@ get_libc_srcs = rule(
},
)
-def _get_libc_textual_hdrs_impl(ctx):
+def _libc_textual_hdrs_filegroup_impl(ctx):
return DefaultInfo(
files = depset(transitive = [
fn[LibcLibraryInfo].textual_hdrs
@@ -162,10 +156,9 @@ def _get_libc_textual_hdrs_impl(ctx):
]),
)
-# get_libc_textual_hdrs returns the list of textual headers required to compile
-# all specified libraries.
-get_libc_textual_hdrs = rule(
- implementation = _get_libc_textual_hdrs_impl,
+_libc_textual_hdrs_filegroup = rule(
+ doc = "Returns all textual headers for compiling the specified libraries.",
+ implementation = _libc_textual_hdrs_filegroup_impl,
attrs = {
"libs": attr.label_list(
mandatory = True,
@@ -189,12 +182,12 @@ def libc_release_library(
**kwargs: Other arguments relevant to cc_library.
"""
- get_libc_srcs(
+ _libc_srcs_filegroup(
name = name + "_srcs",
libs = libc_functions,
)
- get_libc_textual_hdrs(
+ _libc_textual_hdrs_filegroup(
name = name + "_textual_hdrs",
libs = libc_functions,
)
@@ -229,12 +222,12 @@ def libc_header_library(name, hdrs, deps = [], **kwargs):
**kwargs: All other attributes relevant for the cc_library rule.
"""
- get_libc_srcs(
+ _libc_srcs_filegroup(
name = name + "_hdr_deps",
libs = deps,
)
- get_libc_textual_hdrs(
+ _libc_textual_hdrs_filegroup(
name = name + "_textual_hdrs",
libs = deps,
)
More information about the llvm-commits
mailing list