[rfc] Frame based register cache / frame->unwind

[Andrew Cagney <ac131313 at cygnus dot com>]

Hello,

To pull together several apparently random threads.  The attached is a 
work in progress to add a frame based register cache to GDB.  It appears 
to work - NetBSD/PPC shows no regressions.

Performance is hard to quantify(1) but looks positive.  Using a native 
GDB (not typical for me :-) it appears that each frame is ~2% slower to 
create (``(gdb) up'').  Once created, the frame is ~20-25% faster 
(``(gdb) info registers'').

I'm not too worried about the apparent 2% overhead per frame create 
though.  With the patch applied, the code ends up maintaining both this 
new cache and the old ->saved_regs table.  Rewriting a target to just 
use the ->unwind_cache, should, I think, claw back the 2% and then some 
- less need to go out to the target.

There are two parts to the change and I'll describe each in turn.  The 
final patch will likely be committed as two parts.


FRAME->register_unwind(FRAME, REGNUM, ...)

This is a new per-frame method.  Given FRAME and REGNUM, it unwinds the 
register returning the value of that register as it would have been in 
the previous (calling) frame.

An implementation may be recursive.  The value of one of the previous 
frames registers may be in the next frame (available via the call 
get_saved_register(frame) == frame_register_unwind(frame->next).  The 
value of a previous frame's register isn't going to be be found in the 
previous frame :-)

The patch includes two register_unwind methods: one that handles a dummy 
frame; and one that handles ->saved_regs based frames.  The unwind 
function being selected according to the current value of the PC.


FRAME->unwind_cache;

This is used to cache the values returned by FRAME->register_unwind().

Even though the ->register_unwind() method returns the value of a 
register in the previous frame, that value is not cached in the previous 
frame.  Two reasons: the previous frame may not have been created; it is 
a cache to avoid calling ->register_unwind() and not a cache of register 
values.


The key to this all is a mind set change.  Instead of ``pulling'' 
register values from prev->next.  They are pushed/unwound from FRAME. 
When unwinding a register, you can't assume that FRAME->prev exists.


Where to from here?

Rationalize the way frames are created.  Cf other posts, something like:
   create_frame (frame_chain(fp), frame_saved_pc(fp), fp);
should ``just work'' rofl.

Change get_current_frame() to start out with an initial regcache frame 
(one that returns registers from regcache) so that even the initial 
frame can be created with:
   create_frame (...);
rof with lol.

Tie the dwarf2cfi stuff into this ...

thoughts,
Andrew



(1) ``(gdb) maint time 1'' but the numbers are really small.  Hacked it 
to give real/user/sys times but they were even smaller!
2002-04-14  Andrew Cagney  <ac131313@redhat.com>

	* frame.h (cache_generic_get_saved_register): Declare.
	(frame_supply_unwound_register): Declare.
	(frame_register_unwind_ftype): Define.
	(struct frame_info): Add fields unwind_cache and register_unwind.
	* frame.c (struct frame_unwind_cache): Define.
	(frame_unwind_cache_alloc): New function.
	(frame_supply_unwound_register): New function.
	(frame_register_unwind): New function.
	(cache_generic_get_saved_register): New function.
	* blockframe.c: Include "gdb_assert.h".
	(set_unwind_by_pc): New function.
	(create_new_frame, get_prev_frame): Call set_unwind_by_pc.
	(generic_call_dummy_register_unwind): New function.
	(generic_frame_register_unwind): New function.
	* rs6000-tdep.c (rs6000_gdbarch_init): Set get_saved_registers to
	cache_generic_get_saved_registers.
	
Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.24
diff -u -r1.24 blockframe.c
--- blockframe.c	14 Apr 2002 13:38:06 -0000	1.24
+++ blockframe.c	14 Apr 2002 19:29:47 -0000
@@ -34,9 +34,21 @@
 #include "inferior.h"		/* for read_pc */
 #include "annotate.h"
 #include "regcache.h"
+#include "gdb_assert.h"
 
 /* Prototypes for exported functions. */
 
+static void generic_call_dummy_register_unwind (struct frame_info *frame,
+						int regnum, int *optimized,
+						CORE_ADDR *addrp,
+						enum lval_type *lval,
+						void *raw_buffer);
+static void generic_frame_register_unwind (struct frame_info *frame,
+					   int regnum, int *optimized,
+					   CORE_ADDR *addrp,
+					   enum lval_type *lval, void *buffer);
+
+
 void _initialize_blockframe (void);
 
 /* A default FRAME_CHAIN_VALID, in the form that is suitable for most
@@ -208,6 +220,23 @@
   current_frame = frame;
 }
 
+
+/* Using the PC, select a mechanism for unwinding a frame returning
+   the previous frame.  The register unwind function should, on
+   demand, initialize the ->context object.  */
+
+static void
+set_unwind_by_pc (CORE_ADDR pc, CORE_ADDR fp,
+		  frame_register_unwind_ftype **unwind)
+{
+  if (!USE_GENERIC_DUMMY_FRAMES)
+    *unwind = NULL;
+  else if (PC_IN_CALL_DUMMY (pc, fp, fp))
+    *unwind = generic_call_dummy_register_unwind;
+  else
+    *unwind = generic_frame_register_unwind;
+}
+
 /* Create an arbitrary (i.e. address specified by user) or innermost frame.
    Always returns a non-NULL value.  */
 
@@ -229,6 +258,9 @@
   find_pc_partial_function (pc, &name, (CORE_ADDR *) NULL, (CORE_ADDR *) NULL);
   fi->signal_handler_caller = IN_SIGTRAMP (fi->pc, name);
 
+  /* Select/initialize an unwind function.  */
+  set_unwind_by_pc (fi->pc, fi->frame, &fi->register_unwind);
+
   if (INIT_EXTRA_FRAME_INFO_P ())
     INIT_EXTRA_FRAME_INFO (0, fi);
 
@@ -456,6 +488,12 @@
 	}
     }
 
+  /* Initialize the code used to unwind the frame PREV based on the PC
+     (and probably other architectural information).  The PC lets you
+     check things like the debug info at that point (dwarf2cfi?) and
+     use that to decide how the frame should be unwound.  */
+  set_unwind_by_pc (prev->pc, prev->frame, &prev->register_unwind);
+
   find_pc_partial_function (prev->pc, &name,
 			    (CORE_ADDR *) NULL, (CORE_ADDR *) NULL);
   if (IN_SIGTRAMP (prev->pc, name))
@@ -1241,6 +1279,96 @@
 			struct value **args, struct type *type, int gcc_p)
 {
   return;
+}
+
+/* Given a call-dummy dummy-frame, return the registers.  Here the
+   register value is taken from the local copy of the register buffer.  */
+
+static void
+generic_call_dummy_register_unwind (struct frame_info *frame, int regnum,
+				    int *optimized, CORE_ADDR *addrp,
+				    enum lval_type *lval, void *raw_buffer)
+{
+  char *registers;
+  gdb_assert (PC_IN_CALL_DUMMY (frame->pc, frame->frame, frame->frame));
+
+  /* For a dummy_frame, cache the register buffer address in
+     frame->context.  */
+  registers = (char *) (frame->context);
+  if (registers == NULL)
+    {
+      registers = generic_find_dummy_frame (frame->pc, frame->frame);
+      frame->context = (void *) registers;
+    }
+  gdb_assert (registers != NULL);
+
+  if (optimized != NULL)
+    *optimized = 0;
+  if (addrp)
+    *addrp = 0;
+  if (lval)
+    *lval = not_lval;
+  if (raw_buffer != NULL)
+    memcpy (raw_buffer, registers + REGISTER_BYTE (regnum),
+	    REGISTER_RAW_SIZE (regnum));
+}
+
+/* Return the register saved in the generic frame, if it isn't found
+   here, try the next inner frame.  */
+
+static void
+generic_frame_register_unwind (struct frame_info *frame, int regnum,
+			       int *optimized, CORE_ADDR *addrp,
+			       enum lval_type *lval, void *raw_buffer)
+{
+  /* There is always a frame at this point.  And THIS is the frame
+     we're interested in.  */
+  gdb_assert (frame != NULL);
+  gdb_assert (!PC_IN_CALL_DUMMY (frame->pc, frame->frame, frame->frame));
+
+  if (frame->saved_regs == NULL)
+    FRAME_INIT_SAVED_REGS (frame);
+  if (frame->saved_regs != NULL
+      && frame->saved_regs[regnum] != 0)
+    {
+      /* Found it saved on the stack.  */
+      if (optimized != NULL)
+	*optimized = 0;
+      if (addrp)
+	*addrp = 0;
+      if (lval)
+	    *lval = lval_memory;
+      if (regnum == SP_REGNUM)
+	{
+	  /* FIXME: cagney/2002-04-10: I suspect that this is done so
+             that the SP_REGNUM can be cached in the current frame.
+             Code should instead use a function like
+             frame_supply_register() and explicit add register values
+             into the corresponding cache.  It gets tricky as the
+             place to save this register value is in the previous
+             frame and that may not yet have been created.  Hmm,
+             perhaps the regcache should cache the unwind values for
+             the previous frame in this frame.  Anyway, not the
+             problem of this function and this bit of code.  */
+	  /* SP register treated specially.  */
+	  if (raw_buffer)
+	    store_address (raw_buffer, REGISTER_RAW_SIZE (regnum),
+			   frame->saved_regs[regnum]);
+	}
+      else
+	{
+	  /* Any other register.  */
+	  if (addrp)
+	    *addrp = frame->saved_regs[regnum];
+	  if (raw_buffer)
+	    read_memory (frame->saved_regs[regnum], raw_buffer,
+			 REGISTER_RAW_SIZE (regnum));
+	}
+      return;
+    }
+
+  /* No luck, try the next frame along.  */
+  get_saved_register (raw_buffer, optimized, addrp, frame, regnum, lval);
 }
 
 /* Function: get_saved_register
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.6
diff -u -r1.6 frame.c
--- frame.c	12 Apr 2002 18:18:57 -0000	1.6
+++ frame.c	14 Apr 2002 19:29:47 -0000
@@ -1,4 +1,4 @@
-/* Cache and manage the values of registers for GDB, the GNU debugger.
+/* Cache and manage frames for GDB, the GNU debugger.
 
    Copyright 1986, 1987, 1989, 1991, 1994, 1995, 1996, 1998, 2000,
    2001, 2002 Free Software Foundation, Inc.
@@ -157,6 +157,135 @@
     }
   if (addrp != NULL)
     *addrp = addr;
+}
+
+/* The frame cache.  Cache entries are populated by two functions
+   frame_supply_register() and get_saved_register().  */
+
+struct frame_unwind_cache
+{
+  int optimized;
+  CORE_ADDR addr;
+  enum lval_type lval;
+  /* First byte of cached register value.  NOTE: Uses the HACK of
+     overallocating the structure and then running the value off the
+     end of the struct.  */
+  char buffer[1];
+};
+
+/* Return a freshly allocated cache entry for REGNUM.  */
+
+static struct frame_unwind_cache *
+frame_unwind_cache_alloc (struct frame_info *frame, int regnum)
+{
+  if (frame->unwind_cache == NULL)
+    {
+      long size = (sizeof (struct frame_unwind_cache *)
+		   * (NUM_REGS + NUM_PSEUDO_REGS));
+      frame->unwind_cache = frame_obstack_alloc (size);
+      memset (frame->unwind_cache, 0, size);
+    }
+  frame->unwind_cache[regnum]
+    = frame_obstack_alloc (sizeof (struct frame_info)
+			   + REGISTER_RAW_SIZE (regnum));
+  return frame->unwind_cache[regnum];
+}
+
+/* Force a register value into the unwind cache.  If an entry is
+   already there, update it.  */
+
+void
+frame_supply_unwound_register (struct frame_info *frame, int regnum,
+			       int optimized, CORE_ADDR addr,
+			       enum lval_type lval, void *buffer)
+{
+  struct frame_unwind_cache *cache;
+  if (frame->unwind_cache == NULL || frame->unwind_cache[regnum] == NULL)
+    cache = frame_unwind_cache_alloc (frame, regnum);
+  else
+    cache = frame->unwind_cache[regnum];
+  cache->optimized = optimized;
+  cache->addr = addr;
+  cache->lval = lval;
+  if (buffer != NULL)
+    memcpy (cache->buffer, buffer, REGISTER_RAW_SIZE (regnum));
+  else
+    memset (cache, 0, REGISTER_RAW_SIZE (regnum));
+}
+
+static void
+frame_register_unwind (struct frame_info *frame, int regnum,
+		       int *optimized, CORE_ADDR *addrp,
+		       enum lval_type *lval, void *raw_buffer)
+{
+  struct frame_unwind_cache *cache;
+  if (frame->unwind_cache == NULL || frame->unwind_cache[regnum] == NULL)
+    {
+      /* The cache is empty (for this entry).  Allocate an entry (and
+         possibly the entire cache) and then fill it.  */
+      cache = frame_unwind_cache_alloc (frame, regnum);
+      /* Ask the frame to unwind the required register, enter the
+         value directly into the unwind cache.  */
+      frame->register_unwind (frame, regnum, &cache->optimized,
+			      &cache->addr, &cache->lval,
+			      cache->buffer);
+    }
+  else
+    cache = frame->unwind_cache[regnum];
+
+  /* Supply the value from the frame's register cache.  */
+  if (raw_buffer != NULL)
+    memcpy (raw_buffer, cache->buffer, REGISTER_RAW_SIZE (regnum));
+  if (optimized != NULL)
+    *optimized = cache->optimized;
+  if (addrp != NULL)
+    *addrp = cache->addr;
+  if (lval != NULL)
+    *lval = cache->lval;
+}
+
+
+void
+cache_generic_get_saved_register (char *raw_buffer,
+				  int *optimized,
+				  CORE_ADDR *addrp,
+				  struct frame_info *frame,
+				  int regnum,
+				  enum lval_type *lval)
+{
+  if (!target_has_registers)
+    error ("No registers.");
+
+  /* Reached the the bottom (youngest, inner most) of the frame chain
+     (youngest, inner most) frame, go direct to the hardware register
+     cache (do not pass go, do not try to cache the value, ...).  The
+     unwound value would have been cached in frame->next but that
+     doesn't exist.  This doesn't matter as the hardware register
+     cache is stopping any unnecessary accesses to the target.  */
+  /* NOTE: cagney/2002-04-14: It would be nice if, instead of a
+     special case, there was instead always an inner frame dedicated
+     to the hardware registers.  Unfortunatly, there is too much
+     unwind code around that looks up/down the frame chain while
+     making the assumption that each frame level is using the same
+     unwind code.  */
+  if (frame == NULL || frame->next == NULL)
+    {
+      if (raw_buffer != NULL)
+	read_register_gen (regnum, raw_buffer);
+      if (optimized != NULL)
+	*optimized = 0;
+      if (addrp != NULL)
+	*addrp = REGISTER_BYTE (regnum);
+      if (lval != NULL)
+	*lval = lval_register;
+      return;
+    }
+
+  /* Ask the next frame to unwind the register value.  The unwind
+     might even find the value, already computed in the unwind cache
+     (which would be nice).  */
+  frame_register_unwind (frame->next, regnum, optimized, addrp, lval,
+			 raw_buffer);
 }
 
 #if !defined (GET_SAVED_REGISTER)
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.12
diff -u -r1.12 frame.h
--- frame.h	12 Apr 2002 18:18:57 -0000	1.12
+++ frame.h	14 Apr 2002 19:29:48 -0000
@@ -23,6 +23,14 @@
 #if !defined (FRAME_H)
 #define FRAME_H 1
 
+struct frame_unwind_cache;
+
+typedef void (frame_register_unwind_ftype) (struct frame_info *frame,
+					    int regnum, int *optimized,
+					    CORE_ADDR *addrp,
+					    enum lval_type *lval,
+					    void *buffer);
+
 /* Describe the saved registers of a frame.  */
 
 #if defined (EXTRA_FRAME_INFO) || defined (FRAME_FIND_SAVED_REGS)
@@ -112,6 +120,23 @@
        related unwind data.  */
     struct unwind_contect *context;
 
+    /* Return the value of REGNUM for the previous (older, up) frame.
+       It is strongly recommended that implementors tune this function
+       to, on-demand, create data structures needed to unwind
+       registers.  Care should be taken on register-window
+       architectures, for instance with the SPARC, REGNUM "o0" for the
+       previous frame would be found in register "i0" in this FRAME.  */
+    frame_register_unwind_ftype *register_unwind;
+
+    /* Per-frame unwound register cache.  This is used to cache the
+       unwound register value returned by register_unwind().  Note:
+       while the unwound values can be thought of as part of the
+       previous frame, their values are not cached there.  This is
+       because that previous frame may not yet have been created -
+       code creating this frame may be able to supply a number of
+       unwound register values during its initialization.  */
+    struct frame_unwind_cache **unwind_cache;
+
     /* Pointers to the next (down, inner) and previous (up, outer)
        frame_info's in the frame cache.  */
     struct frame_info *next; /* down, inner */
