Re: [patch,avr] PR70677: Use -fno-caller-saves for avr

2016-08-02 Thread Senthil Kumar Selvaraj

Georg-Johann Lay writes:

> On 02.08.2016 06:50, Senthil Kumar Selvaraj wrote:
>>
>> Denis Chertykov writes:
>>
>>> 2016-08-01 15:17 GMT+03:00 Georg-Johann Lay :
 Problem with -fcaller-saves is that there are situations where it triggers
 an expensive frame just to store a variable around a function call even
 though there are plenty of call-saved registers.

 Example:

 typedef __UINT8_TYPE__ uint8_t;

 extern uint8_t uart0_getc (void);

 void foo (uint8_t *buffer, uint8_t cnt)
 {
   while (--cnt)
 {
   *buffer++ = uart0_getc();
 }
 }

 $ avr-gcc -Os -S -dp -mmcu=atmega8 loop-buf.c

 $ avr-gcc gcc -B$TV -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c &&
 avr-size loop-buf.o
textdata bss dec hex filename
  50   0   0  50  32 loop-buf.o

 $ avr-gcc -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c -fno-caller-saves
 && avr-size loop-buf.o
textdata bss dec hex filename
  32   0   0  32  20 loop-buf.o

 I actually came never across a situation where -fcaller-saves improved the
 code performance, hence this patch proposes to switch off -fcaller-saved 
 per
 default.
>>
>> Like you mentioned in the bug report, would fixing the costs be a better
>> way to fix this rather than a blanket disabling of the option?
>
> What costs specifically?  Where could the costs of different epilogues / 
> prologues be described?

I don't know either - I'd have to dig in to find that out too. Let me
check that out.

Regards
Senthil


Re: [patch,avr] PR70677: Use -fno-caller-saves for avr

2016-08-02 Thread Georg-Johann Lay

On 02.08.2016 06:50, Senthil Kumar Selvaraj wrote:


Denis Chertykov writes:


2016-08-01 15:17 GMT+03:00 Georg-Johann Lay :

Problem with -fcaller-saves is that there are situations where it triggers
an expensive frame just to store a variable around a function call even
though there are plenty of call-saved registers.

Example:

typedef __UINT8_TYPE__ uint8_t;

extern uint8_t uart0_getc (void);

void foo (uint8_t *buffer, uint8_t cnt)
{
  while (--cnt)
{
  *buffer++ = uart0_getc();
}
}

$ avr-gcc -Os -S -dp -mmcu=atmega8 loop-buf.c

$ avr-gcc gcc -B$TV -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c &&
avr-size loop-buf.o
   textdata bss dec hex filename
 50   0   0  50  32 loop-buf.o

$ avr-gcc -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c -fno-caller-saves
&& avr-size loop-buf.o
   textdata bss dec hex filename
 32   0   0  32  20 loop-buf.o

I actually came never across a situation where -fcaller-saves improved the
code performance, hence this patch proposes to switch off -fcaller-saved per
default.


Like you mentioned in the bug report, would fixing the costs be a better
way to fix this rather than a blanket disabling of the option?


What costs specifically?  Where could the costs of different epilogues / 
prologues be described?


Johann



Regards
Senthil





Re: [patch,avr] PR70677: Use -fno-caller-saves for avr

2016-08-01 Thread Senthil Kumar Selvaraj

Denis Chertykov writes:

> 2016-08-01 15:17 GMT+03:00 Georg-Johann Lay :
>> Problem with -fcaller-saves is that there are situations where it triggers
>> an expensive frame just to store a variable around a function call even
>> though there are plenty of call-saved registers.
>>
>> Example:
>>
>> typedef __UINT8_TYPE__ uint8_t;
>>
>> extern uint8_t uart0_getc (void);
>>
>> void foo (uint8_t *buffer, uint8_t cnt)
>> {
>>   while (--cnt)
>> {
>>   *buffer++ = uart0_getc();
>> }
>> }
>>
>> $ avr-gcc -Os -S -dp -mmcu=atmega8 loop-buf.c
>>
>> $ avr-gcc gcc -B$TV -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c &&
>> avr-size loop-buf.o
>>textdata bss dec hex filename
>>  50   0   0  50  32 loop-buf.o
>>
>> $ avr-gcc -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c -fno-caller-saves
>> && avr-size loop-buf.o
>>textdata bss dec hex filename
>>  32   0   0  32  20 loop-buf.o
>>
>> I actually came never across a situation where -fcaller-saves improved the
>> code performance, hence this patch proposes to switch off -fcaller-saved per
>> default.

Like you mentioned in the bug report, would fixing the costs be a better
way to fix this rather than a blanket disabling of the option?

Regards
Senthil


Re: [patch,avr] PR70677: Use -fno-caller-saves for avr

2016-08-01 Thread Denis Chertykov
2016-08-01 15:17 GMT+03:00 Georg-Johann Lay :
> Problem with -fcaller-saves is that there are situations where it triggers
> an expensive frame just to store a variable around a function call even
> though there are plenty of call-saved registers.
>
> Example:
>
> typedef __UINT8_TYPE__ uint8_t;
>
> extern uint8_t uart0_getc (void);
>
> void foo (uint8_t *buffer, uint8_t cnt)
> {
>   while (--cnt)
> {
>   *buffer++ = uart0_getc();
> }
> }
>
> $ avr-gcc -Os -S -dp -mmcu=atmega8 loop-buf.c
>
> $ avr-gcc gcc -B$TV -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c &&
> avr-size loop-buf.o
>textdata bss dec hex filename
>  50   0   0  50  32 loop-buf.o
>
> $ avr-gcc -Os -c -save-temps -dp -mmcu=atmega8 loop-buf.c -fno-caller-saves
> && avr-size loop-buf.o
>textdata bss dec hex filename
>  32   0   0  32  20 loop-buf.o
>
> I actually came never across a situation where -fcaller-saves improved the
> code performance, hence this patch proposes to switch off -fcaller-saved per
> default.
>
> I can test the patch without regressions, but what bothers me is the
> following lines in ira-color.c:allocno_reload_assign()
>
>   if (ALLOCNO_CALLS_CROSSED_NUM (a) != 0
>   && ira_hard_reg_set_intersection_p (hard_regno, ALLOCNO_MODE (a),
>   call_used_reg_set))
> {
>   ira_assert (flag_caller_saves);
>   caller_save_needed = 1;
> }
>
> What's not clear is whether this assertion is about the inner working of IRA
> as alloc depends on caller-saves in other places of IRA, or if caller-saves
> is needed because otherwise IRA cannot resolve complicated reload situations
> and hence the proposed change might trigger ICEs for complex programs.
>
> Therefore CCed Vladimir who added the assertion to IRA.
>
> Ok to apply if IRA can do without caller-saves?


Ok.



>
> Johann
>
>
> PR 70677
> * common/config/avr/avr-common.c (avr_option_optimization_table)
> [OPT_LEVELS_ALL]: Turn off -fcaller-saves.
>
>