psas/stm32

Code Review Issue: gcc (cortex-m4) flags review

Closed this issue · 4 comments

kwilsonpdx commented

It may be a valuable exercise to have a group review of all the complier and linker flags
we have chosen to use for STM32 flight-_ applications.

  • We are controlling our own fork of ChibiOS and can change the compile flags if we choose.
  • We can review issues such as: Why -O2 and not -O3 or -Os? -fstack-protector? What about link options? Why not -Werror? etc.
  • Document why we chose specific flags (This could have been the title for this issue)

Here is an example of some of the flags we are currently using during compile:

arm-none-eabi-gcc -c -mcpu=cortex-m4 -O2 -ggdb -fomit-frame-pointer -falign-functions=16 -Wno-main -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/lst/utils_hal.lst -DCORTEX_USE_FPU=FALSE -DTHUMB_PRESENT -mno-thumb-interwork -DTHUMB_NO_INTERWORKING -MD -MP -MF .dep/utils_hal.o.d -mthumb -DTHUMB ...

BartMassey commented

Aren't you using the STM32F4? Why would you have the epic FPU turned off?

kwilsonpdx commented

Aren't you using the STM32F4? Why would you have the epic FPU turned off?

PSAS development is a Traveling Carnival Of Wonder.

(I'll open an issue about turning the FPU ON)

ThirteenFish commented

For clarity the options have been broken out into sections:

Optimizations

  • -O2 - Optimise to level "even more". This is probably ok though I've experienced (but don't currently have a reproducible test case) asserts (in lwip in this instance) being disabled going from -O0 to -O2. For the debug target we should have -Og as it's recommended for optimizing the debugging experience.
  • -fomit-frame-pointer - This is on by default in any optimization level and can be removed.
  • -falign-functions=16 - We're unsure of the effects of this at the moment. The internet claims it's because the flash data bus is 128 bits (16 bytes) wide, and being aligned is good, but alignment set in linker script so I'm still unclear if this is needed.

The following are brought in by setting USE_LINK_GC to yes which seems to imply they're for link time garbage collection. On flight-rnh it reduced binaries by about 12% and I didn't need memstreams.c

  • -ffunction-sections

  • -fdata-sections

  • -fno-common
    While investigating I found the linker flag which I'm probably not understanding but it seems like it might do a similar thing :--gc-sections. It has no noticeable effect though.

    We're explicitly not using -flto, -Ofast, and other major opt flags that interfere with debugging. Once development has neared completion on a project these can be considered for a build for flight target, but the resulting binaries will still need to be verified that optimization hasn't broken anything.

Warnings

  • -Wall
  • -Wextra
  • -Wno-main - -Wall enables -Wmain but since main should never return in our projects we're declaring it as void. -ffreestanding should be looked into as a more correct way of doing this.
  • -Wstrict-prototypes - Catches functions accidentally defined without arguments. A quirk of C is that functions without arguments can be (uselessly) passed any arguments, which is bad.
  • -Wnested-externs - Warn if an extern declaration is encountered within a function. Seems good to have.
  • -Werror - can possibly cause builds to break when upgrading compiler versions so it's off by default, but in build for flight it should absolutely be on.

We can't use -std=c99 or -Wpedantic because ChibiOS makes heavy use of GCCs extensions to asm. -std=gnu99 should be used instead.

Debugging

-ggdb - like -g but with extra info for gdb specifically

Assembler Flags

-Wa,-alms=build/lst/

Arch

-mcpu=cortex-m4
-mno-thumb-interwork

ChibiOS Specific

-DCORTEX_USE_FPU=FALSE - When set to true mfloat-abi should be set to hard instead of softp like it is now
-DTHUMB_PRESENT
-DTHUMB_NO_INTERWORKING

Build

-MD
-MP
-MF .dep/build.d

TODO: Investigate what features newlib offers

Warnings that might be of interest

There are a large number of warning flags that for one reason or another aren't enabled by -Wall and -Wextra. This is a preliminary list of those that might be of interest. With some of them ChibiOS generates a large number of warnings so they're only really useful as a one off thing.

Other flags

  • -ffreestanding (Wno-main)
  • -02 and above (Warray-bounds)

Warnings

  • -Waggressive-loop-optimizations
  • -Wdisabled-optimization
  • -Wunsafe-loop-optimizations - with -funsafe-loop-opts
  • -Wc++-compat (-Wjump-misses-init)? might be like pedantic
  • -Wcast-qual
  • -Wconversion
  • -Wdouble-promotion - tied in with enablefpu
  • -Wunsuffixed-float-constants - may be useful for fpu stuff
  • -Wfloat-equal
    • -Wformat=2
  • -Wlogical-op
  • -Wmissing-include-dirs
  • -Wpacked -Wpadded - padded excessive
  • -Wpointer-arith
  • -Wredundant-decls
  • -Wshadow - chibios hits it
  • -Wstack-usage=len
  • -Wstrict-overflow=5/-fstrict-overflow
  • -Wsuggest-attribute=[pure|const|noreturn|format] - lots of chibios
  • -Wmissing-format-attribute == -Wsuggest-attribute=format
  • -Wundef - !! ChibiOS bug RTC_SUPPORTS_CALLBACKS
  • -Wwrite-strings
  • nonull function attribute?
ThirteenFish commented

There's always more flags to explore, but for the upcoming flight I think we're largely satisfied with the review that's already taken place. We can re-open this later.