Build and install shared library for consumers and backends#18560
Build and install shared library for consumers and backends#18560tomeuv wants to merge 4 commits intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18560
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 3 New Failures, 2 Cancelled Jobs, 2 Unrelated FailuresAs of commit 444a4c3 with merge base e6efe18 ( NEW FAILURES - The following jobs have failed:
CANCELLED JOBS - The following jobs were cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @tomeuv! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
Adds packaging-oriented build/install capabilities so external consumers (including out-of-tree backends) can link against ExecuTorch more easily, with a consolidated shared library option and a few toolchain compatibility fixes.
Changes:
- Introduce
EXECUTORCH_BUILD_SHAREDand build/install a consolidatedlibexecutorchshared library plus aexecutorch.pcpkg-config file. - Install additional runtime backend headers for downstream/backend builds.
- Toolchain fixes: disable
flatccWerror and add missing<cstdint>includes in CoreML inmemoryfs headers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/cmake/preset/default.cmake | Adds EXECUTORCH_BUILD_SHARED build option. |
| tools/cmake/executorch.pc.in | New pkg-config template for installed shared library consumers. |
| third-party/CMakeLists.txt | Passes/forces FLATCC_ALLOW_WERROR=OFF and fixes arg expansion for ExternalProject. |
| backends/apple/coreml/runtime/inmemoryfs/memory_buffer.hpp | Adds missing <cstdint> include for fixed-width integer types. |
| backends/apple/coreml/runtime/inmemoryfs/inmemory_filesystem.hpp | Adds missing <cstdint> include for fixed-width integer types. |
| CMakeLists.txt | Sets project version, installs backend headers, and adds consolidated shared library build/install + pkg-config install. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Hi @tomeuv could you please follow up with the comments from copilot? |
Sorry, it took me some time to be able to get back to it. I addressed those and also fixed an issue found by the linter. |
|
Had to install two more shared libraries so Executorch-using applications can run the CPU operations not delegated to my backend. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add missing #include <cstdint> in CoreML inmemoryfs headers for compilers that don't implicitly include it, disable -Werror in the flatcc ExternalProject_Add and add_subdirectory builds, and fix a missing dollar sign in the flatcc ExternalProject_Add invocation. Co-authored-by: Claude <noreply@anthropic.com>
…ORCH_BUILD_SHARED=ON) When EXECUTORCH_BUILD_SHARED=ON is passed, all static libraries are built with PIC and a new executorch_shared target bundles executorch_core plus commonly-used extensions (data_loader, flat_tensor, named_data_map, module, tensor) into a single libexecutorch.so via whole-archive linking. We add a helper to Utils.cmake to avoid duplicating the definitions of other libraries that we will export in later commits. A pkg-config file is also generated and installed for consumer discovery. When EXECUTORCH_BUILD_SHARED is OFF (the default), behavior is unchanged. Co-authored-by: Claude <noreply@anthropic.com>
Create libexecutorch_portable_ops.so under EXECUTORCH_BUILD_SHARED. Allows applications to run operations on the CPU that their backend of choice doesn't support natively. Co-authored-by: Claude <noreply@anthropic.com>
Create libexecutorch_quantized_ops.so under EXECUTORCH_BUILD_SHARED. Allows applications to run (de)quantization on the CPU when their backend of choice doesn't support natively. Co-authored-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if(EXECUTORCH_BUILD_SHARED) | ||
| executorch_add_shared_library( | ||
| executorch_portable_ops portable_ops_lib portable_kernels executorch_shared |
There was a problem hiding this comment.
executorch_add_shared_library(... executorch_shared) is evaluated while executorch_shared has not been created yet (it’s defined later in the top-level CMakeLists). In this situation CMake treats executorch_shared as a plain library name (e.g. -lexecutorch_shared) rather than a target, which will fail because the consolidated library’s output name is executorch.
Create executorch_shared before adding the kernels subdirectories (so it exists when this line runs), or avoid linking to executorch_shared here (e.g., link to the correct target or add the dependency from the top-level after the target exists).
| executorch_portable_ops portable_ops_lib portable_kernels executorch_shared | |
| executorch_portable_ops portable_ops_lib portable_kernels |
| if(EXECUTORCH_BUILD_SHARED) | ||
| executorch_add_shared_library( | ||
| executorch_quantized_ops quantized_ops_lib quantized_kernels | ||
| executorch_shared |
There was a problem hiding this comment.
Same issue as in portable kernels: this links against executorch_shared before that target is created (it’s defined later in the root CMakeLists). CMake will treat it as a raw library name and try to link -lexecutorch_shared, which won’t exist (the consolidated library is output as libexecutorch).
Fix by ensuring executorch_shared is created before processing this subdirectory, or by restructuring so this dependency is added after executorch_shared exists / using the correct target name.
| executorch_shared | |
| $<TARGET_NAME_IF_EXISTS:executorch_shared> |
| set_target_properties( | ||
| executorch_shared | ||
| PROPERTIES OUTPUT_NAME executorch | ||
| EXPORT_NAME executorch |
There was a problem hiding this comment.
Setting EXPORT_NAME executorch on executorch_shared will collide with the already-exported executorch target (installed earlier via install(TARGETS executorch executorch_core EXPORT ExecuTorchTargets ...)). CMake export/install generation requires unique exported target names within an export set; this will break the ExecuTorchTargets export.
To fix: don’t reuse the executorch export name unless you also stop exporting/rename the existing executorch target when EXECUTORCH_BUILD_SHARED is ON. Alternative: export the shared library under a distinct target name (keep OUTPUT_NAME executorch for the file name), and optionally add a separate alias/imported target in the package config for consumer convenience.
| EXPORT_NAME executorch |
| # Consolidated shared library: bundles executorch_core plus commonly used | ||
| # extensions into a single libexecutorch.so. | ||
| if(EXECUTORCH_BUILD_SHARED) | ||
| executorch_add_shared_library(executorch_shared) | ||
| set_target_properties( |
There was a problem hiding this comment.
executorch_shared is created after the portable/quantized kernel subdirectories are added, but those subdirectories attempt to link against executorch_shared when EXECUTORCH_BUILD_SHARED is enabled. Because the target doesn’t exist yet at that point, those links won’t bind to the intended target.
Consider creating executorch_shared earlier (right after executorch / executorch_core are defined) and then adding the extension/kernel WHOLE_ARCHIVE dependencies later, so subdirectories can safely reference the target.
|
@claude review |
|
Claude finished @kirklandsign's task in 2m 12s —— View job Review of PR #18560: Build and install shared library for consumers and backends
Overall this is a useful contribution for distro packaging and out-of-tree backends. The toolchain fixes and Issues1. Target ordering:
CMake will treat Options to fix:
2. Export name collision: At install(
TARGETS executorch executorch_core
EXPORT ExecuTorchTargets
...
)This will cause CMake export-set generation to fail with duplicate target names. The shared library should either:
3.
4. In Headers are typically included as Minor / Nits5. Bug fix in 6. 7. The 8. Missing Looks Good
|
|
Thanks for putting this up. I'm taking a deeper look, but overall the direction looks good. I'm checking some of the specific CMake changes, but I should be able to get back to you within a day or two. |
There was a problem hiding this comment.
@tomeuv Overall, looks sound to me. I played around with a few build configurations locally, and the WHOLE_ARCHIVE logic seems to work properly. Though it is a bit fragile on our side.
The only issue I see is the export naming conflict between the static and shared executorch. Both are exported as "executorch", which causes a failure when trying to use via find_package. Could we either export it as executorch-shared, or alternatively, only export the shared or static version based on EXECUTORCH_BUILD_SHARED?
If we can resolve that, as well as rebase and lint, I think we should be ready to land it. Thanks for the contribution!
Summary
These are the changes that I needed so that both my Mesa backend (tentatively named Torx), and my test suite can link to Executorch. This should be also what Linux distributions such as Debian and Fedora would need to package Executorch.
There are some fixes as well for recent toolchains, whichi will be needed as well by distributions.
I also install the headers for runtime/backend/, which weren't installed before, to make out-of-tree backends to build cleanly.
Test plan
Build with:
Then run:
And check that the headers and shared library (libexecutorch.so.1.3.0) were installed.
cc @kimishpatel @YifanShenSZ @cymbalrush @metascroy