From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 129903 invoked by alias); 18 Nov 2015 09:11: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 129864 invoked by uid 89); 18 Nov 2015 09:11:54 -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; Wed, 18 Nov 2015 09:11:52 +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-22-Qz3rSUkjSTKhhJ6jf2G3Hw-1; Wed, 18 Nov 2015 09:11:47 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 18 Nov 2015 09:11:45 +0000 Message-ID: <564C40D1.80409@arm.com> Date: Wed, 18 Nov 2015 09:11: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: Bernd Schmidt , GCC Patches CC: Jeff Law Subject: Re: [PATCH][RTL-ree] PR rtl-optimization/68194: Restrict copy instruction in presence of conditional moves References: <5649E333.4090904@arm.com> <564A2339.3030308@redhat.com> <564AEE94.3070708@arm.com> <564B1934.6050300@redhat.com> <564B259A.90206@arm.com> <564BB42C.1020401@redhat.com> In-Reply-To: <564BB42C.1020401@redhat.com> X-MC-Unique: Qz3rSUkjSTKhhJ6jf2G3Hw-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02171.txt.bz2 On 17/11/15 23:11, Bernd Schmidt wrote: > On 11/17/2015 02:03 PM, Kyrill Tkachov wrote: >> + || !reg_overlap_mentioned_p (tmp_reg, SET_SRC (PATTERN (cand->ins= n)))) >> return false; > >> Well, I think the statement we want to make is >> "return false from this function if the two expressions contain the same >> register number". > > I looked at it again and I think I'll need more time to really figure out= what's going on in this pass. > > However, about the above... either I'm very confused, or the actual state= ment here is "return false _unless_ the two expressions contain the same re= gister number". In the testcase, the regs in question are ax and bx, which = is then=20 > rejected if the patch has been applied. I'm sorry, it is my mistake in explaining. I meant to say: "return false from this function unless the two expressions contain the same register number" > > (gdb) p tmp_reg > (reg:SI 0 ax) > (gdb) p cand->insn > (insn 59 117 60 7 (set (reg:SI 4 si [orig:107 D.1813 ] [107]) > (sign_extend:SI (reg:HI 3 bx [orig:99 D.1813 ] [99]))) > > And I think it would be better to strengthen that to "return false unless= registers are identical". (From the above it's clear however that a plain = rtx_equal_p wouldn't work since there's an extension in the operand). So the three relevant instructions are: I1: (set (reg:HI 0 ax) (mem:HI (symbol_ref:DI ("f")))) I2: (set (reg:SI 3 bx) (if_then_else:SI (eq (reg:CCZ 17 flags) (const_int 0)) (reg:SI 0 ax) (reg:SI 3 bx))) I3: (set (reg:SI 4 si) (sign_extend:SI (reg:HI 3 bx))) I1 is def_insn, I3 is cand->insn. tmp_reg is 'ax'. What we want to do is re= ject this transformation because the destination of def_insn (aka I1), that is 'ax', is not the oper= and of the extend operation in cand->insn (aka I3). As you said, rtx_equal won't work on just SET_SRC (= PATTERN (cand->insn)) because it's an extend operation. So reg_overlap_mentioned should be appropriate. Hope this helps. > > Also, I had another look at the testcase. It uses __builtin_printf and dg= -output, which is at least unusual. Try to use the normal "if (cond) abort = ()". > I did not try to remove the __builtin_printf because the use of 'f' there i= s needed to trigger this. There are at least two other dups of this PR: 68328 and 68185 the testcases= for which have an "if (cond) abort ();" form. I'll use them instead. > > Bernd >