-
Notifications
You must be signed in to change notification settings - Fork 688
Arm Zephyr cmake Preset #11923
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
Arm Zephyr cmake Preset #11923
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11923
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 Cancelled Job, 3 Unrelated FailuresAs of commit f404ad8 with merge base de24e18 ( CANCELLED JOB - The following job was cancelled. Please retry:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs failed but was 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. |
@pytorchbot label "release notes: none" |
5736206
to
f14844f
Compare
@@ -0,0 +1,111 @@ | |||
# |
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.
examples/arm/ethos-u-setup/arm-none-eabi-gcc.cmake
can we overload this file, and introduce perhaps PREFIX
or TRIPLET
i.e. arm-none-eabi-
vs. arm-zephyr-eabi-
?
See $ANDROID_NDK/build/cmake/android.toolchain.cmake
as an example.
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.
Hm, I'm been trying to make changes to support this, but in doing so, the changes are becoming quite a bit more invasive. Every arm script (e.g. examples/arm/setup.sh
, examples/arm/run.sh
, backends/arm/scripts/build_executorch.sh
...) would need to take additional flags to use the arm-none-eabi-gcc.cmake
but overloaded with a flag to specify that we shouldn't use the bare metal toolchain, but use the zephyr one instead. It is a bit of file duplication, but changes to the existing scripts are much more minimal if we retain arm-zephyr-eabi-gcc.cmake
file separately. What do you think? I'm of the preference of keeping the arm-zephyr-eabi-gcc.cmake
file for now, and maybe a later PR could consolidate the two?
@zingo - FYI |
42b1860
to
f404ad8
Compare
### Summary This change adds a cmake preset for Arm Zephyr RTOS toolchain. This requires the toolchain cmake compilation target to be added to `PATH` (see pytorch#12077 for more details). This change includes update to CI flows, modifying the CI `build_size_test` to run the test using either arm bare metal or Zephyr toolchains. Additionally, the CI for building presets is updated to target the new cmake Zephyr preset. ### Test plan Tested with `test/build_size_test.sh` to determine the correct threshold for running the size test for the arm zephyr toolchain. When the CI runs the size test for the arm zephyr toolchain, the new cmake preset is used directly. The following commands are run by the CI, and can be manually invoked to confirm functionality: ``` CXXFLAGS="-fno-exceptions -fno-rtti -Wall -Werror -Wno-int-in-bool-context -DET_HAVE_PREAD=0" cmake --preset zephyr -DCMAKE_BUILD_TYPE=Release -DEXECUTORCH_OPTIMIZE_SIZE=ON -DCMAKE_INSTALL_PREFIX=cmake-out -Bcmake-out . cmake --build cmake-out -j9 --target install --config Release CXXFLAGS="-fno-exceptions -fno-rtti -Wall -Werror -Wno-int-in-bool-context -DET_HAVE_PREAD=0" cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=$(realpath examples/zephyr/arm-x86-64-eabi-gcc.cmake) -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=cmake-out -Bcmake-out/test test cmake --build cmake-out/test -j9 --config Release arm-zephyr-eabi-strip cmake-out/test/size_test ls -la cmake-out/test/size_test ``` Output should corroborate CI test threshold, reporting a resultant `size_test` binary size of ~125Kb (threshold set to 130KB toin CI test tolerate variability). --------- Co-authored-by: ZephyrUser <[email protected]> Co-authored-by: Github Executorch <[email protected]>
Summary
This change adds a cmake preset for Arm Zephyr RTOS toolchain. This requires the toolchain cmake compilation target to be added to
PATH
(see #12077 for more details). This change includes update to CI flows, modifying the CIbuild_size_test
to run the test using either arm bare metal or Zephyr toolchains. Additionally, the CI for building presets is updated to target the new cmake Zephyr preset.Test plan
Tested with
test/build_size_test.sh
to determine the correct threshold for running the size test for the arm zephyr toolchain. When the CI runs the size test for the arm zephyr toolchain, the new cmake preset is used directly. The following commands are run by the CI, and can be manually invoked to confirm functionality:Output should corroborate CI test threshold, reporting a resultant
size_test
binary size of ~125Kb (threshold set to 130KB toin CI test tolerate variability).