From 9e5dbd7dee9e7d822bf94fd2dc0df231167366aa Mon Sep 17 00:00:00 2001 From: shoumikhin Date: Fri, 31 Jul 2026 13:57:19 -0700 Subject: [PATCH 1/5] Update [ghstack-poisoned] --- .ci/scripts/wheel/test_cpp_sdk.py | 10 ++++++++++ configurations/CMakeLists.txt | 8 ++++++++ setup.py | 17 ++++++++++++++++- tools/cmake/Codegen.cmake | 25 +++++++++++++++++++++++-- 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/.ci/scripts/wheel/test_cpp_sdk.py b/.ci/scripts/wheel/test_cpp_sdk.py index 6de24259f9f..cae8715a343 100644 --- a/.ci/scripts/wheel/test_cpp_sdk.py +++ b/.ci/scripts/wheel/test_cpp_sdk.py @@ -40,6 +40,10 @@ # oversubscribes the CPU because each pool sizes itself to all cores. _THREADPOOL_SYMBOLS = ("executorch::extension::threadpool::get_threadpool",) +# A representative operator from the merged CPU kernels. A second definer means +# the operators are registered twice, which aborts at startup. +_KERNEL_SYMBOLS = ("torch::executor::native::abs_out",) + # `nm -DC` prints " " for a definition and # " U " for an undefined reference. _DEFINED = re.compile(r"^[0-9a-fA-F]+\s+(?P[A-Za-z])\s+(?P.+)$") @@ -132,6 +136,11 @@ def test_single_threadpool() -> None: _assert_single_definer(_THREADPOOL_SYMBOLS, "thread pool") +def test_single_kernel_registration() -> None: + """Exactly one shipped library may define the merged CPU kernels.""" + _assert_single_definer(_KERNEL_SYMBOLS, "set of CPU kernels") + + def test_cpp_consumer(work_dir: Path) -> None: """A standalone C++ app builds and runs against the installed wheel.""" assert shutil.which("cmake") is not None, "cmake is required to build a consumer" @@ -187,4 +196,5 @@ def test_cpp_consumer(work_dir: Path) -> None: def run_tests(work_dir: Path) -> None: test_single_backend_registry() test_single_threadpool() + test_single_kernel_registration() test_cpp_consumer(work_dir) diff --git a/configurations/CMakeLists.txt b/configurations/CMakeLists.txt index fb154ff88bc..d47c6333d43 100644 --- a/configurations/CMakeLists.txt +++ b/configurations/CMakeLists.txt @@ -50,7 +50,15 @@ if(EXECUTORCH_BUILD_KERNELS_OPTIMIZED) else() set(_optimized_native_cpu_ops_lib_portable_kernels_lib portable_kernels) endif() + # Ship this as a shared library in the wheel so the kernels are registered + # once per process instead of once per component that links them. + if(EXECUTORCH_BUILD_SHARED) + set(_merged_cpu_ops_shared SHARED) + else() + set(_merged_cpu_ops_shared "") + endif() gen_operators_lib( + ${_merged_cpu_ops_shared} LIB_NAME "optimized_native_cpu_ops_lib" KERNEL_LIBS diff --git a/setup.py b/setup.py index d0b3d0448ff..79ec60bdf55 100644 --- a/setup.py +++ b/setup.py @@ -1120,7 +1120,7 @@ def run(self): # noqa C901 BuiltFile( src_dir="%CMAKE_CACHE_DIR%/extension/threadpool/", src_name=( - "libexecutorch_threadpool.so." f"{get_runtime_soname_major()}.*" + f"libexecutorch_threadpool.so.{get_runtime_soname_major()}.*" ), dst=( "executorch/lib/libexecutorch_threadpool.so." @@ -1128,6 +1128,21 @@ def run(self): # noqa C901 ), dependent_cmake_flags=["EXECUTORCH_BUILD_SHARED"], ), + # Install the merged CPU kernels beside them, so the operators are + # registered once per process rather than once per component. + BuiltFile( + src_dir="%CMAKE_CACHE_DIR%/configurations/", + src_name=( + "libexecutorch_optimized_native_cpu_ops_lib.so." + f"{get_runtime_soname_major()}.*" + ), + dst=( + "executorch/lib/" + "libexecutorch_optimized_native_cpu_ops_lib.so." + f"{get_runtime_soname_major()}" + ), + dependent_cmake_flags=["EXECUTORCH_BUILD_SHARED"], + ), # Install the prebuilt pybindings extension wrapper for the runtime, # portable kernels, and a selection of backends. This lets users # load and execute .pte files from python. diff --git a/tools/cmake/Codegen.cmake b/tools/cmake/Codegen.cmake index 66d72914e67..50c61a9fd21 100644 --- a/tools/cmake/Codegen.cmake +++ b/tools/cmake/Codegen.cmake @@ -268,9 +268,13 @@ function(gen_custom_ops_aot_lib) endfunction() # Generate a runtime lib for registering operators in Executorch +# +# SHARED opts this library into being a shared object. It is opt-in because most +# callers want the default static library, and only the one shipped in the wheel +# needs to be shared so a process has a single copy of the kernels. function(gen_operators_lib) set(multi_arg_names LIB_NAME KERNEL_LIBS DEPS DTYPE_SELECTIVE_BUILD) - cmake_parse_arguments(GEN "" "" "${multi_arg_names}" ${ARGN}) + cmake_parse_arguments(GEN "SHARED" "" "${multi_arg_names}" ${ARGN}) message(STATUS "Generating operator lib:") message(STATUS " LIB_NAME: ${GEN_LIB_NAME}") @@ -283,7 +287,24 @@ function(gen_operators_lib) set(_opvariant_h ${_out_dir}/selected_op_variants.h) endif() - add_library(${GEN_LIB_NAME}) + if(GEN_SHARED) + add_library(${GEN_LIB_NAME} SHARED) + set_target_properties( + ${GEN_LIB_NAME} + PROPERTIES OUTPUT_NAME executorch_${GEN_LIB_NAME} + VERSION "${PROJECT_VERSION}" + SOVERSION "${PROJECT_VERSION_MAJOR}" + ) + if(NOT APPLE) + # Ships beside the runtime in the wheel's lib/ directory. + set_target_properties( + ${GEN_LIB_NAME} PROPERTIES BUILD_RPATH "$ORIGIN" INSTALL_RPATH + "$ORIGIN" + ) + endif() + else() + add_library(${GEN_LIB_NAME}) + endif() set(_srcs_list ${_out_dir}/RegisterCodegenUnboxedKernelsEverything.cpp ${_out_dir}/Functions.h ${_out_dir}/NativeFunctions.h From a665ad56385b07079907c871823ddf458089f937 Mon Sep 17 00:00:00 2001 From: shoumikhin Date: Sat, 1 Aug 2026 08:37:01 -0700 Subject: [PATCH 2/5] Update [ghstack-poisoned] --- .ci/scripts/wheel/test_cpp_sdk.py | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/.ci/scripts/wheel/test_cpp_sdk.py b/.ci/scripts/wheel/test_cpp_sdk.py index 2719c9d347f..7905873fca9 100644 --- a/.ci/scripts/wheel/test_cpp_sdk.py +++ b/.ci/scripts/wheel/test_cpp_sdk.py @@ -44,6 +44,15 @@ # the operators are registered twice, which aborts at startup. _KERNEL_SYMBOLS = ("torch::executor::native::abs_out",) +# The registry entry points, kept separate from the kernel implementations above. +# A library that carries its own copy of these has its own registration code, which +# is what this split is meant to prevent: one owner of the operator table. Checking +# only a kernel implementation would miss that entirely. +_KERNEL_REGISTRY_SYMBOLS = ( + "executorch::runtime::register_kernels", + "executorch::runtime::get_registered_kernels", +) + # `nm -DC` prints " " for a definition and # " U " for an undefined reference. _DEFINED = re.compile(r"^[0-9a-fA-F]+\s+(?P[A-Za-z])\s+(?P.+)$") @@ -114,6 +123,26 @@ def _defines_symbol(library: Path, symbol: str) -> bool: return False +def _report_definers(symbols, what: str) -> None: + """Print which shipped libraries define each symbol, without asserting. + + Used where the desired invariant is one owner but the wheel does not reach it + yet for reasons outside this change. Printing keeps the number visible in CI so + a regression is noticeable, without failing on a pre-existing condition. + """ + package_dir = _installed_package_dir() + libraries = _shipped_shared_objects(package_dir) + for symbol in symbols: + definers = [ + str(library.relative_to(package_dir)) + for library in libraries + if _defines_symbol(library, symbol) + ] + print(f"- {what}: {symbol.split('::')[-1]} defined by {len(definers)}") + for definer in definers: + print(f" {definer}") + + def _assert_single_definer(symbols, what: str) -> None: """Exactly one shipped library may define each of `symbols`.""" assert shutil.which("nm") is not None, "nm is required to inspect the wheel" @@ -146,6 +175,12 @@ def test_single_threadpool() -> None: def test_single_kernel_registration() -> None: """Exactly one shipped library may define the merged CPU kernels.""" _assert_single_definer(_KERNEL_SYMBOLS, "set of CPU kernels") + # Ownership of the operator table, not just of a kernel implementation. This is + # reported rather than asserted because two extension modules still link the + # static core and carry their own copy, which predates this split: a released + # wheel has five definers where this one has three. Asserting here would fail + # on those pre-existing copies rather than on anything this change introduced. + _report_definers(_KERNEL_REGISTRY_SYMBOLS, "operator registry") def test_cpp_consumer(work_dir: Path) -> None: From 561435733e1be220978db098336ab058e1d161de Mon Sep 17 00:00:00 2001 From: shoumikhin Date: Sat, 1 Aug 2026 14:29:37 -0700 Subject: [PATCH 3/5] Update [ghstack-poisoned] --- .ci/scripts/wheel/test_cpp_sdk.py | 16 +++++++++++----- tools/cmake/Codegen.cmake | 7 +++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/.ci/scripts/wheel/test_cpp_sdk.py b/.ci/scripts/wheel/test_cpp_sdk.py index 7905873fca9..14fe7b7bf02 100644 --- a/.ci/scripts/wheel/test_cpp_sdk.py +++ b/.ci/scripts/wheel/test_cpp_sdk.py @@ -123,12 +123,13 @@ def _defines_symbol(library: Path, symbol: str) -> bool: return False -def _report_definers(symbols, what: str) -> None: - """Print which shipped libraries define each symbol, without asserting. +def _report_definers(symbols, what: str, limit: int) -> None: + """Print which shipped libraries define each symbol and hold a ceiling. Used where the desired invariant is one owner but the wheel does not reach it - yet for reasons outside this change. Printing keeps the number visible in CI so - a regression is noticeable, without failing on a pre-existing condition. + yet for reasons outside this change. Printing alone would let a regression + from a few definers to many stay green, so the count is also capped at the + number that exists today. The cap is meant to be lowered, never raised. """ package_dir = _installed_package_dir() libraries = _shipped_shared_objects(package_dir) @@ -141,6 +142,11 @@ def _report_definers(symbols, what: str) -> None: print(f"- {what}: {symbol.split('::')[-1]} defined by {len(definers)}") for definer in definers: print(f" {definer}") + assert len(definers) <= limit, ( + f"{what}: {symbol} is defined by {len(definers)} shipped libraries, " + f"more than the {limit} that existed when this check was added, so a " + f"new duplicate crept in: {definers}" + ) def _assert_single_definer(symbols, what: str) -> None: @@ -180,7 +186,7 @@ def test_single_kernel_registration() -> None: # static core and carry their own copy, which predates this split: a released # wheel has five definers where this one has three. Asserting here would fail # on those pre-existing copies rather than on anything this change introduced. - _report_definers(_KERNEL_REGISTRY_SYMBOLS, "operator registry") + _report_definers(_KERNEL_REGISTRY_SYMBOLS, "operator registry", limit=2) def test_cpp_consumer(work_dir: Path) -> None: diff --git a/tools/cmake/Codegen.cmake b/tools/cmake/Codegen.cmake index 50c61a9fd21..6e39adc66c5 100644 --- a/tools/cmake/Codegen.cmake +++ b/tools/cmake/Codegen.cmake @@ -314,6 +314,13 @@ function(gen_operators_lib) endif() target_sources(${GEN_LIB_NAME} PRIVATE ${_srcs_list}) target_link_libraries(${GEN_LIB_NAME} PRIVATE ${GEN_DEPS}) + if(GEN_SHARED) + # Resolve the runtime from the shared library rather than from the static + # core in GEN_DEPS. Linking the static core gives this library its own copy + # of the operator table, so its static initializer registers into a table + # nothing else reads and the operators appear missing at run time. + executorch_target_link_shared_runtime(${GEN_LIB_NAME}) + endif() set(portable_kernels_check "portable_kernels") if(GEN_KERNEL_LIBS) From e7efa2411d0120c37a975e2d8ace606fbeb4e904 Mon Sep 17 00:00:00 2001 From: shoumikhin Date: Sat, 1 Aug 2026 16:48:39 -0700 Subject: [PATCH 4/5] Update [ghstack-poisoned] --- .ci/scripts/wheel/test_cpp_sdk.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/.ci/scripts/wheel/test_cpp_sdk.py b/.ci/scripts/wheel/test_cpp_sdk.py index 14fe7b7bf02..772cfa17e1f 100644 --- a/.ci/scripts/wheel/test_cpp_sdk.py +++ b/.ci/scripts/wheel/test_cpp_sdk.py @@ -123,32 +123,6 @@ def _defines_symbol(library: Path, symbol: str) -> bool: return False -def _report_definers(symbols, what: str, limit: int) -> None: - """Print which shipped libraries define each symbol and hold a ceiling. - - Used where the desired invariant is one owner but the wheel does not reach it - yet for reasons outside this change. Printing alone would let a regression - from a few definers to many stay green, so the count is also capped at the - number that exists today. The cap is meant to be lowered, never raised. - """ - package_dir = _installed_package_dir() - libraries = _shipped_shared_objects(package_dir) - for symbol in symbols: - definers = [ - str(library.relative_to(package_dir)) - for library in libraries - if _defines_symbol(library, symbol) - ] - print(f"- {what}: {symbol.split('::')[-1]} defined by {len(definers)}") - for definer in definers: - print(f" {definer}") - assert len(definers) <= limit, ( - f"{what}: {symbol} is defined by {len(definers)} shipped libraries, " - f"more than the {limit} that existed when this check was added, so a " - f"new duplicate crept in: {definers}" - ) - - def _assert_single_definer(symbols, what: str) -> None: """Exactly one shipped library may define each of `symbols`.""" assert shutil.which("nm") is not None, "nm is required to inspect the wheel" @@ -186,7 +160,7 @@ def test_single_kernel_registration() -> None: # static core and carry their own copy, which predates this split: a released # wheel has five definers where this one has three. Asserting here would fail # on those pre-existing copies rather than on anything this change introduced. - _report_definers(_KERNEL_REGISTRY_SYMBOLS, "operator registry", limit=2) + _assert_single_definer(_KERNEL_REGISTRY_SYMBOLS, "operator registry") def test_cpp_consumer(work_dir: Path) -> None: From 317e37616ea64233038441d3941657688c318a44 Mon Sep 17 00:00:00 2001 From: shoumikhin Date: Tue, 4 Aug 2026 19:35:23 -0700 Subject: [PATCH 5/5] Update [ghstack-poisoned] --- tools/cmake/executorch-wheel-config.cmake | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tools/cmake/executorch-wheel-config.cmake b/tools/cmake/executorch-wheel-config.cmake index 0a701292640..3e10a1ece8a 100644 --- a/tools/cmake/executorch-wheel-config.cmake +++ b/tools/cmake/executorch-wheel-config.cmake @@ -308,6 +308,22 @@ endfunction() executorch_define_component(threadpool executorch_threadpool) +# The merged CPU kernels. Documented as a component and asserted by the release checks, so it has +# to be defined here or a consumer following the documentation gets a bare name that CMake hands +# to the linker as a literal flag. +executorch_define_component(kernels executorch_optimized_native_cpu_ops_lib) + +# A consumer that links the thread pool has to see the same switch a source build sets, or the +# parallel helpers in the runtime headers compile their serial fallback instead and the library +# they linked is never used. +if(TARGET executorch::threadpool) + set_property( + TARGET executorch::threadpool + APPEND + PROPERTY INTERFACE_COMPILE_DEFINITIONS ET_USE_THREADPOOL + ) +endif() + # Find prebuilt _portable_lib..so. This is the legacy contract used # to build custom-op extensions against the Python module, and is kept working