From patchwork Wed Apr 23 15:33:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robert Suchanek X-Patchwork-Id: 341905 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)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 575541400FB for ; Thu, 24 Apr 2014 01:33:22 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:content-transfer-encoding:mime-version; q=dns; s= default; b=khn6Lo9anIp9hQkFVX0TsWTVtB1Ff7eJDSdEmnUFOcwYrkMnyKE9W aNzuQyPvVhe7/tvlNrM+Gh0ht2NuLp/QERTiWPCS7NVVXrcjrcQ8xWx53v9zA1oD ye3KgFNXmdoO8wXwGHTyzPOQ3DS0WXk8csCDkGjIZ+f3NT7MOvAVmY= 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:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:content-transfer-encoding:mime-version; s=default; bh=65gNc5gukRSigxLz9dKN1nQzJcA=; b=IKTxYXm9Y2ad4jj7Kc6lDhqwim5S Uzlhn5J0krDmZnhz+sVmToODGetEgQt8WhWJa46iW1RrKbfSvgnEE5DoHe55e+yD QTRz2Y1wbUG3jDDt4+ks94pSQzk/KDu14oRGeThVfkNyX3fgbhhAGgQsYqfKy+fi eCs/SOf8daOl/N4= Received: (qmail 791 invoked by alias); 23 Apr 2014 15:33:15 -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 767 invoked by uid 89); 23 Apr 2014 15:33:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.89.28.114) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 23 Apr 2014 15:33:11 +0000 Received: from KLMAIL01.kl.imgtec.org (unknown [192.168.5.35]) by Websense Email Security Gateway with ESMTPS id CE754DBEE7935; Wed, 23 Apr 2014 16:33:03 +0100 (IST) Received: from KLMAIL02.kl.imgtec.org (192.168.5.97) by KLMAIL01.kl.imgtec.org (192.168.5.35) with Microsoft SMTP Server (TLS) id 14.3.181.6; Wed, 23 Apr 2014 16:33:06 +0100 Received: from KLMAIL01.kl.imgtec.org ([192.168.5.35]) by klmail02.kl.imgtec.org ([192.168.5.97]) with mapi id 14.03.0181.006; Wed, 23 Apr 2014 16:33:06 +0100 From: Robert Suchanek To: Richard Sandiford CC: "gcc-patches@gcc.gnu.org" , "vmakarov@redhat.com" Subject: RE: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend Date: Wed, 23 Apr 2014 15:33:05 +0000 Message-ID: References: <87d2h51dm6.fsf@talisman.default> <87d2gqfb7t.fsf@talisman.default> <87ob02jodp.fsf@talisman.default> <87fvl6hnw2.fsf@talisman.default> <87ioq0w37w.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> In-Reply-To: <87ioq0w37w.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> MIME-Version: 1.0 > Hmm, in that case maybe we should just leave it failing. The alternative > would be to skip the test altogther for MIPS, with a PR referencing it, > but that seems a bit over-the-top. I'd leave it as it is for now until the consensus regarding the 'X' constraint is reached. > Please use comments of the form: > > /* Implement TARGET_FOO. */ > > above all three functions (instead of the current one in the case of > mips_register_priority), just so that it's painfully obvious that > these are target hooks. Modified as requested and attached the patch below. I tried to keep to the conventions but apparently I seem to overlook certain things. I'll remember this part now :). > Out of interest, do you see any difference if you include $sp in SPILL_REGS? > That obviously doesn't make much conceptual sense, but it would give a > cleaner class hierarchy. Including $sp does not make any difference, exactly the same code size. Although I haven't thoroughly tested it, I limited the check to "-Os". Regards, Robert 2014-03-26 Robert Suchanek * lra-constraints.c (base_to_reg): New function. (process_address): Use new function. * config/mips/constraints.md ("d"): BASE_REG_CLASS replaced by "TARGET_MIPS16 ? M16_REGS : GR_REGS". * config/mips/mips.c (mips_regno_mode_ok_for_base_p): Remove use !strict_p for MIPS16. (mips_register_priority): New function that implements the target hook TARGET_REGISTER_PRIORITY. (mips_spill_class): Likewise for TARGET_SPILL_CLASS (mips_lra_p): Likewise for TARGET_LRA_P. * config/mips/mips.h (reg_class): Add M16_SP_REGS and SPILL_REGS classes. (REG_CLASS_NAMES): Likewise. (REG_CLASS_CONTENTS): Likewise. (BASE_REG_CLASS): Use M16_SP_REGS. * config/mips/mips.md (*mul_acc_si, *mul_sub_si): Add alternative tuned for LRA. New set attribute to enable alternatives depending on the register allocator used. (*lea64): Disable pattern for MIPS16. * config/mips/mips.opt (mlra): New option diff --git gcc/config/mips/constraints.md gcc/config/mips/constraints.md index f6834fd..fa33c30 100644 --- gcc/config/mips/constraints.md +++ gcc/config/mips/constraints.md @@ -19,7 +19,7 @@ ;; Register constraints -(define_register_constraint "d" "BASE_REG_CLASS" +(define_register_constraint "d" "TARGET_MIPS16 ? M16_REGS : GR_REGS" "An address register. This is equivalent to @code{r} unless generating MIPS16 code.") diff --git gcc/config/mips/mips.c gcc/config/mips/mips.c index 45256e9..f8d90b2 100644 --- gcc/config/mips/mips.c +++ gcc/config/mips/mips.c @@ -655,7 +655,7 @@ const enum reg_class mips_regno_to_class[FIRST_PSEUDO_REGISTER] = { M16_REGS, M16_STORE_REGS, LEA_REGS, LEA_REGS, LEA_REGS, LEA_REGS, LEA_REGS, LEA_REGS, T_REG, PIC_FN_ADDR_REG, LEA_REGS, LEA_REGS, - LEA_REGS, LEA_REGS, LEA_REGS, LEA_REGS, + LEA_REGS, M16_SP_REGS, LEA_REGS, LEA_REGS, FP_REGS, FP_REGS, FP_REGS, FP_REGS, FP_REGS, FP_REGS, FP_REGS, FP_REGS, @@ -2241,22 +2241,9 @@ mips_regno_mode_ok_for_base_p (int regno, enum machine_mode mode, return true; /* In MIPS16 mode, the stack pointer can only address word and doubleword - values, nothing smaller. There are two problems here: - - (a) Instantiating virtual registers can introduce new uses of the - stack pointer. If these virtual registers are valid addresses, - the stack pointer should be too. - - (b) Most uses of the stack pointer are not made explicit until - FRAME_POINTER_REGNUM and ARG_POINTER_REGNUM have been eliminated. - We don't know until that stage whether we'll be eliminating to the - stack pointer (which needs the restriction) or the hard frame - pointer (which doesn't). - - All in all, it seems more consistent to only enforce this restriction - during and after reload. */ + values, nothing smaller. */ if (TARGET_MIPS16 && regno == STACK_POINTER_REGNUM) - return !strict_p || GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8; + return GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8; return TARGET_MIPS16 ? M16_REG_P (regno) : GP_REG_P (regno); } @@ -12115,6 +12102,18 @@ mips_register_move_cost (enum machine_mode mode, return 0; } +/* Implement TARGET_REGISTER_PRIORITY. */ + +static int +mips_register_priority (int hard_regno) +{ + /* Treat MIPS16 registers with higher priority than other regs. */ + if (TARGET_MIPS16 + && TEST_HARD_REG_BIT (reg_class_contents[M16_REGS], hard_regno)) + return 1; + return 0; +} + /* Implement TARGET_MEMORY_MOVE_COST. */ static int @@ -18897,6 +18896,25 @@ mips_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update) *update = build2 (COMPOUND_EXPR, void_type_node, *update, atomic_feraiseexcept_call); } + +/* Implement TARGET_SPILL_CLASS. */ + +static reg_class_t +mips_spill_class (reg_class_t rclass ATTRIBUTE_UNUSED, + enum machine_mode mode ATTRIBUTE_UNUSED) +{ + if (TARGET_MIPS16) + return SPILL_REGS; + return NO_REGS; +} + +/* Implement TARGET_LRA_P. */ + +static bool +mips_lra_p (void) +{ + return mips_lra_flag; +} /* Initialize the GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP @@ -18960,6 +18978,8 @@ mips_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update) #define TARGET_VALID_POINTER_MODE mips_valid_pointer_mode #undef TARGET_REGISTER_MOVE_COST #define TARGET_REGISTER_MOVE_COST mips_register_move_cost +#undef TARGET_REGISTER_PRIORITY +#define TARGET_REGISTER_PRIORITY mips_register_priority #undef TARGET_MEMORY_MOVE_COST #define TARGET_MEMORY_MOVE_COST mips_memory_move_cost #undef TARGET_RTX_COSTS @@ -19134,6 +19154,11 @@ mips_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update) #undef TARGET_ATOMIC_ASSIGN_EXPAND_FENV #define TARGET_ATOMIC_ASSIGN_EXPAND_FENV mips_atomic_assign_expand_fenv +#undef TARGET_SPILL_CLASS +#define TARGET_SPILL_CLASS mips_spill_class +#undef TARGET_LRA_P +#define TARGET_LRA_P mips_lra_p + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-mips.h" diff --git gcc/config/mips/mips.h gcc/config/mips/mips.h index b25865b..fb37999 100644 --- gcc/config/mips/mips.h +++ gcc/config/mips/mips.h @@ -1872,10 +1872,12 @@ enum reg_class NO_REGS, /* no registers in set */ M16_STORE_REGS, /* microMIPS store registers */ M16_REGS, /* mips16 directly accessible registers */ + M16_SP_REGS, /* mips16 + $sp */ T_REG, /* mips16 T register ($24) */ M16_T_REGS, /* mips16 registers plus T register */ PIC_FN_ADDR_REG, /* SVR4 PIC function address register */ V1_REG, /* Register $v1 ($3) used for TLS access. */ + SPILL_REGS, /* All but $sp and call preserved regs are in here */ LEA_REGS, /* Every GPR except $25 */ GR_REGS, /* integer registers */ FP_REGS, /* floating point registers */ @@ -1910,10 +1912,12 @@ enum reg_class "NO_REGS", \ "M16_STORE_REGS", \ "M16_REGS", \ + "M16_SP_REGS", \ "T_REG", \ "M16_T_REGS", \ "PIC_FN_ADDR_REG", \ "V1_REG", \ + "SPILL_REGS", \ "LEA_REGS", \ "GR_REGS", \ "FP_REGS", \ @@ -1951,10 +1955,12 @@ enum reg_class { 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* NO_REGS */ \ { 0x000200fc, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* M16_STORE_REGS */ \ { 0x000300fc, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* M16_REGS */ \ + { 0x200300fc, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* M16_SP_REGS */ \ { 0x01000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* T_REG */ \ { 0x010300fc, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* M16_T_REGS */ \ { 0x02000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* PIC_FN_ADDR_REG */ \ { 0x00000008, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* V1_REG */ \ + { 0x0303fffc, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* SPILL_REGS */ \ { 0xfdffffff, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* LEA_REGS */ \ { 0xffffffff, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* GR_REGS */ \ { 0x00000000, 0xffffffff, 0x00000000, 0x00000000, 0x00000000, 0x00000000 }, /* FP_REGS */ \ @@ -1987,7 +1993,7 @@ enum reg_class valid base register must belong. A base register is one used in an address which is the register value plus a displacement. */ -#define BASE_REG_CLASS (TARGET_MIPS16 ? M16_REGS : GR_REGS) +#define BASE_REG_CLASS (TARGET_MIPS16 ? M16_SP_REGS : GR_REGS) /* A macro whose definition is the name of the class to which a valid index register must belong. An index register is one used diff --git gcc/config/mips/mips.md gcc/config/mips/mips.md index f914ab6..4377a69 100644 --- gcc/config/mips/mips.md +++ gcc/config/mips/mips.md @@ -1622,40 +1622,66 @@ ;; copy instructions. Reload therefore thinks that the second alternative ;; is two reloads more costly than the first. We add "*?*?" to the first ;; alternative as a counterweight. +;; +;; LRA simulates reload but the cost of reloading scratches is lower +;; than of the classic reload. For the time being, removing the counterweight +;; for LRA is more profitable. (define_insn "*mul_acc_si" - [(set (match_operand:SI 0 "register_operand" "=l*?*?,d?") - (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d") - (match_operand:SI 2 "register_operand" "d,d")) - (match_operand:SI 3 "register_operand" "0,d"))) - (clobber (match_scratch:SI 4 "=X,l")) - (clobber (match_scratch:SI 5 "=X,&d"))] + [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d?") + (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d,d") + (match_operand:SI 2 "register_operand" "d,d,d")) + (match_operand:SI 3 "register_operand" "0,0,d"))) + (clobber (match_scratch:SI 4 "=X,X,l")) + (clobber (match_scratch:SI 5 "=X,X,&d"))] "GENERATE_MADD_MSUB && !TARGET_MIPS16" "@ madd\t%1,%2 + madd\t%1,%2 #" [(set_attr "type" "imadd") (set_attr "accum_in" "3") (set_attr "mode" "SI") - (set_attr "insn_count" "1,2")]) + (set_attr "insn_count" "1,1,2") + (set (attr "enabled") + (cond [(and (eq_attr "alternative" "0") + (match_test "!mips_lra_flag")) + (const_string "yes") + (and (eq_attr "alternative" "1") + (match_test "mips_lra_flag")) + (const_string "yes") + (eq_attr "alternative" "2") + (const_string "yes")] + (const_string "no")))]) ;; The same idea applies here. The middle alternative needs one less ;; clobber than the final alternative, so we add "*?" as a counterweight. (define_insn "*mul_acc_si_r3900" - [(set (match_operand:SI 0 "register_operand" "=l*?*?,d*?,d?") - (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d,d") - (match_operand:SI 2 "register_operand" "d,d,d")) - (match_operand:SI 3 "register_operand" "0,l,d"))) - (clobber (match_scratch:SI 4 "=X,3,l")) - (clobber (match_scratch:SI 5 "=X,X,&d"))] + [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d*?,d?") + (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d,d,d") + (match_operand:SI 2 "register_operand" "d,d,d,d")) + (match_operand:SI 3 "register_operand" "0,0,l,d"))) + (clobber (match_scratch:SI 4 "=X,X,3,l")) + (clobber (match_scratch:SI 5 "=X,X,X,&d"))] "TARGET_MIPS3900 && !TARGET_MIPS16" "@ madd\t%1,%2 + madd\t%1,%2 madd\t%0,%1,%2 #" [(set_attr "type" "imadd") (set_attr "accum_in" "3") (set_attr "mode" "SI") - (set_attr "insn_count" "1,1,2")]) + (set_attr "insn_count" "1,1,1,2") + (set (attr "enabled") + (cond [(and (eq_attr "alternative" "0") + (match_test "!mips_lra_flag")) + (const_string "yes") + (and (eq_attr "alternative" "1") + (match_test "mips_lra_flag")) + (const_string "yes") + (eq_attr "alternative" "2,3") + (const_string "yes")] + (const_string "no")))]) ;; Split *mul_acc_si if both the source and destination accumulator ;; values are GPRs. @@ -1859,20 +1885,31 @@ ;; See the comment above *mul_add_si for details. (define_insn "*mul_sub_si" - [(set (match_operand:SI 0 "register_operand" "=l*?*?,d?") - (minus:SI (match_operand:SI 1 "register_operand" "0,d") - (mult:SI (match_operand:SI 2 "register_operand" "d,d") - (match_operand:SI 3 "register_operand" "d,d")))) - (clobber (match_scratch:SI 4 "=X,l")) - (clobber (match_scratch:SI 5 "=X,&d"))] + [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d?") + (minus:SI (match_operand:SI 1 "register_operand" "0,0,d") + (mult:SI (match_operand:SI 2 "register_operand" "d,d,d") + (match_operand:SI 3 "register_operand" "d,d,d")))) + (clobber (match_scratch:SI 4 "=X,X,l")) + (clobber (match_scratch:SI 5 "=X,X,&d"))] "GENERATE_MADD_MSUB" "@ msub\t%2,%3 + msub\t%2,%3 #" [(set_attr "type" "imadd") (set_attr "accum_in" "1") (set_attr "mode" "SI") - (set_attr "insn_count" "1,2")]) + (set_attr "insn_count" "1,1,2") + (set (attr "enabled") + (cond [(and (eq_attr "alternative" "0") + (match_test "!mips_lra_flag")) + (const_string "yes") + (and (eq_attr "alternative" "1") + (match_test "mips_lra_flag")) + (const_string "yes") + (eq_attr "alternative" "2") + (const_string "yes")] + (const_string "no")))]) ;; Split *mul_sub_si if both the source and destination accumulator ;; values are GPRs. @@ -4139,7 +4176,10 @@ [(set (match_operand:DI 0 "register_operand" "=d") (match_operand:DI 1 "absolute_symbolic_operand" "")) (clobber (match_scratch:DI 2 "=&d"))] - "TARGET_EXPLICIT_RELOCS && ABI_HAS_64BIT_SYMBOLS && cse_not_expected" + "!TARGET_MIPS16 + && TARGET_EXPLICIT_RELOCS + && ABI_HAS_64BIT_SYMBOLS + && cse_not_expected" "#" "&& reload_completed" [(set (match_dup 0) (high:DI (match_dup 3))) diff --git gcc/config/mips/mips.opt gcc/config/mips/mips.opt index 6ee5398..c8e840d 100644 --- gcc/config/mips/mips.opt +++ gcc/config/mips/mips.opt @@ -380,6 +380,10 @@ msynci Target Report Mask(SYNCI) Use synci instruction to invalidate i-cache +mlra +Target Report Var(mips_lra_flag) Init(1) Save +Use LRA instead of reload + mtune= Target RejectNegative Joined Var(mips_tune_option) ToLower Enum(mips_arch_opt_value) -mtune=PROCESSOR Optimize the output for PROCESSOR diff --git gcc/lra-constraints.c gcc/lra-constraints.c index 7d5103f..9c72670 100644 --- gcc/lra-constraints.c +++ gcc/lra-constraints.c @@ -2630,6 +2630,39 @@ valid_address_p (struct address_info *ad) return ok_p; } +/* Make reload base reg from address AD. */ +static rtx +base_to_reg (struct address_info *ad) +{ + enum reg_class cl; + int code = -1; + rtx new_inner = NULL_RTX; + rtx new_reg = NULL_RTX; + rtx insn; + rtx last_insn = get_last_insn(); + + lra_assert (ad->base == ad->base_term && ad->disp == ad->disp_term); + cl = base_reg_class (ad->mode, ad->as, ad->base_outer_code, + get_index_code (ad)); + new_reg = lra_create_new_reg (GET_MODE (*ad->base_term), NULL_RTX, + cl, "base"); + new_inner = simplify_gen_binary (PLUS, GET_MODE (new_reg), new_reg, + ad->disp_term == NULL + ? gen_int_mode (0, ad->mode) + : *ad->disp_term); + if (!valid_address_p (ad->mode, new_inner, ad->as)) + return NULL_RTX; + insn = emit_insn (gen_rtx_SET (ad->mode, new_reg, *ad->base_term)); + code = recog_memoized (insn); + if (code < 0) + { + delete_insns_since (last_insn); + return NULL_RTX; + } + + return new_inner; +} + /* Make reload base reg + disp from address AD. Return the new pseudo. */ static rtx base_plus_disp_to_reg (struct address_info *ad) @@ -2847,6 +2880,8 @@ process_address (int nop, rtx *before, rtx *after) 3) the address is a frame address with an invalid offset. + 4) the address is a frame address with an invalid base. + All these cases involve a non-autoinc address, so there is no point revalidating other types. */ if (ad.autoinc_p || valid_address_p (&ad)) @@ -2928,14 +2963,19 @@ process_address (int nop, rtx *before, rtx *after) int regno; enum reg_class cl; rtx set, insns, last_insn; + /* Try to reload base into register only if the base is invalid + for the address but with valid offset, case (4) above. */ + start_sequence (); + new_reg = base_to_reg (&ad); + /* base + disp => new base, cases (1) and (3) above. */ /* Another option would be to reload the displacement into an index register. However, postreload has code to optimize address reloads that have the same base and different displacements, so reloading into an index register would not necessarily be a win. */ - start_sequence (); - new_reg = base_plus_disp_to_reg (&ad); + if (new_reg == NULL_RTX) + new_reg = base_plus_disp_to_reg (&ad); insns = get_insns (); last_insn = get_last_insn (); /* If we generated at least two insns, try last insn source as