[libc][bazel] Simplify libc_build_rules by grouping release copts

[vonosmas]

Extract all compiler options used to build "release" versions of libc API functions into a separate helper function, instead of burying this logic inside libc_function() macro.

With this change, we further split two "flavors" of cc_library() produced for each libc public function:

• <function>.__internal__ library used in unit tests is not built with release copts and is thus indistinguishable from regular libc_support_library(). Arguably, it's a good thing, because all sources in a unit test are built with the same set of compiler flags, instead of "franken-build" when a subset of sources is always built with -O3. If a user needs to run the tests in optimized mode, they should really be using Bazel invocation-level compile flags instead.
• <function> library that libc users can use to construct their own static archive is built with the same release copts as before. There is a pre-existing problem that its libc_support_library() dependencies are not built with the same copts. We're not addressing it here now.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Alexey Samsonov (vonosmas)

Changes

Extract all compiler options used to build "release" versions of libc API functions into a separate helper function, instead of burying this logic inside libc_function() macro.

With this change, we further split two "flavors" of cc_library() produced for each libc public function:

• <function>.__internal__ library used in unit tests is not built with release copts and is thus indistinguishable from regular libc_support_library(). Arguably, it's a good thing, because all sources in a unit test are built with the same set of compiler flags, instead of "franken-build" when a subset of sources is always built with -O3. If a user needs to run the tests in optimized mode, they should really be using Bazel invocation-level compile flags instead.
• <function> library that libc users can use to construct their own static archive is built with the same release copts as before. There is a pre-existing problem that its libc_support_library() dependencies are not built with the same copts. We're not addressing it here now.

---

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

1 Files Affected:

• (modified) utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl (+28-37)

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 82e65a728bc61b..9c9fd50117cf6a 100644
--- a/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl
@@ -22,7 +22,29 @@ def libc_common_copts():
         "-DLIBC_NAMESPACE=" + LIBC_NAMESPACE,
     ]
 
-def _libc_library(name, hidden, copts = [], deps = [], local_defines = [], **kwargs):
+def libc_release_copts():
+    copts = [
+        "-DLIBC_COPT_PUBLIC_PACKAGING",
+        # This is used to explicitly give public symbols "default" visibility.
+        # See src/__support/common.h for more information.
+        "-DLLVM_LIBC_FUNCTION_ATTR='[[gnu::visibility(\"default\")]]'",
+        # All other libc sources need to be compiled with "hidden" visibility.
+        "-fvisibility=hidden",
+        "-O3",
+        "-fno-builtin",
+        "-fno-lax-vector-conversions",
+        "-ftrivial-auto-var-init=pattern",
+        "-fno-omit-frame-pointer",
+        "-fstack-protector-strong",
+    ]
+
+    platform_copts = selects.with_or({
+        PLATFORM_CPU_X86_64: ["-mno-omit-leaf-frame-pointer"],
+        "//conditions:default": [],
+    })
+    return copts + platform_copts
+
+def _libc_library(name, copts = [], deps = [], local_defines = [], **kwargs):
     """Internal macro to serve as a base for all other libc library rules.
 
     Args:
@@ -30,15 +52,9 @@ def _libc_library(name, hidden, copts = [], deps = [], local_defines = [], **kwa
       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.
-      hidden: Whether the symbols should be explicitly hidden or not.
       **kwargs: All other attributes relevant for the cc_library rule.
     """
 
-    # We want all libc sources to be compiled with "hidden" visibility.
-    # The public symbols will be given "default" visibility explicitly.
-    # See src/__support/common.h for more information.
-    if hidden:
-        copts = copts + ["-fvisibility=hidden"]
     native.cc_library(
         name = name,
         copts = copts + libc_common_copts(),
@@ -52,13 +68,13 @@ def _libc_library(name, hidden, copts = [], deps = [], local_defines = [], **kwa
 # 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, hidden = False, **kwargs)
+    _libc_library(name = name, **kwargs)
 
 def libc_function(
         name,
         srcs,
         weak = False,
-        copts = None,
+        copts = [],
         local_defines = [],
         **kwargs):
     """Add target for a libc function.
@@ -81,25 +97,6 @@ def libc_function(
       **kwargs: Other attributes relevant for a cc_library. For example, deps.
     """
 
-    # We use the explicit equals pattern here because append and += mutate the
-    # original list, where this creates a new list and stores it in deps.
-    copts = copts or []
-    copts = copts + [
-        "-O3",
-        "-fno-builtin",
-        "-fno-lax-vector-conversions",
-        "-ftrivial-auto-var-init=pattern",
-        "-fno-omit-frame-pointer",
-        "-fstack-protector-strong",
-    ]
-
-    # x86 targets have -mno-omit-leaf-frame-pointer.
-    platform_copts = selects.with_or({
-        PLATFORM_CPU_X86_64: ["-mno-omit-leaf-frame-pointer"],
-        "//conditions:default": [],
-    })
-    copts = copts + platform_copts
-
     # We compile the code twice, the first target is suffixed with ".__internal__" and contains the
     # C++ functions in the "LIBC_NAMESPACE" namespace. This allows us to test the function in the
     # presence of another libc.
@@ -111,22 +108,16 @@ def libc_function(
         **kwargs
     )
 
-    # This second target is the llvm libc C function with either a default or hidden visibility.
-    # All other functions are hidden.
+    # This second target is the llvm libc C function with default visibility.
     func_attrs = [
         "LLVM_LIBC_FUNCTION_ATTR_" + name + "='LLVM_LIBC_EMPTY, [[gnu::weak]]'",
     ] if weak else []
 
-    local_defines = (local_defines +
-                     ["LIBC_COPT_PUBLIC_PACKAGING"] +
-                     ["LLVM_LIBC_FUNCTION_ATTR='[[gnu::visibility(\"default\")]]'"] +
-                     func_attrs)
     _libc_library(
         name = name,
-        hidden = True,
         srcs = srcs,
-        copts = copts,
-        local_defines = local_defines,
+        copts = copts + libc_release_copts(),
+        local_defines = local_defines + func_attrs,
         **kwargs
     )