The regcache abstraction (Was Re: PATCH ARM initial support for different floating-point models)

The regcache abstraction (Was Re: PATCH ARM initial support for different floating-point models)

[Richard Earnshaw]

> Hmm, I'm afraid you may have just stepped into the 
> write_register_bytes() bear trap :-( Have a look at the comments in 
> regcache.c for the history.
> 

Well if I was confused before, I'm totally flummoxed now.

The more I try to wade through the different legacy interfaces to that 
file the more confused I become...

> 
> > Index: arm-tdep.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> > retrieving revision 1.47
> > diff -p -r1.47 arm-tdep.c
> > *** arm-tdep.c	2002/02/19 13:57:35	1.47
> > --- arm-tdep.c	2002/02/19 19:14:40
> > *************** arm_extract_return_value (struct type *t
> > *** 2139,2145 ****
> >   			  char *valbuf)
> >   {
> >     if (TYPE_CODE_FLT == TYPE_CODE (type))
> > !     convert_from_extended (&regbuf[REGISTER_BYTE (ARM_F0_REGNUM)], valbuf);
> >     else
> >       memcpy (valbuf, &regbuf[REGISTER_BYTE (ARM_A1_REGNUM)],
> >   	    TYPE_LENGTH (type));
> 
> Here, unfortunatly, directly pokeing around the regcache buffer (a 
> parameter) is still the only way to do this.

Yep, this is awful, but that's what it has always done there (you quote 
the old code).


> >   	    TYPE_LENGTH (type));
> > *************** arm_store_return_value (struct type *typ
> > *** 2256,2270 ****
> >   {
> >     if (TYPE_CODE (type) == TYPE_CODE_FLT)
> >       {
> >         char buf[MAX_REGISTER_RAW_SIZE];
> >   
> > !       convert_to_extended (valbuf, buf);
> > !       /* XXX Is this correct for soft-float?  */
> > !       write_register_bytes (REGISTER_BYTE (ARM_F0_REGNUM), buf,
> > ! 			    MAX_REGISTER_RAW_SIZE);
> 
> I think your changes to this function can be rewritten to use 
> write_register_gen(REGNUM,BUF).

And you quote the old code again here.

> Need to be careful though.  If the code is assuming a floating point 
> value should be stored across multiple adjacent registers then the code 
> will need to be broken down into separate explicit writes.

I desperately need a high-level overview of how the regcache interface is 
*supposed* to work in the brave new world (ie without all the legacy 
interfaces lying around).  Let me start by trying to draw a little diagram 
to explain how I think it might work; then we can expand on that, or 
re-draw it as appropriate.

Lets start the model with four main units.  The inferior, the regcache, 
the MI routines and the tdep support.  The interfaces are then as follows:

          +--------------------------------+
          |          Inferior              |
          +--------------------------------+
                        ^ |
                        | | A
                        | v
          +--------------------------------+
          |          Regcache              |
          +--------------------------------+
                ^ |                ^ |
                | | B              | | C
                | v                | v
          +-------------+    +-------------+
          |     MI      |<==>|    Tdep     |
          +-------------+ D  +-------------+

That gives us three potential interfaces to the regcache, A, B and C.  In 
addition, there is an interface D, which is effectively the gdbarch vector.

Now, as I understand it, interface A is quite straight-forward and 
consists of three functions:

	supply_register()
	collect_register()
	set_register_cached()

[The last of these is needed because a target may be unable to recover 
some registers at certain times, and may need to tell the regcache that.]

This interface is used by the 'nat' code or anything else that talks to a 
target machine (ie it lies at the callee side of a target_ops vector).

However, after that it starts to become fuzzy.  The first problem is that 
it isn't completely clear whether the regcache should contain a raw cache 
of the physical registers, or some munged view of them.

I'd be inclined to argue the regcache should be pretty much a raw cache of 
the machine registers, and that interface B is therefore bogus (since it 
can't know how to interpret the information); therefore all accesses to 
the regcache by MI code should be, at least conceptually, via the tdep 
code using interface D.

Of course, it may be that many targets will have near identical routines 
for mapping D<->C and there's no reason why that shouldn't be extracted 
out so that the gdbarch vector can handle it cleanly.

So my questions are: Do we have interface B?  If so, why? what functions 
comprise C? and what functions comprise D?

> 
> enjoy,

Not a chance :-)

R.

Re: The regcache abstraction (Was Re: PATCH ARM initial support for different floating-point models)

[Andrew Cagney]

Richard what follows is the short obscure answer (sorry).  I'll try to 
do the long (add to documentation answer, in about a week - this really 
does need to be documented.

> retrieving revision 1.47
>> > diff -p -r1.47 arm-tdep.c
>> > *** arm-tdep.c	2002/02/19 13:57:35	1.47
>> > --- arm-tdep.c	2002/02/19 19:14:40
>> > *************** arm_extract_return_value (struct type *t
>> > *** 2139,2145 ****
>> > char *valbuf)
>> > {
>> > if (TYPE_CODE_FLT == TYPE_CODE (type))
>> > !     convert_from_extended (&regbuf[REGISTER_BYTE (ARM_F0_REGNUM)], valbuf);
>> > else
>> > memcpy (valbuf, &regbuf[REGISTER_BYTE (ARM_A1_REGNUM)],
>> > TYPE_LENGTH (type));
> 
>> 
>> Here, unfortunatly, directly pokeing around the regcache buffer (a 
>> parameter) is still the only way to do this.
> 
> 
> Yep, this is awful, but that's what it has always done there (you quote 
> the old code).

Yes, I figured you're intended modifications were just re-aranging the 
existing code.

For the above case it isn't possible to do any better.  Fixing the 
problem requires further changes to the regcache interface - perhaphs a 
mechanism for saving/restoring the regcache or better the ``target 
register state''.

> And you quote the old code again here.
> 
> 
>> Need to be careful though.  If the code is assuming a floating point 
>> value should be stored across multiple adjacent registers then the code 
>> will need to be broken down into separate explicit writes.
> 
> 
> I desperately need a high-level overview of how the regcache interface is 
> *supposed* to work in the brave new world (ie without all the legacy 
> interfaces lying around).  Let me start by trying to draw a little diagram 
> to explain how I think it might work; then we can expand on that, or 
> re-draw it as appropriate.
> 
> Lets start the model with four main units.  The inferior, the regcache, 
> the MI routines and the tdep support.  The interfaces are then as follows:
> 
>           +--------------------------------+
>           |          Inferior              |
>           +--------------------------------+
>                         ^ |
>                         | | A
>                         | v
>           +--------------------------------+
>           |          Regcache              |
>           +--------------------------------+
>                 ^ |                ^ |
>                 | | B              | | C
>                 | v                | v
>           +-------------+    +-------------+
>           |     MI      |<==>|    Tdep     |
>           +-------------+ D  +-------------+
> 
> That gives us three potential interfaces to the regcache, A, B and C.  In 
> addition, there is an interface D, which is effectively the gdbarch vector.
> 
> Now, as I understand it, interface A is quite straight-forward and 
> consists of three functions:
> 
> 	supply_register()
> 	collect_register()
> 	set_register_cached()
> 
> [The last of these is needed because a target may be unable to recover 
> some registers at certain times, and may need to tell the regcache that.]

Yes.  In a ``pure'' new model the regcache only contains raw values.

> This interface is used by the 'nat' code or anything else that talks to a 
> target machine (ie it lies at the callee side of a target_ops vector).
> 
> However, after that it starts to become fuzzy.  The first problem is that 
> it isn't completely clear whether the regcache should contain a raw cache 
> of the physical registers, or some munged view of them.
> 
> I'd be inclined to argue the regcache should be pretty much a raw cache of 
> the machine registers, and that interface B is therefore bogus (since it 
> can't know how to interpret the information); therefore all accesses to 
> the regcache by MI code should be, at least conceptually, via the tdep 
> code using interface D.
> 
> Of course, it may be that many targets will have near identical routines 
> for mapping D<->C and there's no reason why that shouldn't be extracted 
> out so that the gdbarch vector can handle it cleanly.
> 
> So my questions are: Do we have interface B?  If so, why? what functions 
> comprise C? and what functions comprise D?

(core gdb is safer than MI, although core GDB still has problems).

In theory the call pattern is:

	core-gdb
	-> read_register_gen
	-> gdbarch_register_read()
	-> regcache_read() OR -> memory_read()
	-> get value from raw regcache.


All accesses to the regcache are ment to go via gdbarch_register_read() 
which your architecture controls.
If a register is really in memory, register_read() can fetch it from 
memory.  If a register is a composite of several raw registers, 
register_read() can compose it (on the fly) from the sub registers. 
Conversely for writes.  gdbarch_register_write() can scatter the 
register's value to the correct raw register and memory locations.

The theory breaks down on two fronts:

Core code still uses read_register_bytes() to do things like extract 
struct values from two or more registers assuming they are adjacent. 
That code is broken since, for instance, when a 64 bit architecture is 
executing 32 bit compat code - structs are across 32 and not 64 bit 
registers.

Core code still uses read_register_bytes() to save the register cache 
and to apply operations on a saved register set (eg extract return 
value).  That code is broken since it saves/restores all registers and 
not just the registers (and memory) relevant to the ABI.


Andrew