Re: RFA: PowerPC: add segment register numbers

[Kevin Buettner <kevinb at redhat dot com>]

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