Re: [patch,avr] PR70677: Use -fno-caller-saves for avr
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
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
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 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. > >