Re: [RFA] mips: Fix "info registers" output

[Daniel Jacobowitz <drow at mvista dot com>]

On Sun, Mar 10, 2002 at 11:30:09AM -0500, Andrew Cagney wrote:
> >
> >FP_REGISTER_DOUBLE describes a property of the ABI.  I don't really
> >think it's the appropriate check when printing floating-point
> >registers; we always take care to print the single-precision value even
> >if FP_REGISTER_DOUBLE, because they might be used in single-precision
> >anyway.
> 
> True, sort of.  The decision is a function of that FP bit, 
> FP_REGISTER_DOUBLE and the user typing ``(gdb) set mips 
> fp-register-double on, damit!'' (the user is always right :-).

(said `set' does not exist, of course)

> If the FP register bit is used by just this code, other parts of GDB are 
> going to be inconsistent since they are still using FP_REGISTER_DOUBLE 
> when [un]packing FP registers.  Can I suggest using FP_REGISTER_DOUBLE 
> initially (#if 0 #else #endif the code in mips2_fp_compat()) and bug 
> report the need to change everyting to use mips2_fp_compat() as a 
> separate change.

But there is nowhere else that we really unpack FP registers... well, I
suppose there is actually.  A fixme and PR it is.  The PR will include
a question about what to do with the displayed types of these
registers.

I have a few ideas, involving gdbarch, on how to solve this properly. 
I'll get back to it in a few months, thus the PR :)

> Apart from that, I think the code is brilliant.  Just suggest a few 
> comment tweaks before the commit.
> 
> +      if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
> + 
> {
> + 
>   mips_read_fp_register_single (regno, rare_buffer + 4);
> + 
>   mips_read_fp_register_single (regno + 1, rare_buffer);
> + 
> }
> +      else
> + 
> {
> + 
>   mips_read_fp_register_single (regno, rare_buffer);
> + 
>   mips_read_fp_register_single (regno + 1, rare_buffer + 4);
> + 
> }
> 
> Suggest mentioning that mips_read_fp_register_single() handles the 
> problem of extracting the correct four bytes from from each register.

OK, will do.

> >-  /* use HI and LO to control the order of combining two flt regs */
> >-  int HI = (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG);
> >-  int LO = (TARGET_BYTE_ORDER != BFD_ENDIAN_BIG);
> 
> Yes! In 20:20 hindsight that was a very confusing idea.
> 
> >+      /* 4-byte registers: we can fit two registers per row.  */
> >+      /* Also print every pair of 4-byte regs as an 8-byte double.  */
> >+      mips_read_fp_register_single (regnum, raw_buffer);
> >+      flt1 = unpack_double (builtin_type_float, raw_buffer, &inv1);
> >+
> >+      mips_read_fp_register_single (regnum + 1, raw_buffer);
> >+      flt2 = unpack_double (builtin_type_float, raw_buffer, &inv2);
> > 
> >+      mips_read_fp_register_double (regnum, raw_buffer);
> >+      doub = unpack_double (builtin_type_double, raw_buffer, &inv3);
> >+      
> >       printf_filtered (" %-5s", REGISTER_NAME (regnum));
> 
> Suggest a FIXME and bug report here.  It isn't safe to assume things 
> like builtin_type_double is 64 bit.  The code should use the ABI 
> independant builtin_type_ieee_BLAH.  But this is a separate bug and not 
> your problem :-)

  builtin_type_double =
    init_type (TYPE_CODE_FLT, TARGET_DOUBLE_BIT / TARGET_CHAR_BIT,
               0,
               "double", (struct objfile *) NULL);


  set_gdbarch_double_bit (gdbarch, 64);


Why isn't it safe to assume that a double is 64-bit when we explicitly
set it that way?  I assume that the builtin types get swapped out when
we change gdbarch... yes, they do.  Besides, is MIPS FP actually IEEE? 
Oh, I suppose the values probably are and only some of the math isn't.

Committed without that last FIXME; I'll add it if it's really
necessary.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer