Skip to content

Commit 54f9f3e

Browse files
kirklandsignfacebook-github-bot
authored andcommitted
Set kernel default visibility to hidden (#3060)
Summary: Pull Request resolved: #3060 When we compile the kernel into a shared library, we don't know whether the definition of kernel implementation symbol can be dropped or not based on op registry. The kernel itself is just a normal function and the user can find it. We set its visibility to hidden by default. Then these kernels are gone when we do `objdump -TC` This reduces binary size. --- This is not done in fbcode so far. When we compile in fbcode, seems that all dependency libraries is compiled into shared library, not static library. For example, op tests depends on op implementation through shared library. In that case, the hidden symbols are not exposed and could cause link time failure. In xplat, these dependencies are set to static libraries so it has no impact. Only when we explicitly build a shared library (for android), we hide the symbols and rely on op registry to store the impl. --- This applies to internal build only for now. We will re-visit this for OSS later. It's a step needed to make use of selective build for building shared library (android use case mainly) Reviewed By: dbort Differential Revision: D56167833 fbshipit-source-id: 98cd47836b616fc33dbc9af284d9e758b242b3a3
1 parent 4b6d2c3 commit 54f9f3e

File tree

2 files changed

+13
-3
lines changed

2 files changed

+13
-3
lines changed

shim/xplat/executorch/build/env_interface.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ env = struct(
216216
# @lint-ignore BUCKLINT: native and fb_native are explicitly forbidden in fbcode.
217217
genrule = native.genrule,
218218
is_oss = True,
219-
is_xplat = False,
219+
is_xplat = lambda: False,
220220
patch_deps = _patch_deps,
221221
patch_cxx_compiler_flags = _patch_cxx_compiler_flags,
222222
patch_executorch_genrule_cmd = _patch_executorch_genrule_cmd,

shim/xplat/executorch/kernels/portable/op_registration_util.bzl

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("@fbsource//xplat/executorch/build:runtime_wrapper.bzl", "runtime")
1+
load("@fbsource//xplat/executorch/build:runtime_wrapper.bzl", "is_xplat", "runtime")
22
load("@fbsource//xplat/executorch/build:selects.bzl", "selects")
33

44
def op_target(name, deps = [], android_deps = [], _allow_third_party_deps = False, _aten_mode_deps = []):
@@ -122,7 +122,17 @@ def define_op_library(name, deps, android_deps, aten_target, _allow_third_party_
122122
fbandroid_platform_deps = android_deps,
123123
# kernels often have helpers with no prototypes just disabling the warning here as the headers
124124
# are codegend and linked in later
125-
compiler_flags = ["-Wno-missing-prototypes"],
125+
compiler_flags = ["-Wno-missing-prototypes"] + (
126+
# For shared library build, we don't want to expose symbols of
127+
# kernel implementation (ex torch::executor::native::tanh_out)
128+
# to library users. They should use kernels through registry only.
129+
# With visibility=hidden, linker won't expose kernel impl symbols
130+
# so it can prune unregistered kernels.
131+
# Currently fbcode linkes all dependent libraries through shared
132+
# library, and it blocks users like unit tests to use kernel
133+
# implementation directly. So we enable this for xplat only.
134+
["-fvisibility=hidden"] if is_xplat() else []
135+
),
126136
deps = [
127137
"//executorch/runtime/kernel:kernel_includes" + aten_suffix,
128138
] + deps,

0 commit comments

Comments
 (0)