From e6a56909db88b12001cdc55a2cfe1863a726c0f5 Mon Sep 17 00:00:00 2001 From: Alexey Samsonov Date: Sat, 4 Jan 2025 00:00:13 -0800 Subject: [PATCH] [libc][bazel] Simplify libc_build_rules by extracting/grouping release copts. 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() rules that each libc public function produces: * ".__internal__" rule used in unit tests is *not* built with release copts and is thus indistinguishable from regular libc_support_library() rule. 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 half of sources are always built with -O3. If a user needs to run the tests in optimized mode, they should really be using Bazel invocation-level commandline flags instead. * "" rule 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. --- .../libc/libc_build_rules.bzl | 65 ++++++++----------- 1 file changed, 28 insertions(+), 37 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 82e65a728bc61..9c9fd50117cf6 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 )