modm-io / modm

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

GCC 13 warning: Redefinition of __FPU_PRESENT in CMSIS module #1181

Open calebchalmers opened 3 weeks ago

calebchalmers commented 3 weeks ago

While compiling an STM32F427 project with arm-none-eabi-gcc 13.2.rel1-2, I'm seeing several warnings of the following form:

In file included from taproot/modm/src/modm/platform/core/../device.hpp:25,
                 from taproot/modm/src/modm/platform/core/atomic_lock_impl.hpp:17,
                 from taproot/modm/src/modm/architecture/interface/atomic_lock.hpp:79,
                 from taproot/modm/src/modm/architecture/interface.hpp:23,
                 from taproot/modm/src/modm/architecture.hpp:18,
                 from taproot/modm/src/modm/platform.hpp:13,
                 from taproot/src/tap/board/board.hpp:42,
                 from taproot/src/tap/communication/gpio/pwm.cpp:27:
taproot/modm/ext/cmsis/device/stm32f427xx.h:51: warning: "__FPU_PRESENT" redefined
   51 | #define __FPU_PRESENT             1U       /*!< FPU present                                   */
      | 
In file included from taproot/modm/ext/cmsis/dsp/arm_math.h:34,
                 from taproot/src/tap/algorithms/cmsis_mat.hpp:34,
                 from taproot/src/tap/algorithms/math_user_utils.hpp:37,
                 from taproot/src/tap/communication/gpio/pwm.cpp:26:
taproot/modm/ext/cmsis/dsp/arm_math_types.h:18: note: this is the location of the previous definition
   18 | #define __FPU_PRESENT 1
      | 
In file included from taproot/modm/src/modm/platform/core/../device.hpp:25,
                 from taproot/modm/src/modm/platform/core/atomic_lock_impl.hpp:17,
                 from taproot/modm/src/modm/architecture/interface/atomic_lock.hpp:79,
                 from taproot/modm/src/modm/architecture/interface.hpp:23,
                 from taproot/modm/src/modm/architecture.hpp:18,
                 from taproot/modm/src/modm/platform.hpp:13,
                 from taproot/src/tap/architecture/clock.hpp:30,
                 from taproot/src/tap/architecture/timeout.hpp:29,
                 from taproot/src/tap/control/setpoint/commands/move_absolute_command.hpp:29,
                 from taproot/src/tap/control/setpoint/commands/move_absolute_command.cpp:24:
taproot/modm/ext/cmsis/device/stm32f427xx.h:51: warning: "__FPU_PRESENT" redefined
   51 | #define __FPU_PRESENT             1U       /*!< FPU present                                   */
      | 
In file included from taproot/modm/ext/cmsis/dsp/arm_math.h:34,
                 from taproot/src/tap/algorithms/cmsis_mat.hpp:34,
                 from taproot/src/tap/algorithms/math_user_utils.hpp:37,
                 from taproot/src/tap/control/setpoint/commands/move_absolute_command.hpp:27:
taproot/modm/ext/cmsis/dsp/arm_math_types.h:18: note: this is the location of the previous definition
   18 | #define __FPU_PRESENT 1
      | 
...

These warnings ultimately boil down to arm_math_types.h and stm32f427xx.h both being included and both defining __FPU_PRESENT.

I see that arm_math_types.h has a guard in place but stm32f427xx.h does not, and in my case, the latter is usually being included first.

Apologies since I'm not familiar with CMSIS; if this is a question for one of the ARM repos please let me know and I'll submit it over there instead.

salkinium commented 3 weeks ago

stm32f427xx.h must be included first, otherwise arm_math_types.h will choose the wrong defaults. This is a bug in modm's include order, we should probably explicitly include <modm/platform/device.hpp> inside arm_math_types.h?

calebchalmers commented 2 weeks ago

@salkinium that makes sense to me, since device.hpp would contain the relevant configuration.

Are these lines in arm_math_types.h necessary?

#define ARM_MATH_CM4
#define __ARM_FEATURE_MVE 0

I can't see any reference to ARM_MATH_CM4, and from what I can tell, not defining __ARM_FEATURE_MVE would be sufficient rather than defining it to be 0?

I'm thinking we can replace all the current defines in arm_math_types.h with a device.hpp include.