From patchwork Fri Dec 6 19:38:31 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oleg Endo X-Patchwork-Id: 298233 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 7EF5A2C00A9 for ; Sat, 7 Dec 2013 06:39:13 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:to:date:in-reply-to:references :content-type:mime-version; q=dns; s=default; b=N1gcF+Gd5+D+9sjZ ThAXeVeLI2s37qNWOZxzw4YW+3Z2hnQT6VNi/O8u6ZkRZcKVe6mLRum9b5y+8zUc iZAdHh4xVNd43Q/38EcJR8PXSOXNwtfxh88RTdd4+sxxQjFcEib2sKyRW7qqSzba +tP2dEEgL40AEcAPXNK1V55U81E= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:to:date:in-reply-to:references :content-type:mime-version; s=default; bh=tIVeRrA1jswopXVWz4Ngb5 s9zy8=; b=n8SqhwRFQaFz5NJXvFBEE0WcLjNGBlv5xvfklRyMvoO8fAPw6RGNFZ 7h4RRHVxxInNeElTVpPptOXO35cwUDVILKVx9dBVPZKzMP8GeU8PhJcajrjGYBuC ruxR5yzQMQnuw+LoDxwXTpGCn0DyWIviRVf5PVLzisX0R9DnAbhdY= Received: (qmail 16527 invoked by alias); 6 Dec 2013 19:39:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 16517 invoked by uid 89); 6 Dec 2013 19:39:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, UNPARSEABLE_RELAY autolearn=ham version=3.3.2 X-HELO: mailout05.t-online.de Received: from Unknown (HELO mailout05.t-online.de) (194.25.134.82) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Dec 2013 19:39:02 +0000 Received: from fwd14.aul.t-online.de (fwd14.aul.t-online.de ) by mailout05.t-online.de with smtp id 1Vp1Ea-0004WK-Pr; Fri, 06 Dec 2013 20:38:45 +0100 Received: from [192.168.0.103] (V+oNWrZDZhwaeuJ9wW3rSAfNaRzSMrzpmKywu9V0+Q1D4cTpa7nc05sk7FhEOqtwKM@[87.155.245.172]) by fwd14.t-online.de with esmtp id 1Vp1EW-49ayXY0; Fri, 6 Dec 2013 20:38:40 +0100 Message-ID: <1386358711.14008.98.camel@yam-132-YW-E178-FTW> Subject: Re: [SH] Fix PR 58314 - Rework *movqi / *movhi patterns From: Oleg Endo To: gcc-patches Date: Fri, 06 Dec 2013 20:38:31 +0100 In-Reply-To: <1385464704.5048.26.camel@yam-132-YW-E178-FTW> References: <1385464704.5048.26.camel@yam-132-YW-E178-FTW> Mime-Version: 1.0 X-IsSubscribed: yes On Tue, 2013-11-26 at 12:18 +0100, Oleg Endo wrote: > 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. I've just committed the attached patch to the 4.8 branch as rev 205759. Tested with make -k check RUNTESTFLAGS="--target_board=sh-sim \{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}" Cheers, Oleg gcc/ChangeLog: Backport from mainline 2013-11-26 Oleg Endo 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_reg_reg): Remove it. (*movqi, *movhi): Merge both insns into... (*mov): ... this new insn. Replace generic 'm' constraints 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 205733) +++ 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 205734) +++ gcc/config/sh/sh.md (working copy) @@ -6831,34 +6831,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_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. %1,%0 - mov. %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_store_mem_disp04" @@ -6908,38 +6883,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" + [(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) + || arith_reg_operand (operands[1], mode))" "@ + 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")]) - -(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. %1,%0 + mov. %1,%0 + mov. %1,%0 + mov. %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 205733) +++ gcc/config/sh/predicates.md (working copy) @@ -389,6 +389,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") @@ -413,6 +419,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 205733) +++ gcc/config/sh/sh-protos.h (working copy) @@ -159,6 +159,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 205733) +++ gcc/config/sh/sh.c (working copy) @@ -310,9 +310,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&, @@ -3628,8 +3626,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 @@ -3665,8 +3663,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)); @@ -3703,12 +3701,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 @@ -10218,7 +10216,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. @@ -10404,7 +10402,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; @@ -13165,7 +13163,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,