Skip to content

Rename executorch cmake target to prim_ops_lib - #23177

Open
TejasweeSulekh wants to merge 3 commits into
pytorch:mainfrom
TejasweeSulekh:rename-executorch-target
Open

TejasweeSulekh wants to merge 3 commits into
pytorch:mainfrom
TejasweeSulekh:rename-executorch-target

Conversation

@TejasweeSulekh

@TejasweeSulekh TejasweeSulekh commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR renames the confusing executorch CMake target to prim_ops_lib. This change clearly reflects its purpose as the Primitive Operations Library and brings it in line with the naming convention of portable_ops_lib.

Key Changes

  • Renamed the core executorch target definition in CMakeLists.txt to prim_ops_lib, alongside its source file arrays (_prim_ops_lib__srcs and PRIM_OPS_LIB_SRCS).
  • Maintained Backwards Compatibility:
    • Created an INTERFACE CMake target named executorch that proxies to prim_ops_lib. Downstream consumers calling target_link_libraries(myapp executorch) will not break.
    • Used set_target_properties(... OUTPUT_NAME "executorch") to ensure the compiled archive is still named libexecutorch.a. This prevents breakages in the dozens of internal CI shell scripts and iOS Xcode .pbxproj configurations that explicitly look for that file.
  • Updated internal library usages (Backends, Llama, Muse-Glimmer, Devtools, Docs) to reference prim_ops_lib.
  • Fixed a regression where primitive operators were stripped from LLaMA by properly restoring executorch_target_link_options_shared_lib(prim_ops_lib) to its CMake file.

Fixes #11761

Test plan

Configured, built, and executed a model at runtime to guarantee no primitive operator symbols were stripped by the linker.

# Clean and Configure
source .venv/bin/activate
rm -rf cmake-out
cmake -B cmake-out .

# Build to guarantee linkage succeeds
cmake --build cmake-out -j$(nproc)

# Runtime verification (Generated a dummy add.pte model locally to prove aten::sym_size and other prim ops weren't stripped)
./cmake-out/executor_runner --model_path /tmp/add.pte
# Outputs: OutputX 0: tensor(sizes=[1], [2.])

cc @nil-is-all

@pytorch-bot

pytorch-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23177

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 056bfe5 with merge base 78a4734 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 27, 2026
@metascroy

Copy link
Copy Markdown
Contributor

@claude review this code. Does _executorch__srcs (which defined the old executorch library) really only contain primp op sources? If so, should _executorch__srcs be renamed?

@metascroy

Copy link
Copy Markdown
Contributor

cc @JacobSzwejbka for core runtime

@claude

claude Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Claude finished @metascroy's task in 2m 5s —— View job


Review: Rename executorch CMake target to prim_ops_lib

  • Gather diff and context
  • Check what _executorch__srcs contains
  • Review downstream usages / config changes
  • Post review

Answer: does _executorch__srcs contain only prim op sources?

Yes. _executorch__srcs gets filled from EXECUTORCH_SRCS in shim_et/xplat/executorch/build/build_variables.bzl (the mapping is in tools/cmake/Codegen.cmake:564-626). That list has three files:

EXECUTORCH_SRCS = [
    "kernels/prim_ops/et_copy_index.cpp",
    "kernels/prim_ops/et_view.cpp",
    "kernels/prim_ops/register_prim_ops.cpp",
]

Program, Method, the platform code and the rest of the runtime all sit in EXECUTORCH_CORE_SRCS / executorch_core. So the comment above the target (CMakeLists.txt:703-709, "executorch: Primary runtime library… Provides the Program and Method interfaces") is out of date. The target only compiles prim ops and pulls executorch_core in transitively through its PRIVATE link.

Should it be renamed? Yes. The file lists follow a _<target>__srcs convention (_executorch_core__srcs, _portable_kernels__srcs, …). With this PR, _executorch__srcs would be the only one that doesn't match its target's name. I suggest renaming EXECUTORCH_SRCS to PRIM_OPS_LIB_SRCS in build_variables.bzl and _executorch__srcs to _prim_ops_lib__srcs in Codegen.cmake, and rewriting the comment at CMakeLists.txt:703-709. No Buck targets.bzl loads EXECUTORCH_SRCS, so the only other user is Codegen.cmake.

Fix this →


Blocking issues

