Re: [WIP/RFC] MIPS registers overhaul

Andrew Cagney
2003-05-10T20:30:13Z

> The patch below represents the results of trying to fix two problems
> related to floating point registers, stumbling at various points along
> the way, and realizing that I needed to fix even more...
>
> The two problems related to floating point registers that I started out
> trying to fix are the following:
>
> 1) On mips64, it's not possible to set a floating point register to
> a single precision value.

Hmm, ``oops''.

> 2) Again, on mips64, when running a program using the o32 ABI, floating
> point doubles were not being fetched from GDB's regcache correctly.
> For o32, floating point doubles are represented using register pairs,
> but, for mips64, each register is stored in a 64-bit container. When
> attempting to retrieve the value of a 64-bit double that should
> be retrieved by taking 32-bits from a pair, the single 64-bit container
> was being considered to be the value of the double.

For a long long time people were trying to solve this using the
convertible methods. It never really worked.

> Problem 1 was solved by introducing a union type for floating point
> registers. When attempting to display a value using ``print'', you'll
> see (with my patch) something like this:
>
> (gdb) p $f20
> $1 = {i = 4621199872640208077, f = -107374184, d = 8.9000000000000004}
>
> (If someone can think of more meaningful, but still terse field names for
> the above, please let me know.)

I'd try to be consistent with the other register unions, uint64 for
instance. As for the d/f, I don't know.

> Problem 2 was solved by introducing pseudo-registers for the floating point
> registers. Along the way (last Friday, I think), I ran into an assertion
> failure which caused me to make even more extensive changes to the MIPS code.

Ya! Finally!

> BTW, the raw floating point registers are still accessible. Doing
> "info registers raw" will display all of the raw registers. Or, if
> you know the names of the registers you want to display, you can do,
> e.g, "info registers raw_f20".

Hmm, is this necessary? Confusing? ``maint print
{raw-,cooked-,}registers'' are already available and provide access to
the underlying values.

``info registers'' should always display the target's underlying
register set. In the case of o32 running on a 64 bit ISA, the 64 bit
registers should be displayed.

> Which reminds me... I've introduced two sets of register numbers. I
> called these the "cooked" and "raw" regnums. In many cases, the
> cooked and raw numbers are the same. (They'll usually be the same
> when the raw and cooked types are the same.) In the cases where the
> numbers are different, the cooked number is a pseudo register number
> and the corresponding raw register is (usually) used to determine the
> value of cooked value. Raw registers numbers should be used by the
> lower layers of GDB which communicate with the target and need to
> populate GDB's regcache. The cooked registers *should* be used almost
> everywhere else. Except they aren't at the moment. In particular,
> most of mips-tdep.c wants to still operate on "raw" register data.
> (Anywhere you see a byte order check, that's a clue that raw register
> numbers should still be used.) So, in order to prevent this patch from
> becoming even larger than it already is, much of mips-tdep.c still
> uses raw register numbers. These should (probably) be cleaned up some
> day to use cooked numbers instead. (I need to check to make sure I
> have this as a comment in the code somewhere...)
>
> For the o32 case that I was originally trying to fix, we end up
> creating 16 64-bit cooked floating point registers of the union type
> mentioned above. The raw floating point registers (in pairs) are used
> to construct the cooked value. It's also possible to end up with no
> cooked floating point registers or 32-floating floating point
> registers, either 32- or 64-bit. (The values of FP_REGISTER_DOUBLE
> and MIPS_FPU_TYPE influence what exactly gets done.)

Ok.

> Index: mdebugread.c

The below isn't right.

RA_REGNUM is defined by both Alpha and MIPS. There are a number of long
standing problems with mdebug read vis:
tm-alpha.h:64:#define mips_extra_func_info alpha_extra_func_info
and hard-wiring the reference to RA_REGNUM just wouldn't work.

