Re: [RFA] regcache.c (register_fetched) + related changes

Re: [RFA] regcache.c (register_fetched) + related changes

[David Taylor]

    Date: Wed, 28 Feb 2001 18:14:29 -0500
    From: Andrew Cagney <ac131313@cygnus.com>

    David Taylor wrote:
    > 
    >     From: Andrew Cagney <ac131313@cygnus.com>
    >     Date: Tue, 27 Feb 2001 20:38:30 -0500
    > 
    >     David Taylor wrote:
    > 
    >     > I propose that we:
    >     >
    >     > . add register_fetched
    > 
    >     David,
    > 
    >     The functionality of register_fetched() overlaps with
    >     set_register_cached() and supply_register().  Rather than add a
    >     redundant method, could an existing interface be used or the current
    >     interfaces rationalized slightly?
    > 
    > Andrew,
    > 
    > Supply register does more than register_fetched; register_fetched only
    > affects register_valid -- the register must have been supplied via
    > some other method.

    Yes, that is what I mean by their functionality overlapping.  There is
    much legacy code of the form:

	    blat the registers[] array
	    set register_valid[] to 1

But, there also exists code that does not directly blat the
registers[] array, but nonetheless still needs to mark one or more
registers as cached -- i.e. has to set one or more register_valid[]
locations to 1!

    The accepted ``correct fix'' for that code is to replace it with
    supply_register().  It is just an unfortunate reality that no one has
    the time to do the work. I have a feeling that, right now GDB is
    deleting this code faster than it is being fixed!

The STORE_PSEUDO_REGISTER routines do not get the register contents as
an argument.  But they potentially need to set and clear various
register_valid locations.

To clear a location (i.e., mark it as "invalid") they can use the
existing register_changed function; but there is currently no
functional interface for them to set (i.e., mark as "cached" or
"valid") a location.

That is the point of my proposed function:

void
register_fetched (int regnum)
{
  set_register_cached (regnum, 1);
}

And the rest of the changes (excluding the one line declaration in a
header file) are just to modify existing targets to use a functional
interface rather than referencing register_valid directly.

    Given that reailty, if this code is going to be changed, I would prefer
    it changed in a way that made it very clear that this is not the correct
    way to do things.  Taking a lead from DEPRECATED_SERIAL_FD I think those
    blocks can be converted to something like:

I wasn't changing the accesses to the registers[] array.  Nor do I
currently have the time to spend to make such a change just to get the
above one line function accepted.  I've already spent more time on
this than I should have.

I guess for now I'll just have to say register_valid[...] = 1; in my
tdep file.  Maybe when the revolution comes... but until then...  Very
big sigh.

	    for (;;)
	      *deprecated_register_buffer(...) = ...;
	    deprecated_register_valid();

    > set_register_cached is -- with one exception -- only used within
    > regcache.c.  The one exception is remote.c (remote_fetch_registers).
    > 
    > I feel that a function should be created (register_unavailable?) and
    > the call in remote.c to set_register_cached replaced with that call.
    > Then set_register_cached should be made static.
    > 
    > To call set_register_cached, you have to know what the meanings are of
    > the various possible values of register_valid.  This is knowledge that
    > shouldn't really exist outside of regcache.c.

    Yes.  My suggestion, hinted at in the last e-mail, would be to combine
    the two disjoint operations:

	    supply_register(....);
	    set_register_cache(..., -1)

    into an atomic cache transaction:

	    supply_unavailable_register(....)

    >     Keep in mind that the long term goal is to tighten regcache's interface
    >     signficantly.  That is, eliminate register_valid[], registers[] and
    >     possibly even set_register_cached() replacing them with a small set of
    >     functions such as:
    >             supply_register()
    >             supply_unavailable_register()
    >     If you are thinking of proposing further changes then you may want to
    >     keep that in mind.
    > 
    > My change advances the goal of eliminating register_valid!  It reduces
    > significantly the number of files that even know that register_valid
    > exists!  Outside of regcache.c, only two files would reference it (for
    > a total of three references).  Those two files could also have their
    > references to it removed with a little bit more work.

    I agree, eliminating register_valid[] and registers[] are important
    goals.  The thing to be wary of is a change that just substitutes
    something like registers[] and register_valid[] without actually
    improving the overall quality of the code.  I think it is important to
    ensure that any proposed change to GDB moves its overall code quality
    forward (and not just sideways).

    Grubbing around for other cases.

    --

    Grubbing through the code, the other case where register_valid[] is
    being blatted is:

      /* These (for the most part) are pseudo registers and are obtained
	 by other means.  Those that aren't are already handled by the
	 code above.  */
      for (regi = IA64_GR32_REGNUM; regi <= IA64_GR127_REGNUM; regi++)
	register_valid[regi] = 1;
      for (regi = IA64_PR0_REGNUM; regi <= IA64_PR63_REGNUM; regi++)
	register_valid[regi] = 1;
      for (regi = IA64_VFP_REGNUM; regi <= NUM_REGS; regi++)
	register_valid[regi] = 1;

    I'm guessing that code somewhere is looking at register_valid[] to
    decide if a pseudo is available.  The actual value (hopefully) being
    supplied by the pseudo-register method.

Unless a pseudo shares nothing with any real register, with the
current interfaces a tdep file should *NEVER* allow register_valid to
be 1 for that pseudo register.  Otherwise, when something updates one
of the contributing registers, the pseudo will still be marked as
valid when it might not be valid.

    While that particular hack could eventually be fixed in other ways
    (short term by having the ``register_cached()'' method ask gdbarch if a
    pseudo is valid; and long term by having the register_valid() method
    bound to a frame and always asking gdbarch if i is valid) that ain't
    going to solve the hear and now.

    Given that, I think this case (pseudo) is different/separate to the
    legacy code case, I don't think it makes sense to use an interface like
    deprecated_register_valid().  Instead, I think a separate:

	    pseudo_register_valid(REGNUM)

Are you proposing this as setting the location?  Or are you proposing
this as a query function?  I would welcome a such a query function.
If you are proposing a "set" function, then until other changes are
made (namely, changes that allow the implementor to keep the pseudo
bits current), I would object to encouraging a false sense of security
in people.

    Interface should be available.  I think this also helps to differentiate
    the more recent pseudo from legacy code.

    --

    The other case I'm aware of, is with registers[] and core-gdb poking
    around in that instead of using the read/write register functions.  I
    think, there, the deprecated_register_buffer() method would be fine. 
    That code should be using the read_register*() functions and not pokeing
    around in that buffer.

    >     With regard to regcache.h, yes the two clash.  It both moves code around
    >     and changes the set_register_cached() interface.  If anything regcache.h
    >     makes life easier because it is finally clear what the regcache
    >     interfaces really are.
    > 
    >             Andrew
    > 
    > What change is this that you are referring to?  The message with subject
    > 
    >     [rfc] Re-merged regcache.h patch

    I also mentioned:

	    o       The changes adding:
    + #define REGISTER_CACHE_UNAVAILABLE (1)
    + #define REGISTER_CACHE_VALID       (0)
    + #define REGISTER_CACHE_INVALID     (-1)
		    (or enum equivalents) were stripped out.
		    These changes badly clashed with Davids
		    proposed register_*() change so, I'll
		    leave them for later.

Which I saw; but, they aren't part of the patch.  (And either the
un-included part changes the meanings of the register_valid slots, or
the above defines are wrong.)

    I was actually wrong here.  The changes were only in my local sandpit
    and not in the original patch.

    >   +/* Character array containing the current state of each register
    >   +   (unavailable<0, valid=0, invalid>0). */

    See inferior.h:register_valid.  I'll fix the comment.  Thanks for
    noticing this.

Yes, the comment in inferior.h is wrong.

I didn't delete the declaration as part of my proposed patch because,
as I said, there were still three locations (spread between two files)
that still referenced register_valid.

    ----

    Any way, to summarize, I'm proposing:

	    replace
		    supply_register();
		    set_register_cached();
	    with:
		    supply_unavailable_register()

	    replace
		    blat registers[];
		    register_valid[] = 1;
	    with
		    blat deprecated_register_buffer(...);
		    depreciated_register_valid/fetched(...);
	    where this I/F would only work on raw registers.

	    replace
		    registers_valid[pseudo] = 1;
	    with
		    pseudo_register_valid/fetched(REGNUM)
	    where the register must be a pseudo.

	    replace remaining
		    ... = registers[];
	    with
		    ... = depreciated_register_buffer();

    I think this is a reasonable balance between the immediate desire of
    eliminate the globals registers_valid[] and registers[] and the long
    term objective of eliminating code that blats arbitrary bytes in the
    registers[] array.

	    Andrew

And, as I said, there are times when you need to get the effect of
register_valid[regno] = 1 when you have not blatted the registers[]
array.  The question, in my mind, is whether to continue to directly
set it or to instead have a functional interface.  I would prefer that
there be a functional interface -- then it would be possible to make
register_valid be static (i.e., private to the regcache.c file) [and
to eventually make it go away].

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

David Taylor wrote:

>             blat the registers[] array
>             set register_valid[] to 1
> 
> But, there also exists code that does not directly blat the
> registers[] array, but nonetheless still needs to mark one or more
> registers as cached -- i.e. has to set one or more register_valid[]
> locations to 1!

Can you give an example of where this is happening?  I'm currently aware
of two cases:

	[0 .. NUM_REGS)
		where supply_register() should be used

	[NUM_REGS .. NUM_REGS + PSEUDO_NUM_REGS)
		I suspect your case.

> The STORE_PSEUDO_REGISTER routines do not get the register contents as
> an argument.  But they potentially need to set and clear various
> register_valid locations.

Yes, agreed.

> To clear a location (i.e., mark it as "invalid") they can use the
> existing register_changed function; but there is currently no
> functional interface for them to set (i.e., mark as "cached" or
> "valid") a location.

> That is the point of my proposed function:

Is:

	[NUM_REGS .. NUM_REGS + PSEUDO_NUM_REGS)
		I suspect your case.

your case?

>     Given that reailty, if this code is going to be changed, I would prefer
>     it changed in a way that made it very clear that this is not the correct
>     way to do things.  Taking a lead from DEPRECATED_SERIAL_FD I think those
>     blocks can be converted to something like:
> 
> I wasn't changing the accesses to the registers[] array.  Nor do I
> currently have the time to spend to make such a change just to get the
> above one line function accepted.  I've already spent more time on
> this than I should have.

Yes, I know.

For my part, I've the following:

	o	your patch to clean up register_valid[]

	o	the very old regcache.h patch

	o	Nick's proposal to change registers[]
		to a function

	o	the proposal to make ``frame''
		the center of the universe

	o	the standing order that
		registers[] should not be used
		in all future code.

	o	Steven's pseudo size patch

	o	the request to eliminate
		ARCH_NUM_REGS.

to reconcile.

It is just really unfortunate that your patch landed on my table at the
exact same time that I was working on resolving the other issues listed
above.

You'll note that I've now resolved the regcache.h and pseudo size
patches.  As for ARCH_NUM_REGS, the predicate - eliminate harris/88k -
has been started.

In case you're wondering, I'm not expecting you to solve all the above
problems, nor submit a new patch :-)

> I guess for now I'll just have to say register_valid[...] = 1; in my
> tdep file.  Maybe when the revolution comes... but until then...  Very
> big sigh.

If you submited the target today, then yes, I would honestly recommend
doing that!  It confines the changes to an existing well defined
interface.

However, if you're willing to help resolve this thread, then you'll
hopefully find that your patch, or one very like it is integrated into
GDB.

> Unless a pseudo shares nothing with any real register, with the
> current interfaces a tdep file should *NEVER* allow register_valid to
> be 1 for that pseudo register.  Otherwise, when something updates one
> of the contributing registers, the pseudo will still be marked as
> valid when it might not be valid.
> 
>     While that particular hack could eventually be fixed in other ways
>     (short term by having the ``register_cached()'' method ask gdbarch if a
>     pseudo is valid; and long term by having the register_valid() method
>     bound to a frame and always asking gdbarch if i is valid) that ain't
>     going to solve the hear and now.
> 
>     Given that, I think this case (pseudo) is different/separate to the
>     legacy code case, I don't think it makes sense to use an interface like
>     deprecated_register_valid().  Instead, I think a separate:
> 
>             pseudo_register_valid(REGNUM)

  pseudo_register_valid(REGNUM) == register_fetched(REGNUM)

when REGNUM in PSEUDOs. 

  set_pseudo_register_valid(REGNUM)
or
  pseudo_register_fetched(REGNUM)

would probably better names.

> Are you proposing this as setting the location?  Or are you proposing
> this as a query function?  I would welcome a such a query function.
> If you are proposing a "set" function, then until other changes are
> made (namely, changes that allow the implementor to keep the pseudo
> bits current), I would object to encouraging a false sense of security
> in people.

On existing implementations, it would have the effect of:

	register_valid[REGNUM] = 1;

I don't understand what you're refering to by ``false sense of
security''.

> And, as I said, there are times when you need to get the effect of
> register_valid[regno] = 1 when you have not blatted the registers[]
> array.  The question, in my mind, is whether to continue to directly
> set it or to instead have a functional interface.  I would prefer that
> there be a functional interface -- then it would be possible to make
> register_valid be static (i.e., private to the regcache.c file) [and
> to eventually make it go away].

I know.  I would like to identify an interface that lets that happen
while at the same time ensuring that legacy code (namely registers[])
doesn't get duplicated.

Would functions with initial semantics:

	deprecated_set_raw_register_fetched()
		gdb_assert (regnum >= 0 && regnum < NUM_REGS);
		register_valid[regnum] = 1;

and	set_pseudo_register_fetched()
		gdb_assert (regnum >= NUM_REGS && regnum < NUM_REGS +
NUM_PSEUDO_REGS);
		register_valid[regnum] = 1;

be more applicable?

Why two functions?  The first may eventually be modified to report its
use: ``warning: this target uses the deprecated function ....''.  It may
simply be deleted.  The second may be re-implemented to use the a new
frame-register interface when that becomes available.

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

To follow this up, David went through the problem in detail by phone.  I
belive the below describes the underlying problem.

Consider an architecture with three registers:

	$s0 $s1
	$d0

where $d0 is made up of both $s0 and $s1.  Using the current code, the
developer would lay this out as:

	REGNUM   name  size  offset
	   0      s0     1      0
	   1      s1     1      1
then the pseudo as either
           2      d0     2      0
or
	   2      d0     2      2

The choice is pretty arbitrary.  Under my proposed changes to make
everything bound to a frame, the user would instead lay it out as:

	RAWNUM   rawname  size
	   0      rs0      1
	   1      rs1      1

and then the frame/cooked registers would be arranged as:

	REGNUM  name  size
	   0     s0    1
	   1     s1    1
	   2     d0    2

and d0 would be implemented as:

	read/write rs0
	read/write rs1

Anyway....

The user enters:

	(gdb) set $d0 = value

GDB creates an expression tree that contains an LVAL with (at a guess)
the following:

	register number
	register size
	register address (in regcache or in memory)

this infomation is then used (valops.c) to write the register value
using write_register_bytes().

[Side note one: Previous discussion has agreed that
write_register_bytes() is bad.  The above should be changed so that it
stores the location of the register in some less implementation specific
way.  Fixing this little nastyness is part of binding everything to a
frame.]

write_register_bytes() iterates through all the registers (real and
pseudo) looking to see where those bytes need to go.  In michaels case,
that code would generate one or more calls (depending on how the
registers are layed out).  One of the calls would be:

	write_register_gen (d1regnum, buffer);

Write register gen, then writes bytes into the pseudo register, set
register_valid[] for the pseudo register and finally calls:

	store_register (d1regnum);

The immediate problem is that the pseudo registers register_valid[] bit
has been set.  That is a pseudo register has been marked as valid in the
register cache.

This becomes a problem when a user goes to again display that or one of
the other registers affected by that pseudo.  For instance consider:

	(gdb) set $d0 = 22
	(gdb) set $s1 = 1
	(gdb) set $s0 = 0
	(gdb) print $d0

If you follow the register read side through, you'll find that $d0 can
potentially become inconsistent with s0/s1 because $d0 is considered to
already be valid.

The obvious fix is to modify store_register() so that it clears
register_valid[] and thus ensures that D1 is never valid in the cache.
(an invalid register is always fetched/computed).

Discussion with David suggests a better fix - one that is more in line
with with the direction that the register interface is going - catch
writes to pseudos before they hit the cache so that the inferior-arch
can redirect them as needed.

The first bit of the change is to modify write_register_gen() to ask the
architecture to perform the write vis:

	write_register_gen (regnum, buf)
	   gdbarch_write_register_gen (regnum, buf)

In Davids case, write register gen would look like:

	if (regnum < NUM_REGS)
	   regcache_write (regnum, buff)
	else if (regnum == d0regnum)
	   regcache_write (s0regnum, buff)
	   regcache_write (s1regnum, buff)

regcache_write() would only allow writes to 0 .. NUM_REG.  Legacy code
would use a compatiblity function that would also allow writes to
NUM_REG .. NUM_REG+PSEUDO_NUM_REG.  Sane code would just set
gdbarch_write_register_gen() to regcache_write

Of course, life isn't that simple.  One additional change is to
write_register() which should be calling write_register_bytes().  This
also avoids the write_register_bytes() problem and completly avoids
read_register_bytes() :-(

However, on the bright side.  This does mean that Davids immediate
problem is addressed _and_ the need to blat register_valid[] is
avoided.  As an additional bonus, this pushes the code in the general
direction of separate cooked and raw registers.  In the future, the
above gdbarch_write_register_gen would become part of a write
frame-register function.

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

> Example, for this processor, the frame pointer is a pseudo register.
> The frame pointer is two random 8 bit registers -- 16 bits.  The
> hardware does *NOT* have the ability to operate on those registers as
> a pair.  It cannot load the frame pointer, it cannot store the frame
> pointer, it can only load or store one of the 8 bit registers at a
> time.
> 
> Since the compiler writer simply chose two adjacent 8 bit registers,
> in the right order, there is no need, in REGISTER_BYTES to have a
> special place for the fp and then copy things back and forth --
> instead the value goes directly into the two underlying real
> registers.
> 
> So, when the fp is fetched, there is a need to mark the underlying
> real registers (i.e., registers in the range [0,NUM_REGS) as fetched.

Yes.

That is a design flaw in the way GDB currently handles FP and other
pseudo registers.  At first sight you think that something like
write_pseudo() can be implemented as just:

	read_register_gen(..., first half);
	read_register_gen(..., second half);

Unfortunatly, because of some broken code in the core of gdb still also
need to allocate space in the regcache and pull all sorts of nasty
hacks.

The idea of separating a core-gdb register write from the raw register
cache (so that write_pseudo() can do a scatter.  Should fix that.  I was
planning on such a change as part of binding everything to a frame,
however, that could be separated out.

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

>     Can you give an example of where this is happening?  I'm currently aware
>     of two cases:
> 
>             [0 .. NUM_REGS)
>                     where supply_register() should be used
> 
> Not always.  See below.
> 
>             [NUM_REGS .. NUM_REGS + PSEUDO_NUM_REGS)
>                     I suspect your case.
> 
> I *clear* in the [NUM_REGS, NUM_REGS+PSEUDO_NUM_REGS) range and *set*
> in the [0, NUM_REGS) range.

The code that sets the valid bit in the range [0, NUM_REGS) should be
doing so using either of the functions:

	supply_register() - target side
or	write_register_gen() - core-gdb side

if it isn't it is doing something un-documented and so, in my opinion,
should use a function that makes the use of that un-documented behavour
very clear.

Hence, in your case, I think your code really should be using a function
called something like deprecated_raw_register_fetched().

If I commit a change that replaces register_valid[] with the pair:

	deprecated_raw_register_fetched()
	pseudo_register_fetched()

will this allow your existing code to work?

Looking to the future, when (not if) the frame code is re-vamped to have
register values bound to a frame, will it be possible to use that
mechanism to implement what you're trying to do how using the regcache?

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[David Taylor]

    Date: Thu, 01 Mar 2001 17:05:00 -0500
    From: Andrew Cagney <ac131313@cygnus.com>

    David Taylor wrote:

    >             blat the registers[] array
    >             set register_valid[] to 1
    > 
    > But, there also exists code that does not directly blat the
    > registers[] array, but nonetheless still needs to mark one or more
    > registers as cached -- i.e. has to set one or more register_valid[]
    > locations to 1!

    Can you give an example of where this is happening?  I'm currently aware
    of two cases:

	    [0 .. NUM_REGS)
		    where supply_register() should be used

Not always.  See below.

	    [NUM_REGS .. NUM_REGS + PSEUDO_NUM_REGS)
		    I suspect your case.

I *clear* in the [NUM_REGS, NUM_REGS+PSEUDO_NUM_REGS) range and *set*
in the [0, NUM_REGS) range.

I will probably eventually have to set in the range [NUM_REGS,
NUM_REGS + PSEUDO_NUM_REGS) as well -- if I want to manage 32 bytes of
non register state (though perhaps what I'm already doing will suffice
-- those 32 bytes are only an issue during when gdb makes inferior
function calls).

    > The STORE_PSEUDO_REGISTER routines do not get the register contents as
    > an argument.  But they potentially need to set and clear various
    > register_valid locations.

    Yes, agreed.

And the locations that they potentially need to clear are in the [0,
NUM_REGS) range.

    > To clear a location (i.e., mark it as "invalid") they can use the
    > existing register_changed function; but there is currently no
    > functional interface for them to set (i.e., mark as "cached" or
    > "valid") a location.

    > That is the point of my proposed function:

    Is:

	    [NUM_REGS .. NUM_REGS + PSEUDO_NUM_REGS)
		    I suspect your case.

    your case?

It is inside of my target's STORE_PSEUDO_REGISTER routine that I need
to do it.  So, yes, it is some registers in that range that create the
need to do it, but the registers for which it is done are in the
[0,NUM_REGS) range.

In STORE_PSEUDO_REGISTER, for some pseudo registers, I

. clear register_valid (by calling register_changed) for the pseudo
  register.
  (i.e., a register in the [NUM_REGS,  NUM_REGS+PSEUDO_NUM_REGS) range.)

. set register_valid (directly, since there is no functional
  interface) for two underlying real registers.
  (i.e., two registers in the [0, NUM_REGS) range.)

Example, for this processor, the frame pointer is a pseudo register.
The frame pointer is two random 8 bit registers -- 16 bits.  The
hardware does *NOT* have the ability to operate on those registers as
a pair.  It cannot load the frame pointer, it cannot store the frame
pointer, it can only load or store one of the 8 bit registers at a
time.

Since the compiler writer simply chose two adjacent 8 bit registers,
in the right order, there is no need, in REGISTER_BYTES to have a
special place for the fp and then copy things back and forth --
instead the value goes directly into the two underlying real
registers.

So, when the fp is fetched, there is a need to mark the underlying
real registers (i.e., registers in the range [0,NUM_REGS) as fetched.

    >     Given that reailty, if this code is going to be changed, I would prefer
    >     it changed in a way that made it very clear that this is not the correct
    >     way to do things.  Taking a lead from DEPRECATED_SERIAL_FD I think those
    >     blocks can be converted to something like:
    > 
    > I wasn't changing the accesses to the registers[] array.  Nor do I
    > currently have the time to spend to make such a change just to get the
    > above one line function accepted.  I've already spent more time on
    > this than I should have.

    Yes, I know.

    For my part, I've the following:

	    o       your patch to clean up register_valid[]

	    o       the very old regcache.h patch

	    o       Nick's proposal to change registers[]
		    to a function

Some of the problems that I've had during the port would not have been
present if Nick's proposed interface existed.

	    o       the proposal to make ``frame''
		    the center of the universe

	    o       the standing order that
		    registers[] should not be used
		    in all future code.

	    o       Steven's pseudo size patch

	    o       the request to eliminate
		    ARCH_NUM_REGS.

    to reconcile.

    It is just really unfortunate that your patch landed on my table at the
    exact same time that I was working on resolving the other issues listed
    above.

    You'll note that I've now resolved the regcache.h and pseudo size
    patches.  As for ARCH_NUM_REGS, the predicate - eliminate harris/88k -
    has been started.

    In case you're wondering, I'm not expecting you to solve all the above
    problems, nor submit a new patch :-)

    > I guess for now I'll just have to say register_valid[...] = 1; in my
    > tdep file.  Maybe when the revolution comes... but until then...  Very
    > big sigh.

    If you submited the target today, then yes, I would honestly recommend
    doing that!  It confines the changes to an existing well defined
    interface.

    However, if you're willing to help resolve this thread, then you'll
    hopefully find that your patch, or one very like it is integrated into
    GDB.

    > Unless a pseudo shares nothing with any real register, with the
    > current interfaces a tdep file should *NEVER* allow register_valid to
    > be 1 for that pseudo register.  Otherwise, when something updates one
    > of the contributing registers, the pseudo will still be marked as
    > valid when it might not be valid.
    > 
    >     While that particular hack could eventually be fixed in other ways
    >     (short term by having the ``register_cached()'' method ask gdbarch if a
    >     pseudo is valid; and long term by having the register_valid() method
    >     bound to a frame and always asking gdbarch if i is valid) that ain't
    >     going to solve the hear and now.
    > 
    >     Given that, I think this case (pseudo) is different/separate to the
    >     legacy code case, I don't think it makes sense to use an interface like
    >     deprecated_register_valid().  Instead, I think a separate:
    > 
    >             pseudo_register_valid(REGNUM)

      pseudo_register_valid(REGNUM) == register_fetched(REGNUM)

    when REGNUM in PSEUDOs. 

      set_pseudo_register_valid(REGNUM)
    or
      pseudo_register_fetched(REGNUM)

    would probably better names.

With the existing interfaces, nothing in the tdep file gets notified
when a register changes -- there is no mechanism in place to keep the
pseudo register valid bits accurate.

Given that lack of mechanism, if any real registers contribute to a
particular pseudo register, the ONLY safe value for that pseudo
register's valid field is 0 -- that way, the FETCH_PSEUDO_REGISTER
routine will be called and it can perform any necessary fetches and
updates.

    > Are you proposing this as setting the location?  Or are you proposing
    > this as a query function?  I would welcome a such a query function.
    > If you are proposing a "set" function, then until other changes are
    > made (namely, changes that allow the implementor to keep the pseudo
    > bits current), I would object to encouraging a false sense of security
    > in people.

    On existing implementations, it would have the effect of:

	    register_valid[REGNUM] = 1;

So, it would set the bit, not query it.

    I don't understand what you're refering to by ``false sense of
    security''.

Perhaps 'false sense of security' was a bad phrase.  But, if there is
a set_pseudo_register_valid function, then people will be potentially
lulled into thinking that they there is a mechanism for keeping the
state up to date when there is none.

Without additional changes to the register interface, the proposed
set_pseudo_register_valid function is only safe to call if the pseudo
register represents state that is not shared in any way shape or
fashion with any real register.

I would guess that most existing pseudo registers either are computed
from real registers or they share one or more bits with one or more
real registers.  For such registers you never want
register_valid[REGNUM] to be 1.

    > And, as I said, there are times when you need to get the effect of
    > register_valid[regno] = 1 when you have not blatted the registers[]
    > array.  The question, in my mind, is whether to continue to directly
    > set it or to instead have a functional interface.  I would prefer that
    > there be a functional interface -- then it would be possible to make
    > register_valid be static (i.e., private to the regcache.c file) [and
    > to eventually make it go away].

    I know.  I would like to identify an interface that lets that happen
    while at the same time ensuring that legacy code (namely registers[])
    doesn't get duplicated.

    Would functions with initial semantics:

	    deprecated_set_raw_register_fetched()
		    gdb_assert (regnum >= 0 && regnum < NUM_REGS);
		    register_valid[regnum] = 1;

This is the one that I would need to call in my tdep file.

    and     set_pseudo_register_fetched()
		    gdb_assert (regnum >= NUM_REGS && regnum < NUM_REGS +
    NUM_PSEUDO_REGS);
		    register_valid[regnum] = 1;

I would have no need of this one.

I would assert, having only looked briefly at the places where
register_valid is set that:

. most of them are calling it with a real register number, not a
  pseudo register number.

. most of the places where it is called with a pseudo register number
  are likely bugs.

    be more applicable?

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

Andrew Cagney wrote:

> Yes.  My suggestion, hinted at in the last e-mail, would be to combine
> the two disjoint operations:
> 
>         supply_register(....);
>         set_register_cache(..., -1)
> 
> into an atomic cache transaction:
> 
>         supply_unavailable_register(....)

PS/2:  The comment on supply_register() in regcache.c reads:

   [....]
   If VAL is a NULL pointer, then it's probably an unsupported register.
   We just set its value to all zeros.  We might want to record this
   fact, and report it to the users of read_register and friends.  */

Nothing is recorded.  The register is simply set to zero.

Checking the target code, this ``feature'' is used a lot (eg
alphabsd-nat.c):

      if (CANNOT_FETCH_REGISTER (i))
        supply_register (i, NULL);
      else
        supply_register (i, (char *) &gregsetp->r_regs[i]);

GDB really should record this cache state giving:

	valid
	unavailable
	invalid
	unsupported

Alternativly, the states unavailable and unsupported could be merged.

remote.c, which puts the cache into ``unavailable'' state, also writes
zeros into the register.

Thoughts? Especially from the people that added introspect support.

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

> Interface should be available.  I think this also helps to differentiate
> the more recent pseudo from legacy code.

A PS on this.  Keep in mind that I've put forward a proposal to replace
the pseudo registers with a separated cooked/frame registers and raw
register cache.

Being able to more easily identify pseudo register operations such as
this will help in that long term goal.

	enjoy,
		Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

David Taylor wrote:
> 
>     From: Andrew Cagney <ac131313@cygnus.com>
>     Date: Tue, 27 Feb 2001 20:38:30 -0500
> 
>     David Taylor wrote:
> 
>     > I propose that we:
>     >
>     > . add register_fetched
> 
>     David,
> 
>     The functionality of register_fetched() overlaps with
>     set_register_cached() and supply_register().  Rather than add a
>     redundant method, could an existing interface be used or the current
>     interfaces rationalized slightly?
> 
> Andrew,
> 
> Supply register does more than register_fetched; register_fetched only
> affects register_valid -- the register must have been supplied via
> some other method.

Yes, that is what I mean by their functionality overlapping.  There is
much legacy code of the form:

	blat the registers[] array
	set register_valid[] to 1

The accepted ``correct fix'' for that code is to replace it with
supply_register().  It is just an unfortunate reality that no one has
the time to do the work. I have a feeling that, right now GDB is
deleting this code faster than it is being fixed!

Given that reailty, if this code is going to be changed, I would prefer
it changed in a way that made it very clear that this is not the correct
way to do things.  Taking a lead from DEPRECATED_SERIAL_FD I think those
blocks can be converted to something like:

	for (;;)
	  *deprecated_register_buffer(...) = ...;
	deprecated_register_valid();

> set_register_cached is -- with one exception -- only used within
> regcache.c.  The one exception is remote.c (remote_fetch_registers).
> 
> I feel that a function should be created (register_unavailable?) and
> the call in remote.c to set_register_cached replaced with that call.
> Then set_register_cached should be made static.
> 
> To call set_register_cached, you have to know what the meanings are of
> the various possible values of register_valid.  This is knowledge that
> shouldn't really exist outside of regcache.c.

Yes.  My suggestion, hinted at in the last e-mail, would be to combine
the two disjoint operations:

	supply_register(....);
	set_register_cache(..., -1)

into an atomic cache transaction:

	supply_unavailable_register(....)

>     Keep in mind that the long term goal is to tighten regcache's interface
>     signficantly.  That is, eliminate register_valid[], registers[] and
>     possibly even set_register_cached() replacing them with a small set of
>     functions such as:
>             supply_register()
>             supply_unavailable_register()
>     If you are thinking of proposing further changes then you may want to
>     keep that in mind.
> 
> My change advances the goal of eliminating register_valid!  It reduces
> significantly the number of files that even know that register_valid
> exists!  Outside of regcache.c, only two files would reference it (for
> a total of three references).  Those two files could also have their
> references to it removed with a little bit more work.

I agree, eliminating register_valid[] and registers[] are important
goals.  The thing to be wary of is a change that just substitutes
something like registers[] and register_valid[] without actually
improving the overall quality of the code.  I think it is important to
ensure that any proposed change to GDB moves its overall code quality
forward (and not just sideways).

Grubbing around for other cases.

--

Grubbing through the code, the other case where register_valid[] is
being blatted is:

  /* These (for the most part) are pseudo registers and are obtained
     by other means.  Those that aren't are already handled by the
     code above.  */
  for (regi = IA64_GR32_REGNUM; regi <= IA64_GR127_REGNUM; regi++)
    register_valid[regi] = 1;
  for (regi = IA64_PR0_REGNUM; regi <= IA64_PR63_REGNUM; regi++)
    register_valid[regi] = 1;
  for (regi = IA64_VFP_REGNUM; regi <= NUM_REGS; regi++)
    register_valid[regi] = 1;

I'm guessing that code somewhere is looking at register_valid[] to
decide if a pseudo is available.  The actual value (hopefully) being
supplied by the pseudo-register method.

While that particular hack could eventually be fixed in other ways
(short term by having the ``register_cached()'' method ask gdbarch if a
pseudo is valid; and long term by having the register_valid() method
bound to a frame and always asking gdbarch if i is valid) that ain't
going to solve the hear and now.

Given that, I think this case (pseudo) is different/separate to the
legacy code case, I don't think it makes sense to use an interface like
deprecated_register_valid().  Instead, I think a separate:

	pseudo_register_valid(REGNUM)

Interface should be available.  I think this also helps to differentiate
the more recent pseudo from legacy code.

--

The other case I'm aware of, is with registers[] and core-gdb poking
around in that instead of using the read/write register functions.  I
think, there, the deprecated_register_buffer() method would be fine. 
That code should be using the read_register*() functions and not pokeing
around in that buffer.

>     With regard to regcache.h, yes the two clash.  It both moves code around
>     and changes the set_register_cached() interface.  If anything regcache.h
>     makes life easier because it is finally clear what the regcache
>     interfaces really are.
> 
>             Andrew
> 
> What change is this that you are referring to?  The message with subject
> 
>     [rfc] Re-merged regcache.h patch

I also mentioned:

        o       The changes adding:
+ #define REGISTER_CACHE_UNAVAILABLE (1)
+ #define REGISTER_CACHE_VALID       (0)
+ #define REGISTER_CACHE_INVALID     (-1)
                (or enum equivalents) were stripped out.
                These changes badly clashed with Davids
                proposed register_*() change so, I'll
                leave them for later.

I was actually wrong here.  The changes were only in my local sandpit
and not in the original patch.

>   +/* Character array containing the current state of each register
>   +   (unavailable<0, valid=0, invalid>0). */

See inferior.h:register_valid.  I'll fix the comment.  Thanks for
noticing this.

----

Any way, to summarize, I'm proposing:

	replace
		supply_register();
		set_register_cached();
	with:
		supply_unavailable_register()

	replace
		blat registers[];
		register_valid[] = 1;
	with
		blat deprecated_register_buffer(...);
		depreciated_register_valid/fetched(...);
	where this I/F would only work on raw registers.

	replace
		registers_valid[pseudo] = 1;
	with
		pseudo_register_valid/fetched(REGNUM)
	where the register must be a pseudo.

	replace remaining
		... = registers[];
	with
		... = depreciated_register_buffer();

I think this is a reasonable balance between the immediate desire of
eliminate the globals registers_valid[] and registers[] and the long
term objective of eliminating code that blats arbitrary bytes in the
registers[] array.

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[David Taylor]

    From: Andrew Cagney <ac131313@cygnus.com>
    Date: Tue, 27 Feb 2001 20:38:30 -0500

    David Taylor wrote:

    > I propose that we:
    > 
    > . add register_fetched

    David,

    The functionality of register_fetched() overlaps with
    set_register_cached() and supply_register().  Rather than add a
    redundant method, could an existing interface be used or the current
    interfaces rationalized slightly?

Andrew,

Supply register does more than register_fetched; register_fetched only
affects register_valid -- the register must have been supplied via
some other method.

set_register_cached is -- with one exception -- only used within
regcache.c.  The one exception is remote.c (remote_fetch_registers).

I feel that a function should be created (register_unavailable?) and
the call in remote.c to set_register_cached replaced with that call.
Then set_register_cached should be made static.

To call set_register_cached, you have to know what the meanings are of
the various possible values of register_valid.  This is knowledge that
shouldn't really exist outside of regcache.c.

    Keep in mind that the long term goal is to tighten regcache's interface
    signficantly.  That is, eliminate register_valid[], registers[] and
    possibly even set_register_cached() replacing them with a small set of
    functions such as:
	    supply_register()
	    supply_unavailable_register()
    If you are thinking of proposing further changes then you may want to
    keep that in mind.

My change advances the goal of eliminating register_valid!  It reduces
significantly the number of files that even know that register_valid
exists!  Outside of regcache.c, only two files would reference it (for
a total of three references).  Those two files could also have their
references to it removed with a little bit more work.

    With regard to regcache.h, yes the two clash.  It both moves code around
    and changes the set_register_cached() interface.  If anything regcache.h
    makes life easier because it is finally clear what the regcache
    interfaces really are.

	    Andrew

What change is this that you are referring to?  The message with subject

    [rfc] Re-merged regcache.h patch

that you posted after the above message?  I assume not, but if so...

. It does not move around code within regcache.c.  It adds a comment
  to regcache.c.  I don't see that as a conflict with my patch.

. Yes, it adds a new header file regcache.h.  A header file that is
  long overdue.  But, that does not affect the bulk of my patch.  It
  means that my one line change to gdbcore.h becomes instead a one
  line change to regcache.h.  Not a big deal -- I can change this
  easily enough.

. The posted patch does not change the set_register_cached interface.
  It does add a comment concerning the interface that conflicts with
  reality:

  +/* Character array containing the current state of each register
  +   (unavailable<0, valid=0, invalid>0). */

  but that is a problem with that patch, not with mine.

  [Current reality is: unavailable<0, invalid=0, valid>0.]

David

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

David Taylor wrote:

> I propose that we:
> 
> . add register_fetched

David,

The functionality of register_fetched() overlaps with
set_register_cached() and supply_register().  Rather than add a
redundant method, could an existing interface be used or the current
interfaces rationalized slightly?

Keep in mind that the long term goal is to tighten regcache's interface
signficantly.  That is, eliminate register_valid[], registers[] and
possibly even set_register_cached() replacing them with a small set of
functions such as:
	supply_register()
	supply_unavailable_register()
If you are thinking of proposing further changes then you may want to
keep that in mind.

With regard to regcache.h, yes the two clash.  It both moves code around
and changes the set_register_cached() interface.  If anything regcache.h
makes life easier because it is finally clear what the regcache
interfaces really are.

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[David Taylor]

    Date: Tue, 27 Feb 2001 18:19:35 -0500
    From: Andrew Cagney <ac131313@cygnus.com>

    David, there is an older regcache.h patch that should probably go in
    before this change.

Do my patch and the older patch conflict?  Or are they independent?
Is the older patch blocked somewhere?

Re: [RFA] regcache.c (register_fetched) + related changes

[Andrew Cagney]

David, there is an older regcache.h patch that should probably go in
before this change.

	Andrew

Re: [RFA] regcache.c (register_fetched) + related changes

[Michael Snyder]

David Taylor wrote:
> 
> Right now, we have:
> 
> . void register_changed (int regnum)
>   ==> invalidates a single register REGNUM in the cache.
> 
> . void registers_changed (void)
>   ==> indicate that registers may have changed, so invalidate the
>       cache.
> 
> . void registers_fetched (void)
>   ==> indicate that all registers have been fetched, so mark them
>       all valid.
> 
> But, we don't have:
> 
> . void register_fetched (int regnum)
>   ==> indicate that register REGNUM has been fetched.
> 
> I propose that we:
> 
> . add register_fetched
> 
> . change all the files that say
>   "register_valid[foo] = 1;" to instead say "register_fetched (foo);"
> 
> . change all the files that say
>   "register_valid[foo] = 0;" to instead say "register_changed (foo);"
> 
> . change all the files that say
>   "if (register_valid[regno])" or "if (!register_valid[regno])"
>   to instead say
>   "if (register_cached (regno))" or "if (!register_cached (regno))"
> 
> The following diff gets rid of all but 3 references to register_valid
> from outside of regcache.c by *.c files from within the gdb directory.
> 
> Okay to commit?

Hmmm... I can't approve all of the changes to various target and native
files, but as the original author of regcache.c I whole-heartedly approve
the change in general.


> 
> ChangeLog:
> 
> Tue Feb 27 16:59:10 2001  David Taylor  <taylor@redhat.com>
> 
>         * regcache.c (register_fetched): New function.
>         * gdbcore.h (register_fetched): Declare it.
>         * alpha-nat.c (fetch_elf_core_registers): Replace references to
>         register_valid with appropriate function calls.
>         * arm-linux-nat.c (store_nwfpe_register, store_fpregister,
>         store_fpregs, store_register, store_regs, fill_gregset,
>         fill_fpregset): Ditto.
>         * i386gnu-nat.c (gnu_store_registers): Ditto.
>         * ia64-aix-nat.c (supply_gregset): Ditto.
>         * lynx-nat.c (fetch_inferior_registers, store_inferior_registers):
>         Ditto.
>         * remote-mips.c (mips_load): Ditto.
>         * remote-uid.c (udi_fetch_registers, store_register): Ditto.
>         * rs6000-nat.c (fetch_register): Ditto.
>         * sh-tdep.c (sh_fetch_pseudo_register, sh_store_pseudo_register):
>         Ditto.
>         * sparc-nat.c (fetch_inferior_registers,
>         store_inferior_registers): Ditto.

[diffs pruned for bandwidth]

[RFA] regcache.c (register_fetched) + related changes

[David Taylor]

Right now, we have:

. void register_changed (int regnum)
  ==> invalidates a single register REGNUM in the cache.

. void registers_changed (void)
  ==> indicate that registers may have changed, so invalidate the
      cache.

. void registers_fetched (void)
  ==> indicate that all registers have been fetched, so mark them
      all valid.

But, we don't have:

. void register_fetched (int regnum)
  ==> indicate that register REGNUM has been fetched.

I propose that we:

. add register_fetched

. change all the files that say
  "register_valid[foo] = 1;" to instead say "register_fetched (foo);"

. change all the files that say
  "register_valid[foo] = 0;" to instead say "register_changed (foo);"

. change all the files that say
  "if (register_valid[regno])" or "if (!register_valid[regno])"
  to instead say
  "if (register_cached (regno))" or "if (!register_cached (regno))"

The following diff gets rid of all but 3 references to register_valid
from outside of regcache.c by *.c files from within the gdb directory.

Okay to commit?

ChangeLog:

Tue Feb 27 16:59:10 2001  David Taylor  <taylor@redhat.com>

	* regcache.c (register_fetched): New function.
	* gdbcore.h (register_fetched): Declare it.
	* alpha-nat.c (fetch_elf_core_registers): Replace references to
 	register_valid with appropriate function calls.
	* arm-linux-nat.c (store_nwfpe_register, store_fpregister,
 	store_fpregs, store_register, store_regs, fill_gregset,
 	fill_fpregset): Ditto.
	* i386gnu-nat.c (gnu_store_registers): Ditto.
	* ia64-aix-nat.c (supply_gregset): Ditto.
	* lynx-nat.c (fetch_inferior_registers, store_inferior_registers):
 	Ditto.
	* remote-mips.c (mips_load): Ditto.
	* remote-uid.c (udi_fetch_registers, store_register): Ditto.
	* rs6000-nat.c (fetch_register): Ditto.
	* sh-tdep.c (sh_fetch_pseudo_register, sh_store_pseudo_register):
 	Ditto.
	* sparc-nat.c (fetch_inferior_registers,
 	store_inferior_registers): Ditto.
	
cvs server: Diffing .
Index: alpha-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/alpha-nat.c,v
retrieving revision 1.5
diff -c -r1.5 alpha-nat.c
*** alpha-nat.c	2000/12/07 10:50:50	1.5
--- alpha-nat.c	2001/02/27 21:48:47
***************
*** 157,163 ****
        memcpy (&registers[REGISTER_BYTE (PC_REGNUM)], core_reg_sect + 31 * 8, 8);
        memset (&registers[REGISTER_BYTE (ZERO_REGNUM)], 0, 8);
        memset (&register_valid[V0_REGNUM], 1, 32);
!       register_valid[PC_REGNUM] = 1;
      }
  }
  
--- 157,163 ----
        memcpy (&registers[REGISTER_BYTE (PC_REGNUM)], core_reg_sect + 31 * 8, 8);
        memset (&registers[REGISTER_BYTE (ZERO_REGNUM)], 0, 8);
        memset (&register_valid[V0_REGNUM], 1, 32);
!       register_fetched (PC_REGNUM);
      }
  }
  
Index: arm-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-linux-nat.c,v
retrieving revision 1.8
diff -c -r1.8 arm-linux-nat.c
*** arm-linux-nat.c	2001/02/06 19:59:05	1.8
--- arm-linux-nat.c	2001/02/27 21:48:48
***************
*** 198,204 ****
  void
  store_nwfpe_register (int regno, FPA11 * fpa11)
  {
!   if (register_valid[regno])
      {
         unsigned int fn = regno - F0_REGNUM;
         switch (fpa11->fType[fn])
--- 198,204 ----
  void
  store_nwfpe_register (int regno, FPA11 * fpa11)
  {
!   if (register_cached (regno))
      {
         unsigned int fn = regno - F0_REGNUM;
         switch (fpa11->fType[fn])
***************
*** 337,343 ****
      }
  
    /* Store fpsr.  */
!   if (FPS_REGNUM == regno && register_valid[FPS_REGNUM])
      read_register_gen (FPS_REGNUM, (char *) &fp.fpsr);
  
    /* Store the floating point register.  */
--- 337,343 ----
      }
  
    /* Store fpsr.  */
!   if (FPS_REGNUM == regno && register_cached (FPS_REGNUM))
      read_register_gen (FPS_REGNUM, (char *) &fp.fpsr);
  
    /* Store the floating point register.  */
***************
*** 375,381 ****
      }
  
    /* Store fpsr.  */
!   if (register_valid[FPS_REGNUM])
      read_register_gen (FPS_REGNUM, (char *) &fp.fpsr);
  
    /* Store the floating point registers.  */
--- 375,381 ----
      }
  
    /* Store fpsr.  */
!   if (register_cached (FPS_REGNUM))
      read_register_gen (FPS_REGNUM, (char *) &fp.fpsr);
  
    /* Store the floating point registers.  */
***************
*** 469,475 ****
    int ret, tid;
    struct pt_regs regs;
    
!   if (!register_valid[regno])
      return;
  
    /* Get the thread id for the ptrace call.  */
--- 469,475 ----
    int ret, tid;
    struct pt_regs regs;
    
!   if (!register_cached (regno))
      return;
  
    /* Get the thread id for the ptrace call.  */
***************
*** 513,519 ****
  
    for (regno = A1_REGNUM; regno <= PC_REGNUM; regno++)
      {
!       if (register_valid[regno])
  	read_register_gen (regno, (char *) &regs.uregs[regno]);
      }
  
--- 513,519 ----
  
    for (regno = A1_REGNUM; regno <= PC_REGNUM; regno++)
      {
!       if (register_cached (regno))
  	read_register_gen (regno, (char *) &regs.uregs[regno]);
      }
  
***************
*** 581,598 ****
      {
        int regnum;
        for (regnum = A1_REGNUM; regnum <= PC_REGNUM; regnum++) 
!         if (register_valid[regnum])
  	  read_register_gen (regnum, (char *) &(*gregsetp)[regnum]);
      }
    else if (regno >= A1_REGNUM && regno <= PC_REGNUM)
      {
!       if (register_valid[regno])
  	read_register_gen (regno, (char *) &(*gregsetp)[regno]);
      }
  
    if (PS_REGNUM == regno || -1 == regno)
      {
!       if (register_valid[regno] || -1 == regno)
          {
            if (arm_apcs_32)
  	    read_register_gen (PS_REGNUM, (char *) &(*gregsetp)[CPSR_REGNUM]);
--- 581,598 ----
      {
        int regnum;
        for (regnum = A1_REGNUM; regnum <= PC_REGNUM; regnum++) 
!         if (register_cached (regnum))
  	  read_register_gen (regnum, (char *) &(*gregsetp)[regnum]);
      }
    else if (regno >= A1_REGNUM && regno <= PC_REGNUM)
      {
!       if (register_cached (regno))
  	read_register_gen (regno, (char *) &(*gregsetp)[regno]);
      }
  
    if (PS_REGNUM == regno || -1 == regno)
      {
!       if (-1 == regno || register_cached (regno))
          {
            if (arm_apcs_32)
  	    read_register_gen (PS_REGNUM, (char *) &(*gregsetp)[CPSR_REGNUM]);
***************
*** 645,651 ****
      }
  
    /* Store fpsr.  */
!   if (register_valid[FPS_REGNUM])
      if (FPS_REGNUM == regno || -1 == regno)
        read_register_gen (FPS_REGNUM, (char *) &fp->fpsr);
  }
--- 645,651 ----
      }
  
    /* Store fpsr.  */
!   if (register_cached (FPS_REGNUM))
      if (FPS_REGNUM == regno || -1 == regno)
        read_register_gen (FPS_REGNUM, (char *) &fp->fpsr);
  }
Index: gdbcore.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbcore.h,v
retrieving revision 1.5
diff -c -r1.5 gdbcore.h
*** gdbcore.h	2001/01/27 00:43:25	1.5
--- gdbcore.h	2001/02/27 21:48:55
***************
*** 109,114 ****
--- 109,115 ----
  extern CORE_ADDR register_addr (int regno, CORE_ADDR blockend);
  
  extern void registers_fetched (void);
+ extern void register_fetched (int regnum);
  
  #if !defined (KERNEL_U_ADDR)
  extern CORE_ADDR kernel_u_addr;
Index: i386gnu-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386gnu-nat.c,v
retrieving revision 1.3
diff -c -r1.3 i386gnu-nat.c
*** i386gnu-nat.c	2000/12/17 00:35:58	1.3
--- i386gnu-nat.c	2001/02/27 21:48:55
***************
*** 314,327 ****
  	  proc_debug (thread, "storing all registers");
  
  	  for (i = 0; i < NUM_GREGS; i++)
! 	    if (register_valid[i])
  	      fill (state, i);
  	}
        else
  	{
  	  proc_debug (thread, "storing register %s", REGISTER_NAME (regno));
  
! 	  gdb_assert (register_valid[regno]);
  	  fill (state, regno);
  	}
  
--- 314,327 ----
  	  proc_debug (thread, "storing all registers");
  
  	  for (i = 0; i < NUM_GREGS; i++)
! 	    if (register_cached (i))
  	      fill (state, i);
  	}
        else
  	{
  	  proc_debug (thread, "storing register %s", REGISTER_NAME (regno));
  
! 	  gdb_assert (register_cached (regno));
  	  fill (state, regno);
  	}
  
Index: ia64-aix-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-aix-nat.c,v
retrieving revision 1.1
diff -c -r1.1 ia64-aix-nat.c
*** ia64-aix-nat.c	2001/02/22 03:01:26	1.1
--- ia64-aix-nat.c	2001/02/27 21:48:55
***************
*** 70,80 ****
       by other means.  Those that aren't are already handled by the
       code above.  */
    for (regi = IA64_GR32_REGNUM; regi <= IA64_GR127_REGNUM; regi++)
!     register_valid[regi] = 1;
    for (regi = IA64_PR0_REGNUM; regi <= IA64_PR63_REGNUM; regi++)
!     register_valid[regi] = 1;
    for (regi = IA64_VFP_REGNUM; regi <= NUM_REGS; regi++)
!     register_valid[regi] = 1;
  }
  
  void
--- 70,80 ----
       by other means.  Those that aren't are already handled by the
       code above.  */
    for (regi = IA64_GR32_REGNUM; regi <= IA64_GR127_REGNUM; regi++)
!     register_fetched (regi);
    for (regi = IA64_PR0_REGNUM; regi <= IA64_PR63_REGNUM; regi++)
!     register_fetched (regi);
    for (regi = IA64_VFP_REGNUM; regi <= NUM_REGS; regi++)
!     register_fetched (regi);
  }
  
  void
Index: lynx-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/lynx-nat.c,v
retrieving revision 1.5
diff -c -r1.5 lynx-nat.c
*** lynx-nat.c	2001/02/25 04:45:11	1.5
--- lynx-nat.c	2001/02/27 21:48:56
***************
*** 297,303 ****
        memcpy (&registers[REGISTER_BYTE (G1_REGNUM)], &ec.g1,
  	      4 * REGISTER_RAW_SIZE (G1_REGNUM));
        for (i = G1_REGNUM; i <= G1_REGNUM + 3; i++)
! 	register_valid[i] = 1;
  
        supply_register (PS_REGNUM, (char *) &ec.psr);
        supply_register (Y_REGNUM, (char *) &ec.y);
--- 297,303 ----
        memcpy (&registers[REGISTER_BYTE (G1_REGNUM)], &ec.g1,
  	      4 * REGISTER_RAW_SIZE (G1_REGNUM));
        for (i = G1_REGNUM; i <= G1_REGNUM + 3; i++)
! 	register_fetched (i);
  
        supply_register (PS_REGNUM, (char *) &ec.psr);
        supply_register (Y_REGNUM, (char *) &ec.y);
***************
*** 308,314 ****
        memcpy (&registers[REGISTER_BYTE (O0_REGNUM)], ec.o,
  	      8 * REGISTER_RAW_SIZE (O0_REGNUM));
        for (i = O0_REGNUM; i <= O0_REGNUM + 7; i++)
! 	register_valid[i] = 1;
      }
  
    if (whatregs & WHATREGS_STACK)
--- 308,314 ----
        memcpy (&registers[REGISTER_BYTE (O0_REGNUM)], ec.o,
  	      8 * REGISTER_RAW_SIZE (O0_REGNUM));
        for (i = O0_REGNUM; i <= O0_REGNUM + 7; i++)
! 	register_fetched (i);
      }
  
    if (whatregs & WHATREGS_STACK)
