From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15930 invoked by alias); 10 Dec 2001 20:25:03 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 15541 invoked from network); 10 Dec 2001 20:23:47 -0000 Received: from unknown (HELO cygnus.com) (205.180.230.5) by sources.redhat.com with SMTP; 10 Dec 2001 20:23:47 -0000 Received: from cse.cygnus.com (cse.cygnus.com [205.180.230.236]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id MAA25040; Mon, 10 Dec 2001 12:23:45 -0800 (PST) Received: (from kev@localhost) by cse.cygnus.com (8.9.3/8.9.3) id NAA32507; Mon, 10 Dec 2001 13:23:27 -0700 Date: Mon, 10 Dec 2001 12:25:00 -0000 From: Kevin Buettner Message-Id: <1011210202327.ZM32506@ocotillo.lan> In-Reply-To: Michael Snyder "Re: [RFA] ppc: include register numbers in gdbarch_tdep structure." (Dec 10, 11:30am) References: <15379.50110.429092.453327@krustylu.cygnus.com> <3C150D60.631D281C@cygnus.com> X-Mailer: Z-Mail (4.0.1 13Jan97 Caldera) To: Michael Snyder , Elena Zannoni Subject: Re: [RFA] ppc: include register numbers in gdbarch_tdep structure. Cc: gdb-patches@sources.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2001-12/txt/msg00279.txt.bz2 On Dec 10, 11:30am, Michael Snyder wrote: > Elena Zannoni wrote: > > > +++ ppc-bdm.c 2001/12/09 19:55:12 > > @@ -200,8 +200,8 @@ bdm_ppc_fetch_registers (int regno) > > /* printf("Asking for register %d\n", first_regno); */ > > > > /* if asking for an invalid register */ > > - if ((first_regno == PPC_MQ_REGNUM) || > > - ((first_regno >= FP0_REGNUM) && (first_regno <= FPLAST_REGNUM))) > > + if ((first_regno == gdbarch_tdep (current_gdbarch)->ppc_mq_regnum) > > + || ((first_regno >= FP0_REGNUM) && (first_regno <= FPLAST_REGNUM))) > > [and many similar changes] > > Not to be nit-picky, and I realize it's already been approved and > committed, > but wouldn't this code look prettier if we simply provided something > like: > > #define PPC_MQ_REGNUM gdbarch_tdep (current_gdbarch)->ppc_mq_regnum As with most things we do there are pros and cons. On the pro side, I agree that it looks prettier and is easier to read. Also, having these defines makes it easier to convert the code back to using actual constants some day. These are both excellent reasons to do as Michael suggests. The drawback to using a macro like this is that it hides what's really going on. (But note that it is this very same quality that enhances readability.) In this case, the macro looks like a constant, leading to the expectation that it is a constant and has the usual costs associated with constants. However, the runtime costs associated with using this expression are significantly greater than using a constant. That said, those of us accustomed to working on GDB are used to this by now, aren't we? E.g, consider: for (regno = 0; regno < NUM_REGS; regno++) ... We all realize that a function is being called each time the test ``regno < NUM_REGS'' is performed, don't we? If so, then I'm all in favor of Michael's suggestion. (Actually, I'm in favor of Michael's suggestion anyway. But, I do think we need to be more careful about how we write code that might potentially contain a hidden function call.) Kevin