Skip to content

[FEATURE] Add sparse build to CI #6317

Closed
#6510
Closed
[FEATURE] Add sparse build to CI#6317
#6510
Assignees
marc-hb
Labels
P1Blocker bugs or important features

Description

@lyakh
lyakh
opened on Jun 15, 2022

Is your feature request related to a problem? Please describe.
We added sparse support to Zephyr and we are in the process of adding it to SOF, but without running automated sparse builds on PRs we are guaranteed to regularly break sparse-guarded features like cached vs. uncached memory disambiguation.

Describe the solution you'd like
A Zephyr+sparse build has to be executed for each PR along the lines of

west build -d tgl-sparse -b intel_adsp_cavs25 zephyr/samples/subsys/audio/sof -- -DSPARSE=y

Output has to be verified for regressions. It must be checked, that no sparse errors are added. Additionally we'll have to gradually verify and fix all currently reported sparse warnings.

Additional context
Note, that Xtensa support has only recently been added to upstream sparse, so we need to use the git version from https://git.kernel.org/pub/scm/devel/sparse/sparse.git/ instead of any distribution-provided packages.

Activity

  1. marc-hb commented on Jun 15, 2022

    @marc-hb
    marc-hb
    on Jun 15, 2022
    Collaborator

    Thanks @lyakh for filing this! Some questions:

    1. Is using sparse from git + the west build command you gave enough to run sparse on Zephyr now? No other dependency, command or magic required?
    2. Is the current sparse run mostly clean for Zephyr? If not, what is the order of magnitude of the number of errors left to fix?
    3. You wrote "... and we are in the process of adding it to SOF" but your west build command already points at zephyr/sample/subsys/audio/sof which includes SOF code. Does "adding it to SOF" require a different command/process?

    Before anything gets automated, it must be easy and well defined to run manually first.

  2. lyakh commented on Jun 20, 2022

    @lyakh
    lyakh
    on Jun 20, 2022
    CollaboratorAuthor

    Thanks @lyakh for filing this! Some questions:

    1. Is using sparse from git + the west build command you gave enough to run sparse on Zephyr now? No other dependency, command or magic required?

    @marc-hb I think so, yes, haven't tried to set up a new testing environment from scratch in a while though

    1. Is the current sparse run mostly clean for Zephyr? If not, what is the order of magnitude of the number of errors left to fix?

    At least when I added sparse to Zephyr I fixed all errors that popped up in an SOF build. And since Zephyr also uses other static code analysers their code looks mostly fine for those, yes.

    1. You wrote "... and we are in the process of adding it to SOF" but your west build command already points at zephyr/sample/subsys/audio/sof which includes SOF code. Does "adding it to SOF" require a different command/process?

    No, no different command or process. You can build SOF with Zephyr with sparse using that command now. By "in the process" I meant that we're adding uses of sparse constructs, namely of sparse address spaces. But even without those you can already build SOF with Zephyr with sparse.

    Before anything gets automated, it must be easy and well defined to run manually first.

  3. greg-intel
    added
    P1Blocker bugs or important features
    on Jun 22, 2022

  4. fredoh9 commented on Jun 22, 2022

    @fredoh9
    fredoh9
    on Jun 22, 2022
    Contributor

    Enhancement and P1 is not usual case. If this is P1, we need a assignee. @greg-intel will you take this?

  5. marc-hb commented on Jun 22, 2022

    @marc-hb
    marc-hb
    on Jun 22, 2022
    Collaborator

    Enhancement and P1 is not usual case

    I'm not a sparse expert but I heard it can spot elusive caching issues that tend to require weeks-long task forces otherwise. At least that's how I understand the sales pitch :-)

  6. greg-intel commented on Jun 22, 2022

    @greg-intel
    greg-intel
    on Jun 22, 2022
    Contributor

    @fredoh9 It was set to P1 per the weekly sync, as an important issue and one planned to be completed for this quarter (I believe, the other two requests are specifically mentioned as P2 Q4).

    I thought people more familiar with this had already volunteered, but thanks for the quick explanation, I think I understand at a high level, but a variety of the smaller details are missing.

  7. marc-hb commented on Jun 22, 2022

    @marc-hb
    marc-hb
    on Jun 22, 2022
    Collaborator

    Is using sparse from git + the west build command you gave enough to run sparse on Zephyr now? No other dependency, command or magic required?

    @marc-hb I think so, yes, haven't tried to set up a new testing environment from scratch in a while though

    There is a golden, "from scratch", reference environment always available: the official zephyr-build container maintained by the Zephyr project and used by multiple CIs including SOF CI. I just tried running west build -d tgl-sparse -b intel_adsp_cavs25 zephyr/samples/subsys/audio/sof -- -DSPARSE=y in it and it failed in an unexpected way. west build -d tgl-sparse -b intel_adsp_cavs25 zephyr/samples/subsys/audio/sof works fine in the exact same environement (after deleting the stale tgl-sparse/ build directory). @lyakh can you please find what's wrong?

    docker pull zephyrprojectrtos/zephyr-build
# in a different window while the above is running
west init zeptest
cd zeptest
git log --oneline
d1d5acd2cd (HEAD -> main, origin/main, origin/HEAD) coding guidelines: comply with MISRA C:2012 Rule 8.2
05ec016157 MAINTAINERS: Add keith-packard as C library collaborator
d326683b56 MAINTAINERS: Add picolibc entry

west update sof hal_extensa
cd ..

    docker run -u 1000 -ti -v "$(pwd)":/workdir docker.io/zephyrprojectrtos/zephyr-build:latest /bin/bash
chown -R 1000 zeptest
cd zeptest

export ZEPHYR_BASE=/workdir/zeptest/zephyr

# just works, obviously because it's routinely tested in CI
west build -d tgl-sparse -b intel_adsp_cavs25 zephyr/samples/subsys/audio/sof

# fails
rm -rf tgl-sparse
west build -d tgl-sparse -b intel_adsp_cavs25 zephyr/samples/subsys/audio/sof -- -DSPARSE=y

-- Generated device_extern.h: /workdir/zeptest/tgl-sparse/zephyr/include/generated/device_extern.h
-- Including generated dts.cmake file: /workdir/zeptest/tgl-sparse/zephyr/dts.cmake
Parsing /workdir/zeptest/zephyr/Kconfig
Loaded configuration '/workdir/zeptest/zephyr/boards/xtensa/intel_adsp_cavs25/intel_adsp_cavs25_defconfig'
Merged configuration '/workdir/zeptest/zephyr/samples/subsys/audio/sof/prj.conf'
Merged configuration '/workdir/zeptest/zephyr/samples/subsys/audio/sof/boards/intel_adsp_cavs25.conf'
Configuration saved to '/workdir/zeptest/tgl-sparse/zephyr/.config'
Kconfig header saved to '/workdir/zeptest/tgl-sparse/zephyr/include/generated/autoconf.h'

warning: DAI_SSP_HAS_POWER_CONTROL (defined at drivers/dai/intel/ssp/Kconfig.ssp:16) was assigned
the value 'y' but got the value 'n'. Check these unsatisfied dependencies: DAI_INTEL_SSP (=n), DAI
(=n). See http://docs.zephyrproject.org/latest/kconfig.html#CONFIG_DAI_SSP_HAS_POWER_CONTROL and/or
look up DAI_SSP_HAS_POWER_CONTROL in the menuconfig/guiconfig interface. The Application Development
Primer, Setting Configuration Values, and Kconfig - Tips and Best Practices sections of the manual
might be helpful too.

CMake Error at /workdir/zeptest/zephyr/cmake/compiler/gcc/target.cmake:17 (message):
  C compiler
  /opt/toolchains/zephyr-sdk-0.14.2/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):
  /workdir/zeptest/zephyr/cmake/modules/target_toolchain.cmake:63 (include)
  /workdir/zeptest/zephyr/cmake/modules/zephyr_default.cmake:121 (include)
  /workdir/zeptest/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
  /workdir/zeptest/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:91 (include_boilerplate)
  CMakeLists.txt:5 (find_package)


