GCC 9.x compatibility - Githubissues

chax commented 5 years ago

Hi, i'm a Solus user and maintainer of avr-gcc and arm-none-eabi-gcc packages in Solus. Solus currently has main GCC version upgraded to 9.2.0, but I as a maintainer of avr/arm variants held back GCC to version 8.3.0 because of some compatibiliy issues with GCC 9.x and QMK.

Recently this PR #5947 has also been merged into QMK, giving Solus users ability to build their firmwares on their Solus machines. Since Solus is rolling release distro (like Arch or openSUSE Tumbleweed), all users who keep their system up to date will get latest versions of every package in repo, which includes avr/arm GCC if i update them.

Currently i've been testing new versions of GCC 9.2.0 locally and i had some issues building both avr and arm versions of QMK firmware.

First to tackle AVR boards, i tested by trying to build planck rev5 and rev4 (which i don't have in possession but i can at least try to build it). I successfully built firmware but resulting .hex file appears to be to large and it finishes with error:

Copying planck_rev5_default.hex to qmk_firmware folder                                              [OK]
Checking file size of planck_rev5_default.hex                                                      
 * The firmware is too large! 30216/28672 (1544 bytes over)

Copying planck_rev4_default.hex to qmk_firmware folder                                              [OK]
Checking file size of planck_rev4_default.hex                                                      
 * The firmware is too large! 30212/28672 (1540 bytes over)

I manage to bring this down a little by modifying makefile tmk_core/avr.mk and adding flags -flto -Os but it's still too large.

Checking file size of planck_rev5_default.hex                                                      
 * The firmware is too large! 29634/28672 (962 bytes over)

For arm board i have both planck rev6 and preonic rev3 so i tested by building default keymap. I stumbled upon some deprecation warnings which were treated like errors.

Compiling: tmk_core/common/chibios/bootloader.c                                                    In file included from ./lib/chibios/os/common/ext/CMSIS/ST/STM32F3xx/stm32f303xc.h:168,
                 from ./lib/chibios/os/common/ext/CMSIS/ST/STM32F3xx/stm32f3xx.h:153,
                 from ./lib/chibios/os/common/startup/ARMCMx/devices/STM32F3xx/cmparams.h:72,
                 from ./lib/chibios/os/common/ports/ARMCMx/chcore.h:70,
                 from ./lib/chibios/os/rt/include/ch.h:81,
                 from tmk_core/common/chibios/bootloader.c:3:
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h: In function 'enter_bootloader_mode_if_requested':
./lib/chibios/os/common/ext/CMSIS/include/core_cm4.h:93:28: error: listing the stack pointer register 'sp' in a clobber list is deprecated [-Werror=deprecated]
   93 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler */
      |                            ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h:190:3: note: in expansion of macro '__ASM'
  190 |   __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : "sp");
      |   ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/core_cm4.h:93:28: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
   93 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler */
      |                            ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h:190:3: note: in expansion of macro '__ASM'
  190 |   __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : "sp");
      |   ^~~~~
cc1: all warnings being treated as errors

After that i tried to modify tmk_core/arm_atsam.mk to ALLOW_WARNINGS=yes, but i also had to modify rules.mk to add -Wno-deprecated to CFLAGS and CPPFLAGS because -Wall infers -Wdeprecated flag. After these modifications i was able to successfully build and flash working firmware for planck rev6.

I also saw that for some compatibility issues with GCC 9.x utils/linux_install.sh script has been modified for Arch/Manjaro to pin avr-gcc to version 8.3.0.

For now, i'm still holding back upgrade to GCC 9.x on Solus, but if these issues could be fixed in QMK firmware, that would be great. Maybe even avr needs just a little more tweaking of flags and makefiles to turn down compiled firmware size.

Keep in mind that i'm not C/C++ guru, so i'm leaving this for experts.

chax commented 5 years ago

I guess I'm hitting this issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91189

yanfali commented 5 years ago

