From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 112154 invoked by alias); 3 Aug 2015 14:12:04 -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 112141 invoked by uid 89); 3 Aug 2015 14:12:04 -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) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Aug 2015 14:12:02 +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-2-bqes-OltT3CnATqD9OYzew-1; Mon, 03 Aug 2015 15:11:58 +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 15:11:58 +0100 Message-ID: <55BF76AD.5000406@arm.com> Date: Mon, 03 Aug 2015 14:12: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: <55BF6FE5.8020902@arm.com> X-MC-Unique: bqes-OltT3CnATqD9OYzew-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00070.txt.bz2 On 03/08/15 14:43, Kyrill Tkachov wrote: > 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 c= ase >>>>> when diff =3D=3D STORE_FLAG_VALUE or diff =3D=3D -STORE_FLAG_V= ALUE more >>>>> explicit. Prefer to add the flag whenever possible. >>>>> (noce_process_if_block): Try noce_try_store_flag_constants bef= ore >>>>> 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 [length= =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 enough. > How about trying an emit_conditional_move and checking that the result is= a > single insn? Hmm, looking deeper into it, I see that: /* if (test) x =3D a; else x =3D b; =3D> x =3D (-(test !=3D 0) & (b - a)) + a; */ is exactly the transformation that: cmpl %edi, %esi sbbl %eax, %eax andl $-10, %eax addl $5, %eax is supposed to represent, right? It seems that noce_try_store_flag_constant= s is just not emitting the optimal rtx forms for x86 So, if before the patch the code in noce_try_store_flag_constants was never= being called what performed this transformation? Turns out it's the conditional move exp= ander in emit_conditional_move that ends up calling the ix86_expand_int_movcc that p= erforms lots of these complex transformations, including this one in an x86-specific way. I suppose that's why noce_try_store_flag_constants was tried after noce_try= _cmove. Because, it is expected that noce_try_store_flag_constants will call the co= nditional move expander and the target-specific conditional move expander is expected to d= o all the complex transformations beneficial to the target. I suppose the comment "Try only s= imple constants and registers here" above noce_try_cmove is somewhat misleading then. I see a couple of avenues here. We try both noce_try_cmove and noce_try_sto= re_flag_constants, compare their seq_cost and pick the cheapest, which sounds somewhat ugly to me, or in noc= e_try_store_flag_constants we try to be more fine-grained and try to restrict some of the transformations= , but on what criteria? Kyrill > > Kyrill > > >> Uros. >>