-- Configuring incomplete, errors occurred!
FATAL ERROR: command exited with status 1: /usr/local/bin/cmake -DWEST_PYTHON=/usr/bin/python3 -B/workdir/zeptest/tgl-sparse -S/workdir/zeptest/zephyr/samples/subsys/audio/sof -GNinja -DBOARD=intel_adsp_cavs25 -DSPARSE=y

    /opt/toolchains/zephyr-sdk-0.14.2/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc does exist in the container.

  8. lyakh commented on Jun 23, 2022

    @lyakh
    lyakh
    on Jun 23, 2022
    CollaboratorAuthor

    @marc-hb right, the error message comes out misleading. sparse isn't installed. As explained, you have to install it manually in your docker image from git. For an explanation (and eventual fox for the error message) see this snippet
    https://github.com/zephyrproject-rtos/zephyr/blob/ee22ae79a4aaa2efb90580888737257724cb5e7d/cmake/compiler/gcc/target.cmake#L8-L18

  9. marc-hb commented on Jun 23, 2022

    @marc-hb
    marc-hb
    on Jun 23, 2022
    Collaborator

    Wow, this error message is one of the most irrelevant-looking I've ever seen. Could you file a new Zephyr bug about it? The Zephyr project has incredibly good CMake experts; we can ask them for ideas on how to fix this error message.

    Also, can you please add sparse to this list? It's a one-line change.
    https://github.com/zephyrproject-rtos/docker-image/blob/master/Dockerfile

    After a manual apt install sparse in the container the build now fails like this below, it looks like it's not using an appropriate toolchain?

    Before adding sparse to CI it needs to be reasonably easy to use outside CI. Failures in CI needs to be reproducible outside CI, otherwise CI is just wasting energy producing reports that can't be acted upon.

    [1/260] Building C object zephyr/CMakeFiles/offsets.dir/arch/xtensa/core/offsets/offsets.c.obj
FAILED: zephyr/CMakeFiles/offsets.dir/arch/xtensa/core/offsets/offsets.c.obj 
ccache /usr/bin/cgcc -DKERNEL -D_FORTIFY_SOURCE=2 -D__ZEPHYR__=1 -I/workdir/zeptest/zephyr/kernel/include -I/workdir/zeptest/zephyr/arch/xtensa/include -I/workdir/zeptest/zephyr/include/zephyr -I/workdir/zeptest/zephyr/include -Izephyr/include/generated -I/workdir/zeptest/zephyr/soc/xtensa/intel_adsp/cavs_v15 -I/workdir/zeptest/zephyr/soc/xtensa/intel_adsp/common/include -I/workdir/zeptest/modules/hal/xtensa/include -I/workdir/zeptest/modules/hal/xtensa/zephyr/soc/intel_apl_adsp -I/workdir/zeptest/modules/audio/sof/zephyr/include -I/workdir/zeptest/modules/audio/sof/zephyr/../src/platform/intel/cavs/include -I/workdir/zeptest/modules/audio/sof/zephyr/../src/platform/apollolake/include -isystem /workdir/zeptest/zephyr/lib/libc/minimal/include -isystem /opt/toolchains/zephyr-sdk-0.14.1/xtensa-intel_apl_adsp_zephyr-elf/bin/../lib/gcc/xtensa-intel_apl_adsp_zephyr-elf/10.3.0/include -isystem /opt/toolchains/zephyr-sdk-0.14.1/xtensa-intel_apl_adsp_zephyr-elf/bin/../lib/gcc/xtensa-intel_apl_adsp_zephyr-elf/10.3.0/include-fixed -O2 -imacros /workdir/zeptest/tgl-sparse/zephyr/include/generated/autoconf.h -ffreestanding -fno-common -g -gdwarf-4 -fdiagnostics-color=always --sysroot=/opt/toolchains/zephyr-sdk-0.14.1/xtensa-intel_apl_adsp_zephyr-elf/xtensa-intel_apl_adsp_zephyr-elf -D__CHECKER__ -Wno-attributes -imacros /workdir/zeptest/zephyr/include/zephyr/toolchain/zephyr_stdint.h -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wno-main -Wno-pointer-sign -Wpointer-arith -Wexpansion-to-defined -Wno-unused-but-set-variable -Werror=implicit-int -fno-asynchronous-unwind-tables -fno-pie -fno-pic -fstrict-overflow -fno-reorder-functions -fno-defer-pop -fmacro-prefix-map=/workdir/zeptest/zephyr/samples/subsys/audio/sof=CMAKE_SOURCE_DIR -fmacro-prefix-map=/workdir/zeptest/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/workdir/zeptest=WEST_TOPDIR -ffunction-sections -fdata-sections -mlongcalls -std=c99 -nostdinc -MD -MT zephyr/CMakeFiles/offsets.dir/arch/xtensa/core/offsets/offsets.c.obj -MF zephyr/CMakeFiles/offsets.dir/arch/xtensa/core/offsets/offsets.c.obj.d -o zephyr/CMakeFiles/offsets.dir/arch/xtensa/core/offsets/offsets.c.obj -c /workdir/zeptest/zephyr/arch/xtensa/core/offsets/offsets.c
/workdir/zeptest/modules/hal/xtensa/include/xtensa/config/core.h:54:10: error: unable to open 'core-isa.h'

  10. nklayman commented on Jun 23, 2022

    @nklayman
    nklayman
    on Jun 23, 2022
    Contributor

    After a manual apt install sparse in the container the build now fails like this below, it looks like it's not using an appropriate toolchain?

    @marc-hb I think you need to build sparse from source in order for it to support the xtensa architecture, as per @lyakh's earlier comment:

    Note, that Xtensa support has only recently been added to upstream sparse, so we need to use the git version from https://git.kernel.org/pub/scm/devel/sparse/sparse.git/ instead of any distribution-provided packages.

    That would explain why you got an error about using the wrong toolchain.

  11. marc-hb commented on Jun 23, 2022

    @marc-hb
    marc-hb
    on Jun 23, 2022
    Collaborator

    Thanks @nklayman , good catch.

    @lyakh, from the top of your head could you share a quick and barely tested set of commands that would build sparse inside the container?

  12. mengdonglin
    assigned
    keqiaozhang
    on Jun 24, 2022

  13. lyakh commented on Jun 24, 2022

    @lyakh
    lyakh
    on Jun 24, 2022
    CollaboratorAuthor

    Thanks @nklayman , good catch.

    @lyakh, from the top of your head could you share a quick and barely tested set of commands that would build sparse inside the container?

    @marc-hb

    git clone https://git.kernel.org/pub/scm/devel/sparse/sparse.git/
