Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Kevin Buettner <kevinb@redhat.com>
To: Michael Snyder <msnyder@cygnus.com>, Elena Zannoni <ezannoni@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] ppc: include register numbers in gdbarch_tdep structure.
Date: Mon, 10 Dec 2001 12:25:00 -0000	[thread overview]
Message-ID: <1011210202327.ZM32506@ocotillo.lan> (raw)
In-Reply-To: Michael Snyder <msnyder@cygnus.com> "Re: [RFA] ppc: include register numbers in gdbarch_tdep structure." (Dec 10, 11:30am)

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


  reply	other threads:[~2001-12-10 20:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-12-09 11:58 Elena Zannoni
2001-12-09 13:24 ` Kevin Buettner
2001-12-09 13:38   ` Elena Zannoni
2001-12-09 15:09   ` Andrew Cagney
2001-12-12 12:16     ` Mark Kettenis
2001-12-10 11:33 ` Michael Snyder
2001-12-10 12:25   ` Kevin Buettner [this message]
2001-12-10 12:41     ` Elena Zannoni
2001-12-10 13:22       ` Kevin Buettner
2001-12-10 14:34         ` Andrew Cagney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1011210202327.ZM32506@ocotillo.lan \
    --to=kevinb@redhat.com \
    --cc=ezannoni@cygnus.com \
    --cc=gdb-patches@sources.redhat.com \
    --cc=msnyder@cygnus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox