From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 130437 invoked by alias); 31 Jul 2015 10:49:53 -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 130428 invoked by uid 89); 31 Jul 2015 10:49:52 -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; Fri, 31 Jul 2015 10:49:51 +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-6-8RsIt-YXSdatK3-PDjfetA-1; Fri, 31 Jul 2015 11:49:46 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 31 Jul 2015 11:49:46 +0100 Message-ID: <55BB52CA.1010506@arm.com> Date: Fri, 31 Jul 2015 10:58: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: Ramana Radhakrishnan , GCC Patches CC: Richard Earnshaw Subject: Re: [PATCH][ARM][2/3] Make if_neg_move and if_move_neg into insn_and_split References: <55B219AB.3040901@arm.com> <55BB4B5B.1030202@foss.arm.com> <55BB4CEA.6090203@arm.com> <55BB4F26.6070301@foss.arm.com> In-Reply-To: <55BB4F26.6070301@foss.arm.com> X-MC-Unique: 8RsIt-YXSdatK3-PDjfetA-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg02652.txt.bz2 On 31/07/15 11:34, Ramana Radhakrishnan wrote: >>> >>> So, we have a predicate that doesn't cover all the constraints - in thi= s case aren't we forcing everything into operand0. What happens if we just = delete this pattern instead of turning it into an insn_and_split - after al= l we have other parts of the backend where conditional negates and conditio= nal moves will be caught and cond-exec probably post dates some of these if= -then-else patterns. >> Hmmm yes, I think operand 1 should be tightened to s_register_operand. >> The reason I want this pattern is so that I can expand to it in patch 3/= 3 where I want to create >> a conditional negate expression. However, I can't just emit a COND_EXEC = at expand time. I found that >> reload doesn't handle the dataflow through them properly. With this patt= ern I can carry the if_then_else >> around and split it into the conditional negate only after reload when i= t's safe. > But don't we loose because the immediate alternatives have been lost ? i.= e. the original pattern allowed us to express conditional negates where the= else condition was a move of an immediate. Thus one didn't require an addi= tional register. Or are you arguing that this is no longer required ? I am arguing that this is no longer required. In the original pattern the c= ases where operand 1 is an immediate just outputs: mov%D4\\t%0, %1\;rsb%d4\\t%0, %2, #0 or mvn%D4\\t%0, #%B1\;rsb%d4\\t%0, %2, #0 It doesn't do anything smart. I can build SPEC2006 with and without this patch to check for suspect code = differences, but I suspect there won't be much that matches it. For example, even without this patch it doesn't get used for: int foo (int a) { return a > 5 ? -a : 8; } Bottom line: I'll have a look at codegen differences in SPEC to see if ther= e's codegen degradation Thanks, Kyrill >=20=20=20 > > Not enough coffee in system yet. > > regards > Ramana > >> Kyrill >> >>> >>>> + "TARGET_32BIT" >>>> + "#" >>>> + "&& reload_completed" >>>> + [(cond_exec (match_op_dup 4 [(match_dup 3) (const_int 0)]) >>>> + (set (match_dup 0) (neg:SI (match_dup 2))))] >>>> + "" >>>> [(set_attr "conds" "use") >>>> - (set_attr "length" "4,8,8") >>>> - (set_attr "type" "logic_shift_imm,multiple,multiple")] >>>> + (set_attr "length" "4") >>>> + (set_attr "arch" "t2,32") >>>> + (set_attr "enabled_for_depr_it" "yes,no") >>>> + (set_attr "type" "logic_shift_imm")] >>>> ) >>>>=20=20=20=20 >>>> (define_insn "*ifcompare_move_neg" >>>> @@ -10097,21 +10100,34 @@ (define_insn "*ifcompare_move_neg" >>>> (set_attr "type" "multiple")] >>>> ) >>>>=20=20=20=20 >>>> -(define_insn "*if_move_neg" >>>> - [(set (match_operand:SI 0 "s_register_operand" "=3Dr,r,r") >>>> +(define_insn_and_split "*if_move_neg" >>>> + [(set (match_operand:SI 0 "s_register_operand" "=3Dl,r") >>>> (if_then_else:SI >>>> (match_operator 4 "arm_comparison_operator" >>>> [(match_operand 3 "cc_register" "") (const_int 0)]) >>>> - (match_operand:SI 1 "arm_not_operand" "0,?rI,K") >>>> - (neg:SI (match_operand:SI 2 "s_register_operand" "r,r,r"))))] >>>> - "TARGET_ARM" >>>> - "@ >>>> - rsb%D4\\t%0, %2, #0 >>>> - mov%d4\\t%0, %1\;rsb%D4\\t%0, %2, #0 >>>> - mvn%d4\\t%0, #%B1\;rsb%D4\\t%0, %2, #0" >>>> + (match_operand:SI 1 "arm_not_operand" "0,0") >>>> + (neg:SI (match_operand:SI 2 "s_register_operand" "l,r"))))] >>>> + "TARGET_32BIT" >>>> + "#" >>>> + "&& reload_completed" >>>> + [(cond_exec (match_dup 5) >>>> + (set (match_dup 0) (neg:SI (match_dup 2))))] >>>> + { >>>> + machine_mode mode =3D GET_MODE (operands[3]); >>>> + rtx_code rc =3D GET_CODE (operands[4]); >>>> + >>>> + if (mode =3D=3D CCFPmode || mode =3D=3D CCFPEmode) >>>> + rc =3D reverse_condition_maybe_unordered (rc); >>>> + else >>>> + rc =3D reverse_condition (rc); >>>> + >>>> + operands[5] =3D gen_rtx_fmt_ee (rc, VOIDmode, operands[3], const0= _rtx); >>>> + } >>>> [(set_attr "conds" "use") >>>> - (set_attr "length" "4,8,8") >>>> - (set_attr "type" "logic_shift_imm,multiple,multiple")] >>>> + (set_attr "length" "4") >>>> + (set_attr "arch" "t2,32") >>>> + (set_attr "enabled_for_depr_it" "yes,no") >>>> + (set_attr "type" "logic_shift_imm")] >>>> ) >>> Same as above. >>> >>>>=20=20=20=20 >>>> (define_insn "*arith_adjacentmem"