***************
*** 322,334 ****
  			  &registers[REGISTER_BYTE (I0_REGNUM)],
  			  8 * REGISTER_RAW_SIZE (I0_REGNUM));
        for (i = I0_REGNUM; i <= I7_REGNUM; i++)
! 	register_valid[i] = 1;
  
        target_read_memory (sp + FRAME_SAVED_L0,
  			  &registers[REGISTER_BYTE (L0_REGNUM)],
  			  8 * REGISTER_RAW_SIZE (L0_REGNUM));
        for (i = L0_REGNUM; i <= L0_REGNUM + 7; i++)
! 	register_valid[i] = 1;
      }
  
    if (whatregs & WHATREGS_FLOAT)
--- 322,334 ----
  			  &registers[REGISTER_BYTE (I0_REGNUM)],
  			  8 * REGISTER_RAW_SIZE (I0_REGNUM));
        for (i = I0_REGNUM; i <= I7_REGNUM; i++)
! 	register_fetched (i);
  
        target_read_memory (sp + FRAME_SAVED_L0,
  			  &registers[REGISTER_BYTE (L0_REGNUM)],
  			  8 * REGISTER_RAW_SIZE (L0_REGNUM));
        for (i = L0_REGNUM; i <= L0_REGNUM + 7; i++)
! 	register_fetched (i);
      }
  
    if (whatregs & WHATREGS_FLOAT)
***************
*** 346,352 ****
        memcpy (&registers[REGISTER_BYTE (FP0_REGNUM)], fc.f.fregs,
  	      32 * REGISTER_RAW_SIZE (FP0_REGNUM));
        for (i = FP0_REGNUM; i <= FP0_REGNUM + 31; i++)
! 	register_valid[i] = 1;
  
        supply_register (FPS_REGNUM, (char *) &fc.fsr);
      }
--- 346,352 ----
        memcpy (&registers[REGISTER_BYTE (FP0_REGNUM)], fc.f.fregs,
  	      32 * REGISTER_RAW_SIZE (FP0_REGNUM));
        for (i = FP0_REGNUM; i <= FP0_REGNUM + 31; i++)
! 	register_fetched (i);
  
        supply_register (FPS_REGNUM, (char *) &fc.fsr);
      }
***************
*** 409,415 ****
  
        if (regno == -1 || regno == SP_REGNUM)
  	{
! 	  if (!register_valid[L0_REGNUM + 5])
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  target_write_memory (sp + FRAME_SAVED_I0,
  			      &registers[REGISTER_BYTE (I0_REGNUM)],
--- 409,415 ----
  
        if (regno == -1 || regno == SP_REGNUM)
  	{
! 	  if (!register_cached (L0_REGNUM + 5))
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  target_write_memory (sp + FRAME_SAVED_I0,
  			      &registers[REGISTER_BYTE (I0_REGNUM)],
***************
*** 421,427 ****
  	}
        else if (regno >= L0_REGNUM && regno <= I7_REGNUM)
  	{
! 	  if (!register_valid[regno])
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  if (regno >= L0_REGNUM && regno <= L0_REGNUM + 7)
  	    regoffset = REGISTER_BYTE (regno) - REGISTER_BYTE (L0_REGNUM)
--- 421,427 ----
  	}
        else if (regno >= L0_REGNUM && regno <= I7_REGNUM)
  	{
! 	  if (!register_cached (regno))
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  if (regno >= L0_REGNUM && regno <= L0_REGNUM + 7)
  	    regoffset = REGISTER_BYTE (regno) - REGISTER_BYTE (L0_REGNUM)
Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.17
diff -c -r1.17 regcache.c
*** regcache.c	2001/02/20 22:39:03	1.17
--- regcache.c	2001/02/27 21:48:57
***************
*** 190,195 ****
--- 190,205 ----
       Fetching all real regs might not account for all pseudo-regs.  */
  }
  
+ /* REGISTER_FETCHED
+ 
+    Indicate that register REGNUM has been fetched, so mark it as valid.  */
+ 
+ void
+ register_fetched (int regnum)
+ {
+   set_register_cached (regnum, 1);
+ }
+ 
  /* read_register_bytes and write_register_bytes are generally a *BAD*
     idea.  They are inefficient because they need to check for partial
     updates, which can only be done by scanning through all of the
Index: remote-mips.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-mips.c,v
retrieving revision 1.17
diff -c -r1.17 remote-mips.c
*** remote-mips.c	2001/02/25 04:45:11	1.17
--- remote-mips.c	2001/02/27 21:49:00
***************
*** 3440,3446 ****
        /* Work around problem where PMON monitor updates the PC after a load
           to a different value than GDB thinks it has. The following ensures
           that the write_pc() WILL update the PC value: */
!       register_valid[PC_REGNUM] = 0;
      }
    if (exec_bfd)
      write_pc (bfd_get_start_address (exec_bfd));
--- 3440,3446 ----
        /* Work around problem where PMON monitor updates the PC after a load
           to a different value than GDB thinks it has. The following ensures
           that the write_pc() WILL update the PC value: */
!       register_changed (PC_REGNUM);
      }
    if (exec_bfd)
      write_pc (bfd_get_start_address (exec_bfd));
Index: remote-udi.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-udi.c,v
retrieving revision 1.12
diff -c -r1.12 remote-udi.c
*** remote-udi.c	2001/02/02 23:04:14	1.12
--- remote-udi.c	2001/02/27 21:49:01
***************
*** 665,671 ****
    if (err = UDIRead (From, To, Count, Size, &CountDone, HostEndian))
      error ("UDIRead() failed in udi_fetch_registers");
  
!   register_valid[GR1_REGNUM] = 1;
  
  #if defined(GR64_REGNUM)	/* Read gr64-127 */
  
--- 665,671 ----
    if (err = UDIRead (From, To, Count, Size, &CountDone, HostEndian))
      error ("UDIRead() failed in udi_fetch_registers");
  
!   register_fetched (GR1_REGNUM);
  
  #if defined(GR64_REGNUM)	/* Read gr64-127 */
  
***************
*** 679,685 ****
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = GR64_REGNUM; i < GR64_REGNUM + 32; i++)
!     register_valid[i] = 1;
  
  #endif /*  GR64_REGNUM */
  
--- 679,685 ----
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = GR64_REGNUM; i < GR64_REGNUM + 32; i++)
!     register_fetched (i);
  
  #endif /*  GR64_REGNUM */
  
***************
*** 693,699 ****
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = GR96_REGNUM; i < GR96_REGNUM + 32; i++)
!     register_valid[i] = 1;
  
  /* Local Registers */
  
--- 693,699 ----
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = GR96_REGNUM; i < GR96_REGNUM + 32; i++)
!     register_fetched (i);
  
  /* Local Registers */
  
***************
*** 705,711 ****
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = LR0_REGNUM; i < LR0_REGNUM + 128; i++)
!     register_valid[i] = 1;
  
  /* Protected Special Registers */
  
--- 705,711 ----
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = LR0_REGNUM; i < LR0_REGNUM + 128; i++)
!     register_fetched (i);
  
  /* Protected Special Registers */
  
***************
*** 717,723 ****
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = SR_REGNUM (0); i < SR_REGNUM (0) + 15; i++)
!     register_valid[i] = 1;
  
    if (USE_SHADOW_PC)
      {				/* Let regno_to_srnum() handle the register number */
--- 717,723 ----
      error ("UDIRead() failed in udi_fetch_registers");
  
    for (i = SR_REGNUM (0); i < SR_REGNUM (0) + 15; i++)
!     register_fetched (i);
  
    if (USE_SHADOW_PC)
      {				/* Let regno_to_srnum() handle the register number */
***************
*** 735,741 ****
  	error ("UDIRead() failed in udi_fetch_registers");
  
        for (i = SR_REGNUM (128); i < SR_REGNUM (128) + 135 - 128 + 1; i++)
! 	register_valid[i] = 1;
      }
  
    if (remote_debug)
--- 735,741 ----
  	error ("UDIRead() failed in udi_fetch_registers");
  
        for (i = SR_REGNUM (128); i < SR_REGNUM (128) + 135 - 128 + 1; i++)
! 	register_fetched (i);
      }
  
    if (remote_debug)
***************
*** 1506,1513 ****
  
        /* Writing to this loc actually changes the values of pc0 & pc1 */
  
!       register_valid[PC_REGNUM] = 0;	/* pc1 */
!       register_valid[NPC_REGNUM] = 0;	/* pc0 */
      }
    else
      /* An unprotected or protected special register */
--- 1506,1513 ----
  
        /* Writing to this loc actually changes the values of pc0 & pc1 */
  
!       register_changed (PC_REGNUM);	/* pc1 */
!       register_changed (NPC_REGNUM);	/* pc0 */
      }
    else
      /* An unprotected or protected special register */
Index: rs6000-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-nat.c,v
retrieving revision 1.14
diff -c -r1.14 rs6000-nat.c
*** rs6000-nat.c	2001/02/10 11:05:39	1.14
--- rs6000-nat.c	2001/02/27 21:49:02
***************
*** 233,239 ****
      }
  
    if (!errno)
!     register_valid[regno] = 1;
    else
      {
  #if 0
--- 233,239 ----
      }
  
    if (!errno)
!     register_fetched (regno);
    else
      {
  #if 0
Index: sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.30
diff -c -r1.30 sh-tdep.c
*** sh-tdep.c	2001/02/08 06:03:53	1.30
--- sh-tdep.c	2001/02/27 21:49:04
***************
*** 1649,1655 ****
                target_fetch_registers (base_regnum + portion);
  
          }
!       register_valid [reg_nr] = 1;
      }
  }
  
--- 1649,1655 ----
                target_fetch_registers (base_regnum + portion);
  
          }
!       register_fetched (reg_nr);
      }
  }
  
***************
*** 1666,1672 ****
        /* Write the real regs for which this one is an alias.  */
        for (portion = 0; portion < 2; portion++)
  	{
! 	  register_valid[base_regnum + portion] = 1;
  	  target_store_registers (base_regnum + portion);
  	}
      }
--- 1666,1672 ----
        /* Write the real regs for which this one is an alias.  */
        for (portion = 0; portion < 2; portion++)
  	{
! 	  register_fetched (base_regnum + portion);
  	  target_store_registers (base_regnum + portion);
  	}
      }
***************
*** 1678,1684 ****
        /* Write the real regs for which this one is an alias.  */
        for (portion = 0; portion < 4; portion++)
  	{
! 	  register_valid[base_regnum + portion] = 1;
  	  target_store_registers (base_regnum + portion);
  	}
      }
--- 1678,1684 ----
        /* Write the real regs for which this one is an alias.  */
        for (portion = 0; portion < 4; portion++)
  	{
! 	  register_fetched (base_regnum + portion);
  	  target_store_registers (base_regnum + portion);
  	}
      }
Index: sparc-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-nat.c,v
retrieving revision 1.5
diff -c -r1.5 sparc-nat.c
*** sparc-nat.c	2001/02/25 04:45:11	1.5
--- sparc-nat.c	2001/02/27 21:49:04
***************
*** 70,76 ****
       to the stack pointer.  */
    if (regno < O7_REGNUM		/* including -1 */
        || regno >= Y_REGNUM
!       || (!register_valid[SP_REGNUM] && regno < I7_REGNUM))
      {
        if (0 != ptrace (PTRACE_GETREGS, inferior_pid,
  		       (PTRACE_ARG3_TYPE) & inferior_registers, 0))
--- 70,76 ----
       to the stack pointer.  */
    if (regno < O7_REGNUM		/* including -1 */
        || regno >= Y_REGNUM
!       || (!register_cached (SP_REGNUM) && regno < I7_REGNUM))
      {
        if (0 != ptrace (PTRACE_GETREGS, inferior_pid,
  		       (PTRACE_ARG3_TYPE) & inferior_registers, 0))
***************
*** 85,100 ****
        *(int *) &registers[REGISTER_BYTE (Y_REGNUM)] = inferior_registers.r_y;
  
        for (i = G0_REGNUM; i <= O7_REGNUM; i++)
! 	register_valid[i] = 1;
!       register_valid[Y_REGNUM] = 1;
!       register_valid[PS_REGNUM] = 1;
!       register_valid[PC_REGNUM] = 1;
!       register_valid[NPC_REGNUM] = 1;
        /* If we don't set these valid, read_register_bytes() rereads
           all the regs every time it is called!  FIXME.  */
!       register_valid[WIM_REGNUM] = 1;	/* Not true yet, FIXME */
!       register_valid[TBR_REGNUM] = 1;	/* Not true yet, FIXME */
!       register_valid[CPS_REGNUM] = 1;	/* Not true yet, FIXME */
      }
  
    /* Floating point registers */
--- 85,100 ----
        *(int *) &registers[REGISTER_BYTE (Y_REGNUM)] = inferior_registers.r_y;
  
        for (i = G0_REGNUM; i <= O7_REGNUM; i++)
! 	register_fetched (i);
!       register_fetched (Y_REGNUM);
!       register_fetched (PS_REGNUM);
!       register_fetched (PC_REGNUM);
!       register_fetched (NPC_REGNUM);
        /* If we don't set these valid, read_register_bytes() rereads
           all the regs every time it is called!  FIXME.  */
!       register_fetched (WIM_REGNUM);		/* Not true yet, FIXME */
!       register_fetched (TBR_REGNUM);		/* Not true yet, FIXME */
!       register_fetched (CPS_REGNUM);		/* Not true yet, FIXME */
      }
  
    /* Floating point registers */
***************
*** 112,119 ****
  	      &inferior_fp_registers.Fpu_fsr,
  	      sizeof (FPU_FSR_TYPE));
        for (i = FP0_REGNUM; i <= FP0_REGNUM + 31; i++)
! 	register_valid[i] = 1;
!       register_valid[FPS_REGNUM] = 1;
      }
  
    /* These regs are saved on the stack by the kernel.  Only read them
--- 112,119 ----
  	      &inferior_fp_registers.Fpu_fsr,
  	      sizeof (FPU_FSR_TYPE));
        for (i = FP0_REGNUM; i <= FP0_REGNUM + 31; i++)
! 	register_fetched (i);
!       register_fetched (FPS_REGNUM);
      }
  
    /* These regs are saved on the stack by the kernel.  Only read them
***************
*** 124,140 ****
  			  &registers[REGISTER_BYTE (L0_REGNUM)],
  			  16 * REGISTER_RAW_SIZE (L0_REGNUM));
        for (i = L0_REGNUM; i <= I7_REGNUM; i++)
! 	register_valid[i] = 1;
      }
    else if (regno >= L0_REGNUM && regno <= I7_REGNUM)
      {
        CORE_ADDR sp = *(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)];
        i = REGISTER_BYTE (regno);
!       if (register_valid[regno])
  	printf_unfiltered ("register %d valid and read\n", regno);
        target_read_memory (sp + i - REGISTER_BYTE (L0_REGNUM),
  			  &registers[i], REGISTER_RAW_SIZE (regno));
!       register_valid[regno] = 1;
      }
  }
  
--- 124,140 ----
  			  &registers[REGISTER_BYTE (L0_REGNUM)],
  			  16 * REGISTER_RAW_SIZE (L0_REGNUM));
        for (i = L0_REGNUM; i <= I7_REGNUM; i++)
! 	register_fetched (i);
      }
    else if (regno >= L0_REGNUM && regno <= I7_REGNUM)
      {
        CORE_ADDR sp = *(CORE_ADDR *) & registers[REGISTER_BYTE (SP_REGNUM)];
        i = REGISTER_BYTE (regno);
!       if (register_cached (regno))
  	printf_unfiltered ("register %d valid and read\n", regno);
        target_read_memory (sp + i - REGISTER_BYTE (L0_REGNUM),
  			  &registers[i], REGISTER_RAW_SIZE (regno));
!       register_fetched (regno);
      }
  }
  
***************
*** 194,200 ****
  
        if (regno < 0 || regno == SP_REGNUM)
  	{
! 	  if (!register_valid[L0_REGNUM + 5])
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  target_write_memory (sp,
  			       &registers[REGISTER_BYTE (L0_REGNUM)],
--- 194,200 ----
  
        if (regno < 0 || regno == SP_REGNUM)
  	{
! 	  if (!register_cached (L0_REGNUM + 5))
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  target_write_memory (sp,
  			       &registers[REGISTER_BYTE (L0_REGNUM)],
***************
*** 202,208 ****
  	}
        else
  	{
! 	  if (!register_valid[regno])
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  target_write_memory (sp + REGISTER_BYTE (regno) - REGISTER_BYTE (L0_REGNUM),
  			       &registers[REGISTER_BYTE (regno)],
--- 202,208 ----
  	}
        else
  	{
! 	  if (!register_cached (regno))
  	    internal_error (__FILE__, __LINE__, "failed internal consistency check");
  	  target_write_memory (sp + REGISTER_BYTE (regno) - REGISTER_BYTE (L0_REGNUM),
  			       &registers[REGISTER_BYTE (regno)],
***************
*** 213,219 ****
  
    if (wanna_store & INT_REGS)
      {
!       if (!register_valid[G1_REGNUM])
  	internal_error (__FILE__, __LINE__, "failed internal consistency check");
  
        memcpy (&inferior_registers.r_g1, &registers[REGISTER_BYTE (G1_REGNUM)],
--- 213,219 ----
  
    if (wanna_store & INT_REGS)
      {
!       if (!register_cached (G1_REGNUM))
  	internal_error (__FILE__, __LINE__, "failed internal consistency check");
  
        memcpy (&inferior_registers.r_g1, &registers[REGISTER_BYTE (G1_REGNUM)],
***************
*** 235,241 ****
  
    if (wanna_store & FP_REGS)
      {
!       if (!register_valid[FP0_REGNUM + 9])
  	internal_error (__FILE__, __LINE__, "failed internal consistency check");
        memcpy (&inferior_fp_registers, &registers[REGISTER_BYTE (FP0_REGNUM)],
  	      sizeof inferior_fp_registers.fpu_fr);
--- 235,241 ----
  
    if (wanna_store & FP_REGS)
      {
!       if (!register_cached (FP0_REGNUM + 9))
  	internal_error (__FILE__, __LINE__, "failed internal consistency check");
        memcpy (&inferior_fp_registers, &registers[REGISTER_BYTE (FP0_REGNUM)],
  	      sizeof inferior_fp_registers.fpu_fr);