Re: push r1, pop r0

Hi David,

What is exactly wrong with my code?

Thanks by the way for __get_PSP and all your help.

This is what it looks like on my end:
__STATIC_INLINE uint32_t __get_PSP(void)
{
  register uint32_t __regProcessStackPointer  __ASM("psp");
  return(__regProcessStackPointer);
}
in
C:\Program Files (x86)\Atmel\Atmel Toolchain\ARM
GCC\Native\4.8.1443\CMSIS_Atmel\CMSIS\Include\core_cmFunc.h

Which for me looks a lot like my code except for volatile.

Actually I already had inline asm version, but I prefer to use C if I can:
unsigned int GetStackPointer()
{
//asm volatile ("mov %0, r13" : "=l" (sp));
volatile register unsigned int sp asm("r13");
return sp;
}


So why volatile?
Because it generates smaller code:

    while (1)
    {
        unsigned int sp=GetStackPointer();
        sum+=sp;
  400270: 9b01      ldr r3, [sp, #4]
  400272: 446b      add r3, sp
  400274: 9301      str r3, [sp, #4]
  400276: e7fb      b.n 400270 <main+0xc>

Without it:

    //volatile
    register unsigned int sp asm("r13");
    return sp;
  400270: 466a      mov r2, sp
    volatile
    uint32_t sum=0;
    while (1)
    {
        unsigned int sp=GetStackPointer();
        sum+=sp;
  400272: 9b01      ldr r3, [sp, #4]
  400274: 4413      add r3, r2
  400276: 9301      str r3, [sp, #4]
  400278: e7fb      b.n 400272 <main+0xe>
the code gets bigger.


__builtin_frame_address(0) returns r7 not sp. Which should be te same, but
forces the use of r7:

  400264: b580      push {r7, lr}
  400266: b082      sub sp, #8
  400268: af00      add r7, sp, #0

My version doesn't have this:
  400264: b500      push {lr}
  400266: b083      sub sp, #12


Here's te complete test case with code size statistics, in case someone
want's to try it with different compiler versions...


#include "sam.h"

static inline unsigned int GetStackPointer() {
    volatile register unsigned int sp asm("r13");
    return sp;
}
static inline unsigned int GetStackPointerAsm() {
    register uint32_t sp;
    asm volatile ("mov %0, r13" : "=l" (sp));
    return sp;
}
static inline unsigned int GetStackPointerNV() {
    register unsigned int sp asm("r13");
    return sp;
}

__attribute__( ( always_inline ) ) __STATIC_INLINE uint32_t
GetStackPointerAsm2(void)
{
    register uint32_t result;
    __ASM volatile ("MRS %0, psp\n"  : "=r" (result) );
    return(result);
}
static inline uint32_t GetStackPointerAsm3(void)
{
    uint32_t sp;
    asm volatile("mov %0, r13" : "=r" (sp));
    return sp;
}

int main(void)
{
    SystemInit();

    volatile uint32_t sum=0;
    while (1) {
        ///                                  Program Memory Usage
        sum+=GetStackPointer();          /// 2080 bytes
        //sum+=GetStackPointerAsm();       /// 2084 bytes
        //sum+=GetStackPointerAsm2();      /// 2084 bytes
        //sum+=GetStackPointerAsm3();      /// 2084 bytes
        //sum+=GetStackPointerNV();        /// 2084 bytes
        //sum+=__get_PSP();                /// 2084 bytes
        //sum+=__builtin_frame_address(0); /// 2084 bytes
    }
}

I agree that the use of volatile is a hack, but that's the best solution I
had found back when I wrote it.


We might consider moving the discussion off the list, since it no longer
AVR related.


Best regards,
Szikra Istvan

On Thu, Nov 9, 2017 at 10:26 AM, David Brown <[email protected]> wrote:

> On 09/11/17 04:57, Szikra Istvan wrote:
> > Hi
> >
> > Thanks for the SP, I missed that.
> > And apparently Atmel Studio also cannot find it, and underlines it with
> > red error marker.
> > It does compile, and I have found it in avr/common.h, it's probably a
> > problem with __AVR_ARCH__ handling by AS...
> > I guess that's what I get for trusting the IDE:)
> >
> > I know that just reading SP is not enough, I do also use stack
> > watermarking. It's just an additional diagnostic information.
> > Note: SP can also be used to re-mark the unused stack to trace stack
> > usage over time...
> > (or mark stack on platforms without .init* sections.)
> >
> > Note #999:
> > "the problem is in your code."
> > This isn't actually my code. My code was written for ARM and looked
> > something like this:
> > unsigned int GetStackPointer() {
> >     volatile register unsigned int sp asm("r13");
> >     return sp;
> > }
> > that someone ported to AVR, and now I'm fixing it... ;)
> >
>
> That code is wrong for the ARM too.  You should not fix a variable to an
> assembly register that is used specifically by the compiler.
>
> Depending on the ARM in question, you should probably use a CMSIS
> function like _get_PSP().  The standard definition for gcc and Cortex-M is:
>
> __attribute__( ( always_inline ) ) __STATIC_INLINE uint32_t __get_PSP(void)
> {
>   register uint32_t result;
>
>   __ASM volatile ("MRS %0, psp\n"  : "=r" (result) );
>   return(result);
> }
>
>
> Just to read the r13 register, the code would be something like this (I
> haven't checked the details here) :
>
> static inline uint32_t GetStackPointer(void)
> {
>         uint32_t sp;
>         asm volatile("mov %0, r13" : "=r" (sp));
>         return sp;
> }
>
>
> And you can probably just use __builtin_frame_address() - that should,
> in theory, work on the AVR and the ARM (I have not tested it on either
> target).
>
>
>
>

_______________________________________________
AVR-GCC-list mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/avr-gcc-list