Hello,
The attached patch is the same as posted in the PR as attachment 31283.
In addition to the testing done by Kaz and Christian, I've also tested
it with
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"
on rev 205313 with no new failures.
OK for trunk?
The additional test case from PR 58314 will follow.
I'll also post a version of the patch for 4.8 after testing.
Cheers,
Oleg
gcc/ChangeLog:
PR target/58314
PR target/50751
* config/sh/sh.c (max_mov_insn_displacement,
disp_addr_displacement): Prefix function names with 'sh_'.
Make them non-static.
* config/sh/sh-protos.h (sh_disp_addr_displacement,
sh_max_mov_insn_displacement): Add declarations.
* config/sh/constraints.md (Q): Reject QImode.
(Sdd): Use match_code "mem".
(Snd): Fix erroneous matching of non-memory operands.
* config/sh/predicates.md (short_displacement_mem_operand): New
predicate.
(general_movsrc_operand): Disallow PC relative QImode loads.
* config/sh/sh.md (*mov<mode>_reg_reg): Remove it.
(*movqi, *movhi): Merge both insns into...
(*mov<mode>): ... this new insn. Replace generic 'm'
constraint with 'Snd' and 'Sdd' constraints. Calculate insn
length dynamically based on the operand types.
Index: gcc/config/sh/constraints.md
===================================================================
--- gcc/config/sh/constraints.md (revision 205190)
+++ gcc/config/sh/constraints.md (working copy)
@@ -221,6 +221,7 @@
(define_constraint "Q"
"A pc relative load operand."
(and (match_code "mem")
+ (match_test "GET_MODE (op) != QImode")
(match_test "IS_PC_RELATIVE_LOAD_ADDR_P (XEXP (op, 0))")))
(define_constraint "Bsc"
@@ -295,13 +296,15 @@
(define_memory_constraint "Sdd"
"A memory reference that uses displacement addressing."
- (and (match_test "MEM_P (op) && GET_CODE (XEXP (op, 0)) == PLUS")
+ (and (match_code "mem")
+ (match_test "GET_CODE (XEXP (op, 0)) == PLUS")
(match_test "REG_P (XEXP (XEXP (op, 0), 0))")
(match_test "CONST_INT_P (XEXP (XEXP (op, 0), 1))")))
(define_memory_constraint "Snd"
"A memory reference that excludes displacement addressing."
- (match_test "! satisfies_constraint_Sdd (op)"))
+ (and (match_code "mem")
+ (match_test "! satisfies_constraint_Sdd (op)")))
(define_memory_constraint "Sbv"
"A memory reference, as used in SH2A bclr.b, bset.b, etc."
Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md (revision 205190)
+++ gcc/config/sh/sh.md (working copy)
@@ -6993,34 +6993,9 @@
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")])
-
+;; Specifying the displacement addressing load / store patterns separately
+;; before the generic movqi / movhi pattern allows controlling the order
+;; in which load / store insns are selected in a more fine grained way.
;; 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"
@@ -7070,38 +7045,44 @@
[(set_attr "type" "load")
(set_attr "length" "2,2,4")])
-;; The m constraints basically allow any kind of addresses to be used with any
-;; source/target register as the other operand. This is not true for
-;; 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"))]
+;; The order of the constraint alternatives is important here.
+;; Q/r has to come first, otherwise PC relative loads might wrongly get
+;; placed into delay slots. Since there is no QImode PC relative load, the
+;; Q constraint and general_movsrc_operand will reject it for QImode.
+;; The Snd alternatives should come before Sdd in order to avoid a preference
+;; of using r0 als the register operand for addressing modes other than
+;; displacement addressing.
+;; The Sdd alternatives allow only r0 as register operand, even though on
+;; SH2A any register could be allowed by switching to a 32 bit insn.
+;; Generally sticking to the r0 is preferrable, since it generates smaller
+;; code. Obvious r0 reloads can then be eliminated with a peephole on SH2A.
+(define_insn "*mov<mode>"
+ [(set (match_operand:QIHI 0 "general_movdst_operand"
+ "=r,r,r,Snd,r, Sdd,z, r,l")
+ (match_operand:QIHI 1 "general_movsrc_operand"
+ "Q,r,i,r, Snd,z, Sdd,l,r"))]
"TARGET_SH1
- && (arith_reg_operand (operands[0], QImode)
- || arith_reg_operand (operands[1], QImode))"
+ && (arith_reg_operand (operands[0], <MODE>mode)
+ || arith_reg_operand (operands[1], <MODE>mode))"
"@
+ mov.<bw> %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")])
-
-(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"))]
- "TARGET_SH1
- && (arith_reg_operand (operands[0], HImode)
- || arith_reg_operand (operands[1], HImode))"
- "@
- mov.w %1,%0
mov %1,%0
- mov.w %1,%0
- mov.w %1,%0
+ mov.<bw> %1,%0
+ mov.<bw> %1,%0
+ mov.<bw> %1,%0
+ mov.<bw> %1,%0
sts %1,%0
lds %1,%0"
- [(set_attr "type" "pcload,movi8,load,store,prget,prset")])
+ [(set_attr "type" "pcload,move,movi8,store,load,store,load,prget,prset")
+ (set (attr "length")
+ (cond [(and (match_operand 0 "displacement_mem_operand")
+ (not (match_operand 0 "short_displacement_mem_operand")))
+ (const_int 4)
+ (and (match_operand 1 "displacement_mem_operand")
+ (not (match_operand 1 "short_displacement_mem_operand")))
+ (const_int 4)]
+ (const_int 2)))])
(define_insn "*movqi_media"
[(set (match_operand:QI 0 "general_movdst_operand" "=r,r,r,m")
Index: gcc/config/sh/predicates.md
===================================================================
--- gcc/config/sh/predicates.md (revision 205190)
+++ gcc/config/sh/predicates.md (working copy)
@@ -421,6 +421,12 @@
XEXP (XEXP (op, 0), 1),
TARGET_SH2A, true)")))
+;; Returns true if OP is a displacement address that can fit into a
+;; 16 bit (non-SH2A) memory load / store insn.
+(define_predicate "short_displacement_mem_operand"
+ (match_test "sh_disp_addr_displacement (op)
+ <= sh_max_mov_insn_displacement (GET_MODE (op), false)"))
+
;; Returns 1 if the operand can be used in an SH2A movu.{b|w} insn.
(define_predicate "zero_extend_movu_operand"
(and (match_operand 0 "displacement_mem_operand")
@@ -445,6 +451,11 @@
if (t_reg_operand (op, mode))
return 0;
+ /* Disallow PC relative QImode loads, since these is no insn to do that
+ and an imm8 load should be used instead. */
+ if (IS_PC_RELATIVE_LOAD_ADDR_P (op) && GET_MODE (op) == QImode)
+ return false;
+
if (MEM_P (op))
{
rtx inside = XEXP (op, 0);
Index: gcc/config/sh/sh-protos.h
===================================================================
--- gcc/config/sh/sh-protos.h (revision 205190)
+++ gcc/config/sh/sh-protos.h (working copy)
@@ -162,6 +162,8 @@
extern bool sh_cfun_trap_exit_p (void);
extern rtx sh_find_equiv_gbr_addr (rtx cur_insn, rtx mem);
extern int sh_eval_treg_value (rtx op);
+extern HOST_WIDE_INT sh_disp_addr_displacement (rtx mem_op);
+extern int sh_max_mov_insn_displacement (machine_mode mode, bool consider_sh2a);
/* Result value of sh_find_set_of_reg. */
struct set_of_reg
Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c (revision 205191)
+++ gcc/config/sh/sh.c (working copy)
@@ -308,9 +308,7 @@
static void sh_conditional_register_usage (void);
static bool sh_legitimate_constant_p (enum machine_mode, rtx);
static int mov_insn_size (enum machine_mode, bool);
-static int max_mov_insn_displacement (enum machine_mode, bool);
static int mov_insn_alignment_mask (enum machine_mode, bool);
-static HOST_WIDE_INT disp_addr_displacement (rtx);
static bool sequence_insn_p (rtx);
static void sh_canonicalize_comparison (int *, rtx *, rtx *, bool);
static void sh_canonicalize_comparison (enum rtx_code&, rtx&, rtx&,
@@ -3574,8 +3572,8 @@
/* Determine the maximum possible displacement for a move insn for the
specified mode. */
-static int
-max_mov_insn_displacement (enum machine_mode mode, bool consider_sh2a)
+int
+sh_max_mov_insn_displacement (machine_mode mode, bool consider_sh2a)
{
/* The 4 byte displacement move insns are the same as the 2 byte
versions but take a 12 bit displacement. All we need to do is to
@@ -3611,8 +3609,8 @@
}
/* Return the displacement value of a displacement address. */
-static inline HOST_WIDE_INT
-disp_addr_displacement (rtx x)
+HOST_WIDE_INT
+sh_disp_addr_displacement (rtx x)
{
gcc_assert (satisfies_constraint_Sdd (x));
return INTVAL (XEXP (XEXP (x, 0), 1));
@@ -3649,12 +3647,12 @@
HImode and QImode loads/stores with displacement put pressure on
R0 which will most likely require another reg copy. Thus account
a higher cost for that. */
- if (offset > 0 && offset <= max_mov_insn_displacement (mode, false))
+ if (offset > 0 && offset <= sh_max_mov_insn_displacement (mode, false))
return (mode == HImode || mode == QImode) ? 2 : 1;
/* The displacement would fit into a 4 byte move insn (SH2A). */
if (TARGET_SH2A
- && offset > 0 && offset <= max_mov_insn_displacement (mode, true))
+ && offset > 0 && offset <= sh_max_mov_insn_displacement (mode, true))
return 2;
/* The displacement is probably out of range and will require extra
@@ -10167,7 +10165,7 @@
else
{
const HOST_WIDE_INT offset = INTVAL (op);
- const int max_disp = max_mov_insn_displacement (mode, consider_sh2a);
+ const int max_disp = sh_max_mov_insn_displacement (mode, consider_sh2a);
const int align_mask = mov_insn_alignment_mask (mode, consider_sh2a);
/* If the mode does not support any displacement always return false.
@@ -10353,7 +10351,7 @@
effectively disable the small displacement insns. */
const int mode_sz = GET_MODE_SIZE (mode);
const int mov_insn_sz = mov_insn_size (mode, false);
- const int max_disp = max_mov_insn_displacement (mode, false);
+ const int max_disp = sh_max_mov_insn_displacement (mode, false);
const int max_disp_next = max_disp + mov_insn_sz;
HOST_WIDE_INT align_modifier = offset > 127 ? mov_insn_sz : 0;
HOST_WIDE_INT offset_adjust;
@@ -13113,7 +13111,8 @@
the insns must have the appropriate alternatives. */
if ((mode == QImode || mode == HImode) && rclass != R0_REGS
&& satisfies_constraint_Sdd (x)
- && disp_addr_displacement (x) <= max_mov_insn_displacement (mode, false))
+ && sh_disp_addr_displacement (x)
+ <= sh_max_mov_insn_displacement (mode, false))
return R0_REGS;
/* When reload is trying to address a QImode or HImode subreg on the stack,
