Re: [PATCH 3/3] MIPS: Provide FPU info and decode FCSR in `info float'

["Maciej W. Rozycki" <macro@codesourcery.com>]

On Wed, 17 Dec 2014, Pedro Alves wrote:

> > +static void
> > +mips_print_float_info (struct gdbarch *gdbarch, struct ui_file *file,
> > +		      struct frame_info *frame, const char *args)
> > +{
> > +  int fcsr = mips_regnum (gdbarch)->fp_control_status;
> > +  enum mips_fpu_type type = MIPS_FPU_TYPE (gdbarch);
> > +  ULONGEST fcs = 0;
> > +  int i;
> > +
> > +  if (fcsr == -1 || !deprecated_frame_register_read (frame, fcsr, NULL))
> > +    type = MIPS_FPU_NONE;
> 
> "deprecated" method usage alert.  It's better to use methods that return
> values and then print "<unsaved>", "<unavailable>" as appropriate,
> though in this case you may be able to just use read_frame_register_unsigned
> instead.

 For the record, we use `get_frame_register_unsigned' in such cases 
elsewhere, e.g. `micromips_bc1_pc' and we don't expect it to fail on the 
assumption that `fp_control_status' will have been set correctly.  I see 
this change went in already, but I'd like to see it updated with a 
follow-up change for consistency.

 Specifically `get_frame_register_unsigned' is unsuitable here (as we want 
to poke at the actual register file, not the frame state), but I think a 
failure from `read_frame_register_unsigned' should be signalled as an 
error (exception being thrown, just as from the former function) rather 
than being silently treated as if there were no FPU.

 Also I think the registers should be dumped earlier on, with only generic 
FPU parameters preceding, as the extra decoded FCSR information is an 
elaboration of raw register contents, so I find it logical (as far as the 
reading flow of English text is considered) to be presented later, i.e.:

fpu type: ...
reg size: ...
[registers]
cond    : ...
[...]

Or so I think.  It would help reviewing it if the updated version of the 
patch was presented with corresponding output produced accompanying, just 
as I did with the original version.  Otherwise it requires drawing the 
output in one's head which is not necessarily straightforward.

  Maciej