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

[Boyapati, Anitha]

>-----Original Message-----
>From: Georg-Johann Lay [mailto:address@hidden
>Sent: Thursday, October 13, 2011 10:14 PM
>To: Boyapati, Anitha
>Cc: address@hidden
>Subject: Re: [avr-gcc-list] [AVR] RTL prologue/epilogue ver.3
>
>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.


I am using gcc 4.5.1. I will just check the higher versions and trunk.

>
>> 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.

I had not given a thought about these cases. As the follow-up
conversations suggested, DWARF info should suffice.


Thanks for the suggestions.



Anitha