AVR Freaks

For an 8-bit or 32-bit Arduino?

Does it make any difference?

I'm convinced you've tickled a bug (or more).  It took me almost 15 minutes to get an example program that will reproduce your problem, and in the process I found another avr-gcc bug.

I'll start with the code that reproduces your bug:

#include <avr/io.h>
#include <stdint.h>

void foo(long l)
{
    PORTB = ((l >> 12) | l) & 0xFF;
}

void bar(int i) { foo ( (long) i); }

int main()
{
    int i = 1;
    while (1) {
        bar((int16_t) i);
        i += 10000;
        PORTB = 0;
    }
}

With avr-gcc 5.4.0, -Os -flto, I get:

00000030 <main>:
  30:   81 e0           ldi     r24, 0x01       ; 1
  32:   90 e0           ldi     r25, 0x00       ; 0
  34:   a0 e0           ldi     r26, 0x00       ; 0
  36:   b0 e0           ldi     r27, 0x00       ; 0
  38:   ac 01           movw    r20, r24
  3a:   bd 01           movw    r22, r26
  3c:   2c e0           ldi     r18, 0x0C       ; 12
  3e:   75 95           asr     r23
  40:   67 95           ror     r22
  42:   57 95           ror     r21
  44:   47 95           ror     r20
  46:   2a 95           dec     r18
  48:   d1 f7           brne    .-12            ; 0x3e <__SP_H__>
  4a:   48 2b           or      r20, r24
  4c:   48 bb           out     0x18, r20       ; 24
  4e:   18 ba           out     0x18, r1        ; 24
  50:   80 5f           subi    r24, 0xF0       ; 240
  52:   98 4d           sbci    r25, 0xD8       ; 216
  54:   af 4f           sbci    r26, 0xFF       ; 255
  56:   bf 4f           sbci    r27, 0xFF       ; 255
  58:   ef cf           rjmp    .-34            ; 0x38 <main+0x8>

Most of us are aware of C's annoying int promotion rules that sometimes cause avr-gcc to use a 16-bit value even though you specify 8-bit.  However there is no rule that allows the compiler to promote an int to a long.  I suppose one could argue the compiler is free to implement it as 32-bits, as long it works the same as if it were 16-bits.  But before I get to that, I want to explain the reason for PORTB = 0 in the loop.  If you comment it out, here's what you'll get:

00000030 <main>:
  30:   81 e0           ldi     r24, 0x01       ; 1
  32:   88 bb           out     0x18, r24       ; 24
  34:   83 e1           ldi     r24, 0x13       ; 19
  36:   88 bb           out     0x18, r24       ; 24
  38:   85 e2           ldi     r24, 0x25       ; 37
  3a:   88 bb           out     0x18, r24       ; 24
  3c:   87 e3           ldi     r24, 0x37       ; 55
  3e:   88 bb           out     0x18, r24       ; 24

And no, I didn't make any mistake.  The compiler actually removed the loop!  If I compile without lto, it does the loop.  I still find it hard to believe avr-gcc actually does something so stupid.  I re-compiled this several times, checking timestamps, switching on and off lto because at first I thought I must've made a mistake.  I'm used to finding subtle bugs in gcc, but nothing like this.

Going back to the original problem, I wrote a modified test program with i inside a struct, in which case gcc only allocates 2 bytes in the data section.  I used an asm memory clobber in the loop to force gcc save/restore i on each itteration, and it still uses 4 registers in the loop when lto is enabled.  Only the lower 16 bits are saved and restored.  Without lto, it does the more intelligent thing and just uses 2 registers instead of 4.

I suspect what's going on is that gcc internally uses 32-bit values for constant propagation & constant folding for LTO.  I highly doubt avr-gcc will see a patch to improve the lto perfomance.  I even doubt the serious bug I found with the while(1) loop disappearing will get fixed.  And yet Microchip will continue to market the AVR line to the automotive sector, where you need not only robust and reliable hardware, but also a robust and reliable compiler.

p.s. Maybe now more people will understand one of the reasons I like writing in asm for AVR targets.   You don't have to coerce or trick the assembler into generating the code you really want.

