Skip to content

Conversation

@vonosmas
Copy link
Contributor

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.

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.
@llvmbot llvmbot added libc bazel "Peripheral" support tier build system: utils/bazel labels Mar 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 11, 2025

@llvm/pr-subscribers-libc

Author: Alexey Samsonov (vonosmas)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/130694.diff

2 Files Affected:

  • (modified) utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl (+83-21)
  • (modified) utils/bazel/llvm-project-overlay/libc/test/BUILD.bazel (+20)
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"],
+)

Copy link
Contributor

@michaelrj-google michaelrj-google left a 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

Copy link
Contributor

@jhuber6 jhuber6 left a 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?

@vonosmas
Copy link
Contributor Author

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.

@jhuber6
Copy link
Contributor

jhuber6 commented Mar 11, 2025

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.

@vonosmas
Copy link
Contributor Author

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.

@michaelrj-google
Copy link
Contributor

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 libc.a. I'm not opposed to adding this functionality to cmake, but it'd need more than just a cmake cache file. Adding something like this would involve adding a bunch of new cmake to collect all the non-skipped targets' sources, and also creating a new target for the monobuild library.

@vonosmas vonosmas merged commit 10a6a34 into llvm:main Mar 11, 2025
9 checks passed
@vonosmas vonosmas deleted the pr130327 branch March 11, 2025 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel libc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants