Register cache

Register cache

[Mark Kettenis]

A few days ago Eli proposed the following patch:

   http://sources.redhat.com/ml/gdb-patches/2001-02/msg00140.html

It proposes a new interface to supply a single i387 register to GDB's
internal register cache in addition to the existing interface that
supplies the whole FPU state.

In a perhaps overly pedantic mood, I raised some objections to the
patch.  I would prefer not to add yet another interface, and I
suggested to Eli that he'd simply use the existing interface, to which
he responded that that would implicate violating the
target_fetch_registers() API.  Let me explain:

We have the following comment on target_fetch_registers() in target.h:

   /* Fetch register REGNO, or all regs if regno == -1.  No result.  */

This suggests that target_fetch_registers() will fetch exactly one
register if REGNO != -1.  However, quite a few targets do something
different, that is, they fetch at least register REGNO, but actually
fetch a whole bunch of them.  This probably started with SVR4-ish
systems that have a /proc filesystem.  On those systems it isn't
possible to fetch a single register.  Instead one get a whole bunch
and procfs_fetch_registers() supplies them all to GDB's register
cache.  Comments in procfs.c suggest that this is an optimization; by
caching the whole register set on possibly saves a few system calls in
the future.  Other targets like the modern BSDs and GNU/Linux that
have appropriate ptrace() requests followed this example and do the
same thing.

Is this really a good idea?  I think it is, at least on targets where
getting at the registers is relatively expensive since it involves a
system call.  Therefore I propose to make this "official" and change
the comment on target_fetch_registers() to:

   /* Fetch at least register REGNO, or all regs if REGNO == -1.  */

Another question is whether we should make this behaviour (I mean
having target_fetch_registers() supply all registers it managed to get
in one go) recommended practice.

Mark

Re: Register cache

[Nick Duffek]

On 10-Feb-2001, Mark Kettenis wrote:

>Therefore I propose to make this "official" and change
>the comment on target_fetch_registers() to:
>
>   /* Fetch at least register REGNO, or all regs if REGNO == -1.  */

I think that's a great idea.

>Another question is whether we should make this behaviour (I mean
>having target_fetch_registers() supply all registers it managed to get
>in one go) recommended practice.

I don't agree with that.  There are times when GDB just needs one
register, and it may be much more efficient on some targets to fetch one
register than to fetch all of them.

I think there might be places in GDB that fetch all registers by calling
target_fetch_registers(REGNUM) for each REGNUM.  If so, then I vote for
changing those places to call target_fetch_registers(-1) instead of
recommending against the target_fetch_registers(REGNUM) interface.

Nick

Re: Register cache

[Eli Zaretskii]

On Sun, 11 Feb 2001, Nick Duffek wrote:

> >Therefore I propose to make this "official" and change
> >the comment on target_fetch_registers() to:
> >
> >   /* Fetch at least register REGNO, or all regs if REGNO == -1.  */
> 
> I think that's a great idea.
> 
> >Another question is whether we should make this behaviour (I mean
> >having target_fetch_registers() supply all registers it managed to get
> >in one go) recommended practice.
> 
> I don't agree with that.  There are times when GDB just needs one
> register, and it may be much more efficient on some targets to fetch one
> register than to fetch all of them.

So you are telling, in effect, that it's okay to have
i387_supply_fsave get all the FP registers, and x86 targets which
don't like that should provide ther own code instead of using 
i387_supply_fsave?

Re: Register cache

[Eli Zaretskii]

> Date: Mon, 12 Feb 2001 12:53:09 -0500
> From: Nick Duffek <nsd@redhat.com>
> 
> In my opinion, interface expansion is preferable to code duplication, so I
> agree with your patch to put i387_supply_fpreg in i387-nat.c. 

Thanks for the feedback.

> However,
> that's really a coding philosophy issue, and since Mark wrote i387-nat.c,
> I'm inclined to bow to his opinion on how it gets changed.

Perhaps Mark could take your opinion into consideration ;-).

Re: Register cache

[Mark Kettenis]

   Date: Mon, 12 Feb 2001 13:37:25 -0500 (EST)
   From: Eli Zaretskii <eliz@delorie.com>

   > However,
   > that's really a coding philosophy issue, and since Mark wrote i387-nat.c,
   > I'm inclined to bow to his opinion on how it gets changed.

   Perhaps Mark could take your opinion into consideration ;-).

How about the attached patch?  Just give a yell and I'll check it in

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* i387-nat.c: Use regnum instead of regno consistently.  Fix
	comments accordingly.
	(i387_supply_register): New function.
	(i387_supply_fsave): Implement using i387_supply_register.
	* i387-nat.h: Use regnum instead of regno consistently.  Fix
	comments accordingly.

Index: i387-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i387-nat.c,v
retrieving revision 1.2
diff -u -p -r1.2 i387-nat.c
--- i387-nat.c 2000/08/10 14:54:51 1.2
+++ i387-nat.c 2001/02/16 23:19:48
@@ -1,5 +1,5 @@
 /* Native-dependent code for the i387.
-   Copyright 2000 Free Software Foundation, Inc.
+   Copyright 2000, 2001 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -22,15 +22,17 @@
 #include "inferior.h"
 #include "value.h"
 
+#include "i387-nat.h"
+
 /* FIXME: kettenis/2000-05-21: Right now more than a few i386 targets
    define their own routines to manage the floating-point registers in
    GDB's register array.  Most (if not all) of these targets use the
    format used by the "fsave" instruction in their communication with
    the OS.  They should all be converted to use the routines below.  */
 
-/* At fsave_offset[REGNO] you'll find the offset to the location in
+/* At fsave_offset[REGNUM] you'll find the offset to the location in
    the data structure used by the "fsave" instruction where GDB
-   register REGNO is stored.  */
+   register REGNUM is stored.  */
 
 static int fsave_offset[] =
 {
@@ -55,6 +57,32 @@ static int fsave_offset[] =
 #define FSAVE_ADDR(fsave, regnum) (fsave + fsave_offset[regnum - FP0_REGNUM])
 \f
 
+/* Fill register REGNUM in GDB's register array with the appropriate
+   value from *FSAVE.  This function masks off any of the reserved
+   bits in *FSAVE.  */
+
+void
+i387_supply_register (int regnum, char *fsave)
+{
+  /* Most of the FPU control registers occupy only 16 bits in
+     the fsave area.  Give those a special treatment.  */
+  if (regnum >= FIRST_FPU_CTRL_REGNUM
+      && regnum != FCOFF_REGNUM && regnum != FDOFF_REGNUM)
+    {
+      unsigned int val = *(unsigned short *) (FSAVE_ADDR (fsave, regnum));
+
+      if (regnum == FOP_REGNUM)
+	{
+	  val &= ((1 << 11) - 1);
+	  supply_register (regnum, (char *) &val);
+	}
+      else
+	supply_register (regnum, (char *) &val);
+    }
+  else
+    supply_register (regnum, FSAVE_ADDR (fsave, regnum));
+}
+
 /* Fill GDB's register array with the floating-point register values
    in *FSAVE.  This function masks off any of the reserved
    bits in *FSAVE.  */
@@ -65,39 +93,21 @@ i387_supply_fsave (char *fsave)
   int i;
 
   for (i = FP0_REGNUM; i <= LAST_FPU_CTRL_REGNUM; i++)
-    {
-      /* Most of the FPU control registers occupy only 16 bits in
-	 the fsave area.  Give those a special treatment.  */
-      if (i >= FIRST_FPU_CTRL_REGNUM
-	  && i != FCOFF_REGNUM && i != FDOFF_REGNUM)
-	{
-	  unsigned int val = *(unsigned short *) (FSAVE_ADDR (fsave, i));
-
-	  if (i == FOP_REGNUM)
-	    {
-	      val &= ((1 << 11) - 1);
-	      supply_register (i, (char *) &val);
-	    }
-	  else
-	    supply_register (i, (char *) &val);
-	}
-      else
-	supply_register (i, FSAVE_ADDR (fsave, i));
-    }
+    i387_supply_register (i, fsave);
 }
 
-/* Fill register REGNO (if it is a floating-point register) in *FSAVE
-   with the value in GDB's register array.  If REGNO is -1, do this
+/* Fill register REGNUM (if it is a floating-point register) in *FSAVE
+   with the value in GDB's register array.  If REGNUM is -1, do this
    for all registers.  This function doesn't touch any of the reserved
    bits in *FSAVE.  */
 
 void
-i387_fill_fsave (char *fsave, int regno)
+i387_fill_fsave (char *fsave, int regnum)
 {
   int i;
 
   for (i = FP0_REGNUM; i <= LAST_FPU_CTRL_REGNUM; i++)
-    if (regno == -1 || regno == i)
+    if (regnum == -1 || regnum == i)
       {
 	/* Most of the FPU control registers occupy only 16 bits in
            the fsave area.  Give those a special treatment.  */
@@ -125,9 +135,9 @@ i387_fill_fsave (char *fsave, int regno)
 }
 \f
 
-/* At fxsave_offset[REGNO] you'll find the offset to the location in
+/* At fxsave_offset[REGNUM] you'll find the offset to the location in
    the data structure used by the "fxsave" instruction where GDB
-   register REGNO is stored.  */
+   register REGNUM is stored.  */
 
 static int fxsave_offset[] =
 {
@@ -223,18 +233,18 @@ i387_supply_fxsave (char *fxsave)
     }
 }
 
-/* Fill register REGNO (if it is a floating-point or SSE register) in
-   *FXSAVE with the value in GDB's register array.  If REGNO is -1, do
+/* Fill register REGNUM (if it is a floating-point or SSE register) in
+   *FXSAVE with the value in GDB's register array.  If REGNUM is -1, do
    this for all registers.  This function doesn't touch any of the
    reserved bits in *FXSAVE.  */
 
 void
-i387_fill_fxsave (char *fxsave, int regno)
+i387_fill_fxsave (char *fxsave, int regnum)
 {
   int i;
 
   for (i = FP0_REGNUM; i <= MXCSR_REGNUM; i++)
-    if (regno == -1 || regno == i)
+    if (regnum == -1 || regnum == i)
       {
 	/* Most of the FPU control registers occupy only 16 bits in
            the fxsave area.  Give those a special treatment.  */
Index: i387-nat.h
===================================================================
RCS file: /cvs/src/src/gdb/i387-nat.h,v
retrieving revision 1.2
diff -u -p -r1.2 i387-nat.h
--- i387-nat.h 2000/08/10 14:54:51 1.2
+++ i387-nat.h 2001/02/16 23:19:48
@@ -1,5 +1,5 @@
 /* Native-dependent code for the i387.
-   Copyright 2000 Free Software Foundation, Inc.
+   Copyright 2000, 2001 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -21,18 +21,24 @@
 #ifndef I387_NAT_H
 #define I387_NAT_H
 
+/* Fill register REGNO in GDB's register array with the appropriate
+   value from *FSAVE.  This function masks off any of the reserved
+   bits in *FSAVE.  */
+
+extern void i387_supply_register (int regnum, char *fsave);
+
 /* Fill GDB's register array with the floating-point register values
    in *FSAVE.  This function masks off any of the reserved
    bits in *FSAVE.  */
 
 extern void i387_supply_fsave (char *fsave);
 
-/* Fill register REGNO (if it is a floating-point register) in *FSAVE
-   with the value in GDB's register array.  If REGNO is -1, do this
+/* Fill register REGNUM (if it is a floating-point register) in *FSAVE
+   with the value in GDB's register array.  If REGNUM is -1, do this
    for all registers.  This function doesn't touch any of the reserved
    bits in *FSAVE.  */
 
-extern void i387_fill_fsave (char *fsave, int regno);
+extern void i387_fill_fsave (char *fsave, int regnum);
 
 /* Fill GDB's register array with the floating-point and SSE register
    values in *FXSAVE.  This function masks off any of the reserved
@@ -40,11 +46,11 @@ extern void i387_fill_fsave (char *fsave
 
 extern void i387_supply_fxsave (char *fxsave);
 
-/* Fill register REGNO (if it is a floating-point or SSE register) in
-   *FXSAVE with the value in GDB's register array.  If REGNO is -1, do
+/* Fill register REGNUM (if it is a floating-point or SSE register) in
+   *FXSAVE with the value in GDB's register array.  If REGNUM is -1, do
    this for all registers.  This function doesn't touch any of the
    reserved bits in *FXSAVE.  */
 
-extern void i387_fill_fxsave (char *fxsave, int regno);
+extern void i387_fill_fxsave (char *fxsave, int regnum);
 
 #endif /* i387-nat.h */

Re: Register cache

[Eli Zaretskii]

> Date: Sat, 17 Feb 2001 00:21:09 +0100
> From: Mark Kettenis <kettenis@wins.uva.nl>
> 
> How about the attached patch?  Just give a yell and I'll check it in
> 
> Mark
> 
> 
> Index: ChangeLog
> from  Mark Kettenis  <kettenis@gnu.org>
> 
>	* i387-nat.c: Use regnum instead of regno consistently.  Fix
>	comments accordingly.
>	(i387_supply_register): New function.
>	(i387_supply_fsave): Implement using i387_supply_register.
>	* i387-nat.h: Use regnum instead of regno consistently.  Fix
>	comments accordingly.

How can I say anything but "YES, PLEASE!" ?  It's almost the exact
patch I suggested ;-)

Thanks!

Re: Register cache

[Mark Kettenis]

   Date: Sat, 17 Feb 2001 03:28:37 -0500 (EST)
   From: Eli Zaretskii <eliz@delorie.com>

   > Date: Sat, 17 Feb 2001 00:21:09 +0100
   > From: Mark Kettenis <kettenis@wins.uva.nl>
   > 
   > How about the attached patch?  Just give a yell and I'll check it in
   > 
   > Mark
   > 
   > 
   > Index: ChangeLog
   > from  Mark Kettenis  <kettenis@gnu.org>
   > 
   >	* i387-nat.c: Use regnum instead of regno consistently.  Fix
   >	comments accordingly.
   >	(i387_supply_register): New function.
   >	(i387_supply_fsave): Implement using i387_supply_register.
   >	* i387-nat.h: Use regnum instead of regno consistently.  Fix
   >	comments accordingly.

   How can I say anything but "YES, PLEASE!" ?  It's almost the exact
   patch I suggested ;-)

Yeah! Basically I only changed the name of the function and the order
of the paramaters such that it more closely resembles the signature of
supply_register().  And I took the opportunity to sneak in some
gratuitious changes :-).  Anyway, it's checked in now.

Mark

Re: Register cache

[Andrew Cagney]

> Is this really a good idea?  I think it is, at least on targets where
> getting at the registers is relatively expensive since it involves a
> system call.  Therefore I propose to make this "official" and change
> the comment on target_fetch_registers() to:
> 
>    /* Fetch at least register REGNO, or all regs if REGNO == -1.  */

This behavour is pretty much engrained in GDB so yes, the documentation
should be changed to reflect this.

> Another question is whether we should make this behaviour (I mean
> having target_fetch_registers() supply all registers it managed to get
> in one go) recommended practice.

I'd agree with Nick and not go that far.  Keep the contract between
GDB's core and the target simple.

	Andrew

Re: Register Cache.

[Andrew Cagney]

Just FYI,

I'm letting this e-mail slip off my desk and onto the floor.  Someone
will eventually re-visit the idea (since it is very real).?

	Andrew


To : gdb at sources dot redhat dot com
Subject : Register Cache.
From : Steven Johnson <sbjohnson at ozemail dot com dot au>
Date : Wed, 30 Aug 2000 10:47:06 +1000
References : <8AE4B526B977D411841F00A0CC334020052C28@cuz-exchange.sdesigns.net>

GDB caches it's register reads. ie, it will only read a register once, and
will only write a register if it thinks the value has changed.

For Example:

set $myreg=0x12345678     <-- Results in GDB Actually changing the register.
set $myreg=0x12345678     <-- Filtered by GDB and doesnt set register.

Now this seems fine on the surface, but there are many registers that
the act of writing is sometimes more important than the data (like say a
watchdog reset register). You may need to write the same value multiple times.

I Can find no way of forcing GDB to not cache its register accesses (dcache
also has this problem, but it also has a solution) does anyone know of a way to
force GDB to either always update registers and read registers and not cache
them, or of a way to force GDB to set it's status of these registers as unknown
(and hence making it update from the register).

Ive spent all day hunting through the code ("Using the source" as some would
say) but i'm damned if I can find anything. So I am attempting to introduce my
own solution. But obviously I may have missed something. 

Steven Johnson

Re: Register Cache.

[Fernando Nasser]

This would be a serious problem on targets that have access to Control
Registers.   However, I vaguely remember our register cache being a
"write-through" cache, i.e., registers are written to the cache AND to
the actual register (it worked like that on the source code at some
point last year -- there are people using this right now).  I wonder if
this behavior was changed.

The only complain I got from users is that some hardware registers do
not keep the value that are written to them (some of the bits you may
have written to are R/O or you write to a bit to reset it and it is read
as zero afterwards etc.).  As GDB caches the value it has written,
subsequent reads do not reflect the actual register value until you run
the target (a stepi will do).  The solution proposed (never implemented)
was to add a user command to flush the register cache.  In the GUI case,
on user request, the GUI would reread the values after doing that.  This
would prevent the user to do a "stepi" (sometimes not desirable).

Fernando



Andrew Cagney wrote:
> 
> Just FYI,
> 
> I'm letting this e-mail slip off my desk and onto the floor.  Someone
> will eventually re-visit the idea (since it is very real).?
> 
>         Andrew
> 
>   ------------------------------------------------------------------------
> 
> Subject: Register Cache.
> Date: Wed, 30 Aug 2000 10:47:06 +1000
> From: Steven Johnson <sbjohnson@ozemail.com.au>
> To: gdb@sources.redhat.com
> References: <8AE4B526B977D411841F00A0CC334020052C28@cuz-exchange.sdesigns.net>
> 
> GDB caches it's register reads. ie, it will only read a register once, and
> will only write a register if it thinks the value has changed.
> 
> For Example:
> 
> set $myreg=0x12345678     <-- Results in GDB Actually changing the register.
> set $myreg=0x12345678     <-- Filtered by GDB and doesnt set register.
> 
> Now this seems fine on the surface, but there are many registers that
> the act of writing is sometimes more important than the data (like say a
> watchdog reset register). You may need to write the same value multiple times.
> 
> I Can find no way of forcing GDB to not cache its register accesses (dcache
> also has this problem, but it also has a solution) does anyone know of a way to
> force GDB to either always update registers and read registers and not cache
> them, or of a way to force GDB to set it's status of these registers as unknown
> (and hence making it update from the register).
> 
> Ive spent all day hunting through the code ("Using the source" as some would
> say) but i'm damned if I can find anything. So I am attempting to introduce my
> own solution. But obviously I may have missed something.
> 
> Steven Johnson
> 
>   ------------------------------------------------------------------------
> 
> Subject: Re: Register Cache.
> Date: Thu, 31 Aug 2000 14:27:24 +1000
> From: Steven Johnson <sbjohnson@ozemail.com.au>
> To: gdb@sources.redhat.com
> References: <8AE4B526B977D411841F00A0CC334020052C28@cuz-exchange.sdesigns.net> <39AC598A.DFAF67E9@ozemail.com.au>
> 
> I Want to change the way GDB Caches Registers by doing the following:
> 
> 1. Add a Cache Type entry so that each register can be set (via a gdb command)
> to be read cached, write cached or no cache. By default it would be read/write
> cache enabled as is the case now.
> 
> 2. Add a command to flush GDB's knowledge of the cache.
> 
> My Questions are:
> 
> 1. Does anyone have any input they would like to add to this?
> 2. What sort of command should they be maintenance, data or set commands.
> 
> I Would like to have commands like this:
> set register-cache $r5 read
> set register-cache $r5 write
> set register-cache $r5 rw
> set register-cache $r5 disabled
> 
> and another command
> 
> maintenance register-cache flush
> 
> This raises another question:
> 
> 3. How do i look up the $r5 in the command and turn it into a register number?
> 
> I tried finding the code the interprets:
> set $r5=0
> 
> to see how it did it, but it's obscure and I couldn't find it.
> 
> Steven Johnson.

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From unknown@somewhere.org Thu Mar 29 16:26:00 2001
To: egcs@cygnus.com
Subject: No Subject
Date: Thu, 29 Mar 2001 16:26:00 -0000
Message-id: <21326ae5a2c9179cdc607dfa974d7fce@NO-ID-FOUND.mhonarc.org>
X-SW-Source: 2001-03/msg00283.html
Content-length: 0