From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32314 invoked by alias); 3 Aug 2015 15:37:32 -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 32301 invoked by uid 89); 3 Aug 2015 15:37:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Aug 2015 15:37:25 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-21-78qRhRScRtWFDlRYRNLF4g-1; Mon, 03 Aug 2015 16:37:20 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 3 Aug 2015 16:37:20 +0100 Message-ID: <55BF8AAF.7060101@arm.com> Date: Mon, 03 Aug 2015 15:37:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Uros Bizjak CC: "gcc-patches@gcc.gnu.org" , Jeff Law , "H.J. Lu" Subject: Re: [PATCH][RTL-ifcvt] Improve conditional select ops on immediates References: <55BF666A.5000008@arm.com> <55BF6FE5.8020902@arm.com> In-Reply-To: X-MC-Unique: 78qRhRScRtWFDlRYRNLF4g-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00087.txt.bz2 On 03/08/15 15:15, Uros Bizjak wrote: > On Mon, Aug 3, 2015 at 3:43 PM, Kyrill Tkachov w= rote: >> On 03/08/15 14:37, Uros Bizjak wrote: >>> On Mon, Aug 3, 2015 at 3:02 PM, Kyrill Tkachov >>> wrote: >>>> On 03/08/15 13:33, Uros Bizjak wrote: >>>>> Hello! >>>>> >>>>>> 2015-07-30 Kyrylo Tkachov >>>>>> >>>>>> * ifcvt.c (noce_try_store_flag_constants): Make logic of the = case >>>>>> when diff =3D=3D STORE_FLAG_VALUE or diff =3D=3D -STORE_FLAG_= VALUE more >>>>>> explicit. Prefer to add the flag whenever possible. >>>>>> (noce_process_if_block): Try noce_try_store_flag_constants be= fore >>>>>> noce_try_cmove. >>>>>> >>>>>> 2015-07-30 Kyrylo Tkachov >>>>>> >>>>>> * gcc.target/aarch64/csel_bfx_1.c: New test. >>>>>> * gcc.target/aarch64/csel_imms_inc_1.c: Likewise. >>>>> This patch regressed following tests on x86_64: >>>>> >>>>> FAIL: gcc.target/i386/cmov2.c scan-assembler sbb >>>>> FAIL: gcc.target/i386/cmov3.c scan-assembler cmov[^3] >>>> >>>> Sorry for that :( >>>> I could have sworn they passed for me during my run... >>>> >>>>> While cmov3 case is questionable by itself in light of PR56309 [1], >>>>> the cnov2 case regressed from: >>>>> >>>>> cmpl %edi, %esi >>>>> sbbl %eax, %eax >>>>> andl $-10, %eax >>>>> addl $5, %eax >>>>> ret >>>>> >>>>> to: >>>>> >>>>> xorl %eax, %eax >>>>> cmpl %esi, %edi >>>>> setbe %al >>>>> negl %eax >>>>> andl $10, %eax >>>>> subl $5, %eax >>>>> ret >>>>> >>>>> Please note that sbb (aka {*x86_movsicc_0_m1} ) is not generated >>>>> anymore. Non-QImode setcc instructions are problematic on x86, since >>>>> they need to be zero-extended to their full length. >>> IMO, we can check if the target is able to generate insn that >>> implements conditional move between 0 and -1 for certain comparison >>> mode, like: >>> >>> #(insn:TI 27 26 28 2 (parallel [ >>> # (set (reg:SI 0 ax [orig:87 D.1845 ] [87]) >>> # (if_then_else:SI (ltu:SI (reg:CC 17 flags) >>> # (const_int 0 [0])) >>> # (const_int -1 [0xffffffffffffffff]) >>> # (const_int 0 [0]))) >>> # (clobber (reg:CC 17 flags)) >>> # ]) cmov2.c:9 947 {*x86_movsicc_0_m1} >>> # (expr_list:REG_DEAD (reg:CC 17 flags) >>> # (expr_list:REG_UNUSED (reg:CC 17 flags) >>> # (nil)))) >>> sbbl %eax, %eax # 27 *x86_movsicc_0_m1 [lengt= h =3D >>> 2] >>> >>> this is the key insn, and as shown above, further asm sequence is >>> similar between patched and unpatched compiler. >> >> Hmm yes. >> We have a HAVE_conditional_move check, but that's not fine grained enoug= h. >> How about trying an emit_conditional_move and checking that the result i= s a >> single insn? > Looking at the x86 movcc expansion code (ix86_expand_int_movcc) I > don't think this is a good idea. In the expander, there is already > quite some target-dependent code that goes great length to utilize sbb > insn as much as possible, before cmove is used. > > IMO, as far as x86 is concerned, the best solution would be to revert > the change. ix86_expand_int_movcc already does some tricks from your > patch in a target-efficient way. Generic change that was introduced by > your patch now interferes with this expansion. Well, technically the transformation was already there, it was just never reached for an x86 compilation because noce_try_cmove was tried in front of= it and used a target-specific expansion. In any case, how's this proposal? The transformation noce_try_store_flag_constants /* if (test) x =3D a; else x =3D b; =3D> x =3D (-(test !=3D 0) & (b - a)) + a; */ Is a catch-all-immediates transformation in noce_try_store_flag_constants. What if we moved it to noce_try_cmove and performed it only if the target-s= pecific conditional move expansion there failed? That way we can try the x86_64-specific sequence first and still give the o= pportunity to noce_try_store_flag_constants to perform the transformations that can be= nefit targets that don't have highly specific conditional move expanders. Kyrill > > Uros. >