[patch/wip] Save/restore cooked registers

[patch/wip] Save/restore cooked registers

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

Hello,

The attached is work-in-progress to get gdb saving just a subset of 
cooked registers when doing things like an inferior function call.

It appears to work, however, I'll put it on hold until after the 5.3 
branch gets cut.

enjoy,
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 24659 bytes --]

2002-08-25  Andrew Cagney  <ac131313@redhat.com>

	* arch-utils.h (default_next_cooked_register_to_save): Declare.
	(default_next_cooked_register_to_restore): Declare.
	* arch-utils.c (default_next_cooked_register_to_save): New
	function.
	(default_next_cooked_register_to_restore): New function.

	* regcache.c (struct regcache): Replace passthrough_p with
	readonly_p.  Replace raw_registers with registers.  Replace
	raw_register_valid_p with register_valid_p.
	(struct regcache_descr): Replace sizeof_raw_registers with
	sizeof_registers.  Replace sizeof_raw_register_valid_p with
	sizeof_register_valid_p.
	(init_legacy_regcache_descr): Update.
	(init_regcache_descr): Update.
	(regcache_xmalloc): Update.
	(regcache_xfree): Update.
	(regcache_cpy): Update.
	(regcache_cpy_no_passthrough): Update.
	(regcache_valid_p): Update.
	(deprecated_grub_regcache_for_registers): Update.
	(deprecated_grub_regcache_for_register_valid): Update.
	(register_buffer): Update.
	(regcache_raw_read): Update.
	(regcache_raw_write): Update.  Assert that the register cache is
	not read-only.
	(build_regcache): Update.  Clear the readonly_p flag.
	(regcache_cooked_read): Update.  If a read-only cache, check for a
	cached cooked register value.
	(regcache_cooked_write): Assert that the cache isn't read-only.
	(next_cooked_register): New function.
	(regcache_restore): New function.
	(regcache_save): New function.
	(regcache_cpy): Rewrite.

	* gdbarch.sh (next_cooked_register_to_save): New method.
	(next_cooked_register_to_restore): New method.
	* gdbarch.h, gdbarch.c: Regenerate.

Index: arch-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.c,v
retrieving revision 1.67
diff -u -r1.67 arch-utils.c
--- arch-utils.c	24 Aug 2002 00:21:34 -0000	1.67
+++ arch-utils.c	25 Aug 2002 19:06:10 -0000
@@ -493,6 +493,29 @@
   REGISTER_CONVERT_TO_RAW (type, regnum, from, to);
 }
 
+static int
+next_cooked_register (struct gdbarch *gdbarch, int regnum)
+{
+  if (regnum < 0)
+    return 0;
+  regnum++;
+  if (regnum
+      >= (gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch)))
+    return -1;
+  return regnum;
+}
+
+int
+default_next_cooked_register_to_save (struct gdbarch *gdbarch, int regnum)
+{
+  return next_cooked_register (gdbarch, regnum);
+}
+
+int
+default_next_cooked_register_to_restore (struct gdbarch *gdbarch, int regnum)
+{
+  return next_cooked_register (gdbarch, regnum);
+}
 \f
 /* Functions to manipulate the endianness of the target.  */
 
Index: arch-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.h,v
retrieving revision 1.41
diff -u -r1.41 arch-utils.h
--- arch-utils.h	24 Aug 2002 00:21:34 -0000	1.41
+++ arch-utils.h	25 Aug 2002 19:06:11 -0000
@@ -171,6 +171,10 @@
 extern void legacy_register_to_value (int regnum, struct type *type, char *from, char *to);
 extern void legacy_value_to_register (struct type *type, int regnum, char *from, char *to);
 
+/* Iterators that provide the next/prev register to save/restore.  */
+extern int default_next_cooked_register_to_save (struct gdbarch *gdbarch, int regnum);
+extern int default_next_cooked_register_to_restore (struct gdbarch *gdbarch, int regnum);
+
 /* For compatibility with older architectures, returns
    (LEGACY_SIM_REGNO_IGNORE) when the register doesn't have a valid
    name.  */
Index: gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.146
diff -u -r1.146 gdbarch.c
--- gdbarch.c	24 Aug 2002 00:21:34 -0000	1.146
+++ gdbarch.c	25 Aug 2002 19:06:25 -0000
@@ -113,7 +113,7 @@
      Declare set/get functions and define the corresponding
      macro in gdbarch.h.
 
-     gdbarch_alloc(): If zero/NULL is not a suitable default,
+     gdba   gdbarch_alloc(): If zero/NULL is not a suitable default,
      initialize the new field.
 
      verify_gdbarch(): Confirm that the target updated the field
@@ -265,6 +265,8 @@
   gdbarch_dwarf2_build_frame_info_ftype *dwarf2_build_frame_info;
   gdbarch_elf_make_msymbol_special_ftype *elf_make_msymbol_special;
   gdbarch_coff_make_msymbol_special_ftype *coff_make_msymbol_special;
+  gdbarch_next_cooked_register_to_save_ftype *next_cooked_register_to_save;
+  gdbarch_next_cooked_register_to_restore_ftype *next_cooked_register_to_restore;
 };
 
 
@@ -419,6 +421,8 @@
   0,
   0,
   0,
+  default_next_cooked_register_to_save,
+  default_next_cooked_register_to_restore,
   /* startup_gdbarch() */
 };
 
@@ -549,6 +553,8 @@
   current_gdbarch->construct_inferior_arguments = construct_inferior_arguments;
   current_gdbarch->elf_make_msymbol_special = default_elf_make_msymbol_special;
   current_gdbarch->coff_make_msymbol_special = default_coff_make_msymbol_special;
+  current_gdbarch->next_cooked_register_to_save = default_next_cooked_register_to_save;
+  current_gdbarch->next_cooked_register_to_restore = default_next_cooked_register_to_restore;
   /* gdbarch_alloc() */
 
   return current_gdbarch;
@@ -791,6 +797,12 @@
   /* Skip verify of dwarf2_build_frame_info, has predicate */
   /* Skip verify of elf_make_msymbol_special, invalid_p == 0 */
   /* Skip verify of coff_make_msymbol_special, invalid_p == 0 */
+  if ((GDB_MULTI_ARCH > GDB_MULTI_ARCH_PARTIAL)
+      && (gdbarch->next_cooked_register_to_save == default_next_cooked_register_to_save))
+    fprintf_unfiltered (log, "\n\tnext_cooked_register_to_save");
+  if ((GDB_MULTI_ARCH > GDB_MULTI_ARCH_PARTIAL)
+      && (gdbarch->next_cooked_register_to_restore == default_next_cooked_register_to_restore))
+    fprintf_unfiltered (log, "\n\tnext_cooked_register_to_restore");
   buf = ui_file_xstrdup (log, &dummy);
   make_cleanup (xfree, buf);
   if (strlen (buf) > 0)
@@ -821,6 +833,14 @@
                         (long) current_gdbarch->in_function_epilogue_p);
   if (GDB_MULTI_ARCH)
     fprintf_unfiltered (file,
+                        "gdbarch_dump: next_cooked_register_to_restore = 0x%08lx\n",
+                        (long) current_gdbarch->next_cooked_register_to_restore);
+  if (GDB_MULTI_ARCH)
+    fprintf_unfiltered (file,
+                        "gdbarch_dump: next_cooked_register_to_save = 0x%08lx\n",
+                        (long) current_gdbarch->next_cooked_register_to_save);
+  if (GDB_MULTI_ARCH)
+    fprintf_unfiltered (file,
                         "gdbarch_dump: pseudo_register_read = 0x%08lx\n",
                         (long) current_gdbarch->pseudo_register_read);
   if (GDB_MULTI_ARCH)
@@ -4880,6 +4900,44 @@
                                        gdbarch_coff_make_msymbol_special_ftype coff_make_msymbol_special)
 {
   gdbarch->coff_make_msymbol_special = coff_make_msymbol_special;
+}
+
+int
+gdbarch_next_cooked_register_to_save (struct gdbarch *gdbarch, int last_regnum)
+{
+  gdb_assert (gdbarch != NULL);
+  if (gdbarch->next_cooked_register_to_save == 0)
+    internal_error (__FILE__, __LINE__,
+                    "gdbarch: gdbarch_next_cooked_register_to_save invalid");
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_next_cooked_register_to_save called\n");
+  return gdbarch->next_cooked_register_to_save (gdbarch, last_regnum);
+}
+
+void
+set_gdbarch_next_cooked_register_to_save (struct gdbarch *gdbarch,
+                                          gdbarch_next_cooked_register_to_save_ftype next_cooked_register_to_save)
+{
+  gdbarch->next_cooked_register_to_save = next_cooked_register_to_save;
+}
+
+int
+gdbarch_next_cooked_register_to_restore (struct gdbarch *gdbarch, int last_regnum)
+{
+  gdb_assert (gdbarch != NULL);
+  if (gdbarch->next_cooked_register_to_restore == 0)
+    internal_error (__FILE__, __LINE__,
+                    "gdbarch: gdbarch_next_cooked_register_to_restore invalid");
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_next_cooked_register_to_restore called\n");
+  return gdbarch->next_cooked_register_to_restore (gdbarch, last_regnum);
+}
+
+void
+set_gdbarch_next_cooked_register_to_restore (struct gdbarch *gdbarch,
+                                             gdbarch_next_cooked_register_to_restore_ftype next_cooked_register_to_restore)
+{
+  gdbarch->next_cooked_register_to_restore = next_cooked_register_to_restore;
 }
 
 
Index: gdbarch.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.h,v
retrieving revision 1.114
diff -u -r1.114 gdbarch.h
--- gdbarch.h	24 Aug 2002 00:21:34 -0000	1.114
+++ gdbarch.h	25 Aug 2002 19:07:11 -0000
@@ -2483,6 +2483,16 @@
 #endif
 #endif
 
+/* Iterators for the registers to save or restore. */
+
+typedef int (gdbarch_next_cooked_register_to_save_ftype) (struct gdbarch *gdbarch, int last_regnum);
+extern int gdbarch_next_cooked_register_to_save (struct gdbarch *gdbarch, int last_regnum);
+extern void set_gdbarch_next_cooked_register_to_save (struct gdbarch *gdbarch, gdbarch_next_cooked_register_to_save_ftype *next_cooked_register_to_save);
+
+typedef int (gdbarch_next_cooked_register_to_restore_ftype) (struct gdbarch *gdbarch, int last_regnum);
+extern int gdbarch_next_cooked_register_to_restore (struct gdbarch *gdbarch, int last_regnum);
+extern void set_gdbarch_next_cooked_register_to_restore (struct gdbarch *gdbarch, gdbarch_next_cooked_register_to_restore_ftype *next_cooked_register_to_restore);
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.158
diff -u -r1.158 gdbarch.sh
--- gdbarch.sh	24 Aug 2002 00:21:34 -0000	1.158
+++ gdbarch.sh	25 Aug 2002 19:07:12 -0000
@@ -658,6 +658,9 @@
 F:2:DWARF2_BUILD_FRAME_INFO:void:dwarf2_build_frame_info:struct objfile *objfile:objfile:::0
 f:2:ELF_MAKE_MSYMBOL_SPECIAL:void:elf_make_msymbol_special:asymbol *sym, struct minimal_symbol *msym:sym, msym:::default_elf_make_msymbol_special::0
 f:2:COFF_MAKE_MSYMBOL_SPECIAL:void:coff_make_msymbol_special:int val, struct minimal_symbol *msym:val, msym:::default_coff_make_msymbol_special::0
+# Iterators for the cooked registers to save or restore.
+m:::int:next_cooked_register_to_save:int last_regnum:last_regnum:::default_next_cooked_register_to_save
+m:::int:next_cooked_register_to_restore:int last_regnum:last_regnum:::default_next_cooked_register_to_restore
 EOF
 }
 
Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.56
diff -u -r1.56 regcache.c
--- regcache.c	25 Aug 2002 15:36:11 -0000	1.56
+++ regcache.c	25 Aug 2002 19:07:21 -0000
@@ -50,13 +50,11 @@
      for raw and pseudo registers and allow access to both.  */
   int legacy_p;
 
-  /* The raw register cache.  This should contain just [0
+  /* The raw register space.  This should contain just [0
      .. NUM_RAW_REGISTERS).  However, for older targets, it contains
      space for the full [0 .. NUM_RAW_REGISTERS +
      NUM_PSEUDO_REGISTERS).  */
   int nr_raw_registers;
-  long sizeof_raw_registers;
-  long sizeof_raw_register_valid_p;
 
   /* The cooked register space.  Each cooked register in the range
      [0..NR_RAW_REGISTERS) is direct-mapped onto the corresponding raw
@@ -66,6 +64,11 @@
      gdbarch_register_read and gdbarch_register_write.  */
   int nr_cooked_registers;
 
+  /* The cache proper.  A read-only cache can contain cooked values.
+     A read/write cache can not.  */
+  long sizeof_registers;
+  long sizeof_register_valid_p;
+
   /* Offset and size (in 8 bit bytes), of reach register in the
      register cache.  All registers (including those in the range
      [NR_RAW_REGISTERS .. NR_COOKED_REGISTERS) are given an offset.
@@ -94,7 +97,6 @@
   /* FIXME: cagney/2002-05-11: Shouldn't be including pseudo-registers
      in the register buffer.  Unfortunatly some architectures do.  */
   descr->nr_raw_registers = descr->nr_cooked_registers;
-  descr->sizeof_raw_register_valid_p = descr->nr_cooked_registers;
 
   /* FIXME: cagney/2002-05-11: Instead of using REGISTER_BYTE() this
      code should compute the offets et.al. at runtime.  This currently
@@ -115,7 +117,7 @@
     }
 
   /* Come up with the real size of the registers buffer.  */
-  descr->sizeof_raw_registers = REGISTER_BYTES; /* OK use.  */
+  descr->sizeof_registers = REGISTER_BYTES; /* OK use.  */
   for (i = 0; i < descr->nr_cooked_registers; i++)
     {
       long regend;
@@ -130,8 +132,8 @@
          Ulgh!  New targets use gdbarch's register read/write and
          entirely avoid this uglyness.  */
       regend = descr->register_offset[i] + descr->sizeof_register[i];
-      if (descr->sizeof_raw_registers < regend)
-	descr->sizeof_raw_registers = regend;
+      if (descr->sizeof_registers < regend)
+	descr->sizeof_registers = regend;
     }
 }
 
@@ -151,6 +153,14 @@
      either mapped onto raw-registers or memory.  */
   descr->nr_cooked_registers = NUM_REGS + NUM_PSEUDO_REGS;
 
+  /* NOTE: cagney/2002-08-25: Include space for both raw and cooked
+     registers in the register_valid_p table.  A read-only regcache
+     can cache pre-computed cooked values (so needs the space).  A
+     read/write shouldn't need the extra space, unfortunatly existing
+     code is able to access elements of the global register_valid_p[]
+     array in the range [NUM_REGS .. NUM_REGS + NUM_PSEUDO_REGS).  */
+  descr->sizeof_register_valid_p = NUM_REGS + NUM_PSEUDO_REGS;
+
   /* Fill in a table of register types.  */
   descr->register_type = XCALLOC (descr->nr_cooked_registers,
 				  struct type *);
@@ -173,12 +183,6 @@
      into the register cache.  */
   descr->nr_raw_registers = NUM_REGS;
 
-  /* FIXME: cagney/2002-08-13: Overallocate the register_valid_p
-     array.  This pretects GDB from erant code that accesses elements
-     of the global register_valid_p[] array in the range [NUM_REGS
-     .. NUM_REGS + NUM_PSEUDO_REGS).  */
-  descr->sizeof_raw_register_valid_p = NUM_REGS + NUM_PSEUDO_REGS;
-
   /* Lay out the register cache.  The pseud-registers are included in
      the layout even though their value isn't stored in the register
      cache.  Some code, via read_register_bytes() access a register
@@ -208,7 +212,7 @@
        the register array directly using the global registers[].
        Until that code has been purged, play safe and over allocating
        the register buffer.  Ulgh!  */
-    descr->sizeof_raw_registers = offset;
+    descr->sizeof_registers = offset;
     /* = descr->register_offset[descr->nr_raw_registers]; */
   }
 
@@ -225,7 +229,7 @@
       gdb_assert (descr->sizeof_register[i] == REGISTER_VIRTUAL_SIZE (i));
       gdb_assert (descr->register_offset[i] == REGISTER_BYTE (i));
     }
