This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] ppc: include register numbers in gdbarch_tdep structure.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]