-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[libc][bazel] Create libc_release_library for release configurations. #130694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
See PR llvm#130327 for background and motivation. This change expands the libc_support_library and libc_function rules to create filegroups that allow building a collection of llvm-libc functions together, from sources, as a part of a single cc_library that can then be used by the downstream clients. This change also adds an example use of this macro under libc/test/BUILD.bazel to confirm that this macro works as expected.
@llvm/pr-subscribers-libc Author: Alexey Samsonov (vonosmas) ChangesSee PR #130327 for background and motivation. This change expands the libc_support_library and libc_function rules to create filegroups that allow building a collection of llvm-libc functions together, from sources, as a part of a single cc_library that can then be used by the downstream clients. This change also adds an example use of this macro under libc/test/BUILD.bazel to confirm that this macro works as expected. Full diff: https://github.com/llvm/llvm-project/pull/130694.diff 2 Files Affected:
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 09daa9ecb3675..0d60992bbd638 100644
--- a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
@@ -64,50 +64,72 @@ def _libc_library(name, copts = [], deps = [], local_defines = [], **kwargs):
**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
+ weak = False): # @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,
- srcs,
weak = False,
- copts = [],
- local_defines = [],
**kwargs):
"""Add target for a libc function.
- The libc function is eventually available as a cc_library target by name
- "name". LLVM libc implementations of libc functions are in C++. So, this
- rule internally generates a C wrapper for the C++ implementation and adds
- it to the source list of the cc_library. This way, the C++ implementation
- and the C wrapper are both available in the cc_library.
+ 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.
Args:
name: Target name. It is normally the name of the function this target is
for.
- srcs: The .cpp files which contain the function implementation.
weak: Make the symbol corresponding to the libc function "weak".
- copts: The list of options to add to the C++ compilation command.
- local_defines: The preprocessor defines which will be prepended with -D
- and passed to the compile command of this target but not
- its deps.
**kwargs: Other attributes relevant for a cc_library. For example, deps.
"""
- # We compile the code twice, the first target is suffixed with ".__internal__" and contains the
+ # 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
# presence of another libc.
- libc_support_library(
+ _libc_library(
name = libc_internal_target(name),
- srcs = srcs,
- copts = copts,
- local_defines = local_defines,
**kwargs
)
+ _libc_library_filegroups(name = name, is_function = True, **kwargs)
+
+
+ # TODO(PR #130327): Remove this after downstream uses are migrated to libc_release_library.
# This second target is the llvm libc C function with default visibility.
func_attrs = [
"LLVM_LIBC_FUNCTION_ATTR_" + name + "='LLVM_LIBC_EMPTY, [[gnu::weak]]'",
@@ -115,9 +137,49 @@ def libc_function(
_libc_library(
name = name,
- srcs = srcs,
- copts = copts + libc_release_copts(),
- local_defines = local_defines + func_attrs,
+ copts = libc_release_copts(),
+ local_defines = func_attrs,
+ **kwargs
+ )
+
+def libc_release_library(
+ name,
+ libc_functions,
+ weak_symbols = [],
+ **kwargs):
+ """Create the release version of a libc library.
+
+ Args:
+ name: Name of the cc_library target.
+ libc_functions: List of functions to include in the library. They should be
+ created by libc_function macro.
+ weak_symbols: List of function names that should be marked as weak symbols.
+ **kwargs: Other arguments relevant to cc_library.
+ """
+ # Combine all sources into a single filegroup to avoid repeated sources error.
+ native.filegroup(
+ name = name + "_srcs",
+ srcs = [function + "_fn_srcs" for function in libc_functions],
+ )
+
+ native.cc_library(
+ name = name + "_textual_hdr_library",
+ textual_hdrs = [function + "_fn_textual_hdrs" for function in libc_functions],
+ )
+
+ weak_attributes = [
+ "LLVM_LIBC_FUNCTION_ATTR_" + name + "='LLVM_LIBC_EMPTY, [[gnu::weak]]'"
+ for name in weak_symbols
+ ]
+
+ native.cc_library(
+ name = name,
+ srcs = [":" + name + "_srcs"],
+ copts = libc_common_copts() + libc_release_copts(),
+ local_defines = weak_attributes + LIBC_CONFIGURE_OPTIONS,
+ deps = [
+ ":" + name + "_textual_hdr_library",
+ ],
**kwargs
)
diff --git a/utils/bazel/llvm-project-overlay/libc/test/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/BUILD.bazel
index 70b0196469eca..ac9689713fea4 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/BUILD.bazel
@@ -2,6 +2,26 @@
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+load("@bazel_skylib//rules:build_test.bzl", "build_test")
+load("//libc:libc_build_rules.bzl", "libc_release_library")
+
package(default_visibility = ["//visibility:public"])
exports_files(["libc_test_rules.bzl"])
+
+# Sanity check that libc_release_library macro works as intended.
+libc_release_library(
+ name = "libc_release_test",
+ libc_functions = [
+ "//libc:acosf",
+ "//libc:read",
+ ],
+ weak_symbols = [
+ "read",
+ ],
+)
+
+build_test(
+ name = "libc_release_test_build",
+ targets = [":libc_release_test"],
+)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something that could also be provided by a CMake cache file?
Joseph, could you elaborate? Do you suggest taking the list of sources/headers required to build a particular set of targets from the cache file CMake constructs? That probably wouldn't work in the Bazel world, where the CMake is never executed. |
I just mean if we have special release configurations if that's something we should export through CMake as well. |
Ah, OK. I would defer to @michaelrj-google and @lntue here, but I believe that CMake is somewhat ahead in terms of "release configuration" support? E.g. it has a concept and implementation of OS/arch-specific entrypoints and sets of headers under /libc/config. Up to this point, Bazel has no way of assembling a list of standalone libc functions into something like "library", this CL is a first small step towards doing that. |
I don't think we currently have a way for CMake to build all of the library targets as one massive library, right now it's all individual targets that get combined into a |
See PR #130327 for background and motivation. This change expands the libc_support_library and libc_function rules to create filegroups that allow building a collection of llvm-libc functions together, from sources, as a part of a single cc_library that can then be used by the downstream clients.
This change also adds an example use of this macro under libc/test/BUILD.bazel to confirm that this macro works as expected.