Re: push r1, pop r0

Szikra Istvan schrieb:
> Hi all,
> 
> I have this interesting test case for you guys, and girls.
> 
> #include <avr/io.h>
> 
> unsigned int GetStackPointer()
> {
>     volatile unsigned int sp = (SPH << 8) | SPL;

There is "SP" for you.  Ne need to hamper with bytes.

uint16_t GetStackPointer (void)
{
     return SP;
}

>     return sp;
> }
> 
> int main(void)
> {
>     while(1)
>     {
>         PORTA.OUT = GetStackPointer();
>     }
> }
> 
> After building it with Atmel Studio 6.2 (default project settings,
> -mmcu=atxmega128a4u), AVR8/GNU C Compiler/Linker : 4.8.1 the .lss contains
> this gem:
> 
> unsigned int GetStackPointer()
> {
>  21a: cf 93        push r28
>  21c: df 93        push r29
>  21e: 1f 92        push r1
>  220: 1f 92        push r1
>  222: cd b7        in r28, 0x3d ; 61
>  224: de b7        in r29, 0x3e ; 62
>     volatile unsigned int sp = (SPH << 8) | SPL;
>  226: 2e b7        in r18, 0x3e ; 62
>  228: 8d b7        in r24, 0x3d ; 61
>  22a: 90 e0        ldi r25, 0x00 ; 0
>  22c: 92 2b        or r25, r18
>  22e: 89 83        std Y+1, r24 ; 0x01
>  230: 9a 83        std Y+2, r25 ; 0x02
>     return sp;
>  232: 89 81        ldd r24, Y+1 ; 0x01
>  234: 9a 81        ldd r25, Y+2 ; 0x02
> }
>  236: 0f 90        pop r0
>  238: 0f 90        pop r0
>  23a: df 91        pop r29
>  23c: cf 91        pop r28
>  23e: 08 95        ret
> 
> (Atmel Studio 7 (Version: 7.0.1417), gcc version 5.4.0
> (AVR_8_bit_GNU_Toolchain_3.6.0_1734) does the same.)
> (If anyone interested I can attach the whole compressed project.)

A working test case is always preferred over binary clump :-)

> I'm mostly interested in the push r1, pop r0 pairs.
> Why? What are they doing? Who puts them there? Can someone "fix" the
> compiler/binutils?

You used volatile which forces "sp" into the frame of the function.
As avr-gcc has to set up a frame pointer, this needs to push 2 more
bytes.  Y is frame pointer and callee-saved here, this explains total
of 4 bytes of stack usage (not counting return address).

> This seems to be a bug for me.

Not to me.

> Can someone "fix" the compiler/binutils?

The only bug is in your code:

o Forcing sp onto stack by volatile.

o Using -O1 does not set -fomit-frame-pointer, hence even
   without the volatile you get a frame pointer.  To get
   rid of it, use code that doesn't trigger a frame and
   compile with optimization higher than -O1 and / or with
   -fomit-frame-pointer.

o Trying to get free stack by reading SP is a broken design.
   For example, an ISR might consume way more space, which
   will not be detected by your code.  Consider code like
   in "mem-check.c" from here:
 
http://rn-wissen.de/wiki/index.php?title=Speicherverbrauch_bestimmen_mit_avr-gcc#Dynamischer_RAM-Verbrauch

> AFAIK r1 is the zero reg, and r0 is temp reg, so this should not cause
> problems, so it's not a critical bug, only optimization issue.
> (Unnecessary __tmp_reg__ =__zero_reg__ )

As already mentioned, the problem is in your code.

> And my optimal solution is
>     unsigned int GetStackPointer()
>     {
>         union { unsigned int w; unsigned char b[2];} w;
>         w.b[0] = SPL; w.b[1] = SPH;
>         21a: 8d b7        in r24, 0x3d ; 61
>         21c: 9e b7        in r25, 0x3e ; 62
>         return w.w;
>     }
>     21e: 08 95        ret

No need for type punning unions, just use "SP" as explained above.

Johann