Skip to content

Refactor CMake build for examples/models/llama#18825

Open
GregoryComer wants to merge 1 commit intopytorch:mainfrom
GregoryComer:cmake-refactor-llama
Open

Refactor CMake build for examples/models/llama#18825
GregoryComer wants to merge 1 commit intopytorch:mainfrom
GregoryComer:cmake-refactor-llama

Conversation

@GregoryComer
Copy link
Copy Markdown
Member

@GregoryComer GregoryComer commented Apr 10, 2026

Summary

Update the CMake build for examples/models/llama to be in-tree in the top-level build but excluded from all. This means that it can be built in a single step, but will only be built when explicitly requested with --target llama_main.

This simplifies the build quite a bit (as evidenced by how many lines we remove in this PR) and prevents issues with iterative rebuild or mismatched build options between llama_main and the core runtime.

Note that the top-level make llama-cpu command is unmodified, just simplified under the hood. For direct CMake builds, here's a before and after:

Before:

cmake --preset llm -DCMAKE_INSTALL_PREFIX=cmake-out
cmake --build cmake-out --target install
cd examples/models/llama
cmake -B ../../../cmake-out/examples/models/llama -DCMAKE_FIND_ROOT_PATH=../../../cmake-out
cmake --build ../../../cmake-out/examples/models/llama --target llama_main

After:

cmake -B cmake-out --preset llm
cmake --build cmake-out --target llama_main

Note to reviewers: the MLX failures are unrelated. They are due to running extended CI on a forked PR.

cc @larryliu0820

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Apr 10, 2026

🔗 Helpful Links

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

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

❌ 15 New Failures, 2 Cancelled Jobs, 3 Unrelated Failures

As of commit 5bb2f3e with merge base 38fdfae (image):

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.

@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 Apr 10, 2026
@GregoryComer GregoryComer added ciflow/trunk and removed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Apr 10, 2026
@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 Apr 10, 2026
@GregoryComer GregoryComer force-pushed the cmake-refactor-llama branch 6 times, most recently from 3f9c81d to 965a4a9 Compare April 11, 2026 00:10
@GregoryComer GregoryComer force-pushed the cmake-refactor-llama branch from 965a4a9 to 5bb2f3e Compare April 17, 2026 20:56
@GregoryComer
Copy link
Copy Markdown
Member Author

CI should be passing now. The MLX failures are failing due to being on a forked PR. The android emulator and unittest failures are flakes.

@GregoryComer GregoryComer marked this pull request as ready for review April 20, 2026 19:48
@GregoryComer GregoryComer added module: build/install Issues related to the cmake and buck2 builds, and to installing ExecuTorch release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. labels Apr 20, 2026
@GregoryComer GregoryComer removed the request for review from cbilgin April 20, 2026 19:49

if(EXECUTORCH_BUILD_KERNELS_LLM)
list(APPEND link_libraries $<LINK_LIBRARY:WHOLE_ARCHIVE,custom_ops>)
list(APPEND link_libraries custom_ops)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these not needed anymore? Do we rely on whole archive flag added in executorch-config.cmake?

Copy link
Copy Markdown
Contributor

@larryliu0820 larryliu0820 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change itself looks ok. I don't think this approach is scalable though, like we should not make all the example cmake target visible to the top level cmake. I tend to make executorch easy to use such that, when a real application needs to depend on it, they just do add_subdirectory(executorch) and can link to all the executorch libraries and when they do that we should not expose example cmake targets to them. I guess since you exclude llama_main from being built by default it's an ok change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: build/install Issues related to the cmake and buck2 builds, and to installing ExecuTorch 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.

2 participants