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...
