From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96825 invoked by alias); 1 Mar 2016 23:44:38 -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 96815 invoked by uid 89); 1 Mar 2016 23:44:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=diving, amkerchenggmailcom, amker.cheng@gmail.com, Hx-languages-length:2463 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, 01 Mar 2016 23:44:36 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id BD76FC0005D1; Tue, 1 Mar 2016 23:44:34 +0000 (UTC) Received: from slagheap.utah.redhat.com (ovpn-113-147.phx2.redhat.com [10.3.113.147]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u21NiYoc006548; Tue, 1 Mar 2016 18:44:34 -0500 Subject: Re: [PATCH PR69052]Check if loop inv can be propagated into mem ref with additional addr expr canonicalization To: "Bin.Cheng" References: <56BC34CE.20100@redhat.com> <56BD189A.1040409@redhat.com> <56C7962A.4010508@redhat.com> <56CEA194.1090204@redhat.com> Cc: Bin Cheng , "gcc-patches@gcc.gnu.org" , nd From: Jeff Law Message-ID: <56D62962.3010309@redhat.com> Date: Tue, 01 Mar 2016 23:44:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg00112.txt.bz2 On 03/01/2016 10:08 AM, Bin.Cheng wrote: > On Thu, Feb 25, 2016 at 8:47 AM, Bin.Cheng wrote: >> On Thu, Feb 25, 2016 at 6:39 AM, Jeff Law wrote: >>> On 02/22/2016 02:22 AM, Bin.Cheng wrote: >>> >>>>> My only question is why didn't you use FOR_EACH_SUBRTX_VRA from >>>>> rtl-iter.h >>>>> to walk the RTX expressions in collect_address_parts and >>>>> canonicalize_address_mult? >>>> >>>> Hi Jeff, >>>> Nothing special, just I haven't used this before, also >>>> canonicalize_address_mult is alphabetically copied from fwprop.c. One >>>> question is when rewriting SHIFT to MULT, we need to modify rtl >>>> expression in place, does FOR_EACH_SUBRTX iterator support this? If >>>> yes, what is the behavior for modified sub-expression? >>> >>> Hmm. The question of semantics when we change the underlying >>> sub-expressions is an interesting one. >>> >>> While I think we're OK in practice using the iterators, I think that's more >>> of an accident than by design -- if MULT/ASHIFT had a different underlying >>> RTL structure then I'm much less sure using the iterators would be safe. >> Hi Jeff, >> Yes, I thought about this too and finally decided to skip sub rtxes in >> modified MULT/ASHIFT expressions. I think this will make the patch >> with FOR_EACH_SUBRTX iterator safe. Although it doesn't dive into >> MULT/ASHIFT while the original one does, I don't think there is such >> case in practice, after all we can't handle such complicated address >> expression either. >>> >>> Let's go with your original patch that didn't use the iterators. Sorry for >>> making you do the additional work/testing to build the iterator version. >> Not even a problem. >>> But after pondering the issue you raised, I think your original patch is >>> safer. >> So in conclusion, I think both versions should be safe, the iterator >> one is definitely cleaner. It is your call which version I should >> apply. > Hi Jeff, > I tend to apply the new patch with FOR_EACH_SUBRTX because it's > clearer. Does it work for you? Of course if you do think it's not > that safe I will fall back to the old one. Sorry, things are just busy here. I'm reasonably comfortable with both as long as we're not diving inside the subexpressions in the ASHIFT/MULT case. If you'd prefer the FOR_EACH_SUBRTX variant, that's OK with me. jeff