-  /* gdb_assert (descr->sizeof_raw_registers == REGISTER_BYTES (i));  */
+  /* gdb_assert (descr->sizeof_cache == REGISTER_BYTES (i));  */
 #endif
 
   return descr;
@@ -276,11 +280,11 @@
 struct regcache
 {
   struct regcache_descr *descr;
-  char *raw_registers;
-  char *raw_register_valid_p;
-  /* If a value isn't in the cache should the corresponding target be
-     queried for a value.  */
-  int passthrough_p;
+  char *registers;
+  char *register_valid_p;
+  /* A read-only regcache, can cache cooked values.  It is
+     created/updated using the dup/cpy functions.  */
+  int readonly_p;
 };
 
 struct regcache *
@@ -292,11 +296,9 @@
   descr = regcache_descr (gdbarch);
   regcache = XMALLOC (struct regcache);
   regcache->descr = descr;
-  regcache->raw_registers
-    = XCALLOC (descr->sizeof_raw_registers, char);
-  regcache->raw_register_valid_p
-    = XCALLOC (descr->sizeof_raw_register_valid_p, char);
-  regcache->passthrough_p = 0;
+  regcache->registers = XCALLOC (descr->sizeof_registers, char);
+  regcache->register_valid_p = XCALLOC (descr->sizeof_register_valid_p, char);
+  regcache->readonly_p = 1;
   return regcache;
 }
 
@@ -305,8 +307,8 @@
 {
   if (regcache == NULL)
     return;
-  xfree (regcache->raw_registers);
-  xfree (regcache->raw_register_valid_p);
+  xfree (regcache->registers);
+  xfree (regcache->register_valid_p);
   xfree (regcache);
 }
 
@@ -322,6 +324,48 @@
   return make_cleanup (do_regcache_xfree, regcache);
 }
 
+static void
+regcache_save (struct regcache *dst, struct regcache *src)
+{
+  struct gdbarch *gdbarch = dst->descr->gdbarch;
+  int regnum;
+  void *buf = alloca (dst->descr->max_register_size);
+  gdb_assert (src->descr->gdbarch == dst->descr->gdbarch);
+  gdb_assert (dst->readonly_p);
+  /* Clear the dest.  */
+  memset (dst->registers, 0, dst->descr->sizeof_registers);
+  memset (dst->register_valid_p, 0, dst->descr->sizeof_register_valid_p);
+  /* Copy over any relevant registers.  */
+  for (regnum = gdbarch_next_cooked_register_to_save (gdbarch, -1);
+       regnum >= 0;
+       regnum = gdbarch_next_cooked_register_to_save (gdbarch, regnum))
+    {
+      regcache_cooked_read (src, regnum, buf);
+      memcpy (dst->registers + dst->descr->register_offset[regnum],
+	      buf, dst->descr->sizeof_register[regnum]);
+      dst->register_valid_p[regnum] = 1;
+    }
+}
+
+static void
+regcache_restore (struct regcache *dst, struct regcache *src)
+{
+  struct gdbarch *gdbarch = dst->descr->gdbarch;
+  int regnum;
+  void *buf = alloca (dst->descr->max_register_size);
+  gdb_assert (src->descr->gdbarch == dst->descr->gdbarch);
+  gdb_assert (!dst->readonly_p);
+  /* Copy over any relevant registers.  */
+  for (regnum = gdbarch_next_cooked_register_to_restore (gdbarch, -1);
+       regnum >= 0;
+       regnum = gdbarch_next_cooked_register_to_restore (gdbarch, regnum))
+    {
+      memcpy (buf, src->registers + src->descr->register_offset[regnum],
+	      src->descr->sizeof_register[regnum]);
+      regcache_cooked_write (dst, regnum, buf);
+    }
+}
+
 void
 regcache_cpy (struct regcache *dst, struct regcache *src)
 {
@@ -330,33 +374,13 @@
   gdb_assert (src != NULL && dst != NULL);
   gdb_assert (src->descr->gdbarch == dst->descr->gdbarch);
   gdb_assert (src != dst);
-  /* FIXME: cagney/2002-05-17: To say this bit is bad is being polite.
-     It keeps the existing code working where things rely on going
-     through to the register cache.  */
-  if (src == current_regcache && src->descr->legacy_p)
-    {
-      /* ULGH!!!!  Old way.  Use REGISTER bytes and let code below
-	 untangle fetch.  */
-      read_register_bytes (0, dst->raw_registers, REGISTER_BYTES);
-      return;
-    }
-  /* FIXME: cagney/2002-05-17: To say this bit is bad is being polite.
-     It keeps the existing code working where things rely on going
-     through to the register cache.  */
-  if (dst == current_regcache && dst->descr->legacy_p)
-    {
-      /* ULGH!!!!  Old way.  Use REGISTER bytes and let code below
-	 untangle fetch.  */
-      write_register_bytes (0, src->raw_registers, REGISTER_BYTES);
-      return;
-    }
-  buf = alloca (src->descr->max_register_size);
-  for (i = 0; i < src->descr->nr_raw_registers; i++)
-    {
-      /* Should we worry about the valid bit here?  */
-      regcache_raw_read (src, i, buf);
-      regcache_raw_write (dst, i, buf);
-    }
+  gdb_assert (src->readonly_p || dst->readonly_p);
+  if (!src->readonly_p)
+    regcache_save (dst, src);
+  else if (!dst->readonly_p)
+    regcache_restore (dst, src);
+  else
+    regcache_cpy_no_passthrough (dst, src);
 }
 
 void
@@ -369,10 +393,9 @@
      move of data into the current_regcache().  Doing this would be
      silly - it would mean that valid_p would be completly invalid.  */
   gdb_assert (dst != current_regcache);
-  memcpy (dst->raw_registers, src->raw_registers,
-	  dst->descr->sizeof_raw_registers);
-  memcpy (dst->raw_register_valid_p, src->raw_register_valid_p,
-	  dst->descr->sizeof_raw_register_valid_p);
+  memcpy (dst->registers, src->registers, dst->descr->sizeof_registers);
+  memcpy (dst->register_valid_p, src->register_valid_p,
+	  dst->descr->sizeof_register_valid_p);
 }
 
 struct regcache *
@@ -400,19 +423,19 @@
 {
   gdb_assert (regcache != NULL);
   gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
-  return regcache->raw_register_valid_p[regnum];
+  return regcache->register_valid_p[regnum];
 }
 
 char *
 deprecated_grub_regcache_for_registers (struct regcache *regcache)
 {
-  return regcache->raw_registers;
+  return regcache->registers;
 }
 
 char *
 deprecated_grub_regcache_for_register_valid (struct regcache *regcache)
 {
-  return regcache->raw_register_valid_p;
+  return regcache->register_valid_p;
 }
 
 /* Global structure containing the current regcache.  */
@@ -470,7 +493,7 @@
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < current_regcache->descr->nr_raw_registers);
-  current_regcache->raw_register_valid_p[regnum] = state;
+  current_regcache->register_valid_p[regnum] = state;
 }
 
 /* REGISTER_CHANGED
@@ -488,7 +511,7 @@
 static char *
 register_buffer (struct regcache *regcache, int regnum)
 {
-  return regcache->raw_registers + regcache->descr->register_offset[regnum];
+  return regcache->registers + regcache->descr->register_offset[regnum];
 }
 
 /* Return whether register REGNUM is a real register.  */
@@ -669,7 +692,7 @@
   gdb_assert (regcache != NULL && buf != NULL);
   gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
   if (regcache->descr->legacy_p
-      && regcache->passthrough_p)
+      && !regcache->readonly_p)
     {
       gdb_assert (regcache == current_regcache);
       /* For moment, just use underlying legacy code.  Ulgh!!! This
@@ -682,7 +705,7 @@
      to the current thread.  This switching shouldn't be necessary
      only there is still only one target side register cache.  Sigh!
      On the bright side, at least there is a regcache object.  */
-  if (regcache->passthrough_p)
+  if (!regcache->readonly_p)
     {
       gdb_assert (regcache == current_regcache);
       if (! ptid_equal (registers_ptid, inferior_ptid))
@@ -694,7 +717,7 @@
 	target_fetch_registers (regnum);
     }
   /* Copy the value directly into the register cache.  */
-  memcpy (buf, (regcache->raw_registers
+  memcpy (buf, (regcache->registers
 		+ regcache->descr->register_offset[regnum]),
 	  regcache->descr->sizeof_register[regnum]);
 }
@@ -744,6 +767,10 @@
   gdb_assert (regnum < regcache->descr->nr_cooked_registers);
   if (regnum < regcache->descr->nr_raw_registers)
     regcache_raw_read (regcache, regnum, buf);
+  else if (regcache->readonly_p && regcache->register_valid_p[regnum])
+    memcpy (buf, (regcache->registers
+		  + regcache->descr->register_offset[regnum]),
+	    regcache->descr->sizeof_register[regnum]);
   else
     gdbarch_pseudo_register_read (regcache->descr->gdbarch, regcache,
 				  regnum, buf);
@@ -820,9 +847,9 @@
 {
   gdb_assert (regcache != NULL && buf != NULL);
   gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
+  gdb_assert (!regcache->readonly_p);
 
-  if (regcache->passthrough_p
-      && regcache->descr->legacy_p)
+  if (regcache->descr->legacy_p)
     {
       /* For moment, just use underlying legacy code.  Ulgh!!! This
 	 silently and very indirectly updates the regcache's buffers
@@ -837,17 +864,6 @@
   if (CANNOT_STORE_REGISTER (regnum))
     return;
 
-  /* Handle the simple case first -> not write through so just store
-     value in cache.  */
-  if (!regcache->passthrough_p)
-    {
-      memcpy ((regcache->raw_registers
-	       + regcache->descr->register_offset[regnum]), buf,
-	      regcache->descr->sizeof_register[regnum]);
-      regcache->raw_register_valid_p[regnum] = 1;
-      return;
-    }
-
   /* Make certain that the correct cache is selected.  */
   gdb_assert (regcache == current_regcache);
   if (! ptid_equal (registers_ptid, inferior_ptid))
@@ -866,7 +882,7 @@
   target_prepare_to_store ();
   memcpy (register_buffer (regcache, regnum), buf,
 	  regcache->descr->sizeof_register[regnum]);
-  regcache->raw_register_valid_p[regnum] = 1;
+  regcache->register_valid_p[regnum] = 1;
   target_store_registers (regnum);
 }
 
@@ -888,6 +904,7 @@
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < regcache->descr->nr_cooked_registers);
+  gdb_assert (!regcache->readonly_p);
   if (regnum < regcache->descr->nr_raw_registers)
     regcache_raw_write (regcache, regnum, buf);
   else
@@ -1340,7 +1357,7 @@
 build_regcache (void)
 {
   current_regcache = regcache_xmalloc (current_gdbarch);
-  current_regcache->passthrough_p = 1;
+  current_regcache->readonly_p = 0;
   registers = deprecated_grub_regcache_for_registers (current_regcache);
   register_valid = deprecated_grub_regcache_for_register_valid (current_regcache);
 }

Re: [patch/wip] Save/restore cooked registers

[Kevin Buettner]

On Aug 25,  3:16pm, Andrew Cagney wrote:

> The attached is work-in-progress to get gdb saving just a subset of 
> cooked registers when doing things like an inferior function call.

Why is it necessary to save the cooked registers during an inferior
function call?  I would have thought that being able to save/restore
raw registers would be sufficient.

(Or did I miss a thread which explains this?)

Kevin

Re: [patch/wip] Save/restore cooked registers

[Andrew Cagney]

> On Aug 25,  3:16pm, Andrew Cagney wrote:
> 
> 
>> The attached is work-in-progress to get gdb saving just a subset of 
>> cooked registers when doing things like an inferior function call.
> 
> 
> Why is it necessary to save the cooked registers during an inferior
> function call?  I would have thought that being able to save/restore
> raw registers would be sufficient.
> 
> (Or did I miss a thread which explains this?)

See: http://sources.redhat.com/ml/gdb/2002-08/msg00196.html

Andrew

Re: [patch/wip] Save/restore cooked registers

[Elena Zannoni]

Kevin Buettner writes:
 > On Aug 25,  3:16pm, Andrew Cagney wrote:
 > 
 > > The attached is work-in-progress to get gdb saving just a subset of 
 > > cooked registers when doing things like an inferior function call.
 > 
 > Why is it necessary to save the cooked registers during an inferior
 > function call?  I would have thought that being able to save/restore
 > raw registers would be sufficient.
 > 
 > (Or did I miss a thread which explains this?)
 > 
 > Kevin


On ppc the e500 general registers are pseudo (i.e.cooked). Those
are used in inferior function calls, for parameter passing.
I think this is part of the story.
Full explanation:

http://sources.redhat.com/ml/gdb/2002-08/msg00196.html



Elena

Re: [patch/wip] Save/restore cooked registers

[Kevin Buettner]

On Aug 26, 12:29pm, Elena Zannoni wrote:

> Kevin Buettner writes:
>  > On Aug 25,  3:16pm, Andrew Cagney wrote:
>  > 
>  > > The attached is work-in-progress to get gdb saving just a subset of 
>  > > cooked registers when doing things like an inferior function call.
>  > 
>  > Why is it necessary to save the cooked registers during an inferior
>  > function call?  I would have thought that being able to save/restore
>  > raw registers would be sufficient.
>  > 
>  > (Or did I miss a thread which explains this?)
>  > 
>  > Kevin
> 
> 
> On ppc the e500 general registers are pseudo (i.e.cooked). Those
> are used in inferior function calls, for parameter passing.
> I think this is part of the story.

If I'm not mistaken, the pseudos on the e500 are synthesized from the
raw registers without the need for outside sources such as memory. 
That being the case, saving the raw registers (or, more precisely, the
cooked registers corresponding to the raw registers) should be
sufficient.

In fact, for the e500, I think we'll need to take care NOT to write
back the pseudos since this could potentially cause information to
be lost.  It would depend upon the order in which things were done.
If the pseudos are restored after the raw registers that they map
onto, the most significant bits would likely be wiped out.  (In this
case the pseudos are narrower than the raw registers, right?)

> Full explanation:
> 
> http://sources.redhat.com/ml/gdb/2002-08/msg00196.html

Yes, reading this explanation helped quite a lot.  When a pseduo register's
value is synthesized from sources outside of the raw register cache, e.g,
from memory, we need to save these values.  Later when writing them back,
the reverse transformation will occur potentially writing memory (or those
other outside sources).  This makes sense to me.

I've looked over Andrew's WIP patch.  I haven't checked all the details,
but the ideas seem sound and the interfaces look okay.  I'd like to see
the iterators better commented though.

Kevin

Re: [patch/wip] Save/restore cooked registers

[Andrew Cagney]

> I've looked over Andrew's WIP patch.  I haven't checked all the details,
> but the ideas seem sound and the interfaces look okay.  I'd like to see
> the iterators better commented though.

The doco will go in:
gdbint.texinfo:Registers:Saving and Restoring Registers.

Hence ``wip'' :-)

Andrew

Re: [patch/wip] Save/restore cooked registers

[Elena Zannoni]

Kevin Buettner writes:
 > On Aug 26, 12:29pm, Elena Zannoni wrote:
 > 
 > > Kevin Buettner writes:
 > >  > On Aug 25,  3:16pm, Andrew Cagney wrote:
 > >  > 
 > >  > > The attached is work-in-progress to get gdb saving just a subset of 
 > >  > > cooked registers when doing things like an inferior function call.
 > >  > 
 > >  > Why is it necessary to save the cooked registers during an inferior
 > >  > function call?  I would have thought that being able to save/restore
 > >  > raw registers would be sufficient.
 > >  > 
 > >  > (Or did I miss a thread which explains this?)
 > >  > 
 > >  > Kevin
 > > 
 > > 
 > > On ppc the e500 general registers are pseudo (i.e.cooked). Those
 > > are used in inferior function calls, for parameter passing.
 > > I think this is part of the story.
 > 
 > If I'm not mistaken, the pseudos on the e500 are synthesized from the
 > raw registers without the need for outside sources such as memory. 
 > That being the case, saving the raw registers (or, more precisely, the
 > cooked registers corresponding to the raw registers) should be
 > sufficient.
 > 

Yes. I was thinking about this other problem I encountered:
http://sources.redhat.com/ml/gdb-patches/2002-08/msg00689.html

 > In fact, for the e500, I think we'll need to take care NOT to write
 > back the pseudos since this could potentially cause information to
 > be lost.  It would depend upon the order in which things were done.
 > If the pseudos are restored after the raw registers that they map
 > onto, the most significant bits would likely be wiped out.  (In this
 > case the pseudos are narrower than the raw registers, right?)
 > 

The pseudo register write function is written so that it preserves the
upper bits. If you use that technique you should be safe.

Elena


 > > Full explanation:
 > > 
 > > http://sources.redhat.com/ml/gdb/2002-08/msg00196.html
 > 
 > Yes, reading this explanation helped quite a lot.  When a pseduo register's
 > value is synthesized from sources outside of the raw register cache, e.g,
 > from memory, we need to save these values.  Later when writing them back,
 > the reverse transformation will occur potentially writing memory (or those
 > other outside sources).  This makes sense to me.
 > 
 > I've looked over Andrew's WIP patch.  I haven't checked all the details,
 > but the ideas seem sound and the interfaces look okay.  I'd like to see
 > the iterators better commented though.
 > 
 > Kevin

Re: [patch/wip] Save/restore cooked registers

[Kevin Buettner]

On Aug 26,  2:23pm, Elena Zannoni wrote:

>  > If I'm not mistaken, the pseudos on the e500 are synthesized from the
>  > raw registers without the need for outside sources such as memory. 
>  > That being the case, saving the raw registers (or, more precisely, the
>  > cooked registers corresponding to the raw registers) should be
>  > sufficient.
> 
> Yes. I was thinking about this other problem I encountered:
> http://sources.redhat.com/ml/gdb-patches/2002-08/msg00689.html

I see.

>  > In fact, for the e500, I think we'll need to take care NOT to write
>  > back the pseudos since this could potentially cause information to
>  > be lost.  It would depend upon the order in which things were done.
>  > If the pseudos are restored after the raw registers that they map
>  > onto, the most significant bits would likely be wiped out.  (In this
>  > case the pseudos are narrower than the raw registers, right?)
>  > 
> 
> The pseudo register write function is written so that it preserves the
> upper bits. If you use that technique you should be safe.

Okay, good.  (I was concerned that we might need e500-specific iterators
when Andrew commits his patch.  But I guess we won't...)

Kevin

Re: [patch/wip] Save/restore cooked registers

[Kevin Buettner]

On Aug 26,  2:21pm, Andrew Cagney wrote:

> > I've looked over Andrew's WIP patch.  I haven't checked all the details,
> > but the ideas seem sound and the interfaces look okay.  I'd like to see
> > the iterators better commented though.
> 
> The doco will go in:
> gdbint.texinfo:Registers:Saving and Restoring Registers.

As I understand it, functions should be commented in the source
files too.  (Cf. http://www.gnu.org/prep/standards_24.html#SEC24)

When I'm working on gdb, I find it more convenient to go to the
location of the function declaration / definition and read the
associated comment.  I do look at the texinfo document too, but only
as a last resort.  (I trust it less, plus I find that the markup gets
in the way of easy comprehension.)

> Hence ``wip'' :-)

Okay.

Kevin

Re: [patch/wip] Save/restore cooked registers

[Richard Earnshaw]

>  > If I'm not mistaken, the pseudos on the e500 are synthesized from the
>  > raw registers without the need for outside sources such as memory. 
>  > That being the case, saving the raw registers (or, more precisely, the
>  > cooked registers corresponding to the raw registers) should be
>  > sufficient.
>  > 
> 
> Yes. I was thinking about this other problem I encountered:
> http://sources.redhat.com/ml/gdb-patches/2002-08/msg00689.html
> 
>  > In fact, for the e500, I think we'll need to take care NOT to write
>  > back the pseudos since this could potentially cause information to
>  > be lost.  It would depend upon the order in which things were done.
>  > If the pseudos are restored after the raw registers that they map
>  > onto, the most significant bits would likely be wiped out.  (In this
>  > case the pseudos are narrower than the raw registers, right?)
>  > 
> 
> The pseudo register write function is written so that it preserves the
> upper bits. If you use that technique you should be safe.

Ok, this is theoretical since I can't point to a machine where this is in 
fact the case; but I do think it would be a reasonable implementation.

Consider a machine where a floating point register can contain either a 
double or a single-precision value.  The 'type' in the register is stored 
in some special bit in the register that indicates the type.  Now, if we 
start saving and restoring the cooked registers in this context then the 
order in which they are called is critical to restoring the correct value.

Consider the case where the raw register, f0_raw, contains a 'float' value.

Now we extract both the float and the double representations into the two 
pseudos s0 and d0 (what we put into d0 doesn't really matter, it could be 
a 'double' visualization of f0_raw, or it could be some other value, eg 
NaN or 0).  However, when we restore the values then if we restore s0 
before restoring d0 then f0_raw will contain an invalid value and that 
could cause a fault on continuing.

I really don't see what saving and restoring the cooked-but-not-raw 
registers buys us.  These should never have any persistent storage (which 
might need saving) and should always be calculable on demand from the bits 
in the raw registers -- similarly a write to a pseudo should always update 
the corresponding raw registers, so again there is no need to have any 
persistent storage for them.

In summary, I still think that cooked registers should just be considered 
as views of the raw register set and that there should never be any need 
to save or restore the views, just the underlying data that is the 
registers themselves.

Going back to some of the points raised in Andrew's original mail:

> I'd like to propose the following changes to the register cache:

> - save/restore cooked, rather than raw, registers

No, I really think this is a backwards step.

> This would make it possible to save memory based registers across an
> inferior call or return. By default, only [0..NUM_REGS) cooked registers
> would be saved and since they are 1:1 with the raw registers, existing new
> code wouldn't notice the change.

> I suspect this is why the old code was saving the full register range.

I think it saved the entire range because it didn't know any better.

> - disallow writes to a saved copy of the register cache

I don't see that this matters, see below.

> If this isn't done, there is a problem with keeping the cooked registers coherent.

No, the cooked registers can never be incoherrent if you just consider 
them as view-ports onto a register-cache set.  That is, they have no 
persistent state in themselves.

I really think we need to break this implicit link between the raw regs 
and part of the cooked regs, it just causes no-end of confusion.  It's 
fine if we want to say that  some cooked regs are mapped 1:1 onto part of 
the raw regcache, but that should/must be a back-end convenience, and not 
part of gdb's fundamental design (that is asking for the r0 cooked view 
may just happen to fetch the raw r0 register that is at offset zero in the 
regcache, but nothing in GDB-core should assume this).

If you are trying to just add some code that optimizes the storing of 
registers back to the inferior, then surely the easiest way to do this is 
to have a further 'inferior' set of values that we never update, then when 
we need to update a value we check to see if the value we want to write 
matches that which we know the inferior to have and suppress the write if 
there is no change.

R.

Re: [patch/wip] Save/restore cooked registers

[Andrew Cagney]

>> I suspect this is why the old code was saving the full register range.
> 
> 
> I think it saved the entire range because it didn't know any better.

No.  read_register_bytes() was intentionally changed from NUM_REGS to 
NUM_REGS+NUM_PSEUDO_REGS.

I've encountered architectures that use memory mapped register locations 
as part of their ABI and hence, rely on those locations being saved.  I 
suspect that the m69hc11 falls into this category (but it isn't the 
target I've in mind).

Anyway, the branch now defaults to saving just the first [0 .. 
NUM_REGS).  A target is free to replace this with code that saves an 
arbitrary register set in the range [0 .. NUM_REGS+NUM_PSEUDO_REGS) range.

I should note that core gdb knows none of this.  It just asks for a 
register cache to be saved / restored.

>> - disallow writes to a saved copy of the register cache
> 
> 
> I don't see that this matters, see below.

What should the regcache do with a write to a saved register cache when 
the write is ment to go to memory?

Given that GDB never even tries to write to a saved cache, I think 
formalizing this, and disallowing writes to a saved cache, is reasonable.

>> If this isn't done, there is a problem with keeping the cooked registers coherent.
> 
> 
> No, the cooked registers can never be incoherrent if you just consider 
> them as view-ports onto a register-cache set.  That is, they have no 
> persistent state in themselves.

Remember, cooked registers map onto both raw registers and memory.  If 
cooked registers are not saved, gdb will need to start saving chunks of 
memory.

A saved copy of the register cache is simply that, a snapshot.  It just 
needs to ensure that the saved value of all cooked registers (involved 
in the ABI) are available.   How it does this, well you don't want to 
know :-)

> I really think we need to break this implicit link between the raw regs 
> and part of the cooked regs, it just causes no-end of confusion.  It's 
> fine if we want to say that  some cooked regs are mapped 1:1 onto part of 
> the raw regcache, but that should/must be a back-end convenience, and not 
> part of gdb's fundamental design (that is asking for the r0 cooked view 
> may just happen to fetch the raw r0 register that is at offset zero in the 
> regcache, but nothing in GDB-core should assume this).

BTW, it turns out that nothing in core GDB is assuming this (ignoring 
the CONVERTABLE mess).  Elena's e500 port maped everything onto a 
totally cooked register and nothing noticed.  Core gdb just deals with 
cooked registers (or their ulgh offset).

Cooked vs raw only appears:

- in the register cache (where it needs to know what to access when) 
(but the core is clueless to this)

- in the target back-end where it needs to map cooked registers onto raw 
registers or the register cache

Long term, core gdb should find itself only refering to frame_register*()

> If you are trying to just add some code that optimizes the storing of 
> registers back to the inferior, then surely the easiest way to do this is 
> to have a further 'inferior' set of values that we never update, then when 
> we need to update a value we check to see if the value we want to write 
> matches that which we know the inferior to have and suppress the write if 
> there is no change.

I'm not.  The objective is to hand a target that:

- has registers mapped onto memory locations

- has non ABI registers (should not be saved / restored)

enough rope to hang themself while still keeping the common case (just 
save raw registers) simple.

enjoy,
Andrew

Re: [patch/wip] Save/restore cooked registers

[Elena Zannoni]

Andrew Cagney writes:
 > 
 > > I really think we need to break this implicit link between the raw regs 
 > > and part of the cooked regs, it just causes no-end of confusion.  It's 
 > > fine if we want to say that  some cooked regs are mapped 1:1 onto part of 
 > > the raw regcache, but that should/must be a back-end convenience, and not 
 > > part of gdb's fundamental design (that is asking for the r0 cooked view 
 > > may just happen to fetch the raw r0 register that is at offset zero in the 
 > > regcache, but nothing in GDB-core should assume this).
 > 
 > BTW, it turns out that nothing in core GDB is assuming this (ignoring 
 > the CONVERTABLE mess).  Elena's e500 port maped everything onto a 
 > totally cooked register and nothing noticed.  Core gdb just deals with 
 > cooked registers (or their ulgh offset).
 > 

I think there may still be a couple of places where we are using some
wrong assumptions about the register numbers. At least in target ppc
code. Some harcoded 0 instead of tdep->ppc_gp0_regnum and similar
things. Hopefully those will be flushed out overtime.

Elena