Re: [avr-gcc-list] push r1, pop r0

[David Brown]

On 07/11/17 20:36, Szikra Istvan wrote:
> 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;
>     return sp;
> }
> 
> int main(void)
> {
>     while(1)
>     {
>         PORTA.OUT = GetStackPointer();
>     }
> }
> 

You have got an explanation for the pushes and pops.  AVR gcc used to
use "rcall ." to make a little stack space, which also surprised people.

It makes no sense to use "volatile" here - you are not using the
variable "sp" in a volatile manner.  That just means wasted time and
space, and means the value returned by your function is not the stack
pointer at the point of the call.  (Also, if you are writing C rather
than C++, your function declaration is not correct.)  The sensible way
to write this is:

static inline uint16_t GetStackPointer(void)
{
        return (SPH << 8) | SPL;
}

There is no need to use unions here - the compiler should be able to
handle this well.  You don't need a temporary variable here - certainly
not a "volatile" one.  And a static inline function means that the stack
pointer you read is the correct value.  It is good practice to use the
<stdint.h> fixed size types when you want a fixed size - "uint16_t" is
more precise and informative than "unsigned int" in cases like this.