From 5214b5fd8807a302fe0a64cb47bf842f3d1125f9 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Wed, 3 Apr 2024 11:20:30 +0200 Subject: [PATCH 1/3] Add strip optimizations to linker flags Previously, mangled symbols would show up in the exports list on MacOS (as seen in `nm -U` commands on the Python bindings shared objects). The fix here is to supply `-Wl,-x` and `-Wl,-S` to the macOS linker, which ends up producing the same output as CMake does (verified on my machine). Similarly, after checking the Linux ninja build manifest, Linux needs a `-Wl,-s`. --- nanobind.BUILD | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/nanobind.BUILD b/nanobind.BUILD index 944d4e2..fc96f3f 100644 --- a/nanobind.BUILD +++ b/nanobind.BUILD @@ -26,6 +26,7 @@ cc_library( ], "@platforms//os:linux": [ "-fPIC", + "-fvisibility=hidden", "-ffunction-sections", "-fdata-sections", "-fno-strict-aliasing", @@ -37,11 +38,15 @@ cc_library( includes = ["include"], linkopts = select({ "@platforms//os:linux": [ + "-Wl,-s", "-Wl,--gc-sections", ], "@platforms//os:macos": [ # chained fixups on Apple platforms. "-Wl,@$(location :cmake/darwin-ld-cpython.sym)", + "-Wl,-dead_strip", + "-Wl,-x", + "-Wl,-S", ], "//conditions:default": [], }), From 2d17be4ed953505775495e659f947e08ca2425d4 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Wed, 3 Apr 2024 13:32:27 +0200 Subject: [PATCH 2/3] Only apply linker strip optimizations in release builds Otherwise, debugging is impeded. --- BUILD | 20 ++++++++++++++++++++ helpers.bzl | 8 ++++++++ nanobind.BUILD | 11 +++-------- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/BUILD b/BUILD index 92927b2..89c8f76 100644 --- a/BUILD +++ b/BUILD @@ -48,6 +48,26 @@ config_setting( flag_values = {":py-limited-api": "unset"}, ) +config_setting( + name = "MacReleaseBuild", + constraint_values = [ + "@platforms//os:macos", + ], + values = { + "compilation_mode": "opt", + }, +) + +config_setting( + name = "LinuxReleaseBuild", + constraint_values = [ + "@platforms//os:linux", + ], + values = { + "compilation_mode": "opt", + }, +) + selects.config_setting_group( name = "unix", match_any = [ diff --git a/helpers.bzl b/helpers.bzl index 9c5f963..c5905a8 100644 --- a/helpers.bzl +++ b/helpers.bzl @@ -7,6 +7,14 @@ def sizeopts(): "@nanobind_bazel//:without_sizeopts": [], }) +def stripopts(): + """Linker options to strip external and debug symbols from nanobind release builds.""" + return select({ + "@nanobind_bazel//:MacReleaseBuild": ["-Wl,-x", "-Wl,-S"], + "@nanobind_bazel//:LinuxReleaseBuild": ["-Wl,-s"], + "//conditions:default": [], + }) + def sizedefs(): return select({ "@nanobind_bazel//:with_sizeopts": ["NB_COMPACT_ASSERTIONS"], diff --git a/nanobind.BUILD b/nanobind.BUILD index fc96f3f..f74cc2b 100644 --- a/nanobind.BUILD +++ b/nanobind.BUILD @@ -5,7 +5,7 @@ Size optimizations used: -Os, LTO. Linker optimizations used: LTO (clang, gcc) / LTCG (MSVC), linker response file (macOS only). """ -load("@nanobind_bazel//:helpers.bzl", "py_limited_api", "sizedefs", "sizeopts") +load("@nanobind_bazel//:helpers.bzl", "py_limited_api", "sizedefs", "sizeopts", "stripopts") licenses(["notice"]) @@ -37,19 +37,14 @@ cc_library( features = ["-pic"], # use a compiler flag instead. includes = ["include"], linkopts = select({ - "@platforms//os:linux": [ - "-Wl,-s", - "-Wl,--gc-sections", - ], + "@platforms//os:linux": ["-Wl,--gc-sections"], "@platforms//os:macos": [ # chained fixups on Apple platforms. "-Wl,@$(location :cmake/darwin-ld-cpython.sym)", "-Wl,-dead_strip", - "-Wl,-x", - "-Wl,-S", ], "//conditions:default": [], - }), + }) + stripopts(), local_defines = sizedefs(), # sizeopts apply to nanobind only. textual_hdrs = glob( [ From 2defc23dbc0a00f144953eda5ea73d00ba69f7e6 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Wed, 3 Apr 2024 18:38:42 +0200 Subject: [PATCH 3/3] Add nb_ prefix to helpers, change NB_COMPACT_ASSERTIONS condition The latter is only tangentially related to size optimizations, and has to do with debug info, so we condition on release mode. --- BUILD | 19 +++++++++++++++++++ build_defs.bzl | 4 ++-- helpers.bzl | 10 +++++----- nanobind.BUILD | 16 +++++++++++----- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/BUILD b/BUILD index 89c8f76..e1e5b15 100644 --- a/BUILD +++ b/BUILD @@ -68,6 +68,25 @@ config_setting( }, ) +config_setting( + name = "WindowsReleaseBuild", + constraint_values = [ + "@platforms//os:windows", + ], + values = { + "compilation_mode": "opt", + }, +) + +selects.config_setting_group( + name = "releaseBuild", + match_any = [ + ":LinuxReleaseBuild", + ":MacReleaseBuild", + ":WindowsReleaseBuild", + ], +) + selects.config_setting_group( name = "unix", match_any = [ diff --git a/build_defs.bzl b/build_defs.bzl index 0f39058..4cc0c5c 100644 --- a/build_defs.bzl +++ b/build_defs.bzl @@ -11,7 +11,7 @@ which can then be included e.g. as a `data` input in a ``native.py_library``. """ load("@bazel_skylib//rules:copy_file.bzl", "copy_file") -load("@nanobind_bazel//:helpers.bzl", "extension_name", "sizeopts") +load("@nanobind_bazel//:helpers.bzl", "extension_name", "nb_sizeopts") NANOBIND_COPTS = select({ "@platforms//os:macos": [ @@ -27,7 +27,7 @@ NANOBIND_COPTS = select({ "-fdata-sections", ], "//conditions:default": [], -}) + sizeopts() +}) + nb_sizeopts() NANOBIND_DEPS = [Label("@nanobind//:nanobind")] diff --git a/helpers.bzl b/helpers.bzl index c5905a8..59ca75d 100644 --- a/helpers.bzl +++ b/helpers.bzl @@ -1,13 +1,13 @@ """Helper flags for nanobind build options.""" -def sizeopts(): +def nb_sizeopts(): return select({ "@nanobind_bazel//:msvc_and_minsize": ["/Os"], "@nanobind_bazel//:nonmsvc_and_minsize": ["-Os"], "@nanobind_bazel//:without_sizeopts": [], }) -def stripopts(): +def nb_stripopts(): """Linker options to strip external and debug symbols from nanobind release builds.""" return select({ "@nanobind_bazel//:MacReleaseBuild": ["-Wl,-x", "-Wl,-S"], @@ -15,10 +15,10 @@ def stripopts(): "//conditions:default": [], }) -def sizedefs(): +def maybe_compact_asserts(): return select({ - "@nanobind_bazel//:with_sizeopts": ["NB_COMPACT_ASSERTIONS"], - "@nanobind_bazel//:without_sizeopts": [], + "@nanobind_bazel//:releaseBuild": ["NB_COMPACT_ASSERTIONS"], + "//conditions:default": [], }) # Define the Python version hex if stable ABI builds are requested. diff --git a/nanobind.BUILD b/nanobind.BUILD index f74cc2b..6a7cff8 100644 --- a/nanobind.BUILD +++ b/nanobind.BUILD @@ -2,10 +2,16 @@ A cross-platform nanobind Bazel build. Supports size and linker optimizations across all three major operating systems. Size optimizations used: -Os, LTO. -Linker optimizations used: LTO (clang, gcc) / LTCG (MSVC), linker response file (macOS only). +Linker optimizations used: Debug stripping (release mode), linker response file (macOS only). """ -load("@nanobind_bazel//:helpers.bzl", "py_limited_api", "sizedefs", "sizeopts", "stripopts") +load( + "@nanobind_bazel//:helpers.bzl", + "maybe_compact_asserts", + "nb_sizeopts", + "nb_stripopts", + "py_limited_api", +) licenses(["notice"]) @@ -32,7 +38,7 @@ cc_library( "-fno-strict-aliasing", ], "//conditions:default": [], - }) + sizeopts(), + }) + nb_sizeopts(), defines = py_limited_api(), features = ["-pic"], # use a compiler flag instead. includes = ["include"], @@ -44,8 +50,8 @@ cc_library( "-Wl,-dead_strip", ], "//conditions:default": [], - }) + stripopts(), - local_defines = sizedefs(), # sizeopts apply to nanobind only. + }) + nb_stripopts(), + local_defines = maybe_compact_asserts(), textual_hdrs = glob( [ "include/**/*.h",