@chax you have our sympathies. We've had a lot of issues with almost every GCC release. The 8 series was not compatible with QMK until 8.3, it kept producing bad code for us. On ARM we recommend 6.3.1 at this time, though I have been reliably informed that 8.3 is also producing good code at this time. For AVR 8.3 is our current recommendation. We do not currently recommend any 9 series GCC releases for use with AVR and have not done any real testing with ARM, but we do know that 9 has had issues in previous releases. Fingers crossed 9.3 will be a good release for us.

chax commented 5 years ago

Like i said, for ARM (planck rev6) i successfully built and flashed qmk (with makefiles modifications to ignore deprecations) and it works on my planck. For AVR i think this bug report that i posted is the same issue i have. GCC 9.2 works with Arduino and i can easily and successfully flash Arduino UNO through Arduino IDE, other than the issue that i get larger firmware binary image.

yanfali commented 5 years ago

@chax appreciate the suggestions, but we have hundreds of keyboards in our repo, so things take time to verify their impact. Also, arduino is a comparatively small flash size target, we tend to create images that get close to the 28k limit on a pro micro, so we would run into this before most IDE users.

chax commented 5 years ago

I know, i just wrote my observations. For now i will keep holding back upgrade of these GCC packages on Solus, after all, there is no real value-add by upgrading them.

yanfali commented 5 years ago

There's a bigger user base of arduino users than QMK I would wager, so you'll have to make that call. When people run into this kind of issue on a rolling linux distro, we strongly recommend the docker container for consistency.

drashna commented 4 years ago

Also, one thing to note here, is that GCC 9.x generates larger images than previous versions. So we may not want to update, even if we do get this working. 8.3 is probably the best version to use, at this time.

magnusmorton commented 3 years ago

It's looking like this may never be fixed and that GCC may drop AVR support. clang might have better results

itspngu commented 3 years ago

I'm having trouble reproducing this. Built the same keymap on 3 different versions of avr-gcc, see below.

5.4.0

make: 19798/28672 (69%, 8874 bytes free)
make CFLAGS="-Os": 25308/28672 (88%, 3364 bytes free) <- quite obviously overriding some other optimizations I haven't dug into yet, there's a lot of Makefiles involved
make CFLAGS="-Os -flto": 16844/28672 (58%, 11828 bytes free)

8.3.0

make: 19380/28672 (67%, 9292 bytes free)
make CFLAGS="-Os": 24936/28672 (86%, 3736 bytes free)
make CFLAGS="-Os -flto": 16506/28672 (57%, 12166 bytes free)

10.2.0

make: 20172/28672 (70%, 8500 bytes free)
make CFLAGS="-Os": 25828/28672 (90%, 2844 bytes free)
make CFLAGS="-Os -flto": 16452/28672 (57%, 12220 bytes free)

8.3 is an improvement over 5.4 when considering target size, but I don't see the huge regressions people are talking about in 10.2. The -Os -flto build is infact smaller than 8.3's. Am I missing something here (read: being an idiot and doing something wrong)? Is this a corner case that only happens under certain circumstances? Is the increase in binary size perhaps caused by other flags which are absent in my tests?

pngu@smol:~/git/qmk_firmware$ make idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 5.4.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 19798/28672 (69%, 8874 bytes free)

pngu@smol:~/git/qmk_firmware$ make CFLAGS="-Os" idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 5.4.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 25308/28672 (88%, 3364 bytes free)

pngu@smol:~/git/qmk_firmware$ make CFLAGS="-Os -flto" idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 5.4.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 16844/28672 (58%, 11828 bytes free)

pngu@smol:~/git/qmk_firmware$ LD_LIBRARY_PATH=/usr/local/avr/lib:$LD_LIBRARY_PATH PATH=/usr/local/avr/bin:$PATH make idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 8.3.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 19380/28672 (67%, 9292 bytes free)

