add sparse build support
We should try to make this wrapper script smaller, not bigger; moving features to Zephyr's CMake and the rest of the Zephyr build.
This was confirmed in a meeting this morning: a lot of the overlays logic in this script should be moved ASAP to Zephyr "applications" (whatever that means, I believe @aborisovich understands this well). Wrapper scripts add layers of obfuscations and the less often people have to use this script and the more often they can invoke west directly, the better. west is already a wrapper! On top of git and CMake, which is itself on top of ninja...
EDIT1: it's started happening now:
- https://github.com/thesofproject/sof/pull/6004
EDIT2: adding link to:
- https://github.com/zephyrproject-rtos/docker-image/pull/109
What is the goal/need of building sparse?
Is this a requirement for all Xtensa architecture platforms?
Is this sparse build should occur always or be a switchable option?
If so this should be done on much lower level in CMake files not as part of python script.
There is already a -C flag in ./scripts/xtensa-build-zephyr.py:

So, support for sparse build should be done by just calling a command:
./sof/scripts/xtensa-build-zephyr.py <platform> -C '-DBUILD_SPARSE=ON'
Then, in ./sof/src/arch/xtensa/CMakeLists.txt you should use:
if(BUILD_SPARSE):
CMake FetchContent (to clone sparse at configure time)
and then FetchContent_MakeAvailable .
Have a look at how GTest framework does it Googletest quickstart - set up a project .
Alternatively you could try have a go with CMake ExternalProject_Add. It looks easier to implement but less convenient to use because this just creates a new target during Cmake configure and
actual clone, build and all operations are happening at Cmake build time (what can be troublesome if you wish to alter Cmake configure steps of external projects).
I think @keqiaozhang has already been working on a very different alternative
What is the goal/need of building sparse? Is this a requirement for all Xtensa architecture platforms? Is this sparse build should occur always or be a switchable option?
@aborisovich my proposal would be to build it for each PR but we could start with just one configuration, e.g. TGL. Ideally we need an "all-yes-config" for this - to build as much of the source base as possible. Then we should add builds for all other architectures - those, that support Zephyr building. I understand, that we don't get a 100% code base coverage unless we actually build all platforms, so that should be something we have to consider too. And yes, I haven't worked with github CI hooks, but I'd expect it to be a separate GH CI "pipeline" (sorry for a gitlab-ism). And - most importantly - we need to parse an evaluate the resulting log - not just check the return code. But that can also be achieved with simple grep commands. We do already have a "GitHub Actions / gcc-build-only" test, maybe we could extend that with this, or even replace plane gcc building with Zephyr+sparse?
@keqiaozhang @lyakh @aborisovich can we have something simple and very similar to the Linux sparse invocation which everyone understands and uses make (cmake in our case). This would also need to align with Zephyr. https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html#using-sparse
@keqiaozhang @lyakh @aborisovich can we have something simple and very similar to the Linux sparse invocation which everyone understands and uses make (cmake in our case). This would also need to align with Zephyr. https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html#using-sparse
Well I would argue that adding C=1 to make command is intuitive solution... (I'm referring to the link you send). Whatever @keqiaozhang will come up with it will has to be somehow integrated with CMake (unless Sparse is pure Makefile project). I think quite reasonable alternative solution is introducing more west build commands to SOF by adding new west extension commands. Zephyr project gives us "west build" to build a board but we can add out build-sparse or build-tools extension... Just a thought.
I think quite reasonable alternative solution is introducing more west build commands to SOF by adding new west extension commands. Zephyr project gives us "west build" to build a board but we can add out build-sparse or build-tools extension... Just a thought
I'm also good for west support, my ask is that it's a simple command line option.
@aborisovich @marc-hb @keqiaozhang any progress with this? We really want sparse support in CI - we get code added regularly that involves buffer manipulation and having an automated check for its correctness would be very helpful!
@aborisovich @marc-hb @keqiaozhang ping - do we have some alignment yet ? adding @andyross to see how we can all align Zephyr + SOF on SPARSE usage.
I'll submit something tomorrow at the latest
EDIT: first step in
- #6232
@lyakh have you tried -DSPARSE=y with the most recent container and 0.15.0 toolchain?
EDIT: I tried with other platforms and docker image versions and they all fail like this.
In dir: /zep_workspace; running command: west build --build-dir build-tgl --board intel_adsp_cavs25 /zep_workspace/sof/app -p always -- -DSPARSE=y
-- Including generated dts.cmake file: /zep_workspace/build-tgl/zephyr/dts.cmake
Parsing /zep_workspace/zephyr/Kconfig
Loaded configuration '/zep_workspace/zephyr/boards/xtensa/intel_adsp_cavs25/intel_adsp_cavs25_defconfig'
Merged configuration '/zep_workspace/sof/app/prj.conf'
Merged configuration '/zep_workspace/sof/app/boards/intel_adsp_cavs25.conf'
Configuration saved to '/zep_workspace/build-tgl/zephyr/.config'
Kconfig header saved to '/zep_workspace/build-tgl/zephyr/include/generated/autoconf.h'
CMake Error at /zep_workspace/zephyr/cmake/compiler/gcc/target.cmake:17 (message):
C compiler
/opt/toolchains/zephyr-sdk-0.15.0/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc
not found - Please check your toolchain installation
Call Stack (most recent call first):
/zep_workspace/zephyr/cmake/modules/target_toolchain.cmake:63 (include)
/zep_workspace/zephyr/cmake/modules/zephyr_default.cmake:121 (include)
/zep_workspace/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
/zep_workspace/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:91 (include_boilerplate)
CMakeLists.txt:5 (find_package)
@lyakh have you tried
-DSPARSE=ywith the most recent container and 0.15.0 toolchain?
I haven't, but have you also set the env variable? I.e.
export REAL_CC=${HOME}/zephyr-sdk/xtensa/intel_s1000/xtensa-zephyr-elf/bin/xtensa-zephyr-elf-gcc
I actually tried to automate that export in cmake but haven't been able to achieve that. I did add
set(ENV{REAL_CC} ${REAL_CC})
to cmake/compiler/gcc/target.cmake and at least during my testing it was needed too, but it wasn't enough, so both were needed...
I haven't, but have you also set the env variable?
No I haven't. I'm not a sparse expert. Generally speaking CMake does not play well with environment variables (which are of course evil).
Can you please provide somewhere a complete and detailed list of manual steps to compile with sparse? It must be good and complete enough for any reasonably skilled developer to compile with sparse in less than 1h. Trying to automate something that can't be replicated manually is futile and jumping the gun.
Can you please provide somewhere a complete and detailed list of manual steps to compile with sparse?
@marc-hb that's all, that's literally what I do to build with sparse:
export REAL_CC=$HOME/zephyr-sdk/xtensa/intel_s1000/xtensa-zephyr-elf/bin/xtensa-zephyr-elf-gcc
west build -d tgl-sparse -b intel_adsp_cavs25 modules/audio/sof/app -- -DSPARSE=y
if you still cannot get this to work, I guess I'll have to try to reproduce this in Zephyr's docker image myself
@marc-hb also, looking at the error log above - is the path /opt/toolchains/zephyr-sdk-0.15.0/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc actually there?
@marc-hb do I understand it correctly, that all your "sparse" questions are now resolved with #6232 and #6238 ?
export REAL_CC=$HOME/zephyr-sdk/xtensa/intel_s1000/xtensa-zephyr-elf/bin/xtensa-zephyr-elf-gcc
There's no xtensa/intel_s1000/ directory anymore, that was changed months ago. Can you please switch to a recent Zephyr SDK and recent Zephyr main branch?
Don't bother with the container, I can handle the container but we need to all use reasonably recent parts.
is the path /opt/toolchains/zephyr-sdk-0.15.0/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc actually there?
Yes I checked it exists in the container yet CMake fails for some unknown reason. But before we even get into that please upgrade everything to weeks-old versions or even better: days-old versions. Nothing months old.
@marc-hb do I understand it correctly, that all your "sparse" questions are now resolved with #6232 and #6238 ?
Are we good @marc-hb @aborisovich @keqiaozhang now ? Do we need to close this ?
Sorry for hijacking this Pull Request and using it as an issue instead. It's been surprisingly convenient though.
I just had a chat with @lyakh about sparse a couple hours ago. Right now he's busy with other more urgent things so really not in a good place to upgrade his Zephyr SDK. Once he can and we are on the same page then we will resume the troubleshooting together.
In the mean time everyone else is more than welcome to give -DSPARSE=y and REAL_CC a try and report their experience here - with or without the container but only with reasonably (and identified) versions of everything please. See exact commands above.
Sorry, I was busy with other tasks recently, I will try sparse build with latest SOF/Zephyr SDK.
I just tried sparse build with latest sof + zephyr-sdk-0.15.0, build passed without errors.
Here are the steps:
- install the latest
zephyr-sdk-0.15.0 - set the REAL_CC:
export REAL_CC=$INSTALL_PATH/zephyr-sdk-0.15.0/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc - update zephyr project: ./script/xtensa-build-zephyr.py -u
- run sparse build:
west build -d tgl-sparse -b intel_adsp_cavs25 app -- -DSPARSE=y
The output is attached. sparse.txt
I will try to add sparse build support in .github next.
I just tried sparse build with latest
sof+zephyr-sdk-0.15.0, build passed without errors.
@keqiaozhang thanks for checking, I just was going to write the same - updated the toolchain to the newest one, set $REAL_CC correctly, according to the new directory structure and sparse worked. @marc-hb is it because of the docker?
What is current status on this? Anyone can sum up? Are we looking for alternatives?
I will have a look today.
Finally got it working in the container. Not sure what sort of hacks have been implemented to compile with sparse but the error handling in the sparse build is really the worst I've ever seen. I'll add an error message decoder ring in the PR
DONE !!!!
- #6311
DONE !!!!
* [[SKIP CI] Add sparse build to Github Actions #6311](https://github.com/thesofproject/sof/pull/6311)
@marc-hb great, approved, thanks for investigating and fixing it! After that we should add checks for a couple of particularly important for us tests - commented there.
@keqiaozhang close?
@keqiaozhang close?
Yes, closing this PR.