From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26903 invoked by alias); 17 Jul 2015 19:00:48 -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 26893 invoked by uid 89); 17 Jul 2015 19:00:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ie0-f176.google.com Received: from mail-ie0-f176.google.com (HELO mail-ie0-f176.google.com) (209.85.223.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 17 Jul 2015 19:00:46 +0000 Received: by iecuq6 with SMTP id uq6so83370246iec.2 for ; Fri, 17 Jul 2015 12:00:43 -0700 (PDT) X-Received: by 10.107.11.149 with SMTP id 21mr21849342iol.103.1437159643755; Fri, 17 Jul 2015 12:00:43 -0700 (PDT) Received: from [10.19.103.28] (64.2.3.194.ptr.us.xo.net. [64.2.3.194]) by smtp.gmail.com with ESMTPSA id v3sm4321828igk.1.2015.07.17.12.00.41 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 17 Jul 2015 12:00:42 -0700 (PDT) References: <559F8398.8000305@arm.com> <9CCA6AEA-8A0F-44D6-914A-504A2649A7B6@gmail.com> <559F8698.8080301@arm.com> <559FCC97.7060601@arm.com> <55A90A18.1040309@arm.com> Mime-Version: 1.0 (1.0) In-Reply-To: <55A90A18.1040309@arm.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Message-Id: Cc: GCC Patches , James Greenhalgh , Marcus Shawcroft , Richard Earnshaw From: pinskia@gmail.com Subject: Re: [PATCH][AArch64] Improve csinc/csneg/csinv opportunities on immediates Date: Fri, 17 Jul 2015 19:40:00 -0000 To: Kyrill Tkachov X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg01571.txt.bz2 > On Jul 17, 2015, at 9:58 PM, Kyrill Tkachov wrot= e: >=20 >=20 >> On 10/07/15 14:45, Kyrill Tkachov wrote: >>> On 10/07/15 10:00, pinskia@gmail.com wrote: >>>=20 >>>=20 >>>> On Jul 10, 2015, at 1:47 AM, Kyrill Tkachov w= rote: >>>>=20 >>>> Hi Andrew, >>>>=20 >>>>> On 10/07/15 09:40, pinskia@gmail.com wrote: >>>>>=20 >>>>>=20 >>>>>=20 >>>>>> On Jul 10, 2015, at 1:34 AM, Kyrill Tkachov = wrote: >>>>>>=20 >>>>>> Hi all, >>>>>>=20 >>>>>> Currently when evaluating expressions like (a ? 24 : 25) we will mov= e 24 and 25 into >>>>>> registers and perform a csel on them. This misses the opportunity t= o instead move just 24 >>>>>> into a register and then perform a csinc, saving us an instruction a= nd a register use. >>>>>> Similarly for csneg and csinv. >>>>>>=20 >>>>>> This patch implements that idea by allowing such pairs of immediates= in *cmov_insn >>>>>> and adding an early splitter that performs the necessary transformat= ion. >>>>>>=20 >>>>>> The testcase included in the patch demonstrates the kind of opportun= ities that are now picked up. >>>>>>=20 >>>>>> With this patch I see about 9.6% more csinc instructions being gener= ated for SPEC2006 >>>>>> and the generated code looks objectively better (i.e. fewer mov-imme= diates and slightly >>>>>> lower register pressure). >>>>>>=20 >>>>>> Bootstrapped and tested on aarch64. >>>>>>=20 >>>>>> Ok for trunk? >>>>> I think this is the wrong place for this optimization. It should happ= en in expr.c and we should produce cond_expr on the gimple level. >>>> I had considered it, but I wasn't sure how general the conditional inc= rement/negate/inverse operations >>>> are to warrant a midend implementation. Do you mean the expand_cond_ex= pr_using_cmove function in expr.c? >>> Yes and we can expand it to even have a target hook on how to expand th= em if needed. >> I played around in that part and it seems that by the time it gets to ex= pansion the midend >> doesn't have a cond_expr of the two immediates, it's a PHI node with the= immediates already expanded. >> I have not been able to get it to match a cond_expr of two immediates th= ere, although that could be >> because I'm unfamiliar with that part of the codebase. >=20 > So by the time we reach expansion time we don't have a COND_EXPR of two i= mmediates, so I tried getting > the code in expr.c to do the right thing, but it didn't work out. > This patch catches this opportunity at the RTL level and could catch such= cases if they were to be > generated by any of the pre-combine RTL passes. Or do you reckon looking = for these patterns in RTL > ifcvt is the way to go? I think it would be somewhat messy to express the= CSNEG, CSINV opportunities > there as we don't have optabs for conditional negate and invert, but cond= itional increment would work, > though in the aarch64 case we can only do a conditional by 1 rather than = a general conditional add. Right as I said, I have a patch to phiopt to produce the cond_expr when it = is useful. That is create cond_expr before we even get to rtl.=20 Thanks, Andrew >=20 > Kyrill >=20 >=20 >>=20 >> Kyrill >>=20 >>> There is already a standard pattern for condition add so the a ? Const1= : const2 can be handled in the a generic way without much troubles. We sho= uld handle it better in rtl ifcvt too (that should be an easier patch). Th= e neg and not cases are very target specific but can be handled by a target= hook and expand it directly to it. >>>=20 >>>=20 >>>>> I have patches to do both but I have not got around to cleaning the= m up. If anyone wants them, I can send a link to my current gcc 5.1 sources= with them included. >>>> Any chance you can post them on gcc-patches even as a rough idea of wh= at needs to be done? >>> I posted my expr patch a few years ago but I never got around to rth's = comments. This was the generic increment patch. Basically aarch64 should be= implementing that pattern too. >>>=20 >>>=20 >>> The main reason why this should be handled in gimple is that ifcvt on t= he rtl level is not cheap and does not catch all of the cases the simple ex= pansion of phi-opt does. I can dig that patch up and I will be doing that n= ext week anyways. >>>=20 >>> Thanks, >>> Andrew >>>=20 >>>> Thanks, >>>> Kyrill >>>>=20 >>>>> Thanks, >>>>> Andrew >>>>>=20 >>>>>> Thanks, >>>>>> Kyrill >>>>>>=20 >>>>>> 2015-07-10 Kyrylo Tkachov >>>>>>=20 >>>>>> * config/aarch64/aarch64.md (*cmov_insn): Move stricter >>>>>> check for operands 3 and 4 to pattern predicate. Allow immediat= es >>>>>> that can be expressed as csinc/csneg/csinv. New define_split. >>>>>> (*csinv3_insn): Rename to... >>>>>> (csinv3_insn): ... This. >>>>>> * config/aarch64/aarch64.h (AARCH64_IMMS_OK_FOR_CSNEG): New macr= o. >>>>>> (AARCH64_IMMS_OK_FOR_CSINC): Likewise. >>>>>> (AARCH64_IMMS_OK_FOR_CSINV): Likewise. >>>>>> * config/aarch64/aarch64.c (aarch64_imms_ok_for_cond_op_1): >>>>>> New function. >>>>>> (aarch64_imms_ok_for_cond_op): Likewise. >>>>>> * config/aarch64/aarch64-protos.h (aarch64_imms_ok_for_cond_op_1= ): >>>>>> Declare prototype. >>>>>> (aarch64_imms_ok_for_cond_op): Likewise. >>>>>>=20 >>>>>> 2015-07-10 Kyrylo Tkachov >>>>>>=20 >>>>>> * gcc.target/aarch64/cond-op-imm_1.c: New test. >>>>>> >=20