Kevin Buettner <kevinb@redhat.com> writes:
> This portion of the above ChangeLog entry...
> 
> > 	(ppc_supply_fpregset, ppc_collect_fpregset,
> > 	rs6000_push_dummy_call, rs6000_extract_return_value,
> > 	rs6000_store_return_value): Assert that we have floating-point
> 
> ...should be formated differently:
> 
>  	(ppc_supply_fpregset, ppc_collect_fpregset)
>  	(rs6000_push_dummy_call, rs6000_extract_return_value)
>  	(rs6000_store_return_value): Assert that we have floating-point
> 
> Personally, I prefer the way you did it, but I've been told that emacs
> likes the other form better.

Thanks very much.  I knew this rule, and missed that.  I've fixed the
other instances of comma-terminated ChangeLog lines.


> Regarding:
> > *** gdb/ppc-tdep.h	2004-05-06 14:18:00.000000000 -0500
> > --- gdb/ppc-tdep.h	2004-05-06 14:35:11.000000000 -0500
> [...]
> > *************** struct gdbarch_tdep
> > *** 150,158 ****
> > --- 150,162 ----
> >       int ppc_lr_regnum;		/* Link register */
> >       int ppc_ctr_regnum;		/* Count register */
> >       int ppc_xer_regnum;		/* Integer exception register */
> > + 
> > +     /* On RS6000 variants that have no floating-point registers, the
> > +        next two members will be -1.  */
> 
> I'm not comfortable with the term "RS6000 variants" here.  I'd be happier
> with "PPC variants", though that's probably not strictly correct either.
> I suppose you could just say "On cores that have no floating-point
> registers...".

The issue is that people might not immediately see all PPC's as RS6000
variants, right?  Fair enough.  "RS6000 variants" is historically
correct, but it's not like anyone has fabricated an RS6000 in recent
history.  I've changed that to "On PPC and RS6000 variants that ...".

> Regarding:
> 
> > *** gdb/rs6000-nat.c	2004-05-06 14:18:00.000000000 -0500
> > --- gdb/rs6000-nat.c	2004-05-06 14:35:11.000000000 -0500
> [...]
> > *************** fetch_core_registers (char *core_reg_sec
> > *** 583,591 ****
> >         for (regi = 0; regi < 32; regi++)
> >           supply_register (regi, (char *) &regs->r64.gpr[regi]);
> >   
> > !       for (regi = 0; regi < 32; regi++)
> > ! 	supply_register (tdep->ppc_fp0_regnum + regi,
> > !                          (char *) &regs->r64.fpr[regi]);
> >   
> >         supply_register (PC_REGNUM, (char *) &regs->r64.iar);
> >         supply_register (tdep->ppc_ps_regnum, (char *) &regs->r64.msr);
> > --- 589,598 ----
> >         for (regi = 0; regi < 32; regi++)
> >           supply_register (regi, (char *) &regs->r64.gpr[regi]);
> >   
> > !       if (tdep->ppc_fp0_regnum >= 0)
> > !         for (regi = 0; regi < 32; regi++)
> > !           supply_register (tdep->ppc_fp0_regnum + regi,
> > !                            (char *) &regs->r64.fpr[regi]);
> >   
> >         supply_register (PC_REGNUM, (char *) &regs->r64.iar);
> >         supply_register (tdep->ppc_ps_regnum, (char *) &regs->r64.msr);
> 
> I know it's not really related to this patch, but I happened to notice
> that constant 32 in the above (and elsewhere too).  If you get a
> chance, could you change these to use either ppc_num_fprs or
> ppc_num_gprs?  (I mention this because I noticed that you had made
> this kind of change at several points elsewhere in the current patch. 
> A separate patch which addressed these remaining occurrences would
> certainly be welcome.)

Sure --- I was actually thinking about doing that; now I definitely
will.

Here is the revised patch.