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

Elena Zannoni
2001-12-10T20:41:07Z

Kevin Buettner writes:
> 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.
>

Why would you want to convert the code back? Anyway, I find it also
easier to debug the code, if you don't have a macro.

> 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.
>

But having a macro defined to be the same function call is not going
to speed up the evaluation.

> 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?

Sometimes, doing something because it was done in the past is not a
good metric. I think multiarch was developed leaving the macros
because it was a necessary thing to both minimize changes and keep
compatibility with non-multiarched targets. But here I would argue it
is a different situation.

>
> 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.)
>

Sorry, I disagree.

> Kevin

Elena