| Message ID | 4E39D339.1030008@redhat.com |
|---|---|
| State | New |
| Headers | show |
2011/8/4 Richard Henderson <rth@redhat.com>: > When a frame pointer is in use, we can optimize popping all > queued parameters via a simple move from the frame pointer > instead of an addition to the stack pointer. > > The new sequence is 4 insns, the old sequence was 9 insns. > > Committed. It seems strange for me: +;; Notice a special-case when adding N to SP where N results in a +;; zero REG_ARGS_SIZE. This is equivalent to a move from FP. +(define_split + [(set (reg:HI REG_SP) (match_operand:HI 0 "register_operand" ""))] + "reload_completed + && frame_pointer_needed + && !cfun->calls_alloca + && find_reg_note (insn, REG_ARGS_SIZE, const0_rtx)" + [(set (reg:HI REG_SP) (reg:HI REG_Y))] + "") What is it ? ... It's a transition from SP = general-register to SP = REG_Y with set of conditions. Generally, it's seems wrong (SP = REG) isn't equal to (SP = REG_Y). Denis.
Richard Henderson wrote: > When a frame pointer is in use, we can optimize popping all > queued parameters via a simple move from the frame pointer > instead of an addition to the stack pointer. > > The new sequence is 4 insns, the old sequence was 9 insns. > > Committed. > > r~ 4 insns is odd. You cannot move atomically to SP and the sequence finally emit should be something like fiddling with IRQ-Flag. *l = 5; return (AS2 (in,__tmp_reg__,__SREG__) CR_TAB "cli" CR_TAB AS2 (out,__SP_H__,%B1) CR_TAB AS2 (out,__SREG__,__tmp_reg__) CR_TAB AS2 (out,__SP_L__,%A1)); Johann
On 08/03/2011 11:09 PM, Denis Chertykov wrote: > 2011/8/4 Richard Henderson <rth@redhat.com>: >> When a frame pointer is in use, we can optimize popping all >> queued parameters via a simple move from the frame pointer >> instead of an addition to the stack pointer. >> >> The new sequence is 4 insns, the old sequence was 9 insns. >> >> Committed. > > It seems strange for me: > +;; Notice a special-case when adding N to SP where N results in a > +;; zero REG_ARGS_SIZE. This is equivalent to a move from FP. > +(define_split > + [(set (reg:HI REG_SP) (match_operand:HI 0 "register_operand" ""))] > + "reload_completed > + && frame_pointer_needed > + && !cfun->calls_alloca > + && find_reg_note (insn, REG_ARGS_SIZE, const0_rtx)" > + [(set (reg:HI REG_SP) (reg:HI REG_Y))] > + "") > > What is it ? ... It's a transition from SP = general-register to > SP = REG_Y with set of conditions. > Generally, it's seems wrong (SP = REG) isn't equal to (SP = REG_Y). The old sequence is (set tmp SP) (set tmp (plus tmp const_int)) (set SP tmp) Because of the REG_ARGS_SIZE note being 0, we know that this is popping all arguments off the stack. The other conditions, frame pointer existing, and no calls to alloca, mean that we know exactly what the result of the addition is -- the contents of FP. So we transform to (set tmp SP) (set tmp (plus tmp const_int)) (set SP FP) and let the first two insns be deleted as dead code. r~
2011/8/4 Richard Henderson <rth@redhat.com>: > On 08/03/2011 11:09 PM, Denis Chertykov wrote: >> 2011/8/4 Richard Henderson <rth@redhat.com>: >>> When a frame pointer is in use, we can optimize popping all >>> queued parameters via a simple move from the frame pointer >>> instead of an addition to the stack pointer. >>> >>> The new sequence is 4 insns, the old sequence was 9 insns. >>> >>> Committed. >> >> It seems strange for me: >> +;; Notice a special-case when adding N to SP where N results in a >> +;; zero REG_ARGS_SIZE. This is equivalent to a move from FP. >> +(define_split >> + [(set (reg:HI REG_SP) (match_operand:HI 0 "register_operand" ""))] >> + "reload_completed >> + && frame_pointer_needed >> + && !cfun->calls_alloca >> + && find_reg_note (insn, REG_ARGS_SIZE, const0_rtx)" >> + [(set (reg:HI REG_SP) (reg:HI REG_Y))] >> + "") >> >> What is it ? ... It's a transition from SP = general-register to >> SP = REG_Y with set of conditions. >> Generally, it's seems wrong (SP = REG) isn't equal to (SP = REG_Y). > > The old sequence is > > (set tmp SP) > (set tmp (plus tmp const_int)) > (set SP tmp) > > Because of the REG_ARGS_SIZE note being 0, we know that > this is popping all arguments off the stack. > > The other conditions, frame pointer existing, and no > calls to alloca, mean that we know exactly what the > result of the addition is -- the contents of FP. > > So we transform to > > (set tmp SP) > (set tmp (plus tmp const_int)) > (set SP FP) > > and let the first two insns be deleted as dead code. Thank you for explanation. I have a very clean understanding of whole picture. May be better to use define_peephole2 with 3 insns as input and 1 as output for easy understanding. Denis.
On 08/04/2011 09:04 AM, Denis Chertykov wrote: > Thank you for explanation. > I have a very clean understanding of whole picture. > May be better to use define_peephole2 with 3 insns as input and 1 as > output for easy understanding. *shrug* Maybe. Then you also have to check for whether TMP is dead. It just seemed tidier to me to let DCE do its job. r~
2011/8/4 Richard Henderson <rth@redhat.com>: > On 08/04/2011 09:04 AM, Denis Chertykov wrote: >> Thank you for explanation. >> I have a very clean understanding of whole picture. >> May be better to use define_peephole2 with 3 insns as input and 1 as >> output for easy understanding. > > *shrug* Maybe. Then you also have to check for whether TMP > is dead. It just seemed tidier to me to let DCE do its job. Ok. Denis.
Richard Henderson wrote: > On 08/04/2011 09:04 AM, Denis Chertykov wrote: >> Thank you for explanation. >> I have a very clean understanding of whole picture. >> May be better to use define_peephole2 with 3 insns as input and 1 as >> output for easy understanding. > > *shrug* Maybe. Then you also have to check for whether TMP > is dead. It just seemed tidier to me to let DCE do its job. > > > r~ If it's just for understanding, simply add comments! Comments in the source explaining what's going on is always appreciated. Johann
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md index b8560df..b5aa73c 100644 --- a/gcc/config/avr/avr.md +++ b/gcc/config/avr/avr.md @@ -235,6 +235,17 @@ DONE; }) +;; Notice a special-case when adding N to SP where N results in a +;; zero REG_ARGS_SIZE. This is equivalent to a move from FP. +(define_split + [(set (reg:HI REG_SP) (match_operand:HI 0 "register_operand" ""))] + "reload_completed + && frame_pointer_needed + && !cfun->calls_alloca + && find_reg_note (insn, REG_ARGS_SIZE, const0_rtx)" + [(set (reg:HI REG_SP) (reg:HI REG_Y))] + "") + ;;======================================================================== ;; move byte ;; The last alternative (any immediate constant to any register) is