From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16403 invoked by alias); 11 Sep 2010 10:31:23 -0000 Received: (qmail 16394 invoked by uid 22791); 11 Sep 2010 10:31:22 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mail-qw0-f47.google.com (HELO mail-qw0-f47.google.com) (209.85.216.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 11 Sep 2010 10:31:16 +0000 Received: by qwa26 with SMTP id 26so1840317qwa.20 for ; Sat, 11 Sep 2010 03:31:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.2.23 with SMTP id 23mr1189555qch.278.1284201074357; Sat, 11 Sep 2010 03:31:14 -0700 (PDT) Received: by 10.229.65.226 with HTTP; Sat, 11 Sep 2010 03:31:14 -0700 (PDT) In-Reply-To: References: <20100315131650.GV2817@tyan-ft48-01.lab.bos.redhat.com> Date: Sat, 11 Sep 2010 12:41:00 -0000 Message-ID: Subject: Re: [PATCH] Fix VTA sp (and fp) based MEM handling (PR debug/43051, PR debug/43092) From: Steven Bosscher To: Alexandre Oliva Cc: Jakub Jelinek , bernds@codesourcery.com, Richard Guenther , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 X-SW-Source: 2010-09/txt/msg01010.txt.bz2 On Sat, Sep 11, 2010 at 1:34 AM, Alexandre Oliva wrote: > The first patch implements unconditional autoinc support in cselib. =A0I > have regstrapped it on x86_64-linux-gnu and i686-linux-gnu. =A0Ok to > install? You should document what the arguments are here: > +typedef int (*for_each_inc_dec_fn) (rtx, rtx, rtx, rtx, rtx, void *); and here too: > +int > +for_each_inc_dec (rtx *insn, > + for_each_inc_dec_fn fn, > + void *arg) Looks like INSN doesn't have to be an insn, so maybe not call the argument INSN but X just like in for_each_rtx. > - cselib_lookup_from_insn (XEXP (t, 0), address_mode, 1, insn); > - XEXP (t, 0) =3D cselib_subst_to_values (XEXP (t, 0)); > + cselib_lookup_from_insn (XEXP (t, 0), address_mode, 1, > + GET_MODE (x), insn); > + XEXP (t, 0) =3D cselib_subst_to_values (XEXP (t, 0), GET_MODE (x)); GET_MODE (t)? I know t is a copy of x at this point so it makes no real difference, but the existing code looks at the properties of t here and IMHO it would be better to be consistent about that. Likewise in the hunk for sel-sched-dump.c Thanks for working on this, Ciao! Steven