From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42689 invoked by alias); 24 Nov 2015 05:38:18 -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 42674 invoked by uid 89); 24 Nov 2015 05:38:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 24 Nov 2015 05:38:16 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 7E386692; Tue, 24 Nov 2015 05:38:15 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-91.phx2.redhat.com [10.3.113.91]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAO5cE1e025555; Tue, 24 Nov 2015 00:38:14 -0500 Subject: Re: [PR64164] drop copyrename, integrate into expand To: Alexandre Oliva References: <20150810082355.GA31149@arm.com> <55C8BFC3.3030603@redhat.com> <55E72D4C.40705@arm.com> <55FC3171.7040509@arm.com> <56420DC4.3070407@arm.com> <564280E0.7090700@redhat.com> <56458448.7030301@redhat.com> Cc: Alan Lawrence , "gcc-patches@gcc.gnu.org" , Marcus Shawcroft , James Greenhalgh From: Jeff Law Message-ID: <5653F7C6.6000707@redhat.com> Date: Tue, 24 Nov 2015 05:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02820.txt.bz2 On 11/16/2015 05:07 PM, Alexandre Oliva wrote: > > The check is not in my patch, indeed. That's because the previous block > performs the runtime check, and it only lets through two cases: the one > we handle, and the one nobody uses. That was the conclusion I was starting to come to, but expressed so poorly in my last message. Sadly it was non-obvious from staring at the current code. Though I must admit that after a week, I can see it better now. Maybe that's a result of re-reading your message a half-dozen more times with the current code and your patch all visible in windows next to each other :-) Prior to your change we'd just blindly copy from ENTRY_PARM to MEM, which would result in missing the implicit shift if MEM wasn't actually a memory. You're just moving that conditional up and handling the shift explicitly. You've got asserts for the cases you're not handling (and no, I'm not aware of the need for this on any LE architecture, while I am aware of BE architectures that align in both directions). > Any suggestions on how to improve the comments so that they convey > enough of this reasoning to make sense, without our having to write a > book :-) on the topic? Refer back to this thread? :-) Seriously though, looking at things a week later, I can see it much better now. Thanks for your patience on this. OK for the trunk, jeff