pngu@smol:~/git/qmk_firmware$ LD_LIBRARY_PATH=/usr/local/avr/lib:$LD_LIBRARY_PATH PATH=/usr/local/avr/bin:$PATH make CFLAGS="-Os" idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 8.3.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 24936/28672 (86%, 3736 bytes free)

pngu@smol:~/git/qmk_firmware$ LD_LIBRARY_PATH=/usr/local/avr/lib:$LD_LIBRARY_PATH PATH=/usr/local/avr/bin:$PATH make CFLAGS="-Os -flto" idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 8.3.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 16506/28672 (57%, 12166 bytes free)

pngu@smol:~/git/qmk_firmware$ LD_LIBRARY_PATH=/usr/local/avr/lib:$LD_LIBRARY_PATH PATH=/usr/local/avr/bin:$PATH make idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 10.2.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 20172/28672 (70%, 8500 bytes free)

pngu@smol:~/git/qmk_firmware$ LD_LIBRARY_PATH=/usr/local/avr/lib:$LD_LIBRARY_PATH PATH=/usr/local/avr/bin:$PATH make CFLAGS="-Os" idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 10.2.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 25828/28672 (90%, 2844 bytes free)

pngu@smol:~/git/qmk_firmware$ LD_LIBRARY_PATH=/usr/local/avr/lib:$LD_LIBRARY_PATH PATH=/usr/local/avr/bin:$PATH make CFLAGS="-Os -flto" idb/idb_60:via
QMK Firmware 0.9.50
Making idb/idb_60 with keymap via

avr-gcc (GCC) 10.2.0
...
Compiling: keyboards/idb/idb_60/idb_60.c                                                            [OK]
...
Compiling: lib/lufa/LUFA/Drivers/USB/Core/USBTask.c                                                 [OK]
Linking: .build/idb_idb_60_via.elf                                                                  [OK]
Creating load file for flashing: .build/idb_idb_60_via.hex                                          [OK]
Copying idb_idb_60_via.hex to qmk_firmware folder                                                   [OK]
Checking file size of idb_idb_60_via.hex                                                            [OK]
 * The firmware size is fine - 16452/28672 (57%, 12220 bytes free)

tdgrunenwald commented 3 years ago

@itspngu 10.2 may be fixed. I was just able to successfully build + flash with gcc 9.3 where I previously had issues with 9.2

itspngu commented 3 years ago

@itspngu 10.2 may be fixed. I was just able to successfully build + flash with gcc 9.3 where I previously had issues with 9.2

That sounds great, but uh... as outlined in my post I had trouble reproducing the regressions everyone was talking about.

9.3 being fixed (whatever that means) compared to 9.2 seems like a great thing, so perhaps it'd make sense to make the maintainers aware. I suppose if you could provide some more background info, such as build outputs comparing 9.2 to 9.3 and then tag drashna and yanfali as they'd probably be in the best position to evaluate bumping the GCC version from 8.x to 9.x or 10.x :)

tdgrunenwald commented 3 years ago

I did some more testing, and unfortunately I don't have any real breakthroughs to share. I am able to build my keymap on gcc 7.3, 9.2, and 9.3 successfully, but when compiling the production target (keymap + DFU bootloader) I get the same issue I had before in #8391 on the 9.x versions. Also able to reproduce firmware size issue with planck rev 5 & 6 with gcc 9.3

t-8ch commented 2 years ago

I also experience the issue when compiling the lufa DFU bootloader on GCC 11.2.0 The missing space is actually very small (6 Bytes). It turns out that the Product and Manufacturer string embedded by QMK, which are taken from the actual keyboard, evyd13/wasdat in my case) make the difference. By removing three characters (each character takes up 2 bytes) from my PRODUCT the bootloader compiles.

Depending on how much space is missing on other bootloaders it may make sense to use this for a general solution.

qmk / qmk_firmware

GCC 9.x compatibility #6719

5.4.0

8.3.0

10.2.0