cd sparse
make
make install

  14. keqiaozhang commented on Jun 24, 2022

    @keqiaozhang
    keqiaozhang
    on Jun 24, 2022
    Collaborator

    @lyakh I have installed the sparse, but run to below errors:

    -- west build: building application
[1/208] Performing build step for 'smex_ep'
ninja: no work to do.
[4/208] Performing build step for 'sof_logger_ep'
ninja: no work to do.
[10/208] Generating linker_zephyr_pre0.cmd
FAILED: zephyr/linker_zephyr_pre0.cmd /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/linker_zephyr_pre0.cmd
cd /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr && -x assembler-with-cpp -MD -MF linker_zephyr_pre0.cmd.dep -MT linker_zephyr_pre0.cmd -D_LINKER -D_ASMLANGUAGE -imacros /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/include/generated/autoconf.h -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/include/zephyr -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/include -I/home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/include/generated -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/soc/xtensa/intel_adsp/cavs_v25 -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/soc/xtensa/intel_adsp/common/include -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/lib/libc/minimal/include -I/home/keqiao/works/sof-firmware/sof/zephyrproject/modules/hal/atmel/include -I/home/keqiao/works/sof-firmware/sof/zephyr/include -I/home/keqiao/works/sof-firmware/sof/zephyr/../src/platform/intel/cavs/include -I/home/keqiao/works/sof-firmware/sof/zephyr/../src/platform/tigerlake/include -D__GCC_LINKER_CMD__ -DLINKER_DEVICE_HANDLES_PASS1 -E /home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/soc/xtensa/intel_adsp/cavs_v25/linker.ld -P -o linker_zephyr_pre0.cmd && /opt/cmake-3.22.1-linux-x86_64/bin/cmake -E cmake_transform_depfile Ninja gccdepfile /home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/samples/subsys/audio/sof /home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/linker_zephyr_pre0.cmd.dep /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/CMakeFiles/d/c29bd3ec24fe7cb67e788d7df7eda5767658a00aa80f6ad5dd2e3dd8d70c50bc.d
/bin/sh: 1: -x: not found
[11/208] Generating linker_zephyr_pre1.cmd
FAILED: zephyr/linker_zephyr_pre1.cmd /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/linker_zephyr_pre1.cmd
cd /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr && -x assembler-with-cpp -MD -MF linker_zephyr_pre1.cmd.dep -MT linker_zephyr_pre1.cmd -D_LINKER -D_ASMLANGUAGE -imacros /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/include/generated/autoconf.h -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/include/zephyr -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/include -I/home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/include/generated -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/soc/xtensa/intel_adsp/cavs_v25 -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/soc/xtensa/intel_adsp/common/include -I/home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/lib/libc/minimal/include -I/home/keqiao/works/sof-firmware/sof/zephyrproject/modules/hal/atmel/include -I/home/keqiao/works/sof-firmware/sof/zephyr/include -I/home/keqiao/works/sof-firmware/sof/zephyr/../src/platform/intel/cavs/include -I/home/keqiao/works/sof-firmware/sof/zephyr/../src/platform/tigerlake/include -D__GCC_LINKER_CMD__ -DLINKER_ZEPHYR_PREBUILT -E /home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/soc/xtensa/intel_adsp/cavs_v25/linker.ld -P -o linker_zephyr_pre1.cmd && /opt/cmake-3.22.1-linux-x86_64/bin/cmake -E cmake_transform_depfile Ninja gccdepfile /home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr/samples/subsys/audio/sof /home/keqiao/works/sof-firmware/sof/zephyrproject/zephyr /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/zephyr/linker_zephyr_pre1.cmd.dep /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse/CMakeFiles/d/c42cff813fb760f05812ad8d397a5bda6d51365338c80a11420fc4193f6a4808.d
/bin/sh: 1: -x: not found
[19/208] Building C object zephyr/arch/arch/xtensa/core/CMakeFiles/arch__xtensa__core.dir/xtensa-asm2.c.obj
ninja: build stopped: subcommand failed.
FATAL ERROR: command exited with status 1: /usr/local/bin/cmake --build /home/keqiao/works/sof-firmware/sof/zephyrproject/tgl-sparse

    I updated the llvm and west to latest, but didn't help. Any ideas?

  15. keqiaozhang commented on Jun 27, 2022

    @keqiaozhang
    keqiaozhang
    on Jun 27, 2022
    Collaborator

    @lyakh
    I guess you use the default Zephyr SDK gcc xtensa-intel_s1000_zephyr-elf-gcc to build zephyr with Sparse. The default gcc also works for me without errors. The above errors are generated by xcc.

  16. lyakh commented on Jun 27, 2022

    @lyakh
    lyakh
    on Jun 27, 2022
    CollaboratorAuthor

    @lyakh I guess you use the default Zephyr SDK gcc xtensa-intel_s1000_zephyr-elf-gcc to build zephyr with Sparse. The default gcc also works for me without errors. The above errors are generated by xcc.

    @keqiaozhang yes, this is compiler-sensitive and since it's a Zephyr feature I think the main target would be the Zephyr toolchain. And I don't think we need to use xcc for this. We use this to get sparse output - warnings and errors, we don't actually care about compilation results.

  17. keqiaozhang commented on Jun 27, 2022

    @keqiaozhang
    keqiaozhang
    on Jun 27, 2022
    Collaborator

    @lyakh Thanks for explanation.
    I will add a GitHub action to check the sparse build in CI next.
    One tip, we need to set the REAL_CC to Zephyr SDK toolchain, this will let the cgcc use Zephyr SDK toolchain as the compiler to invoke.
    export REAL_CC=/$YOUR_PATH/zephyr-sdk-0.14.2/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc

  18. marc-hb commented on Sep 21, 2022

    @marc-hb
    marc-hb
    on Sep 21, 2022
    Collaborator

    EDIT: first attempt was

    Final PR submitted in

    EDIT: not final after all, see new requirements scattered in

  19. marc-hb
    transferred this issue fromthesofproject/sof-teston Sep 21, 2022
  20. marc-hb
    assigned
    marc-hb
    and unassigned
    keqiaozhang
    on Oct 28, 2022
  21. marc-hb
    added a commit that references this issue on Nov 1, 2022

    .github: fail on errors and address space warnings in sparse output

    6617809
  22. lgirdwood
    closed this as completedin b770818on Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P1Blocker bugs or important features

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions