From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32697 invoked by alias); 6 Aug 2014 18:33:53 -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 32688 invoked by uid 89); 6 Aug 2014 18:33:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_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; Wed, 06 Aug 2014 18:33:52 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s76IXpP2010269 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 6 Aug 2014 14:33:51 -0400 Received: from stumpy.slc.redhat.com (ovpn-113-104.phx2.redhat.com [10.3.113.104]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s76IXoMH021539; Wed, 6 Aug 2014 14:33:50 -0400 Message-ID: <53E2750E.2070907@redhat.com> Date: Wed, 06 Aug 2014 18:33:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Subject: Re: [PATCH 40/50] rtlanal.c:for_each_inc_dec References: <87y4v5d77q.fsf@googlemail.com> <8761i97ieu.fsf@googlemail.com> In-Reply-To: <8761i97ieu.fsf@googlemail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-08/txt/msg00737.txt.bz2 On 08/03/14 08:32, Richard Sandiford wrote: > The old for_each_inc_dec callback had a for_each_rtx-like return value, > with >0 being returned directly, 0 meaning "continue" and <0 meaning > "skip subrtxes". But there's no reason to distinguish the latter two > cases since auto-inc/dec expressions aren't allowed to contain other > auto-inc/dec expressions. And if for_each_rtx is going away, there's > no longer any consistency argument for using the same interface. > > > gcc/ > * rtl.h (for_each_inc_dec_fn): Remove special case for -1. > * cselib.c (cselib_record_autoinc_cb): Update accordingly. > (cselib_record_sets): Likewise. > * dse.c (emit_inc_dec_insn_before, check_for_inc_dec_1) > (check_for_inc_dec): Likewise. > * rtlanal.c (for_each_inc_dec_ops): Delete. > (for_each_inc_dec_find_inc_dec): Take the MEM as argument, > rather than a pointer to the memory address. Replace > for_each_inc_dec_ops argument with separate function and data > arguments. Abort on non-autoinc addresses. > (for_each_inc_dec_find_mem): Delete. > (for_each_inc_dec): Use FOR_EACH_SUBRTX_VAR to visit every > autoinc MEM. So this patch has me a little bit concerned. > @@ -2523,7 +2523,7 @@ cselib_record_sets (rtx insn) > > data.sets = sets; > data.n_sets = n_sets_before_autoinc = n_sets; > - for_each_inc_dec (&insn, cselib_record_autoinc_cb, &data); > + for_each_inc_dec (&PATTERN (insn), cselib_record_autoinc_cb, &data); > n_sets = data.n_sets; So wouldn't this miss an autoincrement operation embedded in a note, such as a REG_EQUAL note? My memory is very fuzzy here, but I can't recall any policy which prohibits an autoincrement addressing mode from appearing in a REG_EQUAL note. Worse yet, I have vague memories of embedded side effects actually showing up in REG_EQUAL notes. Similarly for other places where you're passing down the pattern rather than the insn itself. Thoughts/comments? Does this trigger any disturbing memories for anyone else? jeff