electro-smith
CMake Improvements
Over the past year I've done some work on improving the state of the CMake build system, initially inspired by fixing building on native Windows, and then expanding in scope.
This is part 1 of the work towards fully migrating to CMake as per #606.
Changes in this PR include:
- Fixing the native build on Windows
- Adding support for LLVM/Clang
- Adopting a similar build configuration process as the current Makefile (see example below)
- Auto-detection of embedded build toolchains
- Build the examples with the composite target
examples(these are not part of the defaultalltarget) - Support for all the GoogleTest unit tests via CTest
- Adding matrix toolchain testing to the Github Actions so we can test for regressions on older toolchains
- Cleaning up the GitHub Actions a bit to use cached tools (for both GCC and LLVM). This makes running the test builds much faster.
- Switch from using
TOOLCHAIN_PREFIXto the CMake standardCMAKE_C_COMPILERfor manually selecting a toolchain
Example project:
cmake_minimum_required(VERSION 3.26)
cmake_policy(SET CMP0048 NEW)
# Fetch libDaisy
include(FetchContent)
FetchContent_Declare(daisy
GIT_REPOSITORY https://github.com/stellar-aria/libDaisy
GIT_TAG cmake-only
)
FetchContent_MakeAvailable(daisy)
# Our project declaration
project(daisytest VERSION 0.0.1)
set(FIRMWARE_NAME daisytest)
set(FIRMWARE_SOURCES
${CMAKE_CURRENT_LIST_DIR}/src/test.cpp
)
#set(FIRMWARE_DEBUG_OPT_LEVEL -O0) # (optional) to customize Debug optimization level
#set(FIRMWARE_RELEASE_OPT_LEVEL -O2) # (optional) to customize Release optimization level
# DaisyProject uses FIRMWARE_NAME and FIRMWARE_SOURCES to build a target called ${FIRMWARE_NAME}
include(DaisyProject)
target_include_directories(${FIRMWARE_NAME} PUBLIC include)
Thank you so much for working on this! I did a quick test and for some reason found that the binary cmake is creating is much larger than one done using the old Makefile. I haven't digged into it yet but here's the memory map for both:
Cmake
FLASH: 104468 B 128 KB 79.70%
DTCMRAM: 0 GB 128 KB 0.00%
SRAM: 13644 B 512 KB 2.60%
RAM_D2: 16 KB 288 KB 5.56%
RAM_D3: 0 GB 64 KB 0.00%
BACKUP_SRAM: 12 B 4 KB 0.29%
ITCMRAM: 0 GB 64 KB 0.00%
SDRAM: 0 GB 64 MB 0.00%
QSPIFLASH: 0 GB 8 MB 0.00%
Makefile
FLASH: 63104 B 128 KB 48.14%
DTCMRAM: 0 GB 128 KB 0.00%
SRAM: 12436 B 512 KB 2.37%
RAM_D2: 16 KB 288 KB 5.56%
RAM_D3: 0 GB 64 KB 0.00%
BACKUP_SRAM: 12 B 4 KB 0.29%
ITCMRAM: 0 GB 64 KB 0.00%
SDRAM: 0 GB 64 MB 0.00%
QSPIFLASH: 0 GB 8 MB 0.00%
Thank you so much for working on this! I did a quick test and for some reason found that the binary cmake is creating is much larger than one done using the old Makefile. I haven't digged into it yet but here's the memory map for both:
Is this on the Debug or the Release config? The Makefile currently builds Release by default with an optimization level of -O3. CMake defaults to Debug builds, which with the current toolchain files in this PR have an optimization level of -Og, equivalent to -O1 except for those options that may interfere with debugging. The Release is currently built with -Os, and while that should probably be changed for performance reasons, currently it should be smaller than the Makefile build.
The size of examples/AudioOutput built off:
- HEAD w/
make: 99704 bytes - PR w/
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DDAISY_BUILD_EXAMPLES=YES && cmake --build build: 79172 bytes - cmake but with
-O3in ArmGNUToolchain.cmake: 90980 bytes
So I think this is a config issue, the sizes shouldn't be that different otherwise.
Sizes were checked with arm-none-eabi-size
Thanks for the thorough explanation, after posting I figured it was probably an optimization flag :)
edit tested with -DCMAKE_BUILD_TYPE=Release and built something even smaller as you said
Oh perfect. Yeah, I probably need to think of a better way to have both sane default optimization levels and propagate user-selected ones. I'll poke at that today.
Test Results
150 tests ± 0 150 :white_check_mark: ±0 0s :stopwatch: ±0s 1 suites - 15 0 :zzz: ±0 1 files ± 0 0 :x: ±0
Results for commit 7eee442e. ± Comparison against base commit 39aab667.
This pull request removes 150 and adds 150 tests. Note that renamed tests count towards both.
MAX11300TestFixture ‑ verifyAdcPinConfiguration
MAX11300TestFixture ‑ verifyAutoUpdate
MAX11300TestFixture ‑ verifyDacPinConfiguration
MAX11300TestFixture ‑ verifyDriverInitializationRoutine
MAX11300TestFixture ‑ verifyGpiPinConfiguration
MAX11300TestFixture ‑ verifyGpoPinConfiguration
MAX11300TestFixture ‑ verifyHeterogeneousPinBehavior
MAX11300TestFixture ‑ verifyReadAnalogPin
MAX11300TestFixture ‑ verifyReadAnalogPinMultiple
MAX11300TestFixture ‑ verifyReadDigitalPin
…
MAX11300TestFixture.verifyAdcPinConfiguration ‑ MAX11300TestFixture.verifyAdcPinConfiguration
MAX11300TestFixture.verifyAutoUpdate ‑ MAX11300TestFixture.verifyAutoUpdate
MAX11300TestFixture.verifyDacPinConfiguration ‑ MAX11300TestFixture.verifyDacPinConfiguration
MAX11300TestFixture.verifyDriverInitializationRoutine ‑ MAX11300TestFixture.verifyDriverInitializationRoutine
MAX11300TestFixture.verifyGpiPinConfiguration ‑ MAX11300TestFixture.verifyGpiPinConfiguration
MAX11300TestFixture.verifyGpoPinConfiguration ‑ MAX11300TestFixture.verifyGpoPinConfiguration
MAX11300TestFixture.verifyHeterogeneousPinBehavior ‑ MAX11300TestFixture.verifyHeterogeneousPinBehavior
MAX11300TestFixture.verifyReadAnalogPin ‑ MAX11300TestFixture.verifyReadAnalogPin
MAX11300TestFixture.verifyReadAnalogPinMultiple ‑ MAX11300TestFixture.verifyReadAnalogPinMultiple
MAX11300TestFixture.verifyReadDigitalPin ‑ MAX11300TestFixture.verifyReadDigitalPin
…
:recycle: This comment has been updated with latest results.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Changes:
- Made a separate "daisy-tests" CMake project for test building/running. This allows IDEs such as VSCode and CLion to be able to build (and run) both the unit tests and the main library side-by-side with different compilers, ideal for TDD/BDD development.
- Fixes the github actions changes arising from this
- No longer relies on "CMAKE_CROSSCOMPILING" flag, which was a bit of hack anyways, since it's usually set by the toolchain file with CMAKE_SYSTEM_NAME when it's "Generic"
- Removed the "GoogleTest Adapter" extension and swapped it out for the much better integrated "testMate" extension (which already had configuration options in the settinsg.json, but was not part of the recommended workspace extensions)
@stellar-aria Thank you very much for your work! I tested it on my project and got an undefined reference to 'USBH_MIDI_SetReceiveCallback' linker error. Also there is a warning about changing start of .text section, not sure if it is relevant, but something that I haven't seen with Makefile.
Here is barebones example that builds fine with the Makefile approach and libDaisy from master but fails witch CMake using latest commit from the PR branch.
#include "daisy_seed.h"
using namespace daisy;
DaisySeed hardware;
MidiUsbHandler midiUsb;
int main(void)
{
hardware.Configure();
hardware.Init();
MidiUsbHandler::Config midiUsbConfig;
midiUsbConfig.transport_config.periph = MidiUsbTransport::Config::INTERNAL;
midiUsb.Init(midiUsbConfig);
for(;;)
{
}
}
cmake_minimum_required(VERSION 3.26)
cmake_policy(SET CMP0048 NEW)
# Fetch libDaisy
include(FetchContent)
FetchContent_Declare(daisy
GIT_REPOSITORY https://github.com/stellar-aria/libDaisy
GIT_TAG 2826863c573c30c87975b46780df81ec425259b2
)
FetchContent_MakeAvailable(daisy)
# Our project declaration
project(daisytest VERSION 0.0.1)
set(FIRMWARE_NAME daisytest)
set(FIRMWARE_SOURCES
main.cpp
)
#set(FIRMWARE_DEBUG_OPT_LEVEL -O0) # (optional) to customize Debug optimization level
#set(FIRMWARE_RELEASE_OPT_LEVEL -O2) # (optional) to customize Release optimization level
# DaisyProject uses FIRMWARE_NAME and FIRMWARE_SOURCES to build a target called ${FIRMWARE_NAME}
include(DaisyProject)
target_include_directories(${FIRMWARE_NAME} PUBLIC include)
$ cmake -B build -DCMAKE_BUILD_TYPE=Release
$ cmake --build build
[ 2%] Built target FatFs
[ 61%] Built target STM32H7XX_HAL_DRIVER
[ 64%] Built target STM32_USB_DEVICE_LIBRARY
[ 67%] Built target STM32_USB_HOST_LIBRARY
[ 98%] Built target daisy
[ 98%] Building CXX object CMakeFiles/daisytest.dir/main.cpp.obj
[100%] Linking CXX executable daisytest.elf
/Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: warning: start of section .text changed by 8
/Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/libdaisy.a(usb_midi.cpp.obj): in function `daisy::MidiUsbTransport::Impl::Init(daisy::MidiUsbTransport::Config)':
usb_midi.cpp:(.text._ZN5daisy16MidiUsbTransport4Impl4InitENS0_6ConfigE+0x2e): undefined reference to `USBH_MIDI_SetReceiveCallback'
Memory region Used Size Region Size %age Used
FLASH: 119496 B 128 KB 91.17%
DTCMRAM: 0 GB 128 KB 0.00%
SRAM: 65040 B 512 KB 12.41%
RAM_D2: 17372 B 288 KB 5.89%
RAM_D3: 0 GB 64 KB 0.00%
BACKUP_SRAM: 12 B 4 KB 0.29%
ITCMRAM: 0 GB 64 KB 0.00%
SDRAM: 0 GB 64 MB 0.00%
QSPIFLASH: 0 GB 8 MB 0.00%
collect2: error: ld returned 1 exit status
make[2]: *** [daisytest.elf] Error 1
make[1]: *** [CMakeFiles/daisytest.dir/all] Error 2
make: *** [all] Error 2
Also I was not able to make it work properly with LLVMEmbeddedToolchainForArm 18.0.0 on Mac. With DaisyToolchain installed cmake always detected gnu compiler no matter of TOOLCHAIN_PREFIX variable present. I removed DaisyToolchain from my system and cmake partially detected compilers and compiled project but without a bin file, only elf and hex are there. Also there is no size output with LLVM, maybe worth adding -Xlinker --print-memory-usage - it is supported by lld that is in 18.0.0 and 17.0.1 ARM LLVM toolchains.
Toolchain discovery pretty much could be my fault of not configuring project or flags properly but I tried to follow most obvious steps to make it work so others might stumble into it too.
cmake -B build -G Ninja -D CMAKE_BUILD_TYPE=Release -D TOOLCHAIN_PREFIX=/Applications/LLVMEmbeddedToolchainForArm-18.0.0-Darwin-universal
No compiler specified. Falling back to searching PATH...
Could not find arm-none-eabi-gcc. Assuming clang is part of embedded toolchain...
-- The C compiler identification is Clang 17.0.6
-- The CXX compiler identification is Clang 18.0.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /opt/homebrew/bin/clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/LLVMEmbeddedToolchainForArm-18.0.0-Darwin-universal/bin/clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done (62.6s)
-- Generating done (0.1s)
-- Build files have been written to: /Users/aleksei/ScratchPad/DaisyExamples/seed/Blink/build
$ cmake --build build
[a bunch of warnings from libDaisy code]
[204/204] Linking CXX executable daisytest.elf
$ ls -A1 build
.ninja_deps
.ninja_log
CMakeCache.txt
CMakeFiles
_deps
build.ninja
cmake_install.cmake
daisytest.elf
daisytest.hex
@asavartsov looks like I missed a file in the merge conflict resolution, whoops. Thanks for finding that! That should fix the undefined error for the linker.
Instead of using TOOLCHAIN_PREFIX when you're using the auto-detecting toolchain files, you should use the standard CMAKE_C_COMPILER and CMAKE_CXX_COMPILER command-line settings. This integrates much better with IDEs. The autodetection script will then use that compiler instead of hunting for its own (which as you noticed, tries finding GCC first and then falling back to clang).
EDIT: So for example this is the command line used to create a configuration with LLVM on Windows:
"C:\Program Files\CMake\bin\cmake.EXE" --no-warn-unused-cli -DCMAKE_BUILD_TYPE:STRING=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=TRUE -DCMAKE_C_COMPILER:FILEPATH=C:\Developer\LLVMEmbeddedToolchainForArm\bin\clang.exe -DCMAKE_CXX_COMPILER:FILEPATH=C:\Developer\LLVMEmbeddedToolchainForArm\bin\clang++.exe -SC:/Users/Kate/GitHub/libDaisy -BC:/Users/Kate/GitHub/libDaisy/build -G Ninja
Thanks you! It worked perfectly both for linker error and compiler options. When you mentioned IDE I opened my project in VSCode and it gave me similar command, I should have tried it first 🤦
About bin file generation, it was as simple as set(DAISY_GENERATE_BIN true)
I cannot thank you enough again, CMake/LLVM is such a huge improvement for me because I use autogenerated DSP code which compiles forever with gcc and just in seconds with clang.
I also tested CMake with bootloader targets (sram and qspi) and looks like they won't boot from bootloader without -DBOOT_APP which is set by Makefile but not by CMake.
Adding target_compile_definitions(daisy PRIVATE BOOT_APP) to my project file fixes that (though it probably should be automatic based on DAISY_STORAGE value) but this define also breaks Debug CMake target if using GCC toolchain:
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal.c.obj): in function `HAL_Init':
[build] stm32h7xx_hal.c:(.text.HAL_Init+0x6c): undefined reference to `D1CorePrescTable'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: stm32h7xx_hal.c:(.text.HAL_Init+0x70): undefined reference to `SystemD2Clock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: stm32h7xx_hal.c:(.text.HAL_Init+0x74): undefined reference to `SystemCoreClock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal.c.obj): in function `HAL_InitTick':
[build] stm32h7xx_hal.c:(.text.HAL_InitTick+0x64): undefined reference to `SystemCoreClock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal_dma.c.obj): in function `HAL_DMA_IRQHandler':
[build] stm32h7xx_hal_dma.c:(.text.HAL_DMA_IRQHandler+0x1a8): undefined reference to `SystemCoreClock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal_rcc.c.obj): in function `HAL_RCC_ClockConfig':
[build] stm32h7xx_hal_rcc.c:(.text.HAL_RCC_ClockConfig+0x360): undefined reference to `D1CorePrescTable'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: stm32h7xx_hal_rcc.c:(.text.HAL_RCC_ClockConfig+0x364): undefined reference to `SystemD2Clock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: stm32h7xx_hal_rcc.c:(.text.HAL_RCC_ClockConfig+0x368): undefined reference to `SystemCoreClock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal_rcc.c.obj): in function `HAL_RCC_GetHCLKFreq':
[build] stm32h7xx_hal_rcc.c:(.text.HAL_RCC_GetHCLKFreq+0x54): undefined reference to `D1CorePrescTable'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: stm32h7xx_hal_rcc.c:(.text.HAL_RCC_GetHCLKFreq+0x58): undefined reference to `SystemD2Clock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: stm32h7xx_hal_rcc.c:(.text.HAL_RCC_GetHCLKFreq+0x5c): undefined reference to `SystemCoreClock'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal_rcc.c.obj): in function `HAL_RCC_GetPCLK1Freq':
[build] stm32h7xx_hal_rcc.c:(.text.HAL_RCC_GetPCLK1Freq+0x28): undefined reference to `D1CorePrescTable'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal_rcc.c.obj): in function `HAL_RCC_GetPCLK2Freq':
[build] stm32h7xx_hal_rcc.c:(.text.HAL_RCC_GetPCLK2Freq+0x28): undefined reference to `D1CorePrescTable'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal_rcc_ex.c.obj): in function `HAL_RCCEx_GetD3PCLK1Freq':
[build] stm32h7xx_hal_rcc_ex.c:(.text.HAL_RCCEx_GetD3PCLK1Freq+0x28): undefined reference to `D1CorePrescTable'
[build] /Library/DaisyToolchain/0.2.0/arm/bin/../lib/gcc/arm-none-eabi/10.3.1/../../../../arm-none-eabi/bin/ld: _deps/daisy-build/Drivers/libSTM32H7XX_HAL_DRIVER.a(stm32h7xx_hal_sai.c.obj): in function `SAI_Disable':
[build] stm32h7xx_hal_sai.c:(.text.SAI_Disable+0x68): undefined reference to `SystemCoreClock'
All other targets seem to be fine with GCC and all targets including Debug are fine with LLVM. Looks like GCC optimizes out some references from static lib because they are dependent on
#ifndef BOOT_APP
SystemInit();
#endif
in startup_stm32h750xx.c or something like that 🙂
@asavartsov thank you so much for testing this all out! It looks like because the Makefile was building everything as one large static blob it didn't care so much about the references, but system_stm32h7xx.c actually should be compiled as part of the CMSIS Device library.
I've moved it there and changed the library type to static, as well as adding the compile definitions if the DAISY_STORAGE variable is set to anything other than flash.
This should fix the issues, please let me know if you have any more problems!
@stellar-aria I am really sorry if I annoy you too much! I did a little bit more testing and I have some more comments:
- I tested build with all targets and different optimization options.
FIRMWARE_*_OPT_LEVELdoes not seem to affect libDaisy itself. Also if I build my project in Debug or Release, libDaisy sources compiled without any optimization flags at all (also no ggdb3 for Debug and no -DNDEBUG for Release). Produced ninja FLAGS for daisy files:
Release:
-std=gnu++14 [skip] -Wall
Debug:
-std=gnu++14 [skip] -Wall
MinSizeRel:
-Os -DNDEBUG -std=gnu++14 [skip] -Wall
RelWithDebInfo:
-O2 -g -DNDEBUG -std=gnu++14 [skip] -Wall
It does not look like a desired behaviour...
-
link_libraries(semihost)may be a little bit too bold here, it would be very hard (if even possible) to override in the parent project. For GNU toolchain you haveset(CMAKE_EXE_LINKER_FLAGS "--specs=nano.specs --specs=nosys.specs" CACHE INTERNAL "Linker options")which would allow overriding, may be it is worth to have something similar for LLVM to be on par? By the way, I can confirm that semihosting works like a charm with LLVM. -
I tested ArmGNUToolchain 13.2.Rel1 and LLVMEmbeddedToolchainForArm 18.0.0. GNU seems to be working fine except I could not make semihosting to work. LLVM not great news. I started to test different peripherals and I stumbled into
hw.StartAudiocausing hard fault withBFARVALIDafter the code entersSaiHandle::Impl::StartDmaTransferif libDaisy is an optimized build. I can confirm at least-Og,-O1,-O3triggers that rendering audio useless if compiled with LLVM toolchain. With-O0audio starts fine and callback works. I am not sure if compiler omits something or it is CPU runtime optimization error or both. I am actively debugging it though I am not a real embedded dev 😭 but I hope I will figure it out and fix it separately. I know that it is not related to CMake support at all but more like an info for those who will try the same after this will be merged.
@asavartsov no worries! I'd rather get this all worked out in the PR than have to deal with it in mainline.
-
yeah the current setup will not propagate the build flags. ~~I'll need to think on this one. I know what's causing it but it might be tricky to work around.~~ Figured it out. I've added a new patch.
-
semihostshouldn't be there, that's a whoops from my experiments -
I have no idea what's going on there tbh. This does sound like a bug in the LLVM optimization passes which is something I have 0 experience debugging. If you're using Cortex-Debug or something like Segger's Ozone, you can step-debug the assembly output of the problem function/object file. Doing that and manually checking your registers might be the key here. Also an A/B comparison of the output object files and/or going through and manually enabling the -Og optimizations one by one to find which one is the problem.
Thanks! Turned out that clang optimimizes out while(1); as a loop without side effects, which is by spec but not very compatible with libDaisy code.
So my
int main() {
hw.Init();
hw.StartAudio(callback);
while(1) {}
}
effectively becomes just
int main() {
hw.Init();
hw.StartAudio(callback);
}
and after main it tries to do some runtime on exit stuff which eventually sets sp=0x20002000 and access some offset of it which raises a hard fault.
GCC do not optimize while(1); loops so I think it would be fair to add -fno-finite-loops compiler flag to LLVM toolchain by default so it will be compatible with some default error handlers in libDaisy and probably existing code around.
Clang compiled code without a flag and with it: https://godbolt.org/z/Kx75G7hbc
Yeah considering how heavily it's used in certain places of libDaisy, and even shown in the examples, I think that's a reasonable solution.
Ideally we could convert them to a static hang() function or something that's just while(1) { asm("nop"); } since that doesn't get utterly destroyed by the optimization passes, but that's a separate issue.
@beserge @stephenhensley any ideas when/how this might get merged? Are there any edits that need to happen for it to be done?
Checked out and built, everything appears as it should be. I'll defer to @stephenhensley on the merge timing, but looks good to go on my end! One thing to note, I had to update my CMake install, my out of date version was giving me some trouble, but once that was done it was smooth sailing.
Any update on a merge timeline for this? This would be a very welcome addition!
Had to do manual merge resolution, since usbh_midi.c was already listed in the new refactored CMakeLists.txt. However, there also seems to be some issues with building on LLVM (on the CI side) now, so I'm going to poke about at that to see what's wrong.
Alright, everything's working as it should now.
Had to remove support for older versions of the LLVM Embedded Toolchain for ARM due to major breaking changes where the ARMv7E-M and ARMv7-M runtime libraries were merged, but considering we're only merging this now and 19.1.1 is the most current as of this comment, I think that's an acceptable solution.
@stellar-aria Sounds good! I've been doing a little review/testing on this this week; I'll re-sync everything locally, and continue to test a bit more.
So far, so good. Since it's getting a bit late today, and I'd like to test converting a few projects with the latest version, I'll probably end up merging this on Monday unless any issues arise.
So I am running into one issue that I didn't realize initially. I've verified that I can get this to happen when building on Mac OS and Windows.
On Mac OS, I've built with ARM GCC 10.3-2021.10 (10.3.1 20210824 release) On Windows I've built with ARM GCC 10-2020-q4-major (10.2.1 20201103 (release))
In both cases, programs built seem to run properly after being programmed to the Daisy. After a reset, or a full power cycle, it looks like the program faults immediately from the Reset_Handler.
I've used the DaisyBlinkProject repo using the FetchContent feature, as well as modified the GPIO_Input example within the repo, and had the same experience.
CMakeLists.txt:
cmake_minimum_required(VERSION 3.26)
cmake_policy(SET CMP0048 NEW)
include(FetchContent)
FetchContent_Declare(daisy
GIT_REPOSITORY https://github.com/stellar-aria/libDaisy
GIT_TAG 5086cc0
)
FetchContent_MakeAvailable(daisy)
project(daisyblink VERSION 0.0.1)
set(FIRMWARE_NAME daisyblink)
set(FIRMWARE_SOURCES
${CMAKE_CURRENT_LIST_DIR}/Blink/main.cpp
)
set(DAISY_GENERATE_BIN ON)
include(DaisyProject)
target_include_directories(${FIRMWARE_NAME} PRIVATE include)
main.cpp
#include "daisy_seed.h"
using namespace daisy;
DaisySeed hardware;
int main()
{
hardware.Init();
while (true)
{
hardware.SetLed(true);
hardware.DelayMs(500);
hardware.SetLed(false);
hardware.DelayMs(500);
}
}
Built with the following:
cmake -B build -G Ninja -D CMAKE_BUILD_TYPE=Release
cmake --build build --target all
I vaguely recall running into this a long time ago (possibly an issue with LTO, or something), but it was several years ago.
I'll retry with different optimization, etc. and see if I can narrow down the issue, but it's also possible I'm just overlooking something obvious.
@stephenhensley I've used your test case and it compiles and works (uploaded via the Daisy web programmer) with GCC 13.2.1 (see attached files) even after a cold-boot and power cycle, so next I'm going to try with 10-2020-q4. daisyblink.zip
I can confirm the behavior on 10-2020-q4, I'm going to check versions upwards to see where it resolves
@stellar-aria thanks for stepping through that, and finding where it improved! -- I'll continue testing with a newer compiler.
Aside from this issue everything seems great so far. Last things I want to check are bootloader/linker script related, but I don't anticipate any issues.
yeah I'm not sure where this is going wrong. It seems like there might be a problem with linker stuff due to it just failing in the reset handler. For fun, here's a comparison between 11.3 and 12.2's first few bytes of the bin file. Unfortunately, I've never been good at decoding hex by hand.
edit: it might be some kind of alignment issue?
TLDR: Remove -fno-builtin from stm32h750xx.cmake.
ARM GCC 10-2020-q4-major with example above in Release configuration (-O3) generates floating point instructions (vldr, vstmia) for the memory copy loop (😮) in the Reset_Hanlder() that is running before FPU enabled at SystemInit(). It causes a NOCP hard fault.
GCC will usually optimize this loop to builtin memset but CMake build has -fno-builtin flag set (which usage is strongly discouraged anyway). This flag is not set in any of Makefiles, this is why it always worked before.
In my test with real HW removing -fno-builtin from stm32h750xx.cmake fixes the issue.
Thank you so much for figuring that out @asavartsov. It looks like that was added in one of the first few refactor commits I was doing, probably pulled from somewhere else originally, and it's been so long I have no idea what prompted it being there or what I was thinking 😅
It looks like without it, there's now some size changes in some of the binaries for clang, which is preventing them from fully building, so some of the examples might have to be prodded to work properly or removed from the test build