[avr-gcc-list] Confusing volatile behaviour with 4.6.2

[Andy Warner]

I am seeing some volatile handling behaviour that confuses me, and is
causing problems with some code.

Background: freshly built toolchain (gcc: 4.6.2, binutils 2.21.1,
avr-libc: 1.7.2, linux host.)
Target: Mega128, project is a DMX512 dongle.
DMX512 uses 250Kbaud serial comms, and uses a break to signal the
start of a new frame/packet.

Problem - it appears the compiler is feeling free to optimize/reorder
some register accesses that are marked volatile.

I've checked the archive/google and have found nothing useful -
apologies in advance if I just didn't look hard enough and it's all
been discussed before. I can't remember seeing references to this fly
past on the list recently.

Here is the USART0 rx interrupt handler that demonstrates the problem
(apologies in advance, if gmail mangles this):

#define SEARCHING (-2)
#define BREAK (-1)
#define MAX_DMX (512)

ISR(USART0_RX_vect)
{
        unsigned char status = UCSR0A;
        unsigned char data = UDR0;
        volatile static int slot = SEARCHING;

        if (status & 0x10) {
                if (slot == SEARCHING) {
                        slot++;
                } else {
                        goto search;
                }
        } else {
                if ((slot < BREAK) || (++slot > MAX_DMX)) {
                        goto search;
                }

                print_h2(0, slot & 0xff);
                PORTB = slot & 0x3f;

                if ((slot == 0) && (data != 0)) {
                        goto search;
                }

                if (slot == wanted) {
                        level = data;
                        goto search;
                }
        }

        return;

search:
        slot = SEARCHING;
        return;
}

Comments on goto > /dev/null.

When compiled with:
$ make test.lst
avr-gcc -mmcu=atmega128 -Wall -O2 -g -c test.c
avr-gcc -mmcu=atmega128 -g -o test.elf test.o
avr-size test.elf
   text    data     bss     dec     hex filename
   1094      22       1    1117     45d test.elf
avr-objdump -h -S test.elf > test.lst
$

The code as posted does not work, and examining test.lst shows why:

...
00000260 <__vector_18>:
#define SEARCHING (-2)
#define BREAK (-1)
#define MAX_DMX (512)

ISR(USART0_RX_vect)
{
 260:   1f 92           push    r1
 262:   0f 92           push    r0
 264:   0f b6           in      r0, 0x3f        ; 63
 266:   0f 92           push    r0
 268:   0b b6           in      r0, 0x3b        ; 59
 26a:   0f 92           push    r0
 26c:   11 24           eor     r1, r1
 26e:   2f 93           push    r18
 270:   3f 93           push    r19
 272:   4f 93           push    r20
 274:   5f 93           push    r21
 276:   6f 93           push    r22
 278:   7f 93           push    r23
 27a:   8f 93           push    r24
 27c:   9f 93           push    r25
 27e:   af 93           push    r26
 280:   bf 93           push    r27
 282:   cf 93           push    r28
 284:   ef 93           push    r30
 286:   ff 93           push    r31
        unsigned char status = UCSR0A;
        unsigned char data = UDR0;
 288:   cc b1           in      r28, 0x0c       ; 12
        volatile static int slot = SEARCHING;

        if (status & 0x10) {
 28a:   5c 9b           sbis    0x0b, 4 ; 11
 28c:   23 c0           rjmp    .+70            ; 0x2d4 <__vector_18+0x74>
                if (slot == SEARCHING) {
 28e:   80 91 11 01     lds     r24, 0x0111
 292:   90 91 12 01     lds     r25, 0x0112
 296:   2f ef           ldi     r18, 0xFF       ; 255
 298:   8e 3f           cpi     r24, 0xFE       ; 254
 29a:   92 07           cpc     r25, r18
 29c:   09 f4           brne    .+2             ; 0x2a0 <__vector_18+0x40>
 29e:   54 c0           rjmp    .+168           ; 0x348 <__vector_18+0xe8>
        }
...

Note that it has reordered the read of UCSR0A until after the read of UDR0.

Here is what the preprocessor produced for this code:

$ avr-gcc -mmcu=atmega128 -Wall -O2 -g -E test.c
...
void __vector_18 (void) __attribute__ ((signal,used,
externally_visible)) ; void __vector_18 (void)
{
 unsigned char status = (*(volatile uint8_t *)((0x0B) + 0x20));
 unsigned char data = (*(volatile uint8_t *)((0x0C) + 0x20));
 volatile static int slot = (-2);

 if (status & 0x10) {
  if (slot == (-2)) {
   slot++;
  } else {
   goto search;
  }
 } else {
  if ((slot < (-1)) || (++slot > (512))) {
   goto search;
  }
...

Which I _think_ should be telling the compiler that both registers are
volatile, so reordering is forbidden.

If I add some IO pin wiggling for debug, the problem vanishes:

...
ISR(USART0_RX_vect)
{
        unsigned char status = UCSR0A;
        unsigned char data = UDR0;
        volatile static int slot = SEARCHING;

        PORTB |= 0x40;
        if (status & 0x10) {
                if (slot == SEARCHING) {
                        slot++;
                } else {
                        goto search;
                }
        } else {
...

Results in:

...
ISR(USART0_RX_vect)
{
 260:   1f 92           push    r1
 262:   0f 92           push    r0
 264:   0f b6           in      r0, 0x3f        ; 63
 266:   0f 92           push    r0
 268:   0b b6           in      r0, 0x3b        ; 59
 26a:   0f 92           push    r0
 26c:   11 24           eor     r1, r1
 26e:   2f 93           push    r18
 270:   3f 93           push    r19
 272:   4f 93           push    r20
 274:   5f 93           push    r21
 276:   6f 93           push    r22
 278:   7f 93           push    r23
 27a:   8f 93           push    r24
 27c:   9f 93           push    r25
 27e:   af 93           push    r26
 280:   bf 93           push    r27
 282:   cf 93           push    r28
 284:   ef 93           push    r30
 286:   ff 93           push    r31
        unsigned char status = UCSR0A;
 288:   8b b1           in      r24, 0x0b       ; 11
        unsigned char data = UDR0;
 28a:   cc b1           in      r28, 0x0c       ; 12
        volatile static int slot = SEARCHING;

        PORTB |= 0x40;
 28c:   c6 9a           sbi     0x18, 6 ; 24
        if (status & 0x10) {
 28e:   84 ff           sbrs    r24, 4
 290:   23 c0           rjmp    .+70            ; 0x2d8 <__vector_18+0x78>
                if (slot == SEARCHING) {
 292:   80 91 11 01     lds     r24, 0x0111
 296:   90 91 12 01     lds     r25, 0x0112
 29a:   2f ef           ldi     r18, 0xFF       ; 255
 29c:   8e 3f           cpi     r24, 0xFE       ; 254
 29e:   92 07           cpc     r25, r18
 2a0:   09 f4           brne    .+2             ; 0x2a4 <__vector_18+0x44>
 2a2:   54 c0           rjmp    .+168           ; 0x34c <__vector_18+0xec>
        }
...

You can see now that the register reads are in the correct order, and
the code works as expected.

Comments/suggestions ?

-- 
Andy