From patchwork Fri Aug 14 20:30:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Moore, Catherine" X-Patchwork-Id: 507566 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 79DD81401E7 for ; Sat, 15 Aug 2015 06:30:31 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Mj4OP5vC; dkim-atps=neutral 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=tx14Ml7Wr/2dsAFzcD+qsinFYbSIw7o3EGcQ7KxSlRIJqvzv+1HnR Mlqp7Lr7DDUGSi9qJa9hqgCzn4BfCGn3KKeWoMitJyLVi78tZol2TR0GKchE39J8 VE7hGsA7/Zp85D1sao/p/2Pzc3mt+9IhTTK7PbpqmmklUZ/mS3seGY= 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=nGHP0iZb8CWipqCOdDMVT84TPkM=; b=Mj4OP5vCx7CYeVKm5uUrQ/9PemXK CFYJ7hiAebu53qvlXBm5dUU0ZkfGTTZ4rBKPEtLgUcwegcjhuxmO/3e5Qrze0tKf GN4fIBGfjn31KpRfE3OS8TR9Abpq5+rV01Blf9stkY0v0OOta46jHdp+GiVDi4u4 j3cOVGwWPdcm5mA= Received: (qmail 74042 invoked by alias); 14 Aug 2015 20:30:24 -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 74003 invoked by uid 89); 14 Aug 2015 20:30:21 -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_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Aug 2015 20:30:19 +0000 Received: from svr-orw-fem-03.mgc.mentorg.com ([147.34.97.39]) by relay1.mentorg.com with esmtp id 1ZQLcF-0004dt-3h from Catherine_Moore@mentor.com ; Fri, 14 Aug 2015 13:30:15 -0700 Received: from NA-MBX-04.mgc.mentorg.com ([169.254.4.176]) by SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) with mapi id 14.03.0224.002; Fri, 14 Aug 2015 13:30:14 -0700 From: "Moore, Catherine" To: Matthew Fortune , "'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)" CC: "Moore, Catherine" Subject: RE: [PATCH, MIPS] Compact branch support for MIPS32R6/MIPS64R6 Date: Fri, 14 Aug 2015 20:30:14 +0000 Message-ID: References: <6D39441BF12EF246A7ABCE6654B02353211F2232@LEMAIL01.le.imgtec.org> In-Reply-To: <6D39441BF12EF246A7ABCE6654B02353211F2232@LEMAIL01.le.imgtec.org> MIME-Version: 1.0 Hi Matthew, > -----Original Message----- > From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com] > Sent: Wednesday, July 22, 2015 6:19 PM > Subject: [PATCH, MIPS] Compact branch support for MIPS32R6/MIPS64R6 > This patch looks really good. I have a couple of questions and a couple of nits that need to be fixed up. > A full range of 'compact' branch instructions were introduced to MIPS > as part of Release 6. The compact term is used to identify the fact > that these do not have a delay slot. > > So how does all this work in GCC? > > Compact branches are used based on a branch policy. The polices are: > > never: Only use delay slot branches > optimal: Do whatever is best for the current architecture. This will > generally mean that delay slot branches will be used if the delay > slot gets filled but otherwise a compact branch will be used. A > special case here is that JAL and J will not be used in R6 code > regardless of whether the delay slot could be filled. > always: Never emit a delay slot form of a branch if a compact form exists. > This policy cannot apply 100% as FP branches (and MSA branches when > committed) only have delay slot forms. > > These user choices are combined with the features available in the chosen > architecture and, in particular, the optimal form will get handled like > 'never' when there are no compact branches available and will get handled > like 'always' when there are no delay slot branches available. > Why did you choose to make this a user-selectable option? Why not always generated optimal? I don't have a strong opinion about it, but the options seem to complicate things and I'm interested in your rationale. > > The most complicated aspect of this change is to the MIPS_CALL and > MICROMIPS_J macros. These have been rewritten from scratch as a function > that generates an instruction instead. Thank you for cleaning this up. The new function is much easier to follow. > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c > index c3cd52d..c0f2884 100644 > --- a/gcc/config/mips/mips.c > +++ b/gcc/config/mips/mips.c > > +/* Return the asm template for a call. OPERANDS are the operands, > TARGET_OPNO > + is the operand number of the target. SIZE_OPNO is the operand number > of > + the argument size operand that can optionally hold the call attributes. If > + SIZE_OPNO is not -1 and the call is indirect, use the function symbol from > + the call attributes to attach a R_MIPS_JALR relocation to the call. > + Might as well mention LINK_P here as well. > + When generating GOT code without explicit relocation operators, all calls > + should use assembly macros. Otherwise, all indirect calls should use "jr" > + or "jalr"; we will arrange to restore $gp afterwards if necessary. Finally, > + we can only generate direct calls for -mabicalls by temporarily switching > + to non-PIC mode. > + > + For microMIPS jal(r), we try to generate jal(r)s when a 16-bit > + instruction is in the delay slot of jal(r). > + > + Where compact branches are available, we try to use them if the delay > slot > + has a NOP (or equivalently delay slots were not enabled for the instruction > + anyway). */ > + > +const char * > +mips_output_jump (rtx *operands, int target_opno, int size_opno, bool > link_p) > +{ > @@ -13038,6 +13165,59 @@ mips_output_conditional_branch (rtx_insn > *insn, rtx *operands, > return ""; > } > > +const char * > +mips_output_equal_conditional_branch (rtx_insn* insn, rtx *operands, > + bool inverted_p) This function needs a comment. These need to be documented in invoke.texi. diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index 5bc562e..04fe6d0 100644 --- a/gcc/config/mips/mips.h +++ b/gcc/config/mips/mips.h > @@ -92,6 +92,23 @@ struct mips_cpu_info { > /* True if we are generating position-independent VxWorks RTP code. */ > #define TARGET_RTP_PIC (TARGET_VXWORKS_RTP && flag_pic) > > +/* Set based on a combination of compact branch policy and ISA support. > */ > +#define TARGET_CB_NEVER (mips_cb == MIPS_CB_NEVER \ > + || (mips_cb == MIPS_CB_OPTIMAL \ > + && !ISA_HAS_COMPACT_BRANCHES)) > +#define TARGET_CB_MAYBE (TARGET_CB_ALWAYS \ > + || (mips_cb == MIPS_CB_OPTIMAL \ > + && ISA_HAS_COMPACT_BRANCHES)) > +#define TARGET_CB_ALWAYS (mips_cb == MIPS_CB_ALWAYS \ > + || (mips_cb == MIPS_CB_OPTIMAL \ > + && !ISA_HAS_DELAY_SLOTS)) > + I would appreciate a more detailed comment here ... @@ -871,6 +888,10 @@ struct mips_cpu_info { #define ISA_HAS_JR (mips_isa_rev <= 5) +#define ISA_HAS_DELAY_SLOTS 1 If this is a placeholder for the microMIPS patch, then OK. Otherwise, what's the point? My test run had only one test that executed with the -mcompact-branch= option. That may have been because I used RUNTESTFLAGS (the problem that we discussed at Cauldron). It looks like you put the logic in mips.exp though to have the option throw more often. In any case, I would like to see some explicit testing of the options (if they are kept). You've modified the tests to accept both the compact and non-compact forms. It would be nice to ensure that the compact form is not generated when -mcompact-branch=never. Catherine diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt index 348c6e0..84887d1 100644 --- a/gcc/config/mips/mips.opt +++ b/gcc/config/mips/mips.opt @@ -418,3 +418,20 @@ Driver mload-store-pairs Target Report Var(TARGET_LOAD_STORE_PAIRS) Init(1) Enable load/store bonding. + +mcompact-branches= +Target RejectNegative JoinedOrMissing Var(mips_cb) Report Enum(mips_cb_setting) Init(MIPS_CB_OPTIMAL) +Specify the compact branch usage policy + +Enum +Name(mips_cb_setting) Type(enum mips_cb_setting) +Policies available for use with -mcompact-branches=: + +EnumValue +Enum(mips_cb_setting) String(never) Value(MIPS_CB_NEVER) + +EnumValue +Enum(mips_cb_setting) String(optimal) Value(MIPS_CB_OPTIMAL) + +EnumValue +Enum(mips_cb_setting) String(always) Value(MIPS_CB_ALWAYS)