-
Notifications
You must be signed in to change notification settings - Fork 799
Fix Cmakelists for llama main #16581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Cmakelists for llama main #16581
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16581
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 7f300c5 with merge base a9d8230 ( UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the symbol stripping behavior in the llama_main executable build configuration. Previously, symbols were stripped for all non-Debug builds, which incorrectly included RelWithDebInfo. The fix ensures symbols are only stripped for Release and MinSizeRel builds, preserving debug information in RelWithDebInfo builds.
Changes:
- Modified the conditional check for symbol stripping from a negative check (NOT Debug) to a positive check (Release OR MinSizeRel)
- Added a clarifying comment explaining the stripping behavior
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
examples/models/llama/CMakeLists.txt
Outdated
| if(NOT CMAKE_BUILD_TYPE STREQUAL "Debug") | ||
|
|
||
| # Only strip symbols for Release and MinSizeRel builds. | ||
| if(CMAKE_BUILD_TYPE STREQUAL "Release" OR CMAKE_BUILD_TYPE STREQUAL "MinSizeRel") |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same symbol stripping issue exists in other model CMakeLists files (gemma3, llava, whisper, parakeet, voxtral) which still use if(NOT CMAKE_BUILD_TYPE STREQUAL \"Debug\"). For consistency and to prevent the same RelWithDebInfo stripping issue across the codebase, consider applying this fix to those files as well.
GregoryComer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Can you resolve the lint issue? I can go ahead and merge once that's updated. Instructions for installing and running the linter are here: https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#lintrunner. Alternatively, you can just run cmake-format -i examples/models/llama/CMakeLists.txt, if that's easier.
c3b0025 to
7f300c5
Compare
Done. I’ve run cmake-format and pushed an updated commit. |
|
@pytorchbot label "release notes: none" |
|
Looks good. I'll go ahead and merge. Thanks for the contribution! |
Summary
Do not strip in RelWithDebInfo builds for llama main.
Fixes #16572