I'd leave RA_REGNUM as is (or put the definition in tm-mips*.h so that
it is clear that it still needs to be fixed).

> ===================================================================
> RCS file: /cvs/src/src/gdb/mdebugread.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 mdebugread.c
> --- mdebugread.c 19 Mar 2003 19:45:49 -0000 1.44
> +++ mdebugread.c 9 May 2003 23:36:32 -0000
> @@ -55,6 +55,9 @@
> #include "gdb_assert.h"
> #include "block.h"
>
> +#include "mips-tdep.h"
> +#define RA_REGNUM (mips_cooked_regnums (current_gdbarch)->ra_regnum)
> +
> /* These are needed if the tm.h file does not contain the necessary
> mips specific definitions. */
>
> @@ -69,9 +72,6 @@ typedef struct mips_extra_func_info
> PDR pdr;
> }
> *mips_extra_func_info_t;
> -#ifndef RA_REGNUM
> -#define RA_REGNUM 0
> -#endif
> #endif
>
> #ifdef USG

> Index: mips-tdep.h

Should there be separate raw and cooked num structures?

> +struct mips_regnums
> + {
> + int fp0_regnum; /* First floating point register. */
> + int fplast_regnum; /* Last floating point register. */
> + int last_arg_regnum; /* Last general purpose register used for
> + passing arguments. (a0_regnum is the
> + first.) */
> + int first_fp_arg_regnum; /* First floating point register used for
> + passing floating point arguments. */
> + int last_fp_arg_regnum; /* Last floating point register used for
> + passing floating point arguments. */
> + int zero_regnum; /* The zero register; read-only, always 0. */
> + int v0_regnum; /* Function return value. */
> + int a0_regnum; /* First GPR used for passing arguments. */
> + int t9_regnum; /* Contains address of callee in PIC code. */
> + int sp_regnum; /* Stack pointer. */
> + int ra_regnum; /* Return address. */
> + int ps_regnum; /* Processor status. */
> + int hi_regnum; /* High portion of internal multiply/divide
> + register. */
> + int lo_regnum; /* Low portion of internal multiply/divide
> + register. */
> + int badvaddr_regnum; /* Address associated with
> + addressing exception. */
> + int cause_regnum; /* Describes last exception. */
> + int pc_regnum; /* Program counter. */
> + int fcrcs_regnum; /* FP control/status. */
> + int fcrir_regnum; /* FP implementation/revision. */
> + int first_embed_regnum; /* First CP0 register for embedded use. */
> + int last_embed_regnum; /* Last CP0 register for embedded use. */
> + int prid_regnum; /* Processor ID. */
> + };
> +
> +const struct mips_regnums *mips_raw_regnums (struct gdbarch *gdbarch);
> +const struct mips_regnums *mips_cooked_regnums (struct gdbarch *gdbarch);

or at least keep things like "last_arg_regnum" out this space (they only
belong in one of the two spaces). Having them appear in both makes it
too easy to do the wrong thing.

Can I suggest for this code:

> mips_linux_cannot_fetch_register (int regno)
> {
> + const struct mips_regnums *regnums = mips_raw_regnums (current_gdbarch);

use a consistent nameing schema that makes the register's space clear vis:

rawnums = mips_raw_regnums (...);
...
if (regno == rawnums->ps_regnum)

it should be possible to apply separate patches that:

- add mips_raw_regnums() and their initialization
- roll out the mechanical change
s/BADVADDR_REGNUM/rawnums->badvaddr_regnum/

How do you know that the raw register numbers were computed correctly?

--

-#define REGISTER_PTRACE_ADDR(regno) \
+register_ptrace_addr (int regno)

is obvious (and separate also).

--

Make certain you use "mips-tdep.h" and not <mips-tdep.h>.

--

Delete the lsi33k support as Stan noted, it likely never worked
(separate also).

--

This should greatly reduce the diff to just the change adding the
cooked<->raw map, and code using it.

Andrew