From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13129 invoked by alias); 12 Aug 2014 20:11:55 -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 13064 invoked by uid 89); 12 Aug 2014 20:11:54 -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; Tue, 12 Aug 2014 20:11:49 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7CKBk1H013043 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Aug 2014 16:11:47 -0400 Received: from stumpy.slc.redhat.com (ovpn-113-24.phx2.redhat.com [10.3.113.24]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7CKBj5p012843; Tue, 12 Aug 2014 16:11:46 -0400 Message-ID: <53EA7501.1010508@redhat.com> Date: Tue, 12 Aug 2014 20:11: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> <53E2750E.2070907@redhat.com> <87oavuq8ch.fsf@googlemail.com> In-Reply-To: <87oavuq8ch.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/msg01194.txt.bz2 On 08/09/14 04:13, Richard Sandiford wrote: > Jeff Law writes: >> 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. > > But either: > > (a) those notes would contain side effects that are also present in the > main pattern, e.g.: > > (set (reg Z) (plus (mem (pre_inc X)) (reg Y))) > REG_EQUAL: (plus (mem (pre_inc X)) (const_int Z)) > > (b) those notes would contain side effects that are not present in the > main pattern. > > (b) seems completely invalid to me. REG_EQUAL notes are just a hint > and it's perfectly OK to remove them if they're no longer accurate > (e.g. because a register value used in the note is no longer available). > It's also possible to remove them if the set destination gets combined > with something else. Plus the whole idea of a REG_EQUAL note is that > you could replace the SET_SRC with the note value without changing the > effect of the instruction. > > For (a) the current code would end up recording the same side-effect > twice, so looking at just the pattern is likely to be a bug fix. > But (a) is probably invalid too in practice. The note shows another way to express what appears on the RHS. In theory the note is supposed to be a simpler form. So in the case of (a) we might have a PARALLEL in the pattern, but a REG_INC in the note. I don't see how that'd be terribly helpful though. I agree (b) is invalid. > > Just a guess, but maybe the thing you were thinking of was related to > the old REG_LIBCALL/REG_RETVAL support? Although I only vaguely remember > how that worked now... I thought it was something in reload's reg_equiv handling that I stumbled over at some point. The tiny bit I remember was an auto-inc in the note and something wanting to substitute the note for a use elsewhere in the insn stream. My recollection was the note had an auto-inc addressing mode which significantly complicates the validity of such a transformation. However, the only thread I can find is one from 2009 between myself and Ian, but it's not dealing with an autoinc addressing mode in the note. And at some level it simply doesn't make much sense to have the auto-inc addressing mode in a REG_EQUAL note. I guess we could declare that invalid and cope with it if we ever find one. Perhaps a bit of ENABLE_CHECKING to detect if we ever create such a note? Jeff