[Bug target/54602] [SH] Register pop insn not put in rts delay slot

["olegendo at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54602

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu.org

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-10 00:43:39 UTC ---
I was right after all.  The checks in (define_delay (eq_attr "type" "return") 
somehow got lost in parenthesis and the "*movsi_pop" insn is not required after
all.  The following fixes the issue:

Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md    (revision 192200)
+++ gcc/config/sh/sh.md    (working copy)
@@ -541,22 +541,22 @@
   (eq_attr "needs_delay_slot" "yes")
   [(eq_attr "in_delay_slot" "yes") (nil) (nil)])

+;; Since a normal return (rts) implicitly uses the PR register,
+;; we can't allow PR register loads in an rts delay slot.
 ;; On the SH and SH2, the rte instruction reads the return pc from the stack,
 ;; and thus we can't put a pop instruction in its delay slot.
 ;; On the SH3 and SH4, the rte instruction does not use the stack, so a pop
-;; instruction can go in the delay slot.
-;; Since a normal return (rts) implicitly uses the PR register,
-;; we can't allow PR register loads in an rts delay slot.
+;; instruction can go in the delay slot, unless it references a banked
+;; register (the register bank is switched by rte).
 (define_delay
   (eq_attr "type" "return")
   [(and (eq_attr "in_delay_slot" "yes")
     (ior (and (eq_attr "interrupt_function" "no")
           (eq_attr "type" "!pload,prset"))
          (and (eq_attr "interrupt_function" "yes")
-          (ior
-           (not (match_test "TARGET_SH3"))
-           (eq_attr "hit_stack" "no")
-           (eq_attr "banked" "no"))))) (nil) (nil)])
+          (ior (match_test "TARGET_SH3") (eq_attr "hit_stack" "no"))
+          (eq_attr "banked" "no"))))
+   (nil) (nil)])

 ;; Since a call implicitly uses the PR register, we can't allow
 ;; a PR register store in a jsr delay slot.
@@ -6186,21 +6186,6 @@
   emit_insn (gen_ashlsi3 (operands[5], operands[0], operands[2]));
 })

-;; Define additional pop for SH1 and SH2 so it does not get 
-;; placed in the delay slot.
-(define_insn "*movsi_pop"
-  [(set (match_operand:SI 0 "register_operand" "=r,x,l")
-        (match_operand:SI 1 "sh_no_delay_pop_operand" ">,>,>"))]
-  "(TARGET_SH1 || TARGET_SH2E || TARGET_SH2A)
-   && ! TARGET_SH3"
-  "@
-    mov.l    %1,%0
-    lds.l    %1,%0
-    lds.l    %1,%0"
-  [(set_attr "type" "load_si,mem_mac,pload")
-   (set_attr "length" "2,2,2")
-   (set_attr "in_delay_slot" "no,no,no")])
-
 ;; t/r must come after r/r, lest reload will try to reload stuff like
 ;; (set (subreg:SI (mem:QI (plus:SI (reg:SI SP_REG) (const_int 12)) 0) 0)
 ;; (made from (set (subreg:SI (reg:QI ###) 0) ) into T.


Kaz, could you please also have a pre-look at this?  I might be missing
something...

Also, I've noticed that on SH4 (which has banked regs R0..R7) the banked regs
are also saved / restored in an interrupt function.  This actually defeats the
purpose of the R0..R7 register bank.  Maybe some historic reason, or just
accident?