From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 78683 invoked by alias); 17 Nov 2015 09:08:43 -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 78652 invoked by uid 89); 17 Nov 2015 09:08:42 -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; Tue, 17 Nov 2015 09:08:41 +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-30-ErE0xRRjQAqQRKv3n8DhSA-1; Tue, 17 Nov 2015 09:08:36 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 17 Nov 2015 09:08:35 +0000 Message-ID: <564AEE94.3070708@arm.com> Date: Tue, 17 Nov 2015 09:08: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> In-Reply-To: <564A2339.3030308@redhat.com> X-MC-Unique: ErE0xRRjQAqQRKv3n8DhSA-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02038.txt.bz2 Hi Bernd, On 16/11/15 18:40, Bernd Schmidt wrote: > On 11/16/2015 03:07 PM, Kyrill Tkachov wrote: > >> I've explained in the comments in the patch what's going on but the >> short version is trying to change the destination of a defining insn >> that feeds into an extend insn is not valid if the defining insn >> doesn't feed directly into the extend insn. In the ree pass the only >> way this can happen is if there is an intermediate conditional move >> that the pass tries to handle in a special way. An equivalent fix >> would have been to check on that path (when copy_needed in >> combine_reaching_defs is true) that the state->copies_list vector >> (that contains the conditional move insns feeding into the extend >> insn) is empty. > > I ran this through gdb, and I think I see what's going on. For reference,= here's a comment from the source: > > /* Considering transformation of > (set (reg1) (expression)) > ... > (set (reg2) (any_extend (reg1))) > > into > > (set (reg2) (any_extend (expression))) > (set (reg1) (reg2)) > ... */ > > I was thinking that another possible fix would be to also check !reg_used= _between_p for reg1 to ensure it's not used. I'm thinking this might be a l= ittle clearer - what is your opinion? Yes, I had considered that as well. It should be equivalent. I didn't use != reg_used_between_p because I thought it'd be more expensive than checking reg_overlap_mentioned_p since we must = iterate over a number of instructions and call reg_overlap_mentioned_p on each one. But I suppose this case is ra= re enough that it wouldn't make any measurable difference. Would you prefer to use !reg_used_between_p here? > > The added comment could lead to some confusion since it's placed in front= of an existing if statement that also tests a different condition. Also, i= f we go with your fix, > >> + || !reg_overlap_mentioned_p (tmp_reg, SET_SRC (PATTERN (cand->ins= n)))) > > Shouldn't this really be !rtx_equal_p? > Maybe, will it behave the right way if the two regs have different modes or= when subregs are involved? (will we even hit such a case in this path?) Thanks, Kyrill > > Bernd >