Re: [avr-gcc-list] [AVR] RTL prologue/epilogue ver.3

[Georg-Johann Lay]

Boyapati, Anitha schrieb:
> Hi All,
> 
> I have couple of comments on this patch, which I think are important
> discussing with the avr-gcc community.
> 
> Comment - 1: 
> 
> Has any one verified prologue for functions using global variables? I
> think this patch generates incorrect prologue for such cases.
> 
> int global;
> void func(int p)
> {
>  p=global++;
> }
> 
> int main()
> {
>    int local=42;
>    func(local);
> }
> 
> 
> Code Generated for func() -
> 
> 00000228 <func>:
>  228:   cf 93           push    r28
>  22a:   df 93           push    r29
> 
>  22c:   00 d0           rcall   .+0             ; 0x22e <func+0x6>
>  22e:   0f 92           push    r0
> <<<<
> 
>  230:   cd b7           in      r28, 0x3d       ; FP = SP
>  232:   de b7           in      r29, 0x3e       
>  234:   8b 83           std     Y+3, r24        ; store 'local' to 'p'
>  236:   9c 83           std     Y+4, r25         
>  ...
> 
> Read the code between ">>>>  <<<<" and notice that 3/4 bytes (3 bytes
> for Megas, 4 for Xmegas) have been allocated on stack instead of 2.

I don't see that for trunk. The following test case

void func (int);

void bar (void)
{
    int volatile local = 42;
    func (local);
}

generates prologue with -Os -mmcu=avr6:

        push r28         ;  15  pushqi1/1       [length = 1]
        push r29         ;  16  pushqi1/1       [length = 1]
         ; SP -= 2       ;  20  *addhi3_sp_R    [length = 2]
        push __zero_reg__
        push __zero_reg__
        in r28,__SP_L__  ;  21  *movhi/8        [length = 2]
        in r29,__SP_H__
        /* prologue: function */
        /* frame size = 2 */
        /* stack size = 4 */

and with -Os -mmcu=avr4:

        push r28         ;  15  pushqi1/1       [length = 1]
        push r29         ;  16  pushqi1/1       [length = 1]
         ; SP -= 2       ;  20  *addhi3_sp_R    [length = 1]
        rcall .
        in r28,__SP_L__  ;  21  *movhi/8        [length = 2]
        in r29,__SP_H__
        /* prologue: function */
        /* frame size = 2 */
        /* stack size = 4 */

So it's as expected. Likewise without volatile + -O0.

> Comment -2:
> 
> I think this patch severely affects the step in/step out feature of
> debugger like AVR Studio (here after referred to as AS). AS depends on
> 'rcall'/'ret' instruction sequences to recognize a function call and
> step into. If a matching 'ret' is not found, the debugger gets confused.
> Currently, the problem is worked around but has much lower performance.

The debugger has to cope with that. There are two ways of doing that:

1) Analysing the prologue and knowing what sequences the compiler might
   generate.

2) Using debug information. avr-gcc attaches respective FRAME_RELATED
   debug information (don't know what the dwarf equivalent is) to
   respective prologue instructions.

> Also it is misleading to use 'rcall' for purposes it is not meant for
> (like stack allocation). In my opinion, the hack is not desirable by
> default. I would therefore recommend making it optional using a switch
> like 'mrcall'.
> 
> Thoughts?

You will still run into trouble because there may be function without return
(no-return functions or tail-calls) or functions with multiple RETs.

And I don't see a point in making prologue generation even more complex to
support non-smartness of some tool.

> 
> Anitha
> 

I am completely in line with Eric: Your debugger will have to get smarter and
not the compiler get dumb again.

Johann