embecosm / avr-gcc

A fork of the GCC mainline for work on the AVR tool chain
GNU General Public License v2.0
16 stars 3 forks source link

Edge case optimization produces incorrect machine code #3

Closed ConsciousCode closed 4 years ago

ConsciousCode commented 4 years ago
volatile register gbf asm("18");
enum gbf_Flags {
    MONITOR,
    NEST,
    MINE,
    EOM,
    CMD,
    WHO
};

#define set_flag(flag) asm volatile("sbr %0, %1" :"+r"(gbf):"I"(flag))
#define clear_flag(flag) asm volatile("cbr %0, %1" :"+r"(gbf):"I"(flag))

void main() {
    set_flag(NEST);
    clear_flag(NEST);
}

Compiles to:

main:
    ori 18, 0x01 ; Incorrect behavior!!
    andi 18, 0xFE

Should be:

main:
    sbr 18, 1
    cbr 18, 1

Or:

main:
    ori 18, 0x02
    andi 18, 0xFD

avr-gcc --version: avr-gcc (GCC) 5.4.0 avr-objdump --version: GNU objdump (GNU Binutils) 2.26.20160125 uname -a: Linux Darkmatter 4.15.0-88-generic #88-Ubuntu SMP Tue Feb 11 20:11:34 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux Architecture: ATtiny10 (avr1) Compiled with: avr-gcc -ffreestanding -mmcu attiny10 main.c -o main.o Read with: avr-objdump -m avr1 -D main.o

This workaround can be used instead with no change in code size:

#define set_flag(flag) asm volatile("ori %0, 1<<%1" :"+r"(gbf):"I"(flag))
#define clear_flag(flag) asm volatile("andi %0, ~(1<<%1)" :"+r"(gbf):"I"(flag))

Not sure why, but I can't seem to get the compiler to use the "M" constraint for _BV(flag) without it warning that it doesn't match the constraints, so bit shifting is done in the assembly.

ConsciousCode commented 4 years ago

... I'm an idiot. Took one look at the spec page after posting and realized that sbr and cbr are more or less ori and andi.