From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31565 invoked by alias); 23 Jun 2014 09:18:34 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 31540 invoked by uid 89); 23 Jun 2014 09:18:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 23 Jun 2014 09:18:00 +0000 Received: from KLMAIL01.kl.imgtec.org (unknown [192.168.5.35]) by Websense Email Security Gateway with ESMTPS id 282A08FCD25A; Mon, 23 Jun 2014 10:17:55 +0100 (IST) Received: from PUMAIL01.pu.imgtec.org (192.168.91.250) by KLMAIL01.kl.imgtec.org (192.168.5.35) with Microsoft SMTP Server (TLS) id 14.3.181.6; Mon, 23 Jun 2014 10:17:57 +0100 Received: from PUMAIL01.pu.imgtec.org ([::1]) by PUMAIL01.pu.imgtec.org ([::1]) with mapi id 14.02.0247.003; Mon, 23 Jun 2014 14:47:55 +0530 From: Sameera Deshpande To: Richard Sandiford CC: Matthew Fortune , "gcc-patches@gcc.gnu.org" Subject: RE: [PATCH][MIPS] Enable load-load/store-store bonding Date: Mon, 23 Jun 2014 09:18:00 -0000 Message-ID: <38C8F1E431EDD94A82971C543A11B4FEE0BB2F@PUMAIL01.pu.imgtec.org> References: <38C8F1E431EDD94A82971C543A11B4FEE0A588@PUMAIL01.pu.imgtec.org> <8761ju8c2k.fsf@talisman.default> In-Reply-To: <8761ju8c2k.fsf@talisman.default> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg01736.txt.bz2 Hi Richard, Thanks for your comments. I am working on the review comments, and will sha= re the reworked patch soon. However, here is clarification on some of the issues raised. > > + if (TARGET_FIX_24K && TUNE_P5600) > > + error ("unsupported combination: %s", "-mtune=3Dp5600 -mfix-24k"); > > + > > /* Save the base compression state and process flags as though we > > were generating uncompressed code. */ > > mips_base_compression_flags =3D TARGET_COMPRESSION; >=20 > Although it's a bit of an odd combination, we need to accept -mfix-24k - > mtune=3Dp5600 and continue to implement the 24k workarounds. > The idea is that a distributor can build for a common base architecture, = add - > mfix- options for processors that might run the code, and add -mtune=3D f= or > the processor that's most of interest optimisation-wise. >=20 > We should just make the pairing of stores conditional on !TARGET_FIX_24K. We had offline discussion based on your comment. There is additional view o= n the same. Only ISAs mips32r2, mips32r3 and mips32r5 support P5600. Remaining ISAs do = not support P5600.=20 For mips32r2 (24K) and mips32r3 (micromips), load-store pairing is implemen= ted separately, and hence, as you suggested, P5600 Ld-ST bonding optimizati= on should not be enabled for them. So, is it fine if I emit error for any ISAs other than mips32r2, mips32r3 a= nd mips32r5 when P5600 is enabled, or the compilation should continue by em= itting warning and disabling P5600? Also, the optimization will be enabled only if !TARGET_FIX_24K && !TARGET_M= ICTOMIPS as suggested by you. > > + > > +#define ENABLE_LD_ST_PAIRING \ > > + (TARGET_ENABLE_LD_ST_PAIRING && TUNE_P5600) >=20 > The patch requires -mld-st-pairing to be passed explicitly even for - > mtune=3Dp5600. Is that because it's not a consistent enough win for us to > enable it by default? It sounded from the description like it should be = an > improvement more often that not. >=20 > We should allow pairing even without -mtune=3Dp5600. Performance testing for this patch is not yet done.=20 If the patch proves beneficial in most of the testcases (which we believe w= ill do on P5600) we will enable this optimization by default for P5600 - in= which case this option can be removed. >=20 > Are QImodes not paired in the same way? If so, it'd be worth adding a > comment above the define_mode_iterator saying that QI is deliberately > excluded. The P5600 datasheet mentions bonding of load/stores in HI, SI, SF and DF mo= des only. Hence QI mode is excluded. I will add the comment on the iterator. - Thanks and regards, Sameera D.