From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 41409 invoked by alias); 21 Jan 2016 22:07:18 -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 41392 invoked by uid 89); 21 Jan 2016 22:07:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=eq_attr, em1_nmisc, em1_st, exynosm1 X-HELO: cam-smtp0.cambridge.arm.com Received: from fw-tnat.cambridge.arm.com (HELO cam-smtp0.cambridge.arm.com) (217.140.96.140) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 21 Jan 2016 22:07:17 +0000 Received: from arm.com (e107456-lin.cambridge.arm.com [10.2.206.78]) by cam-smtp0.cambridge.arm.com (8.13.8/8.13.8) with ESMTP id u0LM7DfO017190; Thu, 21 Jan 2016 22:07:13 GMT Date: Thu, 21 Jan 2016 22:07:00 -0000 From: James Greenhalgh To: Evandro Menezes Cc: Wilco Dijkstra , "gcc-patches@gcc.gnu.org" , nd , Andrew Pinski Subject: Re: [PATCH 2/4 v2][AArch64] Add support for FCCMP Message-ID: <20160121220712.GA25892@arm.com> References: <568C3D19.908@samsung.com> <568D7CBF.2070407@samsung.com> <20160121092402.GA8842@arm.com> <56A13867.3010103@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56A13867.3010103@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg01682.txt.bz2 On Thu, Jan 21, 2016 at 01:58:31PM -0600, Evandro Menezes wrote: > Hi, James. > > On 01/21/16 03:24, James Greenhalgh wrote: > >On Wed, Jan 06, 2016 at 02:44:47PM -0600, Evandro Menezes wrote: > >>On 01/06/2016 06:04 AM, Wilco Dijkstra wrote: > >>>>Here's what I had in mind when I inquired about distinguishing FCMP from > >>>>FCCMP. As you can see in the patch, Exynos is the only target that > >>>>cares about it, but I wonder if ThunderX or Xgene would too. > >>>> > >>>>What do you think? > >>>The new attributes look fine (I've got a similar outstanding change), however > >>>please don't add them to non-AArch64 cores. We only need it for thunderx.md, > >>>cortex-a53.md, cortex-a57.md, xgene1.md and exynos-m1.md. > >> Add support for the FCCMP insn types > >> > >> 2016-01-04 Evandro Menezes > >> > >> gcc/ > >> * config/aarch64/aarch64.md (fccmp): Change insn type. > >> (fccmpe): Likewise. > >> * config/aarch64/thunderx.md (thunderx_fcmp): Add > >> "fccmp{s,d}" types. > >> * config/arm/cortex-a53.md (cortex_a53_fpalu): Likewise. > >> * config/arm/cortex-a57.md (cortex_a57_fp_cmp): Likewise. > >> * config/arm/xgene1.md (xgene1_fcmp): Likewise. > >> * config/arm/exynos-m1.md (exynos_m1_fp_ccmp): New insn > >> reservation. > >> * config/arm/types.md (fccmps): Add new insn type. > >> (fccmpd): Likewise. > >> > >>Got it. Here's an updated patch. Again, assuming that your > >>original patch is in place. Perhaps you can build on it. > >If we don't have any targets which care about the fccmps/fccmpd split in > >the code base, do we really need it? Can we just follow the example of > >fcsel? > > The Exynos M1 does care about the difference between FCMP and FCCMP, > as can be seen in the patch. > More explicitly: > > (define_insn_reservation "exynos_m1_fp_cmp" 4 > (and (eq_attr "tune" "exynosm1") > (eq_attr "type" "fcmps, fcmpd")) > "em1_nmisc") > > (define_insn_reservation "exynos_m1_fp_ccmp" 7 > (and (eq_attr "tune" "exynosm1") > (eq_attr "type" "fccmps, fccmpd")) > "em1_st, em1_nmisc") > I think I was unclear. Your exynos-m1 model cares about splitting fcmp[s/d] and fccmp, but it doesn't care about splitting fccmp in to fccmps/fccmpd. It is the split to fccmps/fccmpd that I think is unneccesary at this time. > >>diff --git a/gcc/config/arm/types.md b/gcc/config/arm/types.md > >>index 321ff89..daf7162 100644 > >>--- a/gcc/config/arm/types.md > >>+++ b/gcc/config/arm/types.md > >>@@ -70,6 +70,7 @@ > >> ; f_rint[d,s] double/single floating point rount to integral. > >> ; f_store[d,s] double/single store to memory. Used for VFP unit. > >> ; fadd[d,s] double/single floating-point scalar addition. > >>+; fccmp[d,s] double/single floating-point conditional compare. > >Can we follow the convention fcsel uses of calling out "From ARMv8-A:" > >for this type? > > > > I'm not sure I follow. Though I didn't refer to the ISA spec, I > used the description from it for the *fccmp* type. > > Please, advise. Something like: ; fccmp From ARMv8-A: floating point conditional compare. Just to capture that this instruction is only available for cores implementing ARMv8-A. Thanks, James