1. The rename is incomplete, and configure fails in several builds. No alias is kept, so every remaining executorch link item turns into a plain -lexecutorch that no longer resolves. With CMake's usual behaviour, a missing target like this shows up at link time, not configure time. That is why cmake -B cmake-out . in the test plan passed. References that still use the old name include:

  • examples/portable/custom_ops/CMakeLists.txt:113: gen_operators_lib(... DEPS executorch). This file is in the PR, but only line 109 was changed.
  • examples/models/llama/CMakeLists.txt:91: set(link_libraries executorch gflags). Also in the PR.
  • examples/models/muse-glimmer/CMakeLists.txt:36: set(link_libraries executorch gflags). Also in the PR.
  • test/CMakeLists.txt:56 (size_test), kernels/prim_ops/test/CMakeLists.txt:23, extension/llm/runner/test/CMakeLists.txt:70, extension/training/CMakeLists.txt:60.
  • backends/arm/cmake/ArmRunnerUtilsInternal.cmake:559,561, examples/qualcomm/CMakeLists.txt:70, backends/mlx/examples/llm/CMakeLists.txt:38, examples/llm_server/cpp/CMakeLists.txt:37.
  • Almost every examples/models/*/CMakeLists.txt (whisper, gemma3/4, voxtral*, parakeet, llava, dinov2, sortformer, silero_vad, qwen3_5_moe, …).
  • Multi-line target_link_libraries blocks in zephyr/, extension/android/CMakeLists.txt, examples/arm/*, examples/mediatek, examples/devtools, examples/nxp/..., backends/cadence/CMakeLists.txt, backends/cadence/hifi/operators/tests.

2. The installed archive changes from libexecutorch.a to libprim_ops_lib.a, which breaks scripts that ask for it by file name. For example, scripts/build_apple_frameworks.sh:28 lists libexecutorch.a when it assembles the iOS frameworks. It also turns up in test/build_size_test.sh, .ci/scripts/utils.sh, .ci/scripts/test_llama.sh, examples/apple/coreml/scripts/build_executor_runner.sh, backends/apple/coreml/scripts/build_tests.sh, the CoreML .xcodeproj files, and the docs (using-executorch-ios.md, using-executorch-building-from-source.md, backends-qualcomm.md, .claude/skills/building/SKILL.md).

3. The PR description doesn't match the diff, and the llama change is a regression. The description says executorch_target_link_options_shared_lib(prim_ops_lib) was added to tools/cmake/executorch-config.cmake, but that file's only change is swapping the name in required_lib_list (line 61). Even so, examples/models/llama/CMakeLists.txt drops its executorch_target_link_options_shared_lib(executorch) call. The llama runner would then link prim ops without whole-archive. register_prim_ops.cpp registers through a static initializer, so the linker can drop it, and prim ops such as aten::sym_size fail at runtime with "operator not found". Either add the call to the config file as described, or keep it in llama and rename the target there.

4. Downstream users break. executorch is a documented, installed target that external projects link against directly. Renaming it without a transition path breaks all of them. I'd recommend keeping a compatibility target for at least one release:

add_library(executorch INTERFACE)
target_link_libraries(executorch INTERFACE prim_ops_lib)

Both targets would also need to be in the install(TARGETS …) export set, or the equivalent ALIAS/imported target set up in executorch-config.cmake. The PR should also get a release notes: label that points out the rename.

Minor

  • CMakeLists.txt:704: the comment header still reads # executorch:.
  • docs/source/kernel-library-custom-aten-kernel.md:172: the inline comment still says "executorch is a dependency of it".
  • tools/cmake/executorch-config.cmake:41-48: the comment still talks about libexecutorch.a.

Suggested test plan

The configure-only run in the PR can't catch these problems. At minimum, run a full build plus a model run through executor_runner and llama_main, for example cmake --preset llm && cmake --build cmake-out -j && ./cmake-out/executor_runner --model_path add.pte. That exercises prim-op registration. Also run scripts/build_apple_frameworks.sh if you have access to macOS.

@executorch-triage executorch-triage Bot added the community: contribution PRs coming from community (excluding hardware partners) label Sep 28, 2026
@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, and my bad I completely missed the downstream linking impacts and hardcoded script names.

I'll push an update tomorrow to fix the broken references and add the INTERFACE compatibility target so we don't break existing users.

@nil-is-all nil-is-all added the module: cleanup Issues/PRs which cleanup code across the repository label Sep 28, 2026
@TejasweeSulekh
TejasweeSulekh marked this pull request as draft September 29, 2026 18:10
@TejasweeSulekh
TejasweeSulekh force-pushed the rename-executorch-target branch from 90acc60 to 0059406 Compare September 29, 2026 20:03
@TejasweeSulekh
TejasweeSulekh marked this pull request as ready for review September 29, 2026 20:05
@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "release notes: build"

@pytorch-bot pytorch-bot Bot added the release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. label Sep 29, 2026
@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

Hey @nil-is-all, I've pushed a new commit which is addressing the feedback provided above. Which are as follows:

  • Incomplete Variable Renames: I've updated Codegen.cmake and build_variables.bzl so that _prim_ops_lib__srcs and PRIM_OPS_LIB_SRCS now perfectly match the new target name
  • Backwards Compatibility & Downstream Breakages: I added an INTERFACE CMake alias for executorch that proxies directly to prim_ops_lib. Downstream integrations won't break
  • Broken CI Scripts & libexecutorch.a: I used set_target_properties(prim_ops_lib PROPERTIES OUTPUT_NAME "executorch"). This keeps the compiled archive named libexecutorch.a, guaranteeing that the iOS Xcode projects and internal CI shell scripts remain completely untouched and functional
  • LLaMA Regression: This was a mishap on my end for missing this one. I've correctly restored executorch_target_link_options_shared_lib(prim_ops_lib) in the LLaMA CMake config.
  • Documentation Update: Cleaned up the outdated comment in docs/source/kernel-library-custom-aten-kernel.md and the header in CMakeLists.txt

Just to confirm this is working correctly, I ended up executing a dummy .pte model through executor_runner locally to guarantee the runtime ops aren't being stripped.

I also took the liberty to update the main description of the PR to be more aligned to the final changes.

@TejasweeSulekh
TejasweeSulekh force-pushed the rename-executorch-target branch from 0059406 to d1f2e41 Compare September 30, 2026 03:51
@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

I added a new commit because the CI failed on the MLX, VULKAN, and Android configuration due to an edge case with CMake generator expressions, which I have now fixed across the repository.

Basically, by making executorch an INTERFACE (virtual) library to preserve backwards compatibility, standard linking via target_link_libraries(app executorch) worked correctly.

However, several custom commands in the examples (like MLX and Android) were using the low-level $<TARGET_FILE:executorch> generator expression to extract the absolute filepath of the binary for specific linker flags. Because INTERFACE targets do not have a TARGET_FILE property, CMake threw a fatal error during the Generation step.

To fix this, I did a sweep across all .cmake and CMakeLists.txt files and replaced every instance of $<TARGET_FILE:executorch> (and its associated wrapper macros) with $<TARGET_FILE:prim_ops_lib>.

Because prim_ops_lib is a real STATIC library target, the generator expression cleanly resolves to the libexecutorch.a output path on the hard drive again.

@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

Also @nil-is-all, is it possible for me to run the CI tests in a draft PR? Such that I could confirm the changes are valid and then move the PR to ready for review? As I would feel more comfortable running the CI tests workflow in that setting.

@nil-is-all

Copy link
Copy Markdown
Contributor

Also @nil-is-all, is it possible for me to run the CI tests in a draft PR? Such that I could confirm the changes are valid and then move the PR to ready for review? As I would feel more comfortable running the CI tests workflow in that setting.

Few workflows based on GitHub automations are triggered only when a PR is non-draft; but given that we can review once you flag a PR ready for review. Most contributors address CI test logs on non-draft PRs and ping us once they are satisfied with the PR to be reviewed.

@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

Also @nil-is-all, is it possible for me to run the CI tests in a draft PR? Such that I could confirm the changes are valid and then move the PR to ready for review? As I would feel more comfortable running the CI tests workflow in that setting.

Few workflows based on GitHub automations are triggered only when a PR is non-draft; but given that we can review once you flag a PR ready for review. Most contributors address CI test logs on non-draft PRs and ping us once they are satisfied with the PR to be reviewed.

Thanks for the clarification :D

@TejasweeSulekh
TejasweeSulekh force-pushed the rename-executorch-target branch from d1f2e41 to 056bfe5 Compare October 15, 2026 08:24
@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

Hi @nil-is-all,

I've rebased the branch onto the latest main:

  • Resolved Upstream CI Failure: The lone CI failure previously seen on test-arm-backend-no-driver was a known trunk breakage (relating to the NFRU/NSS checkpoint cache on read-only /mnt/hf_cache and TOSA calibration), which Dr. CI had flagged under BROKEN TRUNK. With the rebase, this is now resolved by the upstream fix.
  • Clean Merge & MSVC Update: Resolved the minor conflict in CMakeLists.txt around executorch_shared, ensuring prim_ops_lib is properly linked and bundled across both MSVC and non-MSVC builds.
  • Verification: Ran a clean CMake configuration

Whenever you get the time can you please run the CI test workflow on this PR?

@TejasweeSulekh

Copy link
Copy Markdown
Contributor Author

Hey @JacobSzwejbka and @nil-is-all, I think the CI tests ended up passing. Let me know if there is something more that might be required in this PR

This branch was successfully deployed

1 active deployment
cadence — 056bfe54 Deployed Oct 15, 2026 by TejasweeSulekh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. community: contribution PRs coming from community (excluding hardware partners) module: cleanup Issues/PRs which cleanup code across the repository release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename executorch cmake target to prim_ops_lib

4 participants