From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72123 invoked by alias); 17 Nov 2015 09:49:07 -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 72098 invoked by uid 89); 17 Nov 2015 09:49:07 -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) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 17 Nov 2015 09:49:05 +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-25-xXq-wnUdQ267kZuX5Rp0qQ-1; Tue, 17 Nov 2015 09:49:01 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 17 Nov 2015 09:49:00 +0000 Message-ID: <564AF80C.3010605@arm.com> Date: Tue, 17 Nov 2015 09:49: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> In-Reply-To: <564AEE94.3070708@arm.com> X-MC-Unique: xXq-wnUdQ267kZuX5Rp0qQ-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02048.txt.bz2 On 17/11/15 09:08, Kyrill Tkachov wrote: > 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_use= d_between_p for reg1 to ensure it's not used. I'm thinking this might be a = little 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 mus= t iterate over a number of instructions > and call reg_overlap_mentioned_p on each one. But I suppose this case is = rare enough that it wouldn't make any > measurable difference. Actually, I tried it out. And while a check reg_used_between_p fixed the te= stcase, it caused code quality regressions on aarch64. Seems it's too aggressive in restricting ree. I'll have a closer look. Kyrill > > Would you prefer to use !reg_used_between_p here? > >> >> The added comment could lead to some confusion since it's placed in fron= t of an existing if statement that also tests a different condition. Also, = if we go with your fix, >> >>> + || !reg_overlap_mentioned_p (tmp_reg, SET_SRC (PATTERN (cand->in= sn)))) >> >> 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 >> >