p.p.s. For anyone trying to test the above code, I only tried it in c++ mode, since Bill's Arduino code would be compiled in c++ mode.  In c, mode, gcc sometimes gives different behavior.

A real issue is how far back does the license from undefined behavior reach?

Also, what should a compiler do when performing optimizations based on undefined behavior?

Methinks that deserves at least a warning.

If the compiler can legitimately remove the entire loop,

why not the entire body of main?

Somewhere in the bowels of advice for GNU developers was the statement

that they need not consider the possibility that int will be smaller than 32 bits.

Whether it is still there, I do not know.

With -fwrapv, the bug I found where gcc silently omits the while(1) loop is "fixed".  Judging by the asm, it looks like it wraps at 16 bits, as expected.  And it only uses 16 bits for i (r19:r18), and casting int to long results in a sign extend of the integer (into r27:r26).

00000030 <main>:
  30:   21 e0           ldi     r18, 0x01       ; 1
  32:   30 e0           ldi     r19, 0x00       ; 0
  34:   c9 01           movw    r24, r18
  36:   03 2e           mov     r0, r19
  38:   00 0c           add     r0, r0
  3a:   aa 0b           sbc     r26, r26
  3c:   bb 0b           sbc     r27, r27
  3e:   4c e0           ldi     r20, 0x0C       ; 12
  40:   b5 95           asr     r27
  42:   a7 95           ror     r26
  44:   97 95           ror     r25
  46:   87 95           ror     r24
  48:   4a 95           dec     r20
  4a:   d1 f7           brne    .-12            ; 0x40 <__SREG__+0x1>
  4c:   82 2b           or      r24, r18
  4e:   88 bb           out     0x18, r24       ; 24
  50:   20 5f           subi    r18, 0xF0       ; 240
  52:   38 4d           sbci    r19, 0xD8       ; 216
  54:   ef cf           rjmp    .-34            ; 0x34 <main+0x4>

Actually, the test above shows it doesn't use 32 bits for i with -flto -fwrapv.  This supports Bill's theory of a snobbish/lazy gcc developer deciding they can do whatever they want, without somuch as a warning, if you do something that the standard says is undefined.  Note that I tried every flag that should have resulted in a warning: -Wall -Wpedantic -pedantic-errors -Werror=pedantic -Wstrict-overflow -Wconversion -Wsign-conversion

That's where I'm convinced the gcc developers have really got it wrong.  No sensible standards organization like ISO would allow "undefined behavior" to mean "all bets are off" for the whole program.  We are talking about a standard that is used for things like medical devices.  Signed integer overflow being undefined means the implementation is not defined.  It could wrap, it could peg to INT_MAX, etc.  Everything else in the program that is well-defined stays well-defined.  What the holy gnu developers have decided is that one single line of code that the standard says is undefined renders everything else undefined as well.

"In contrast, the C standard says that signed integer overflow leads to undefined behavior where a program can do anything, including dumping core or overrunning a buffer. The misbehavior can even precede the overflow."

https://www.gnu.org/software/aut...

Really nasty behavior. Anyway I've got something, that makes it working even with -flto, and if it's commented out, it stops working again. Also String::operator+(int) seems to be working all the times:

void setup() {
  Serial.begin(115200);
}
size_t (Stream::*nasty_print)(int,int) = &Stream::print;

void print_z(int16_t value) {
  Serial.print(static_cast<int16_t>(value));
}

void loop() {
  int16_t i = 1;
  while (i > 0) {
    Serial.print(String{} + i);  // works anytime
    Serial.print(' ');

    // comment out next line and rest of it stops working:
    ((&Serial)->*nasty_print)(i,DEC); // works and causes others to work too, but if it's commented, rest stops working
    // (Serial.*nasty_print)(i,DEC);  // bit more clear than previous

    Serial.print(' ');
    Serial.print(i, DEC); // nope
    Serial.print(' ');
    print_z(i);           // nope
    Serial.println();

    i += 10000;
    delay(1000);
  }
}

So it seems to be just selecting wrong overload of the print/println method.

And maybe that indirect call of function pointer to class method caused it couldn't be optimized by -flto...

Edit: tried it on ARM aaaand... It is wrong on AVR even without -flto