Re: [avr-gcc-list] Problem with delay loop

[David Brown]

There are good reasons for much of the stuff produced by the use of 
"volatile" - since the local variable is volatile, it has to be put on 
the stack.  Generating a stack frame on the AVR needs a bit of messing 
around in the prologue and epilogue, including turning off interrupts 
while modifying the stack pointer (blame Atmel for that whoopsie).

What would be really nice is if gcc accepted the concept of "register 
volatile" variables.  I've used them on other compilers in the past to 
get exactly the effect we are looking for here.  But that was in the 
days when "register" was a useful hint to an otherwise fairly poor 
optimiser - gcc, I believe, ignores "register" for local variables.

Slightly better code can be generated with:

void delay2(unsigned int n) {
        static volatile unsigned int x;
        x = n;
        while (x--) ;
}

But even better is:

void delay3(volatile unsigned int n) {
        while (n--) {
                asm volatile(" " : : );
        };
}


mvh.,

David





Royce Pereira wrote:
> Hi all,
>
> OK fine I agree.
>
> we have to use 'volatile' and all.
>
> But why must it generate such horrid code...
> (I reproduce the comparison again below to get the *real* issue into focus)
>
> The compiler output with the 'correct' code ('volatile' used):
> //---------------------------------------------------
> void delay(unsigned int del_cnt)
>      {
>    2aa: cf 93           push    r28
>    2ac: df 93           push    r29
>    2ae: cd b7           in      r28, 0x3d       ; 61
>    2b0: de b7           in      r29, 0x3e       ; 62
>    2b2: 22 97           sbiw    r28, 0x02       ; 2
>    2b4: 0f b6           in      r0, 0x3f        ; 63
>    2b6: f8 94           cli                     ;<-----disabling interrupts? 
> Why?
>    2b8: de bf           out     0x3e, r29       ; 62
>    2ba: 0f be           out     0x3f, r0        ; 63
>    2bc: cd bf           out     0x3d, r28       ; 61
>         volatile unsigned int n = del_cnt;
>    2be: 9a 83           std     Y+2, r25        ; 0x02
>    2c0: 89 83           std     Y+1, r24        ; 0x01
>         while(n--);
>    2c2: 89 81           ldd     r24, Y+1        ; 0x01
>    2c4: 9a 81           ldd     r25, Y+2        ; 0x02
>    2c6: 01 97           sbiw    r24, 0x01       ; 1
>    2c8: 9a 83           std     Y+2, r25        ; 0x02
>    2ca: 89 83           std     Y+1, r24        ; 0x01
>    2cc: 89 81           ldd     r24, Y+1        ; 0x01
>    2ce: 9a 81           ldd     r25, Y+2        ; 0x02
>    2d0: 8f 5f           subi    r24, 0xFF       ; 255
>    2d2: 9f 4f           sbci    r25, 0xFF       ; 255
>    2d4: b1 f7           brne    .-20            ; 0x2c2 <delay+0x18>
>    2d6: 22 96           adiw    r28, 0x02       ; 2
>    2d8: 0f b6           in      r0, 0x3f        ; 63
>    2da: f8 94           cli
>    2dc: de bf           out     0x3e, r29       ; 62
>    2de: 0f be           out     0x3f, r0        ; 63
>    2e0: cd bf           out     0x3d, r28       ; 61
>    2e2: df 91           pop     r29
>    2e4: cf 91           pop     r28
>    2e6: 08 95           ret
>
>         return;
>      }
> //=======================
>
> Output of the older -WinAVR-20060421-version (with the alleged 'wrong' C code)
> > > //==========================================
> > > void delay(word cnt)
> > >     {
> > >        while(cnt--);
> > >   286:  01 97           sbiw    r24, 0x01       ; 1
> > >   288:  2f ef           ldi     r18, 0xFF       ; 255
> > >   28a:  8f 3f           cpi     r24, 0xFF       ; 255
> > >   28c:  92 07           cpc     r25, r18
> > >   28e:  d9 f7           brne    .-10            ; 0x286 <delay>
> > >   290:  08 95           ret
> > >          return;
> > >     }
> > > //=======================================
>
> So which is the right output we want? Obviously the second.
>
> Agreed not using 'volatile' optimises the code to a mere 'ret'. I'm ok with 
> that.
>
> But should'nt the 'correct' code produce the same output (the shorter version 
> above)??
>
> Thanks,
>
> --Royce.