From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 82453 invoked by alias); 31 Jul 2015 10:24:55 -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 82379 invoked by uid 89); 31 Jul 2015 10:24:55 -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:24:53 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-14-vHYJUGdnQfqzJqy5qZjqiA-1; Fri, 31 Jul 2015 11:24:42 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 31 Jul 2015 11:24:42 +0100 Message-ID: <55BB4CEA.6090203@arm.com> Date: Fri, 31 Jul 2015 10:34: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: Ramana Radhakrishnan , 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> In-Reply-To: <55BB4B5B.1030202@foss.arm.com> X-MC-Unique: vHYJUGdnQfqzJqy5qZjqiA-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg02647.txt.bz2 On 31/07/15 11:18, Ramana Radhakrishnan wrote: > Hmmm, not sure if this is that straightforward just looking at this.. > >> 2015-07-24 Kyrylo Tkachov >> >> * config/arm/arm.md (*if_neg_move): Convert to insn_and_split. >> Enable for TARGET_32BIT. >> (*if_move_neg): Likewise. >> commit 1b495b6cb68c77f628e1c1d672c06dcdf5ccf79b >> Author: Kyrylo Tkachov >> Date: Thu Jul 23 09:20:41 2015 +0100 >> [ARM] Make if_neg_move and if_move_neg into insn_and_split >> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md >> index 0be70a8..f341109 100644 >> --- a/gcc/config/arm/arm.md >> +++ b/gcc/config/arm/arm.md >> @@ -10064,21 +10064,24 @@ (define_insn "*ifcompare_neg_move" >> (set_attr "type" "multiple")] >> ) >>=20=20=20 >> -(define_insn "*if_neg_move" >> - [(set (match_operand:SI 0 "s_register_operand" "=3Dr,r,r") >> +(define_insn_and_split "*if_neg_move" >> + [(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)]) >> - (neg:SI (match_operand:SI 2 "s_register_operand" "r,r,r")) >> - (match_operand:SI 1 "arm_not_operand" "0,?rI,K")))] >> - "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" >> + (neg:SI (match_operand:SI 2 "s_register_operand" "l,r")) >> + (match_operand:SI 1 "arm_not_operand" "0,0")))] > > > So, we have a predicate that doesn't cover all the constraints - in this = case aren't we forcing everything into operand0. What happens if we just de= lete this pattern instead of turning it into an insn_and_split - after all = we have other parts of the backend where conditional negates and conditiona= l moves will be caught and cond-exec probably post dates some of these if-t= hen-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 w= here 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 pattern= I can carry the if_then_else around and split it into the conditional negate only after reload when it's= safe. 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 >> (define_insn "*ifcompare_move_neg" >> @@ -10097,21 +10100,34 @@ (define_insn "*ifcompare_move_neg" >> (set_attr "type" "multiple")] >> ) >>=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_r= tx); >> + } >> [(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 >> (define_insn "*arith_adjacentmem"