|Summary:||Stack patterns for AVR not optimal|
|Product:||gcc||Reporter:||Andy Hutchinson <hutchinsonandy>|
|Component:||target||Assignee:||Richard Henderson <rth>|
|Severity:||minor||CC:||eric.weddington, gcc-bugs, gjl, rth|
|Build:||Known to work:|
|Known to fail:||Last reconfirmed:||2011-07-21 20:48:36|
|Bug Depends on:||49864|
Description Andy Hutchinson 2008-01-20 19:06:28 UTC
There are several instruction patterns related to stack pointer operations. These are not quite right: 1) popqi and poph1 patterns use post_inc codes - when in fact there are pre_inc - this could fail if gcc ever used them outside prolog/epilog 2) Stack moves such as push/pop should be placed before mov patterns, to provide best matching. 3)Stack adjustment (SP=SP+c) is matching with *addhi pattern, which causes reloads of output and input. We really want it to match "*addhi3_sp_R_pc2" when offset is small so *addhi3_sp_R_pc2 needs to be placed before *addhi 4)"*addhi3_sp_R_pc2" does not provide full range of optimal adjustment. For example, SP=SP+8 takes 4 instructions using rcall but around 8 using addition through register. This functionality needs extending accordingly as SP=SP+8 is very common.
Comment 1 Georg-Johann Lay 2011-07-20 15:36:44 UTC
Closed: This was fixed by Anatoly long ago: http://gcc.gnu.org/viewcvs?view=revision&revision=124854
Comment 2 Georg-Johann Lay 2011-07-21 20:48:36 UTC
Set to NEW again, I didn't read r124854 carefully enough :-/
Comment 3 Richard Henderson 2011-08-02 21:04:52 UTC
(1) was fixed at r171295. (2) is done. (3) is done. (4) is hopefully mostly irrelevant now, or shortly post PR49864. As of r177196 (PR49881), we should be generating many more PUSH insns than before, and queueing more stack space to be popped all at once. For the testcase mentioned in that PR, we seem to wait for ~40 bytes to get pushed, and pop them all at once.
Comment 4 Richard Henderson 2011-08-02 21:10:51 UTC
Created attachment 24897 [details] Optimize pop-all The proposed patch for PR49864 introduces REG_ARGS_SIZE. This records a (normally) non-negative number of bytes that have been pushed for arguments. Post-reload, if we see a (mov sp reg) insn that says that it's resetting the stack height, under a (common) set of conditions we can transform that to (mov sp fp) and let the previous (mov reg sp) (add reg N) insns be deleted as dead code. This reduces that common case to 4 insns instead of 9.
Comment 5 Richard Henderson 2011-08-03 22:57:25 UTC
Author: rth Date: Wed Aug 3 22:57:22 2011 New Revision: 177300 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=177300 Log: PR target/34888 * config/avr/avr.md: New splitter for REG_ARGS_SIZE 0. Modified: trunk/gcc/ChangeLog trunk/gcc/config/avr/avr.md
Comment 6 Richard Henderson 2011-08-03 22:58:20 UTC
Comment 7 Richard Henderson 2011-10-07 15:37:40 UTC
Re-open if you think there's still work to do.