@@ -276,6 +301,24 @@
 extern void generic_get_saved_register (char *, int *, CORE_ADDR *,
 					struct frame_info *, int,
 					enum lval_type *);
+
+/* Pair of functions that implement generic_get_saved_register()
+   functionality while also caching any computed values.
+   frame_supply_unwound_register() is for explicitly saving unwound
+   values (remember, they belong to the previous frame) into that
+   frame cache.  cache.  */
+
+extern void frame_supply_unwound_register (struct frame_info *frame,
+					   int regnum, int optimized,
+					   CORE_ADDR addrp,
+					   enum lval_type lval,
+					   void *buffer);
+
+extern void cache_generic_get_saved_register (char *raw_buffer, int *optimized,
+					      CORE_ADDR * addrp,
+					      struct frame_info *frame,
+					      int regnum,
+					      enum lval_type *lval);
 
 extern void get_saved_register (char *raw_buffer, int *optimized,
 				CORE_ADDR * addrp,
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.55
diff -u -r1.55 rs6000-tdep.c
--- rs6000-tdep.c	12 Apr 2002 19:48:36 -0000	1.55
+++ rs6000-tdep.c	14 Apr 2002 19:29:56 -0000
@@ -2652,7 +2652,7 @@
   set_gdbarch_pc_in_call_dummy (gdbarch, generic_pc_in_call_dummy);
   set_gdbarch_call_dummy_p (gdbarch, 1);
   set_gdbarch_call_dummy_stack_adjust_p (gdbarch, 0);
-  set_gdbarch_get_saved_register (gdbarch, generic_get_saved_register);
+  set_gdbarch_get_saved_register (gdbarch, cache_generic_get_saved_register);
   set_gdbarch_fix_call_dummy (gdbarch, rs6000_fix_call_dummy);
   set_gdbarch_push_dummy_frame (gdbarch, generic_push_dummy_frame);
   set_gdbarch_save_dummy_frame_tos (gdbarch, generic_save_dummy_frame_tos);