modm-io / modm

modm: a C++23 library generator for AVR and ARM Cortex-M devices
https://modm.io
Mozilla Public License 2.0
748 stars 132 forks source link

Fix GCC 13 __FPU_PRESENT redefinition warning #1191

Closed calebchalmers closed 2 months ago

calebchalmers commented 3 months ago

Resolves #1181.

Let me know what you think about removing the previous defines, as mentioned here.

calebchalmers commented 3 months ago

Looks like one of the tests failed due to running out of space?

scons: building terminated because of errors.modm/src/modm/platform/core/vectors.c:239:9: fatal error: error writing to /tmp/cchdxs7h.s: No space left on device
salkinium commented 3 months ago

Something in this PR breaks the CMSIS DSP examples on Nucleo-F429. I've split the F4 examples into three jobs now and it still overflows disk space…

calebchalmers commented 3 months ago

Super weird. Seems like there's also an issue with stm32f407vet6_devebox/blinky/main.cpp ?

calebchalmers commented 3 months ago

Ok, I've been digging into the Nucleo-F429 issue. The cause of all the disk usage is two-fold:

  1. By including device.hpp, we are including stm32f429xx.h which is over 1MB in size. (Although the part we actually need is only lines 47-51.)
  2. Many headers files in the DSP library are including arm_math_types.h themselves, which means loading stm32f429xx.h over and over again, causing each object file to increase in size by roughly 800KB.

We could fix the issue by moving the device.hpp include from arm_math_types.h into arm_math.h, so it only gets included once. Thoughts @salkinium ?

salkinium commented 3 months ago

Yes, try it! I think I may have encountered this issue when I initially integrated CMSIS-DSP. Maybe we could also read the first few lines of the header file in Python and regex out the required defines.

calebchalmers commented 2 months ago

Hmm I've been playing around this and I've noticed that in some of the examples, they include both arm_math.h and arm_const_structs.h. (The latter also includes arm_math_types.h.)

The regex thing could work, but it doesn't seem too nice for a long term solution. I wonder if we could design a better interface? Maybe write our own headers over top of the arm ones to ensure the include order is correct. Or, we could put the includes for everything into one dsp.hpp header which would simplify the issue.

I'm not 100% sure what the intended interface for the DSP extension is currently, so guidance here would be appreciated!

calebchalmers commented 2 months ago

I took another look at this and figured it out. I got confused last time, but moving the configuration into arm_math.h fixes the original issue as well as the massive file sizes. I removed arm_math_types.h.in since it isn't needed anymore, and I also updated the docs to reflect that users should only use arm_math.h or arm_math_f16.h, rather than including specific library headers.

This should be good to go now :+1: