[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

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

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

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

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

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to chrbr from comment #5)
> Oleg, I think it time to re-unify those. Doing an experimental resurrection
> of the r,r reload constraints seems to fix it, but without knowing the
> impacts on your T-bit combine optimizations...

OK, I've tried your suggestion by doing ...


Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md    (revision 205190)
+++ gcc/config/sh/sh.md    (working copy)
@@ -6993,34 +6993,6 @@
   prepare_move_operands (operands, QImode);
 })

-;; If movqi_reg_reg is specified as an alternative of movqi, movqi will be
-;; selected to copy QImode regs.  If one of them happens to be allocated
-;; on the stack, reload will stick to movqi insn and generate wrong
-;; displacement addressing because of the generic m alternatives.
-;; With the movqi_reg_reg being specified before movqi it will be initially
-;; picked to load/store regs.  If the regs regs are on the stack reload
-;; try other insns and not stick to movqi_reg_reg, unless there were spilled
-;; pseudos in which case 'm' constraints pertain.
-;; The same applies to the movhi variants.
-;;
-;; Notice, that T bit is not allowed as a mov src operand here.  This is to
-;; avoid things like (set (reg:QI) (subreg:QI (reg:SI T_REG) 0)), which
-;; introduces zero extensions after T bit stores and redundant reg copies.
-;;
-;; FIXME: We can't use 'arith_reg_operand' (which disallows T_REG) as a
-;; predicate for the mov src operand because reload will have trouble
-;; reloading MAC subregs otherwise.  For that probably special patterns
-;; would be required.
-(define_insn "*mov<mode>_reg_reg"
-  [(set (match_operand:QIHI 0 "arith_reg_dest" "=r,m,*z")
-    (match_operand:QIHI 1 "register_operand" "r,*z,m"))]
-  "TARGET_SH1 && !t_reg_operand (operands[1], VOIDmode)"
-  "@
-    mov    %1,%0
-    mov.<bw>    %1,%0
-    mov.<bw>    %1,%0"
-  [(set_attr "type" "move,store,load")])
-
 ;; FIXME: The non-SH2A and SH2A variants should be combined by adding
 ;; "enabled" attribute as it is done in other targets.
 (define_insn "*mov<mode>_store_mem_disp04"
@@ -7075,33 +7047,35 @@
 ;; displacement addressing modes on anything but SH2A.  That's why the
 ;; specialized load/store insns are specified above.
 (define_insn "*movqi"
-  [(set (match_operand:QI 0 "general_movdst_operand" "=r,r,m,r,l")
-    (match_operand:QI 1 "general_movsrc_operand"  "i,m,r,l,r"))]
+  [(set (match_operand:QI 0 "general_movdst_operand" "=r,r,r,m,r,l")
+    (match_operand:QI 1 "general_movsrc_operand"  "r,i,m,r,l,r"))]
   "TARGET_SH1
    && (arith_reg_operand (operands[0], QImode)
        || arith_reg_operand (operands[1], QImode))"
   "@
     mov    %1,%0
+    mov    %1,%0
     mov.b    %1,%0
     mov.b    %1,%0
     sts    %1,%0
     lds    %1,%0"
- [(set_attr "type" "movi8,load,store,prget,prset")])
+ [(set_attr "type" "move,movi8,load,store,prget,prset")])

 (define_insn "*movhi"
-  [(set (match_operand:HI 0 "general_movdst_operand" "=r,r,r,m,r,l")
-    (match_operand:HI 1 "general_movsrc_operand"  "Q,i,m,r,l,r"))]
+  [(set (match_operand:HI 0 "general_movdst_operand" "=r,r,r,r,m,r,l")
+    (match_operand:HI 1 "general_movsrc_operand"  "r,Q,i,m,r,l,r"))]
   "TARGET_SH1
    && (arith_reg_operand (operands[0], HImode)
        || arith_reg_operand (operands[1], HImode))"
   "@
+    mov    %1,%0
     mov.w    %1,%0
     mov    %1,%0
     mov.w    %1,%0
     mov.w    %1,%0
     sts    %1,%0
     lds    %1,%0"
- [(set_attr "type" "pcload,movi8,load,store,prget,prset")])
+ [(set_attr "type" "move,pcload,movi8,load,store,prget,prset")])

 (define_insn "*movqi_media"
   [(set (match_operand:QI 0 "general_movdst_operand" "=r,r,r,m")


... and it fixes the problem.  The CSiBE set doesn't show any size differences
for SH4, which is a good sign.

make -k check-gcc RUNTESTFLAGS="sh.exp
--target_board=sh-sim\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

also doesn't report new failures.

However, I'm still anxious regarding the comment that reload will generate
wrong displacement addresses because of the generic 'm' alternatives in the
*movqi and *movhi insns.  Maybe the additional reg_reg pattern was a wallpaper
fix after all and is not required anymore.

I'm testing the above patch now.

Kaz, could you please run the above patch through your test setup?  I remember
there were some issues triggered by some fortran code in the test suite...