Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Rewrite data cache and use for stack access.
@ 2009-06-29 19:01 Jacob Potter
  2009-06-29 19:32 ` Daniel Jacobowitz
  0 siblings, 1 reply; 6+ messages in thread
From: Jacob Potter @ 2009-06-29 19:01 UTC (permalink / raw)
  To: gdb-patches

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

Hi -

Here's a patch to clean up the data cache code and add an option to
use it for all stack accesses. It's in two parts, first the rewrite of
the cache itself, and then the changes to other code to use it.

The first part of the patch makes the cache set-associative and
write-through. As originally implemented, the cache had writeback
capability; data could be written to the cache and marked as not
having been written to the target yet. However, all the code paths
through the cache actually wrote to the target immediately after each
write command to the cache, so there was no performance advantage and
significant complexity to deal with writeback. Making the cache
set-associative rather than fully-associative allows the size to be
increased significantly without performance penalties. Previously, a
cache lookup cost O(n) as the size of the cache.

Figuring out when to cache is tough.There are plenty of situations
where it's clearly not OK to cache data on GDB's end, most importantly
memory mapped I/O. However, we know that if a given access is known to
be on the stack, it's not going to lie in I/O or otherwise special
memory and it's somewhat less likely to even be shared among threads.
Building a backtrace requires a lot of small accesses all referring to
the stack, so caching them accelerates remote debugging significantly
without affecting correctness.

The old "remotecache" flag didn't actually do anything. Caching was
enabled by adding a cacheable memory region. This patch leaves that
functionality unchanged, but makes the remotecache option explicitly
deprecated and nonfunctional. It's still left around to avoid breaking
scripts, though. There's a new "stackcache" flag which enables the
cache for memory accesses known to be for the stack. It currently
defaults to off, but can always be safely set to on; hopefully the
default can be changed over at some point in the future.

- Jacob


2009-06-26  Jacob Potter  <jdpotter@google.com>

        * dcache.c (DCACHE_SET_SIZE, DCACHE_SET_SIZE_POWER, DCACHE_ASSOC,
        BLOCKNUM): Define.
        (DCACHE_SIZE, ENTRY_INVALID, ENTRY_VALID, ENTRY_DIRTY): Delete.
        (struct dcache_block): Clean up; remove "anydirty" field
        (struct dcache_set): New struct.
        (struct dcache_struct): Redefine as a set (array) of blocks, rather
        than a linked list.
        (dcache_writeback): Remove declaration.
        (dcache_enabled_p): Delete; replaced by the two flags below.
        (dcache_stack_enabled_p, remotecache_enabled_p): New flags.
        (show_dcache_enabled_p): Clarify status message.
        (show_remotecache_enabled_p): New function.
        (dcache_invalidate, dcache_hit): Rewrite for new cache structure.
        (dcache_write_line): Delete.
        (dcache_read_line, dcache_alloc): Rewrite for new cache structure.
        (dcache_write_line): Delete.
        (dcache_peek_byte): Clean up; remove "invalid" state check.
        (dcache_poke_byte): Rewrite for new cache structure; clarify comment.
        (dcache_init, dcache_free): Rewrite for new cache structure.
        (dcache_xfer_memory): Rewrite for new write-through cache structure.
        (dcache_update, dcache_print_line): New functions.
        (dcache_info): Rewrite for new cache structure.
        (_initialize_dcache): Rewrite for new cache structure.
        * dcache.h (dcache_xfer_memory): Update definition.
        (dcache_update, dcache_stack_enabled_p): Declare.
        * target.c (memory_xfer_partial): Update calls to dcache_xfer_memory.

        * corefile.c, gdbcore.h (read_stack): New helper for stack reads.
        * dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark value as stack.
        * frame-unwind.c (frame_unwind_got_memory): Mark value as stack.
        * target.c, target.h: Add new value to enum target_object,
        TARGET_OBJECT_STACK_MEMORY, for stack memory accesses; pass a
        target_object to memory_xfer_partial(). Handle stack accesses via
        dcache_xfer_memory().
        (target_read_stack): New helper for stack reads.
        * valops.c (get_value_at): Refactor common code from value_at() and
        value_at_lazy() into new helper function; add value_at_lazy_stack().
        * value.c, value.h (struct value): Add new field "stack" to
mark values on
        the stack.
        (value_fetch_lazy): Correctly fetch values marked as stack.
        (value_stack, set_value_stack): New accessors.

[-- Attachment #2: part1-dcache-rewrite.patch.txt --]
[-- Type: text/plain, Size: 25799 bytes --]

diff --git a/gdb/dcache.c b/gdb/dcache.c
index ae5a479..3969fa3 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -34,47 +34,25 @@
    comes from the actual caching mechanism, but the major gain is in
    the reduction of the remote protocol overhead; instead of reading
    or writing a large area of memory in 4 byte requests, the cache
-   bundles up the requests into 32 byte (actually LINE_SIZE) chunks.
-   Reducing the overhead to an eighth of what it was.  This is very
-   obvious when displaying a large amount of data,
-
-   eg, x/200x 0 
-
-   caching     |   no    yes 
-   ---------------------------- 
-   first time  |   4 sec  2 sec improvement due to chunking 
-   second time |   4 sec  0 sec improvement due to caching
-
-   The cache structure is unusual, we keep a number of cache blocks
-   (DCACHE_SIZE) and each one caches a LINE_SIZEed area of memory.
-   Within each line we remember the address of the line (always a
-   multiple of the LINE_SIZE) and a vector of bytes over the range.
-   There's another vector which contains the state of the bytes.
-
-   ENTRY_INVALID means that the byte is just plain wrong, and has no
-   correspondence with anything else (as it would when the cache is
-   turned on, but nothing has been done to it).
-
-   ENTRY_DIRTY means that the byte has some data in it which should be
-   written out to the remote target one day, but contains correct
-   data.
-
-   ENTRY_VALID means that the data is the same in the cache as it is in
-   remote memory.
-
-
-   The ENTRY_DIRTY state is necessary because GDB likes to write large
-   lumps of memory in small bits.  If the caching mechanism didn't
-   maintain the DIRTY information, then something like a two byte
-   write would mean that the entire cache line would have to be read,
-   the two bytes modified and then written out again.  The alternative
-   would be to not read in the cache line in the first place, and just
-   write the two bytes directly into target memory.  The trouble with
-   that is that it really nails performance, because of the remote
-   protocol overhead.  This way, all those little writes are bundled
-   up into an entire cache line write in one go, without having to
-   read the cache line in the first place.
- */
+   bundles up the requests into LINE_SIZE chunks, reducing overhead
+   significantly.  This is most useful when accessing a large amount
+   of data, such as when performing a backtrace.
+
+   The cache is essentially a DCACHE_ASSOC-way set-associative cache.  Each
+   each block caches a LINE_SIZE area of memory.  Wtihin each line we remember
+   the address of the line (which must be a multiple of LINE_SIZE) and the
+   actual data block.
+
+   Lines are only allocated as needed, so DCACHE_ASSOC really stores the
+   *maximum* number of lines associated with a given address.  This increases
+   allocation count and CPU use slightly, for the sake of not allocating
+   the maximum cache size unless necessary.
+
+   At present, the cache is write-through rather than writeback: as soon
+   as data is written to the cache, it is also immediately written to
+   the target.  Therefore, cache lines are never "dirty".  Whether a given
+   line is valid or not depends on where it is stored in the dcache_struct;
+   there is no per-block valid flag.  */
 
 /* NOTE: Interaction of dcache and memory region attributes
 
@@ -89,22 +67,23 @@
    the last bit of the .text segment and the first bit of the .data
    segment fall within the same dcache page with a ro/cacheable memory
    region defined for the .text segment and a rw/non-cacheable memory
-   region defined for the .data segment. */
+   region defined for the .data segment.  */
 
-/* This value regulates the number of cache blocks stored.
-   Smaller values reduce the time spent searching for a cache
-   line, and reduce memory requirements, but increase the risk
-   of a line not being in memory */
+/* The number of cache blocks stored per set.  The total size
+   of the cache is equal to DCACHE_SET_SIZE * DCACHE_ASSOC * LINE_SIZE.  */
+#define DCACHE_SET_SIZE_POWER 6
+#define DCACHE_SET_SIZE (1 << DCACHE_SET_SIZE_POWER)
 
-#define DCACHE_SIZE 64
-
-/* This value regulates the size of a cache line.  Smaller values
-   reduce the time taken to read a single byte, but reduce overall
-   throughput.  */
-
-#define LINE_SIZE_POWER (5)
+/* The size of a cache line.  Smaller values reduce the time taken to
+   read a single byte and make the cache more granular, but increase
+   overhead and reduce the effectiveness of the cache as a prefetcher.  */
+#define LINE_SIZE_POWER 6
 #define LINE_SIZE (1 << LINE_SIZE_POWER)
 
+/* The associativity of the cache: how many places a given address
+   could be stored in.  */
+#define DCACHE_ASSOC 4
+
 /* Each cache block holds LINE_SIZE bytes of data
    starting at a multiple-of-LINE_SIZE address.  */
 
@@ -112,59 +91,41 @@
 #define XFORM(x) 	((x) & LINE_SIZE_MASK)
 #define MASK(x)         ((x) & ~LINE_SIZE_MASK)
 
-
-#define ENTRY_INVALID 0 /* data at this byte is wrong */
-#define ENTRY_DIRTY   1 /* data at this byte needs to be written back */
-#define ENTRY_VALID   2 /* data at this byte is same as in memory */
-
-/* For cache state display by "info dcache".
-   The letters I,D,V map to
-   I = ENTRY_INVALID
-   D = ENTRY_DIRTY
-   V = ENTRY_VALID  */
-static const char state_chars[3] = { 'I', 'D', 'V' };
+#define BLOCKNUM(x)	(((x) >> LINE_SIZE_POWER) & (DCACHE_SET_SIZE - 1))
 
 struct dcache_block
-  {
-    struct dcache_block *p;	/* next in list */
-    CORE_ADDR addr;		/* Address for which data is recorded.  */
-    gdb_byte data[LINE_SIZE];	/* bytes at given address */
-    unsigned char state[LINE_SIZE];	/* what state the data is in */
+{
+  struct dcache_block *next;	/* next in list */
+  CORE_ADDR addr;		/* address of data */
+  gdb_byte data[LINE_SIZE];	/* bytes at given address */
 
-    /* whether anything in state is dirty - used to speed up the 
-       dirty scan. */
-    int anydirty;
+  int refs;
+};
 
-    int refs;
-  };
 
+/* The cache is stored as DCACHE_SET_SIZE separate linked lists of cache
+   blocks; each list is no longer than DCACHE_ASSOC.  Thus, only
+   DCACHE_ASSOC blocks need to be searched to find the correct one.
 
-/* FIXME: dcache_struct used to have a cache_has_stuff field that was
-   used to record whether the cache had been accessed.  This was used
-   to invalidate the cache whenever caching was (re-)enabled (if the
-   cache was disabled and later re-enabled, it could contain stale
-   data).  This was not needed because the cache is write through and
-   the code that enables, disables, and deletes memory region all
-   invalidate the cache.
+   Invalidated blocks are never actually free()d except by dcache_free();
+   they are kept around in the free list for reuse.  */
 
-   This is overkill, since it also invalidates cache lines from
-   unrelated regions.  One way this could be addressed by adding a
-   new function that takes an address and a length and invalidates
-   only those cache lines that match. */
+struct dcache_set
+{
+  struct dcache_block *head;
+  struct dcache_block *tail;
+  int num;			/* Number of lines currently stored.  */
+};
 
 struct dcache_struct
-  {
-    /* free list */
-    struct dcache_block *free_head;
-    struct dcache_block *free_tail;
-
-    /* in use list */
-    struct dcache_block *valid_head;
-    struct dcache_block *valid_tail;
+{
+  /* blocks */
+  struct dcache_set blocks[DCACHE_SET_SIZE];
 
-    /* The cache itself. */
-    struct dcache_block *the_cache;
-  };
+  /* free list */
+  struct dcache_block *free_head;
+  struct dcache_block *free_tail;
+};
 
 static struct dcache_block *dcache_hit (DCACHE *dcache, CORE_ADDR addr);
 
@@ -174,21 +135,27 @@ static int dcache_read_line (DCACHE *dcache, struct dcache_block *db);
 
 static struct dcache_block *dcache_alloc (DCACHE *dcache, CORE_ADDR addr);
 
-static int dcache_writeback (DCACHE *dcache);
-
 static void dcache_info (char *exp, int tty);
 
 void _initialize_dcache (void);
 
-static int dcache_enabled_p = 0;
+int dcache_stack_enabled_p = 0;
+
+int remotecache_enabled_p = 0;		/* OBSOLETE */
 
 static void
 show_dcache_enabled_p (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file, _("Cache use for remote targets is %s.\n"), value);
+  fprintf_filtered (file, _("Cache use for stack accesses is %s.\n"), value);
 }
 
+static void
+show_remotecache_enabled_p (struct ui_file *file, int from_tty,
+		       struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Deprecated remotecache flag is %s.\n"), value);
+}
 
 DCACHE *last_cache;		/* Used by info dcache */
 
@@ -199,37 +166,45 @@ void
 dcache_invalidate (DCACHE *dcache)
 {
   int i;
-  dcache->valid_head = 0;
-  dcache->valid_tail = 0;
+  struct dcache_set *set;
 
-  dcache->free_head = 0;
-  dcache->free_tail = 0;
-
-  for (i = 0; i < DCACHE_SIZE; i++)
+  for (i = 0; i < DCACHE_SET_SIZE; i++)
     {
-      struct dcache_block *db = dcache->the_cache + i;
+      set = &dcache->blocks[i];
 
-      if (!dcache->free_head)
-	dcache->free_head = db;
-      else
-	dcache->free_tail->p = db;
-      dcache->free_tail = db;
-      db->p = 0;
+      /* Shuffle all blocks out of each set and into the free list.  */
+      while (set->head)
+	{
+	  struct dcache_block *db = set->head;
+
+	  set->head = db->next;
+
+	  if (!dcache->free_head)
+	    dcache->free_head = db;
+	  else
+	    dcache->free_tail->next = db;
+
+	  dcache->free_tail = db;
+	  db->next = NULL;
+	}
+
+      set->tail = NULL;
+      set->num = 0;
     }
 
   return;
 }
 
 /* If addr is present in the dcache, return the address of the block
-   containing it. */
+   containing it.  */
 
 static struct dcache_block *
 dcache_hit (DCACHE *dcache, CORE_ADDR addr)
 {
   struct dcache_block *db;
 
-  /* Search all cache blocks for one that is at this address.  */
-  db = dcache->valid_head;
+  /* Search cache blocks for one that is at this address.  */
+  db = dcache->blocks[BLOCKNUM(addr)].head;
 
   while (db)
     {
@@ -238,92 +213,14 @@ dcache_hit (DCACHE *dcache, CORE_ADDR addr)
 	  db->refs++;
 	  return db;
 	}
-      db = db->p;
+      db = db->next;
     }
 
   return NULL;
 }
 
-/* Make sure that anything in this line which needs to
-   be written is. */
-
-static int
-dcache_write_line (DCACHE *dcache, struct dcache_block *db)
-{
-  CORE_ADDR memaddr;
-  gdb_byte *myaddr;
-  int len;
-  int res;
-  int reg_len;
-  struct mem_region *region;
-
-  if (!db->anydirty)
-    return 1;
-
-  len = LINE_SIZE;
-  memaddr = db->addr;
-  myaddr  = db->data;
-
-  while (len > 0)
-    {
-      int s;
-      int e;
-      int dirty_len;
-      
-      region = lookup_mem_region(memaddr);
-      if (memaddr + len < region->hi)
-	reg_len = len;
-      else
-	reg_len = region->hi - memaddr;
 
-      if (!region->attrib.cache || region->attrib.mode == MEM_RO)
-	{
-	  memaddr += reg_len;
-	  myaddr  += reg_len;
-	  len     -= reg_len;
-	  continue;
-	}
-
-      while (reg_len > 0)
-	{
-	  s = XFORM(memaddr);
-	  while (reg_len > 0) {
-	    if (db->state[s] == ENTRY_DIRTY)
-	      break;
-	    s++;
-	    reg_len--;
-
-	    memaddr++;
-	    myaddr++;
-	    len--;
-	  }
-
-	  e = s;
-	  while (reg_len > 0) {
-	    if (db->state[e] != ENTRY_DIRTY)
-	      break;
-	    e++;
-	    reg_len--;
-	  }
-
-	  dirty_len = e - s;
-	  res = target_write (&current_target, TARGET_OBJECT_RAW_MEMORY,
-			      NULL, myaddr, memaddr, dirty_len);
-	  if (res < dirty_len)
-	    return 0;
-
-	  memset (&db->state[XFORM(memaddr)], ENTRY_VALID, res);
-	  memaddr += res;
-	  myaddr += res;
-	  len -= res;
-	}
-    }
-
-  db->anydirty = 0;
-  return 1;
-}
-
-/* Read cache line */
+/* Fill a cache line from target memory.  */
 static int
 dcache_read_line (DCACHE *dcache, struct dcache_block *db)
 {
@@ -334,27 +231,22 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
   int reg_len;
   struct mem_region *region;
 
-  /* If there are any dirty bytes in the line, it must be written
-     before a new line can be read */
-  if (db->anydirty)
-    {
-      if (!dcache_write_line (dcache, db))
-	return 0;
-    }
-  
   len = LINE_SIZE;
   memaddr = db->addr;
   myaddr  = db->data;
 
   while (len > 0)
     {
-      region = lookup_mem_region(memaddr);
-      if (memaddr + len < region->hi)
+      /* Don't overrun if this block is right at the end of the region.  */
+      region = lookup_mem_region (memaddr);
+      if (region->hi == 0 || memaddr + len < region->hi)
 	reg_len = len;
       else
 	reg_len = region->hi - memaddr;
 
-      if (!region->attrib.cache || region->attrib.mode == MEM_WO)
+      /* Skip non-readable regions.  The cache attribute can be ignored,
+         since we may be loading this for a stack access.  */
+      if (region->attrib.mode == MEM_WO)
 	{
 	  memaddr += reg_len;
 	  myaddr  += reg_len;
@@ -372,9 +264,6 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
       len -= res;
     }
 
-  memset (db->state, ENTRY_VALID, sizeof (db->data));
-  db->anydirty = 0;
-  
   return 1;
 }
 
@@ -385,62 +274,47 @@ static struct dcache_block *
 dcache_alloc (DCACHE *dcache, CORE_ADDR addr)
 {
   struct dcache_block *db;
+  struct dcache_set *set;
 
-  /* Take something from the free list */
-  db = dcache->free_head;
-  if (db)
+  set = &dcache->blocks[BLOCKNUM(addr)];
+
+  if (set->num < DCACHE_ASSOC)
     {
-      dcache->free_head = db->p;
+      /* Take something from the free list if possible.  */
+      db = dcache->free_head;
+      if (db)
+	dcache->free_head = db->next;
+      else
+	db = xmalloc (sizeof (struct dcache_block));
+
+      set->num++;
     }
   else
     {
-      /* Nothing left on free list, so grab one from the valid list */
-      db = dcache->valid_head;
-
-      if (!dcache_write_line (dcache, db))
-	return NULL;
-      
-      dcache->valid_head = db->p;
+      /* Evict the oldest line first.  LRU would be preferable, but this is
+	 fine for now.  */
+      db = set->head;
+      set->head = db->next;
     }
 
   db->addr = MASK(addr);
   db->refs = 0;
-  db->anydirty = 0;
-  memset (db->state, ENTRY_INVALID, sizeof (db->data));
 
-  /* append this line to end of valid list */
-  if (!dcache->valid_head)
-    dcache->valid_head = db;
+  if (!set->head)
+    set->head = db;
   else
-    dcache->valid_tail->p = db;
-  dcache->valid_tail = db;
-  db->p = 0;
+    set->tail->next = db;
+  set->tail = db;
+  db->next = NULL;
 
   return db;
 }
 
-/* Writeback any dirty lines. */
-static int
-dcache_writeback (DCACHE *dcache)
-{
-  struct dcache_block *db;
-
-  db = dcache->valid_head;
-
-  while (db)
-    {
-      if (!dcache_write_line (dcache, db))
-	return 0;
-      db = db->p;
-    }
-  return 1;
-}
-
 
 /* Using the data cache DCACHE return the contents of the byte at
    address ADDR in the remote machine.  
 
-   Returns 0 on error. */
+   Returns 0 on error.  */
 
 static int
 dcache_peek_byte (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr)
@@ -450,13 +324,8 @@ dcache_peek_byte (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr)
   if (!db)
     {
       db = dcache_alloc (dcache, addr);
-      if (!db)
-	return 0;
-    }
-  
-  if (db->state[XFORM (addr)] == ENTRY_INVALID)
-    {
-      if (!dcache_read_line(dcache, db))
+
+      if (!dcache_read_line (dcache, db))
          return 0;
     }
 
@@ -466,24 +335,24 @@ dcache_peek_byte (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr)
 
 
 /* Write the byte at PTR into ADDR in the data cache.
-   Return zero on write error.
- */
+
+   The caller is responsible for also promptly writing the data
+   through to target memory.
+
+   If addr is not in cache, this function does nothing; writing to
+   an area of memory which wasn't present in the cache doesn't cause
+   it to be loaded in.
+
+   Always return 1 to simplify dcache_xfer_memory.  */
 
 static int
 dcache_poke_byte (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr)
 {
   struct dcache_block *db = dcache_hit (dcache, addr);
 
-  if (!db)
-    {
-      db = dcache_alloc (dcache, addr);
-      if (!db)
-	return 0;
-    }
+  if (db)
+    db->data[XFORM (addr)] = *ptr;
 
-  db->data[XFORM (addr)] = *ptr;
-  db->state[XFORM (addr)] = ENTRY_DIRTY;
-  db->anydirty = 1;
   return 1;
 }
 
@@ -491,28 +360,30 @@ dcache_poke_byte (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr)
 DCACHE *
 dcache_init (void)
 {
-  int csize = sizeof (struct dcache_block) * DCACHE_SIZE;
   DCACHE *dcache;
+  int i;
 
   dcache = (DCACHE *) xmalloc (sizeof (*dcache));
 
-  dcache->the_cache = (struct dcache_block *) xmalloc (csize);
-  memset (dcache->the_cache, 0, csize);
+  for (i = 0; i < DCACHE_SET_SIZE; i++)
+    dcache->blocks[i].head = NULL;
 
   dcache_invalidate (dcache);
 
+  dcache->free_head = NULL;
+  dcache->free_tail = NULL;
+
   last_cache = dcache;
   return dcache;
 }
 
-/* Free a data cache */
+/* Free a data cache.  */
 void
 dcache_free (DCACHE *dcache)
 {
   if (last_cache == dcache)
     last_cache = NULL;
 
-  xfree (dcache->the_cache);
   xfree (dcache);
 }
 
@@ -520,89 +391,182 @@ dcache_free (DCACHE *dcache)
    to or from debugger address MYADDR.  Write to inferior if SHOULD_WRITE is
    nonzero. 
 
-   Returns length of data written or read; 0 for error.  
-
-   This routine is indended to be called by remote_xfer_ functions. */
+   Returns length of data written or read; 0 for error.  */
 
 int
-dcache_xfer_memory (DCACHE *dcache, CORE_ADDR memaddr, gdb_byte *myaddr,
+dcache_xfer_memory (struct target_ops *ops, DCACHE *dcache,
+		    CORE_ADDR memaddr, gdb_byte *myaddr,
 		    int len, int should_write)
 {
   int i;
+  int res;
   int (*xfunc) (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr);
   xfunc = should_write ? dcache_poke_byte : dcache_peek_byte;
 
+  /* Do write-through first, so that if it fails, we don't write to
+     the cache at all.  */
+
+  if (should_write)
+    {
+      res = target_write (ops, TARGET_OBJECT_RAW_MEMORY,
+			  NULL, myaddr, memaddr, len);
+      if (res < len)
+	return 0;
+    }
+      
   for (i = 0; i < len; i++)
     {
       if (!xfunc (dcache, memaddr + i, myaddr + i))
 	return 0;
     }
+    
+  return len;
+}
 
-  /* FIXME: There may be some benefit from moving the cache writeback
-     to a higher layer, as it could occur after a sequence of smaller
-     writes have been completed (as when a stack frame is constructed
-     for an inferior function call).  Note that only moving it up one
-     level to target_xfer_memory() (also target_xfer_memory_partial())
-     is not sufficent, since we want to coalesce memory transfers that
-     are "logically" connected but not actually a single call to one
-     of the memory transfer functions. */
+/* FIXME: There would be some benefit to making the cache write-back and
+   moving the writeback operation to a higher layer, as it could occur
+   after a sequence of smaller writes have been completed (as when a stack
+   frame is constructed for an inferior function call).  Note that only
+   moving it up one level to target_xfer_memory[_partial]() is not
+   sufficient since we want to coalesce memory transfers that are
+   "logically" connected but not actually a single call to one of the
+   memory transfer functions. */
 
-  if (should_write)
-    dcache_writeback (dcache);
+/* Just update any cache lines which are already present.  This is called
+   by memory_xfer_partial in cases where the access would otherwise not go
+   through the cache.  */
+
+void
+dcache_update (DCACHE *dcache, CORE_ADDR memaddr, gdb_byte *myaddr, int len)
+{
+  int i;
+  for (i = 0; i < len; i++)
+    dcache_poke_byte (dcache, memaddr + i, myaddr + i);
+}
+
+static void
+dcache_print_line (int indexno, int lineno)
+{
+  int j;
+  struct dcache_block *p;
+
+  if (!last_cache)
+    {
+      printf_filtered (_("No data cache available.\n"));
+      return;
+    }
+
+  p = last_cache->blocks[indexno].head;
+
+  while (lineno > 0 && p)
+    {
+      p = p->next;
+      lineno--;
+    }
+
+  if (!p)
+    {
+      printf_filtered (_("No such cache line exists.\n"));
+      return;
+    }
     
-  return len;
+  printf_filtered (_("Index %d, line %d: address %lx [%d hits]\n"),
+		  indexno, lineno, p->addr, p->refs);
+
+  for (j = 0; j < LINE_SIZE; j++)
+    {
+      printf_filtered ("%02x ", p->data[j]);
+
+      /* Print a newline every 16 bytes (48 characters) */
+      if ((j % 16 == 15) && (j != LINE_SIZE - 1))
+	printf_filtered ("\n");
+    }
+  printf_filtered ("\n");
 }
 
 static void
 dcache_info (char *exp, int tty)
 {
   struct dcache_block *p;
+  int i, linecount, refcount, lineno;
 
-  printf_filtered (_("Dcache line width %d, depth %d\n"),
-		   LINE_SIZE, DCACHE_SIZE);
+  if (exp)
+    {
+      int indexno, lineno;
+      char *linestart;
+      indexno = strtol (exp, &linestart, 10);
+      if (linestart == exp)
+	{
+	  printf_filtered (_("Usage: info dcache [indexnumber linenumber]\n"));
+          return;
+	}
 
-  if (last_cache)
+      lineno = strtol (linestart + 1, NULL, 10);
+      dcache_print_line (indexno, lineno);
+      return;
+    }
+
+  printf_filtered (_("Dcache line width %d, sets %d, assoc %d\n"),
+		   LINE_SIZE, DCACHE_SET_SIZE, DCACHE_ASSOC);
+
+  if (!last_cache)
     {
-      printf_filtered (_("Cache state:\n"));
+      printf_filtered (_("No data cache available.\n"));
+      return;
+    }
 
-      for (p = last_cache->valid_head; p; p = p->p)
-	{
-	  int j;
-	  printf_filtered (_("Line at %s, referenced %d times\n"),
-			   paddr (p->addr), p->refs);
+  linecount = 0;
+  refcount = 0;
 
-	  for (j = 0; j < LINE_SIZE; j++)
-	    printf_filtered ("%02x", p->data[j] & 0xFF);
-	  printf_filtered (("\n"));
+  for (i = 0; i < DCACHE_SET_SIZE; i++)
+    {
+      p = last_cache->blocks[i].head;
+      lineno = 0;
 
-	  for (j = 0; j < LINE_SIZE; j++)
-	    printf_filtered (" %c", state_chars[p->state[j]]);
-	  printf_filtered ("\n");
+      while (p)
+	{
+          printf_filtered (_("Index %d, line %d: address %lx [%d hits]\n"),
+			  i, lineno, p->addr, p->refs);
+	  linecount++;
+	  lineno++;
+	  refcount += p->refs;
+	  p = p->next;
 	}
     }
+
+  printf_filtered (_("Cache state: %d active lines, %d hits\n"), linecount, refcount);
 }
 
 void
 _initialize_dcache (void)
 {
-  add_setshow_boolean_cmd ("remotecache", class_support,
-			   &dcache_enabled_p, _("\
-Set cache use for remote targets."), _("\
-Show cache use for remote targets."), _("\
-When on, use data caching for remote targets.  For many remote targets\n\
-this option can offer better throughput for reading target memory.\n\
-Unfortunately, gdb does not currently know anything about volatile\n\
-registers and thus data caching will produce incorrect results with\n\
-volatile registers are in use.  By default, this option is off."),
+  add_setshow_boolean_cmd ("stackcache", class_support,
+			   &dcache_stack_enabled_p, _("\
+Set cache use for stack access."), _("\
+Show cache use for stack access."), _("\
+When on, use the data cache for all stack access, regardless of any\n\
+configured memory regions.  This improves remote performance significantly.\n\
+By default, caching for stack access is off."),
 			   NULL,
 			   show_dcache_enabled_p,
 			   &setlist, &showlist);
 
+  add_setshow_boolean_cmd ("remotecache", class_support,
+			   &remotecache_enabled_p, _("\
+Set remotecache flag."), _("\
+Show remotecache flag."), _("\
+This used to enable the data cache for remote targets.  The cache\n\
+functionality is now controlled by the memory region system and the\n\
+\"stackcache\" flag; \"remotecache\" now does nothing and exists only\n\
+for compatibility reasons."),
+			   NULL,
+			   show_remotecache_enabled_p,
+			   &setlist, &showlist);
+
   add_info ("dcache", dcache_info,
 	    _("\
-Print information on the dcache performance.\n\
-The state of each cached byte is represented by a letter:\n\
-  I = invalid\n\
-  D = dirty\n\
-  V = valid"));
+Print information about the data cache.\n\
+With no arguments, this command prints the cache configuration and a\n\
+summary of each line in the cache. Use \"info dcache <indexno> <lineno>\"\n\
+to dump the contents of a given line."));
 }
diff --git a/gdb/dcache.h b/gdb/dcache.h
index 32077a6..fdc22c5 100644
--- a/gdb/dcache.h
+++ b/gdb/dcache.h
@@ -35,7 +35,13 @@ void dcache_free (DCACHE *);
 
 /* Simple to call from <remote>_xfer_memory */
 
-int dcache_xfer_memory (DCACHE *cache, CORE_ADDR mem, gdb_byte *my,
-			int len, int should_write);
+int dcache_xfer_memory (struct target_ops *ops, DCACHE *cache, CORE_ADDR mem,
+			gdb_byte *my, int len, int should_write);
+
+void dcache_update (DCACHE *dcache, CORE_ADDR memaddr, gdb_byte *myaddr,
+		    int len);
+
+/* Should we use the dcache for stack access? */
+extern int dcache_stack_enabled_p;
 
 #endif /* DCACHE_H */
diff --git a/gdb/target.c b/gdb/target.c
index 7a5b768..8c3b2bd 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1214,16 +1214,14 @@ memory_xfer_partial (struct target_ops *ops, void *readbuf, const void *writebuf
 
   if (region->attrib.cache)
     {
-      /* FIXME drow/2006-08-09: This call discards OPS, so the raw
-	 memory request will start back at current_target.  */
       if (readbuf != NULL)
-	res = dcache_xfer_memory (target_dcache, memaddr, readbuf,
+	res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
 				  reg_len, 0);
       else
 	/* FIXME drow/2006-08-09: If we're going to preserve const
 	   correctness dcache_xfer_memory should take readbuf and
 	   writebuf.  */
-	res = dcache_xfer_memory (target_dcache, memaddr,
+	res = dcache_xfer_memory (ops, target_dcache, memaddr,
 				  (void *) writebuf,
 				  reg_len, 1);
       if (res <= 0)

[-- Attachment #3: part2-use-cache-for-stack.patch.txt --]
[-- Type: text/plain, Size: 11107 bytes --]

diff --git a/gdb/corefile.c b/gdb/corefile.c
index e74630b..69971ea 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -228,6 +228,7 @@ memory_error (int status, CORE_ADDR memaddr)
 }
 
 /* Same as target_read_memory, but report an error if can't read.  */
+
 void
 read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
 {
@@ -237,6 +238,17 @@ read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
     memory_error (status, memaddr);
 }
 
+/* Same as target_read_stack, but report an error if can't read.  */
+
+void
+read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
+{
+  int status;
+  status = target_read_stack (memaddr, myaddr, len);
+  if (status != 0)
+    memory_error (status, memaddr);
+}
+
 /* Argument / return result struct for use with
    do_captured_read_memory_integer().  MEMADDR and LEN are filled in
    by gdb_read_memory_integer().  RESULT is the contents that were
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 16d8cee..120269d 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -264,6 +264,7 @@ dwarf2_evaluate_loc_desc (struct symbol *var, struct frame_info *frame,
       retval = allocate_value (SYMBOL_TYPE (var));
       VALUE_LVAL (retval) = lval_memory;
       set_value_lazy (retval, 1);
+      set_value_stack (retval, 1);
       set_value_address (retval, address);
     }
 
diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index 98d6b43..866a156 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -151,7 +151,7 @@ frame_unwind_got_memory (struct frame_info *frame, int regnum, CORE_ADDR addr)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
 
-  return value_at_lazy (register_type (gdbarch, regnum), addr);
+  return value_at_lazy_stack (register_type (gdbarch, regnum), addr);
 }
 
 /* Return a value which indicates that FRAME's saved version of
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index ec3e1a8..d303c44 100644
--- a/gdb/gdbcore.h
+++ b/gdb/gdbcore.h
@@ -47,6 +47,10 @@ extern void memory_error (int status, CORE_ADDR memaddr);
 
 extern void read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len);
 
+/* Like target_read_stack, but report an error if can't read.  */
+
+extern void read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, int len);
+
 /* Read an integer from debugged memory, given address and number of
    bytes.  */
 
diff --git a/gdb/target.c b/gdb/target.c
index 8c3b2bd..7c04b3c 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1132,8 +1132,9 @@ target_section_by_addr (struct target_ops *target, CORE_ADDR addr)
    value are just as for target_xfer_partial.  */
 
 static LONGEST
-memory_xfer_partial (struct target_ops *ops, void *readbuf, const void *writebuf,
-		     ULONGEST memaddr, LONGEST len)
+memory_xfer_partial (struct target_ops *ops, enum target_object object,
+		     void *readbuf, const void *writebuf, ULONGEST memaddr,
+		     LONGEST len)
 {
   LONGEST res;
   int reg_len;
@@ -1212,7 +1213,8 @@ memory_xfer_partial (struct target_ops *ops, void *readbuf, const void *writebuf
       return -1;
     }
 
-  if (region->attrib.cache)
+  if (region->attrib.cache
+      || (dcache_stack_enabled_p && object == TARGET_OBJECT_STACK_MEMORY))
     {
       if (readbuf != NULL)
 	res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
@@ -1234,6 +1236,15 @@ memory_xfer_partial (struct target_ops *ops, void *readbuf, const void *writebuf
 	}
     }
 
+  /* Make sure the cache gets updated no matter what - if we are writing
+     to the stack, even if this write is not tagged as such, we still need
+     to update the cache. */
+  if (readbuf == NULL && !region->attrib.cache &&
+      dcache_stack_enabled_p && object != TARGET_OBJECT_STACK_MEMORY)
+    {
+      dcache_update (target_dcache, memaddr, (void *) writebuf, reg_len);
+    }
+
   /* If none of those methods found the memory we wanted, fall back
      to a target partial transfer.  Normally a single call to
      to_xfer_partial is enough; if it doesn't recognize an object
@@ -1297,8 +1308,9 @@ target_xfer_partial (struct target_ops *ops,
   /* If this is a memory transfer, let the memory-specific code
      have a look at it instead.  Memory transfers are more
      complicated.  */
-  if (object == TARGET_OBJECT_MEMORY)
-    retval = memory_xfer_partial (ops, readbuf, writebuf, offset, len);
+  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY)
+    retval = memory_xfer_partial (ops, object, readbuf,
+				  writebuf, offset, len);
   else
     {
       enum target_object raw_object = object;
@@ -1380,6 +1392,23 @@ target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
     return EIO;
 }
 
+/* Like target_read_memory, but specify explicitly that this is a read from
+   the target's stack. This may trigger different cache behavior. */
+
+int
+target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
+{
+  /* Dispatch to the topmost target, not the flattened current_target.
+     Memory accesses check target->to_has_(all_)memory, and the
+     flattened target doesn't inherit those.  */
+
+  if (target_read (current_target.beneath, TARGET_OBJECT_STACK_MEMORY, NULL,
+		   myaddr, memaddr, len) == len)
+    return 0;
+  else
+    return EIO;
+}
+
 int
 target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, int len)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 89d99bb..efdbcea 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -202,6 +202,10 @@ enum target_object
      Target implementations of to_xfer_partial never need to handle
      this object, and most callers should not use it.  */
   TARGET_OBJECT_RAW_MEMORY,
+  /* Memory known to be part of the target's stack. This is cached even
+     if it is not in a region marked as such, since it is known to be
+     "normal" RAM. */
+  TARGET_OBJECT_STACK_MEMORY,
   /* Kernel Unwind Table.  See "ia64-tdep.c".  */
   TARGET_OBJECT_UNWIND_TABLE,
   /* Transfer auxilliary vector.  */
@@ -663,6 +667,8 @@ extern int target_read_string (CORE_ADDR, char **, int, int *);
 
 extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len);
 
+extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, int len);
+
 extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 				int len);
 
diff --git a/gdb/valops.c b/gdb/valops.c
index 1d19393..a07c495 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -562,6 +562,39 @@ value_one (struct type *type, enum lval_type lv)
   return val;
 }
 
+/* Helper function for value_at, value_at_lazy, and value_at_lazy_stack.  */
+
+static struct value *
+get_value_at (struct type *type, CORE_ADDR addr, int lazy, int stack)
+{
+  struct value *val;
+
+  if (TYPE_CODE (check_typedef (type)) == TYPE_CODE_VOID)
+    error (_("Attempt to dereference a generic pointer."));
+
+  if (lazy)
+    {
+      val = allocate_value_lazy (type);
+    }
+  else
+    {
+      val = allocate_value (type);
+
+      if (stack)
+        read_stack (addr, value_contents_all_raw (val), TYPE_LENGTH (type));
+      else
+        read_memory (addr, value_contents_all_raw (val), TYPE_LENGTH (type));
+    }
+
+  if (stack)
+    set_value_stack (val, 1);
+
+  VALUE_LVAL (val) = lval_memory;
+  set_value_address (val, addr);
+
+  return val;
+}
+
 /* Return a value with type TYPE located at ADDR.
 
    Call value_at only if the data needs to be fetched immediately;
@@ -577,19 +610,7 @@ value_one (struct type *type, enum lval_type lv)
 struct value *
 value_at (struct type *type, CORE_ADDR addr)
 {
-  struct value *val;
-
-  if (TYPE_CODE (check_typedef (type)) == TYPE_CODE_VOID)
-    error (_("Attempt to dereference a generic pointer."));
-
-  val = allocate_value (type);
-
-  read_memory (addr, value_contents_all_raw (val), TYPE_LENGTH (type));
-
-  VALUE_LVAL (val) = lval_memory;
-  set_value_address (val, addr);
-
-  return val;
+  return get_value_at (type, addr, 0, 0);
 }
 
 /* Return a lazy value with type TYPE located at ADDR (cf. value_at).  */
@@ -597,17 +618,17 @@ value_at (struct type *type, CORE_ADDR addr)
 struct value *
 value_at_lazy (struct type *type, CORE_ADDR addr)
 {
-  struct value *val;
-
-  if (TYPE_CODE (check_typedef (type)) == TYPE_CODE_VOID)
-    error (_("Attempt to dereference a generic pointer."));
+  return get_value_at (type, addr, 1, 0);
+}
 
-  val = allocate_value_lazy (type);
+/* Return a lazy value with type TYPE located at ADDR (cf. value_at).  If
+   and when the value is actually fetched, it will be marked as a stack
+   access.  */
 
-  VALUE_LVAL (val) = lval_memory;
-  set_value_address (val, addr);
-
-  return val;
+struct value *
+value_at_lazy_stack (struct type *type, CORE_ADDR addr)
+{
+  return get_value_at (type, addr, 1, 1);
 }
 
 /* Called only from the value_contents and value_contents_all()
@@ -634,8 +655,12 @@ value_fetch_lazy (struct value *val)
       CORE_ADDR addr = value_address (val);
       int length = TYPE_LENGTH (check_typedef (value_enclosing_type (val)));
 
-      if (length)
-	read_memory (addr, value_contents_all_raw (val), length);
+      if (length) {
+	if (value_stack (val))
+	  read_stack (addr, value_contents_all_raw (val), length);
+        else
+	  read_memory (addr, value_contents_all_raw (val), length);
+      }
     }
   else if (VALUE_LVAL (val) == lval_register)
     {
diff --git a/gdb/value.c b/gdb/value.c
index 7566921..9e70b1c 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -190,6 +190,10 @@ struct value
   /* If value is a variable, is it initialized or not.  */
   int initialized;
 
+  /* If value is from the stack. If this is set, read_stack will be
+     used instead of read_memory to enable extra caching. */
+  int stack;
+
   /* Actual contents of the value.  Target byte-order.  NULL or not
      valid if lazy is nonzero.  */
   gdb_byte *contents;
@@ -433,6 +437,18 @@ set_value_lazy (struct value *value, int val)
   value->lazy = val;
 }
 
+int
+value_stack (struct value *value)
+{
+  return value->stack;
+}
+
+void
+set_value_stack (struct value *value, int val)
+{
+  value->stack = val;
+}
+
 const gdb_byte *
 value_contents (struct value *value)
 {
diff --git a/gdb/value.h b/gdb/value.h
index c31cce5..c23b0df 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -214,6 +214,9 @@ extern void *value_computed_closure (struct value *value);
 extern int value_lazy (struct value *);
 extern void set_value_lazy (struct value *value, int val);
 
+extern int value_stack (struct value *);
+extern void set_value_stack (struct value *value, int val);
+
 /* value_contents() and value_contents_raw() both return the address
    of the gdb buffer used to hold a copy of the contents of the lval.
    value_contents() is used when the contents of the buffer are needed
@@ -342,6 +345,7 @@ extern struct value *value_from_decfloat (struct type *type,
 
 extern struct value *value_at (struct type *type, CORE_ADDR addr);
 extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);
+extern struct value *value_at_lazy_stack (struct type *type, CORE_ADDR addr);
 
 extern struct value *value_from_contents_and_address (struct type *,
 						      const gdb_byte *,

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-06-30 21:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-29 19:01 [RFA] Rewrite data cache and use for stack access Jacob Potter
2009-06-29 19:32 ` Daniel Jacobowitz
2009-06-29 19:58   ` Michael Snyder
2009-06-29 20:03     ` Daniel Jacobowitz
2009-06-30 21:16   ` Jacob Potter
2009-06-30 21:28     ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox