[rfc] Regcache revamp (vip)

[Andrew Cagney <ac131313 at cygnus dot com>]

Hello,

	This patch may change your life!

Well, yes, ok, I'm probably being a little dramatic!  However, it
certainly does change the way targets implement pseudo/cooked registers.

The attatched patch revamps the regcache interface along the lines
described in:

 Re: [RFA] regcache.c (register_fetched) + related changes
 http://sources.redhat.com/ml/gdb-patches/2001-03/msg00191.html

In particular it does the following:

	o	adds two new interfaces:
			regcache_read(rawnum, buf)
		  and	regcache_write(rawnum, buf)
		that only accept ``raw'' registers
		or registers in the range [0 .. NUM_REGS)

	o	adds two new architecture methods
			gdbarch_register_read (regnum, buf)
		   and	gdbarch_register_write (regnum, buf)
		and modifes {read,write}_register_gen to
		call them.  It is assumed that these methods
		will be implemented using regcache_{read,write}().

		Legacy code gets the current legach mechaism which
		allows psuedo registers to be directly read/written
		to the reg-cache.

These changes affect the flow of control from core-gdb through to the
target.  A register transfer should now follow the path (read or write):

		core-gdb
		   |
		read_register_gen()
		   |
		gdbarch_register_read()
		   |
		regcache_read () and/or ``d-cache-read()''
		   |
		raw target transfer

Unfortunatly, much of core-gdb bypasses the {read,write}_register_gen()
entry points and, instead, uses registers[], register_valid[] and
register_buffer() to thwack the cache directly :-(  I've addressed the
obvious cases by:

	o	a rewrite (yes, rewrite, gasp!!!)
		or read_register_bytes().
		It now uses read_register_gen()
		to fetch registers.

	o	simplifies read_register(),
		write_regsiter() et.al. along
		the same lines.

This patch has consequences:

	o	The external interfaces
		register[], register_valid[]
		and register_buffer() are
		being documented as deprecated.

		It is thought that this patch
		eliminates any need for these
		interfaces.

	o	Other pseudo-register interfaces
		such as STORE_PSEUDO_REG()
		are put in the fireing line.

		It is thought that this patch will
		eliminate the need for the fetch
		and store pseudo methods since
		that same code can be implemented
		more robustly using the arch
		methods gdbarch_register_{read,write}().

Finally, please keep in mind that the regcache_{read,write}() and
gdbarch_register_{read,write}() interfaces are not frozen.  As part of
multi-arch, all that global state will hopefully be eliminated and that
may well force the addition of a ``struct regcache *'', ``struct
thread_info *'', or ??? parameter.  I didn't add such a parameter now,
since I don't yet know what it should be :-)

Any way, please review carefully and try to figure out where the theory
is flawed :-)  Preliminary testing appears ok.

	Andrew
2001-03-16  Andrew Cagney  <ac131313@redhat.com>

	* gdbarch.sh (gdbarch_register_read, gdbarch_register_write): Add.
	* gdbarch.h, gdbarch.c: Regenerate.
	
	* regcache.h (regcache_read, regcache_write): Declare.
	(registers, register_valid, register_buffer): Add note that these
	interfaces are deprecated.
	
	* regcache.c: Include "gdb_assert.h".
	(legacy_write_register_gen): Rename write_register_gen.
	(legacy_read_register_gen): Rename read_register_gen.
	(regcache_read, regcache_write): New function.
	(read_register_gen, write_register_gen): New function.
	(write_register): Simplify.  Use write_register_gen.
	(read_register): Ditto using read_register_gen.
	(read_signed_register): Ditto.
	(read_register_bytes): Ditto!!!!
	(supply_register): Add note that CLEANUP_REGISTER_VALUE is being
	replaced by gdbarch_register_read.

Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.55
diff -p -r1.55 gdbarch.sh
*** gdbarch.sh	2001/03/17 00:31:22	1.55
--- gdbarch.sh	2001/03/17 01:46:26
*************** f::TARGET_WRITE_FP:void:write_fp:CORE_AD
*** 383,388 ****
--- 383,391 ----
  f::TARGET_READ_SP:CORE_ADDR:read_sp:void:::0:generic_target_read_sp::0
  f::TARGET_WRITE_SP:void:write_sp:CORE_ADDR val:val::0:generic_target_write_sp::0
  #
+ M:::void:register_read:int regnum, char *buf:regnum, buf:
+ M:::void:register_write:int regnum, char *buf:regnum, buf:
+ #
  v:2:NUM_REGS:int:num_regs::::0:-1
  # This macro gives the number of pseudo-registers that live in the
  # register namespace but do not get fetched or stored on the target.
Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.20
diff -p -r1.20 regcache.c
*** regcache.c	2001/03/13 23:31:14	1.20
--- regcache.c	2001/03/17 01:46:26
***************
*** 26,31 ****
--- 26,32 ----
  #include "gdbarch.h"
  #include "gdbcmd.h"
  #include "regcache.h"
+ #include "gdb_assert.h"
  
  /*
   * DATA STRUCTURE
*************** registers_fetched (void)
*** 213,266 ****
     into memory at MYADDR.  */
  
  void
! read_register_bytes (int inregbyte, char *myaddr, int inlen)
  {
!   int inregend = inregbyte + inlen;
    int regnum;
  
-   if (registers_pid != inferior_pid)
-     {
-       registers_changed ();
-       registers_pid = inferior_pid;
-     }
- 
    /* See if we are trying to read bytes from out-of-date registers.  If so,
       update just those registers.  */
  
    for (regnum = 0; regnum < NUM_REGS + NUM_PSEUDO_REGS; regnum++)
      {
!       int regstart, regend;
! 
!       if (register_cached (regnum))
! 	continue;
  
        if (REGISTER_NAME (regnum) == NULL || *REGISTER_NAME (regnum) == '\0')
  	continue;
  
!       regstart = REGISTER_BYTE (regnum);
!       regend = regstart + REGISTER_RAW_SIZE (regnum);
  
!       if (regend <= inregbyte || inregend <= regstart)
  	/* The range the user wants to read doesn't overlap with regnum.  */
  	continue;
  
!       /* We've found an uncached register where at least one byte will be read.
!          Update it from the target.  */
!       fetch_register (regnum);
  
!       if (!register_cached (regnum))
! 	{
! 	  /* Sometimes pseudoregs are never marked valid, so that they 
! 	     will be fetched every time (it can be complicated to know
! 	     if a pseudoreg is valid, while "fetching" them can be cheap). 
! 	     */
! 	  if (regnum < NUM_REGS)
!  	    error ("read_register_bytes:  Couldn't update register %d.", regnum);
! 	}
!     }
  
!   if (myaddr != NULL)
!     memcpy (myaddr, register_buffer (-1) + inregbyte, inlen);
  }
  
  /* Read register REGNUM into memory at MYADDR, which must be large
--- 214,267 ----
     into memory at MYADDR.  */
  
  void
! read_register_bytes (int in_start, char *in_buf, int in_len)
  {
!   int in_end = in_start + in_len;
    int regnum;
+   char *reg_buf = alloca (MAX_REGISTER_RAW_SIZE);
  
    /* See if we are trying to read bytes from out-of-date registers.  If so,
       update just those registers.  */
  
    for (regnum = 0; regnum < NUM_REGS + NUM_PSEUDO_REGS; regnum++)
      {
!       int reg_start;
!       int reg_end;
!       int reg_len;
  
        if (REGISTER_NAME (regnum) == NULL || *REGISTER_NAME (regnum) == '\0')
  	continue;
  
!       reg_start = REGISTER_BYTE (regnum);
!       reg_len = REGISTER_RAW_SIZE (regnum);
!       reg_end = reg_start + reg_len;
  
!       if (reg_end <= in_start || in_end <= reg_start)
  	/* The range the user wants to read doesn't overlap with regnum.  */
  	continue;
  
!       /* Force the cache to fetch the entire register. */
!       read_register_gen (regnum, reg_buf);
  
!       /* Legacy note: This function, for some reason, allows a NULL
!          input buffer.  If the buffer is NULL, the registers are still
!          fetched, just the final transfer is skipped. */
!       if (in_buf == NULL)
! 	continue;
  
!       if (reg_start >= in_start && reg_end <= in_end)
! 	/* register fits inside of in_buf. */
! 	memcpy (in_buf + (reg_start - in_start), reg_buf, reg_len);
!       else if (reg_start >= in_start && reg_end > in_end)
! 	/* register crosses the end of in_buf. */
! 	memcpy (in_buf + (reg_start - in_start), reg_buf, in_end - reg_start);
!       else if (reg_start < in_start && reg_end <= in_end)
! 	/* register crosses the start of in_buf. */
! 	memcpy (in_buf, reg_buf + (in_start - reg_start),
! 		reg_end - in_start);
!       else
! 	internal_error (__FILE__, __LINE__, "read_register_bytes botch");
!     }
  }
  
  /* Read register REGNUM into memory at MYADDR, which must be large
*************** read_register_bytes (int inregbyte, char
*** 268,276 ****
     register is known to be the size of a CORE_ADDR or smaller,
     read_register can be used instead.  */
  
! void
! read_register_gen (int regnum, char *myaddr)
  {
    if (registers_pid != inferior_pid)
      {
        registers_changed ();
--- 269,278 ----
     register is known to be the size of a CORE_ADDR or smaller,
     read_register can be used instead.  */
  
! static void
! legacy_read_register_gen (int regnum, char *myaddr)
  {
+   gdb_assert (regnum >= 0 && regnum < (NUM_REGS + NUM_PSEUDO_REGS));
    if (registers_pid != inferior_pid)
      {
        registers_changed ();
*************** read_register_gen (int regnum, char *mya
*** 284,289 ****
--- 286,311 ----
  	  REGISTER_RAW_SIZE (regnum));
  }
  
+ void
+ regcache_read (int rawnum, char *buf)
+ {
+   gdb_assert (rawnum >= 0 && rawnum < NUM_REGS);
+   /* For moment, just use underlying legacy code. Ulgh!!! */
+   legacy_read_register_gen (rawnum, buf);
+ }
+ 
+ void
+ read_register_gen (int regnum, char *buf)
+ {
+   if (! gdbarch_register_read_p (current_gdbarch))
+     {
+       legacy_read_register_gen (regnum, buf);
+       return;
+     }
+   gdbarch_register_read (current_gdbarch, regnum, buf);
+ }
+ 
+ 
  /* Write register REGNUM at MYADDR to the target.  MYADDR points at
     REGISTER_RAW_BYTES(REGNUM), which must be in target byte-order.  */
  
*************** read_register_gen (int regnum, char *mya
*** 292,301 ****
  #define CANNOT_STORE_REGISTER(regnum) 0
  #endif
  
! void
! write_register_gen (int regnum, char *myaddr)
  {
    int size;
  
    /* On the sparc, writing %g0 is a no-op, so we don't even want to
       change the registers array if something writes to this register.  */
--- 314,324 ----
  #define CANNOT_STORE_REGISTER(regnum) 0
  #endif
  
! static void
! legacy_write_register_gen (int regnum, char *myaddr)
  {
    int size;
+   gdb_assert (regnum >= 0 && regnum < (NUM_REGS + NUM_PSEUDO_REGS));
  
    /* On the sparc, writing %g0 is a no-op, so we don't even want to
       change the registers array if something writes to this register.  */
*************** write_register_gen (int regnum, char *my
*** 326,331 ****
--- 349,373 ----
    store_register (regnum);
  }
  
+ void
+ regcache_write (int rawnum, char *buf)
+ {
+   gdb_assert (rawnum >= 0 && rawnum < NUM_REGS);
+   /* For moment, just use underlying legacy code. Ulgh!!! */
+   legacy_write_register_gen (rawnum, buf);
+ }
+ 
+ void
+ write_register_gen (int regnum, char *buf)
+ {
+   if (! gdbarch_register_write_p (current_gdbarch))
+     {
+       legacy_write_register_gen (regnum, buf);
+       return;
+     }
+   gdbarch_register_write (current_gdbarch, regnum, buf);
+ }
+ 
  /* Copy INLEN bytes of consecutive data from memory at MYADDR
     into registers starting with the MYREGSTART'th byte of register data.  */
  
*************** write_register_bytes (int myregstart, ch
*** 385,401 ****
  ULONGEST
  read_register (int regnum)
  {
!   if (registers_pid != inferior_pid)
!     {
!       registers_changed ();
!       registers_pid = inferior_pid;
!     }
! 
!   if (!register_cached (regnum))
!     fetch_register (regnum);
! 
!   return (extract_unsigned_integer (register_buffer (regnum),
! 				    REGISTER_RAW_SIZE (regnum)));
  }
  
  ULONGEST
--- 427,435 ----
  ULONGEST
  read_register (int regnum)
  {
!   char *buf = alloca (REGISTER_RAW_SIZE (regnum));
!   read_register_gen (regnum, buf);
!   return (extract_unsigned_integer (buf, REGISTER_RAW_SIZE (regnum)));
  }
  
  ULONGEST
*************** read_register_pid (int regnum, int pid)
*** 423,439 ****
  LONGEST
  read_signed_register (int regnum)
  {
!   if (registers_pid != inferior_pid)
!     {
!       registers_changed ();
!       registers_pid = inferior_pid;
!     }
! 
!   if (!register_cached (regnum))
!     fetch_register (regnum);
! 
!   return (extract_signed_integer (register_buffer (regnum),
! 				  REGISTER_RAW_SIZE (regnum)));
  }
  
  LONGEST
--- 457,465 ----
  LONGEST
  read_signed_register (int regnum)
  {
!   void *buf = alloca (REGISTER_RAW_SIZE (regnum));
!   read_register_gen (regnum, buf);
!   return (extract_signed_integer (buf, REGISTER_RAW_SIZE (regnum)));
  }
  
  LONGEST
*************** read_signed_register_pid (int regnum, in
*** 461,498 ****
  void
  write_register (int regnum, LONGEST val)
  {
!   PTR buf;
    int size;
- 
-   /* On the sparc, writing %g0 is a no-op, so we don't even want to
-      change the registers array if something writes to this register.  */
-   if (CANNOT_STORE_REGISTER (regnum))
-     return;
- 
-   if (registers_pid != inferior_pid)
-     {
-       registers_changed ();
-       registers_pid = inferior_pid;
-     }
- 
    size = REGISTER_RAW_SIZE (regnum);
    buf = alloca (size);
    store_signed_integer (buf, size, (LONGEST) val);
! 
!   /* If we have a valid copy of the register, and new value == old value,
!      then don't bother doing the actual store. */
! 
!   if (register_cached (regnum)
!       && memcmp (register_buffer (regnum), buf, size) == 0)
!     return;
! 
!   if (real_register (regnum))
!     target_prepare_to_store ();
! 
!   memcpy (register_buffer (regnum), buf, size);
! 
!   set_register_cached (regnum, 1);
!   store_register (regnum);
  }
  
  void
--- 487,498 ----
  void
  write_register (int regnum, LONGEST val)
  {
!   void *buf;
    int size;
    size = REGISTER_RAW_SIZE (regnum);
    buf = alloca (size);
    store_signed_integer (buf, size, (LONGEST) val);
!   write_register_gen (regnum, buf);
  }
  
  void
*************** supply_register (int regnum, char *val)
*** 546,551 ****
--- 546,557 ----
  
    /* On some architectures, e.g. HPPA, there are a few stray bits in
       some registers, that the rest of the code would like to ignore.  */
+ 
+   /* NOTE: cagney/2001-03-16: The macro CLEAN_UP_REGISTER_VALUE is
+      going to be deprecated.  Instead architectures will leave the raw
+      register value as is and instead clean things up as they pass
+      through the method gdbarch_register_read() clean up the
+      values. */
  
  #ifdef CLEAN_UP_REGISTER_VALUE
    CLEAN_UP_REGISTER_VALUE (regnum, register_buffer (regnum));
Index: regcache.h
===================================================================
RCS file: /cvs/src/src/gdb/regcache.h,v
retrieving revision 1.3
diff -p -r1.3 regcache.h
*** regcache.h	2001/03/06 08:21:11	1.3
--- regcache.h	2001/03/17 01:46:26
***************
*** 22,34 ****
  #ifndef REGCACHE_H
  #define REGCACHE_H
  
! /* Character array containing an image of the inferior programs'
!    registers. */
  
  extern char *registers;
  
! /* Character array containing the current state of each register
!    (unavailable<0, invalid=0, valid>0). */
  
  extern signed char *register_valid;
  
--- 22,41 ----
  #ifndef REGCACHE_H
  #define REGCACHE_H
  
! /* Transfer a raw register [0..NUM_REGS) between core-gdb and the
!    regcache. */
  
+ void regcache_read (int rawnum, char *buf);
+ void regcache_write (int rawnum, char *buf);
+ 
+ /* DEPRECATED: Character array containing an image of the inferior
+    programs' registers for the most recently referenced thread. */
+ 
  extern char *registers;
  
! /* DEPRECATED: Character array containing the current state of each
!    register (unavailable<0, invalid=0, valid>0) for the most recently
!    referenced thread. */
  
  extern signed char *register_valid;
  
*************** extern int register_cached (int regnum);
*** 37,42 ****
--- 44,52 ----
  extern void set_register_cached (int regnum, int state);
  
  extern void register_changed (int regnum);
+ 
+ /* DEPRECATED: Functional interface returning pointer into registers[]
+    array. */
  
  extern char *register_buffer (int regnum);