MCUdude / MegaCoreX

An Arduino hardware package for ATmega4809, ATmega4808, ATmega3209, ATmega3208, ATmega1609, ATmega1608, ATmega809 and ATmega808
GNU Lesser General Public License v2.1
239 stars 47 forks source link

Digital(Read/Write)Fast works under -0s, but fails to compile under -03 or -02. #190

Open devcarbon-com opened 6 months ago

devcarbon-com commented 6 months ago

I'm a bit baffled as to what the compiler is optimizing that would cause this.

it fails even with a very basic test case,

digitalWriteFast(1, 0);
digitalReadFast(1);

I have also tried using #DEFINEs and constexprs, but to no avail.

In function 'check_constant_pin',
    inlined from 'digitalWriteFast' at /home/pacman/.platformio/packages/framework-arduino-megaavr-megacorex/cores/MegaCoreX/wiring_digital.c:208:3:
/home/pacman/.platformio/packages/framework-arduino-megaavr-megacorex/cores/MegaCoreX/api/Common.h:100:5: error: call to 'badArg' declared with attribute error: 
     badArg("Digital pin must be a constant");
     ^
In function 'check_constant_pin',
    inlined from 'digitalReadFast' at /home/pacman/.platformio/packages/framework-arduino-megaavr-megacorex/cores/MegaCoreX/wiring_digital.c:253:3:
/home/pacman/.platformio/packages/framework-arduino-megaavr-megacorex/cores/MegaCoreX/api/Common.h:100:5: error: call to 'badArg' declared with attribute error: 
     badArg("Digital pin must be a constant");
     ^
lto-wrapper: fatal error: avr-g++ returned 1 exit status
compilation terminated.
/home/pacman/.platformio/packages/toolchain-atmelavr/bin/../lib/gcc/avr/7.3.0/../../../../avr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
*** [.pio/build/Upload_UPDI/firmware.elf] Error 1
================== [FAILED] Took 0.42 seconds ==================

Any ideas?

MCUdude commented 6 months ago

Not really. I've only tested the source code using -Os. The MegaCoreX Arduino IDE addon doesn't let the user change the optimization.

devcarbon-com commented 6 months ago

Gotcha. I'm using PlatformIO, so I'll look into options for changing the build flag for the wiring_digital.c file specifically. Thanks!

MCUdude commented 6 months ago

Yes, PlatformIO is really neat, and gives you a whole new level of freedom! Since it's complaining about the input arguments not being compile time constants (which is weird), I'd see if I somehow could make the compiler understand that these are in fact constant. If You're completely stuck, the guys over at the Avrfreaks forum know quite about the avr-gcc compiler.

devcarbon-com commented 6 months ago

Yes, I've been quite happy with it! The flexibility and the reproducibility is so much nicer than trying to work with Arduino in my (limited) experience with Arduino.

Yeah, it does seem rather odd. I tried experimenting with removing the check, and that seems to work fine as a workaround.

Cool, thanks for the tip!

DrItanium commented 3 months ago

So this is really strange, I tried to reproduce this but I can only do so when link time optimization is turned off. But in the original post it is turned on...

The builtin is a strange one so I decided to look at the gcc docs on __builtin_constant_p and found something very interesting:

You can use the built-in function __builtin_constant_p to determine if the expression exp is known to be constant at compile time and hence that GCC can perform constant-folding on expressions involving that value. The argument of the function is the expression to test. The expression is not evaluated, side-effects are discarded. The function returns the integer 1 if the argument is known to be a compile-time constant and 0 if it is not known to be a compile-time constant. Any expression that has side-effects makes the function return 0. A return of 0 does not indicate that the expression is not a constant, but merely that GCC cannot prove it is a constant within the constraints of the active set of optimization options.

...

You may use this built-in function in either a macro or an inline function. However, if you use it in an inlined function and pass an argument of the function as the argument to the built-in, GCC never returns 1 when you call the inline function with a string constant or compound literal (see Compound Literals) and does not return 1 when you pass a constant numeric value to the inline function unless you specify the -O option.

My guess is that somehow -Os works fine but is buggy for -O2 and -O3 on the original poster's machine with LTO enabled. On my machine, it is buggy without LTO... that smells like a compiler bug of some kind.

SpenceKonde commented 2 months ago

My experience attempting to make digitalWriteFast and it's ilk work under -O3 and -O2 was entirely unsuccessful. They also don't work if you turn off LTO.