Re: RFA: PowerPC: add segment register numbers

Kevin Buettner
2004-07-13T00:01:37Z

On 12 Jul 2004 17:08:11 -0500
Jim Blandy <jimb@redhat.com> wrote:

> * ppc-tdep.h (struct gdbarch_tdep): New member: ppc_sr0_regnum.
> * rs6000-tdep.c (rs6000_gdbarch_init): Initialize it.

I have a question and a comment...

First the question: Regarding this portion of the patch...

*************** rs6000_gdbarch_init (struct gdbarch_info
*** 2879,2884 ****
--- 2879,2885 ----
tdep->ppc_mq_regnum = -1;
tdep->ppc_fp0_regnum = 32;
tdep->ppc_fpscr_regnum = power ? 71 : 70;
+ tdep->ppc_sr0_regnum = 71;

...what about the case when ``power'' is non-zero? Do we want
ppc_fpscr_regnum and ppc_sr0_regnum to both be equal to 71?

Now the comment: Regarding the following portion of the patch...

> *************** rs6000_gdbarch_init (struct gdbarch_info
> *** 2939,2945 ****
> --- 2943,2961 ----
> set_gdbarch_pseudo_register_read (gdbarch, e500_pseudo_register_read);
> set_gdbarch_pseudo_register_write (gdbarch, e500_pseudo_register_write);
> break;
> +
> + case bfd_mach_ppc64:
> + case bfd_mach_ppc_620:
> + case bfd_mach_ppc_630:
> + case bfd_mach_ppc_a35:
> + case bfd_mach_ppc_rs64ii:
> + case bfd_mach_ppc_rs64iii:
> + /* These processor's register sets don't have segment registers. */
> + tdep->ppc_sr0_regnum = -1;
> + break;
> }
> + else
> + gdb_assert (0);

...I don't really like the gdb_assert (0).

With this patch, the code is structured as follows:

if (v->arch == bfd_arch_powerpc)
switch (v->mach)
{
...
}
else
gdb_assert (0);

Could you revise your patch so that the above is instead structured
like this...?

gdb_assert(v->arch == bfd_arch_powerpc);
switch (v->mach)
{
...
}

BTW, the reason I don't like the assert (0) is that I like seeing a
somewhat meaningful condition when an assert is triggered. Seeing 0
as the failed condition doesn't give much of a clue about what's going
on.

Kevin