Re: [WIP/RFC] MIPS registers overhaul

Kevin Buettner
2003-05-14T22:00:41Z

On May 10, 4:29pm, Andrew Cagney wrote:

> > 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.

I like Daniel's suggestion for the names: u64, flt, and dbl. (flt and dbl
match the prefixes used by "info float".)

> > 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.

Maybe we need to name the prefix something different than "raw_"?

Whatever we call them, I think it's still useful to have names
associated with them. E.g, you can do any of the following:

print $raw_f20
print/x ($raw_f20 >> 32)
set $raw_f20=0xbadbeef

Most of the time, you'll just want to deal with $f20 though. If you
suspect that something funny is going on with GDB's raw->cooked
translation, being able to more directly manipulate the raw values
could prove to be useful.

Also, due to the fact that most of the code in mips-tdep.c still uses
raw register numbers, you'll see the raw names showing up in "info
frame". E.g.:

(gdb) info frame
...
Saved registers:
gp at 0x7fff7d04, s8 at 0x7fff7d08, ra at 0x7fff7d0c, pc at 0x7fff7d0c,
raw_f20 at 0x7fff7d10, raw_f21 at 0x7fff7d14, f20 at 0x7fff7d10
^^^^^^

In the fullness of time, i.e, when the MIPS code is finally converted
to use cooked registers everywhere that it *should*, the above
references to the "raw" register names can probably go away.

> ``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.

Well, in this case the underlying register set really is only 32-bits
wide. Volume 1 of the MIPS64 Architecture (revision 1.00, dated
August 29, 2002) says:

For compatibility with MIPS32 processors, the FR bit in the CP0
Status register is used by a MIPS64 processor to configure the FPU
in a mode in which the FPRs are treated as 32 32-bit registers,
each of which is capable of storing only 32-bit data types. In
this mode, the double-precision floating point (type D) data type
is stored in even-odd pairs of FPRs, and the long-integer (type L)
and paired single (type PS) data types are not supported.

Figure 2-8 in this manual shows that bits 32-63 (i.e, the high 32 bits)
are "UNPREDICTABLE" in this mode.

We still *could* show these extra bits, but displaying the register in
this way would, most of the time, be next to useless. If the GDB user
really wants to see what the higher order bits are, then the raw_fN
names are useful.

[...]
> > 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).

I'll put RA_REGNUM back into mips-tdep.h along with a comment which
indicates that it should only used by mdebugread.c.

> > 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.

Actually, I think it's useful for the layout raw and cooked regnum
structs to be identical. When initializing the cooked regnum struct,
we can do so via a single assignment:

/* For many registers, the cooked and raw register numbers are the same. */
tdep->cooked_regnums = tdep->raw_regnums;

/* Cooked regnum initializations that differ follow... */

The fact that the structs were identical made it easy and elegant to
define the reg_name() function which is used twice by mips_register_name(),
once for cooked register numbers, and a second time (assuming no suitable
cooked name was found) for the raw numbers. If the layout of the cooked
and raw structs were different, I'd need two separate functions with nearly
identical functionality. Ditto for mips_dump_regnums().

I think that you definitely want "last_arg_regnum" and
"last_fp_arg_regnum" to appear in both structs. At the moment, the
various MIPS *_push_argument() code uses the register names from the
raw regnums struct. I think it may be desirable at some point to make
this code use cooked regnums instead. (It will hopefully simplify a
bunch of code that worries about shifting values to the correct
position within a register.) For the floating point registers, where
the cooked and raw numbers are actually different, we would like
"first_fp_arg_regnum" and "last_fp_arg_regnum" to actually refer to
numbers within this space.

I think it's likely that when/if this conversion occurs, it'll be done
on an incremental basis. We want separate cooked/raw numbers so that
it can be done incrementally.

Also, putting these values in the regnum struct makes it clear exactly
which set they belong to.

> 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)

I'll adopt the following naming scheme:

rawnums - for code which uses and should continue to use raw numbers
cookednums - for code which uses and should continue to use cooked
numbers
regnums - for one of two cases, either code that's currently using
raw numbers that should be converted to be cooked, or
for code which I'm unsure of. (Either case, these'll
be raw numbers. When someone converts the code or decides
that the "raw" usage is correct, the name should be
changed.)

For the last category, I'm open to suggestions for some other name.

> 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/

Okay.

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

I just used the same values as the macros used to use. The user can see
what they are via "maint print architecture".

(Maybe I don't understand your question.)

> -#define REGISTER_PTRACE_ADDR(regno) \
> +register_ptrace_addr (int regno)
>
> is obvious (and separate also).

Okay.

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

Right. There was only one place that this happened. Not sure why.
It's been fixed in my sources.

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

I don't think that this is exactly what Stan noted, but okay.

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

Okay.

Kevin