[RFA] Use data cache for stack accesses

[RFA] Use data cache for stack accesses

[Jacob Potter]

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

This is the second half of the pair of patches I first submitted last
week. Differences from the first pass are:

- Added a NEWS entry for the new option
- Changed the stackcache option to default to on, rather than off
- Got rid of the unnecessary new value_at_lazy_stack() function
- Flush the cache when switching inferiors.

I haven't changed the new read_stack function to take a target_ops,
since it's intended to be consistent with read_memory(); converting
read_memory() to take target_ops would be out of the scope of this
patch (it and target_read_memory have a *lot* of callers). Other than
that, I think I've addressed all the issues with the first pass; is
there anything else to fix?

Thanks!

- Jacob

2009-07-08  Jacob Potter  <jdpotter@google.com>

        * NEWS: Add description of stackcache option.
        * corefile.c (read_stack): New helper for stack reads.
        * gdbcore.h (read_stack): Declare.
        * dcache.c (dcache_enabled_p, remotecache_enabled_p): Rename
        dcache_enabled_p to remotecache_enabled_p.
        (stack_cache_enabled_p): New flag.
        (show_dcache_enabled_p): Change for use with stack cache flag.
        (show_remotecache_enabled_p): New function.
        (_initialize_dcache): Rewrite flag documentation; add new flag.
        * dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark value as stack.
        * frame-unwind.c (frame_unwind_got_memory): Mark value as stack.
        * target.c (memory_xfer_partial): Add a target_object parameter
        to indicate the type of access; use dcache for stack.
        (target_read_stack): New helper for stack reads.
        * target.h (enum target_object): Add TARGET_OBJECT_STACK_MEMORY.
        (target_read_stack): Declare.
        * thread.c (switch_to_thread): Flush cache when pid changes.
        * valops.c (get_value_at): Refactor common code from value_at()
        and value_at_lazy() into new helper function.
        (value_fetch_lazy): Read from stack when necessary.
        * value.c (struct value): Add new field "stack" for values on
        the stack.
        (value_stack, set_value_stack): New accessors.
        * value.h (value_stack, set_value_stack): Declare.

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

diff --git a/gdb/NEWS b/gdb/NEWS
index 68776a5..b30f266 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -342,6 +342,12 @@ show schedule-multiple
   Allow GDB to resume all threads of all processes or only threads of
   the current process.
 
+set stackcache
+show stackcache
+  Use more aggressive caching for accesses to the stack. This improves
+  performance of remote debugging (particularly tracebacks) without
+  affecting correctness.
+
 * Removed commands
 
 info forks
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 6de0772..e70688e 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/dcache.c b/gdb/dcache.c
index 06956cb..f915074 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -116,15 +116,23 @@ static void dcache_info (char *exp, int tty);
 
 void _initialize_dcache (void);
 
-static int dcache_enabled_p = 0;
+int dcache_stack_enabled_p = 1;
+
+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 */
 
@@ -506,20 +514,33 @@ dcache_info (char *exp, int tty)
 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 on."),
 			   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."));
+Print information on the dcache performance.\n\
+With no arguments, this command prints the cache configuration and a\n\
+summary of each line in the cache.  Use \"info dcache <lineno> to dump\"\n\
+the contents of a given line."));
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 7a54f43..23ae052 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -271,6 +271,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 238c6a1..f0c2a64 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -153,8 +153,10 @@ struct value *
 frame_unwind_got_memory (struct frame_info *frame, int regnum, CORE_ADDR addr)
 {
   struct gdbarch *gdbarch = frame_unwind_arch (frame);
+  struct value *v = value_at_lazy (register_type (gdbarch, regnum), addr);
 
-  return value_at_lazy (register_type (gdbarch, regnum), addr);
+  set_value_stack (v, 1);
+  return v;
 }
 
 /* Return a value which indicates that FRAME's saved version of
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index e339c0b..7a7dcb2 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 0623bc3..89810ec 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1143,8 +1143,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;
@@ -1223,7 +1224,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,
@@ -1245,6 +1247,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
@@ -1308,8 +1319,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;
@@ -1391,6 +1403,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 24c803c..fc6b503 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.  */
@@ -667,6 +671,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/thread.c b/gdb/thread.c
index a7ac3c8..0ad3ec7 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -848,6 +848,9 @@ switch_to_thread (ptid_t ptid)
   if (ptid_equal (ptid, inferior_ptid))
     return;
 
+  if (ptid_get_pid (ptid) != ptid_get_pid (inferior_ptid))
+    dcache_invalidate (target_dcache);
+
   inferior_ptid = ptid;
   reinit_frame_cache ();
   registers_changed ();
diff --git a/gdb/valops.c b/gdb/valops.c
index 2d20b41..c64598c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -565,6 +565,32 @@ 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)
+{
+  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);
+      read_memory (addr, value_contents_all_raw (val), TYPE_LENGTH (type));
+    }
+
+  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;
@@ -580,19 +606,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);
 }
 
 /* Return a lazy value with type TYPE located at ADDR (cf. value_at).  */
@@ -600,17 +614,7 @@ 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."));
-
-  val = allocate_value_lazy (type);
-
-  VALUE_LVAL (val) = lval_memory;
-  set_value_address (val, addr);
-
-  return val;
+  return get_value_at (type, addr, 1);
 }
 
 /* Called only from the value_contents and value_contents_all()
@@ -637,8 +641,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 fffc183..fd0df02 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -191,6 +191,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;
@@ -427,6 +431,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 d816156..9259768 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

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Wednesday 08 July 2009 21:08:00, Jacob Potter write:
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -848,6 +848,9 @@ switch_to_thread (ptid_t ptid)
>    if (ptid_equal (ptid, inferior_ptid))
>      return;
>  
> +  if (ptid_get_pid (ptid) != ptid_get_pid (inferior_ptid))
> +    dcache_invalidate (target_dcache);
> +

I'm not sure this would be 100% multi-address space safe.

Do we not have places where we switch inferior_ptid temporarily
before calling reading memory, with save_inferior_ptid, without
going through the high level switch_to_thread ?

What if we do this within dcache itself, similarly
to get_thread_regcache?  That would be probably in memory_xfer_partial.

-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Wednesday 08 July 2009 21:46:40, Pedro Alves wrote:
> What if we do this within dcache itself, similarly
> to get_thread_regcache?  That would be probably in memory_xfer_partial.
> 

Sorry, I meant dcache_xfer_partial.

-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Daniel Jacobowitz]

On Wed, Jul 08, 2009 at 09:46:40PM +0100, Pedro Alves wrote:
> On Wednesday 08 July 2009 21:08:00, Jacob Potter write:
> > --- a/gdb/thread.c
> > +++ b/gdb/thread.c
> > @@ -848,6 +848,9 @@ switch_to_thread (ptid_t ptid)
> >    if (ptid_equal (ptid, inferior_ptid))
> >      return;
> >  
> > +  if (ptid_get_pid (ptid) != ptid_get_pid (inferior_ptid))
> > +    dcache_invalidate (target_dcache);
> > +
> 
> I'm not sure this would be 100% multi-address space safe.
> 
> Do we not have places where we switch inferior_ptid temporarily
> before calling reading memory, with save_inferior_ptid, without
> going through the high level switch_to_thread ?
> 
> What if we do this within dcache itself, similarly
> to get_thread_regcache?  That would be probably in memory_xfer_partial.

Or could we store a dcache per-inferior?  Jacob's right - I thought
there was an 'inferior_data' to store arbitrary data per-inferior,
but there isn't.  I don't like baking knowledge into other modules
of GDB that they can extract the PID and use it to key per-inferior
data.

Or just add it to struct inferior?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Wednesday 08 July 2009 21:51:40, Daniel Jacobowitz wrote:
> Or could we store a dcache per-inferior?  Jacob's right - I thought
> there was an 'inferior_data' to store arbitrary data per-inferior,
> but there isn't.  I don't like baking knowledge into other modules
> of GDB that they can extract the PID and use it to key per-inferior
> data.
> 
> Or just add it to struct inferior?
> 

The reason I didn't suggest that is that we don't have an
address space object --- yet.  My multi-exec patches add one, and
there's a similar mechanism to per-objfile data where we can
do things like these.  More than one inferior can share the
same address space, so we could share a cache between them, but,
if the cache if write-through, it won't be a big problem if we
don't ---  it would be if it was left as writeback.

I don't have a problem with adding this to struct inferior for now,
it just seemed premature.

-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Wednesday 08 July 2009 21:58:44, Pedro Alves wrote:
> On Wednesday 08 July 2009 21:51:40, Daniel Jacobowitz wrote:
> > Or could we store a dcache per-inferior?  Jacob's right - I thought
> > there was an 'inferior_data' to store arbitrary data per-inferior,
> > but there isn't.  

> > I don't like baking knowledge into other modules 
> > of GDB that they can extract the PID and use it to key per-inferior
> > data.
> > 
> > Or just add it to struct inferior?

To complete a thought: if not adding a generic inferior_data so that
modules can put whatever they want there, then putting a new member
on struct inferior exposes some module's internal detail to the rest
of GDB.  I'm not sure I like that better either.  Note that we
always get the inferior from the pid (it's mostly unavoidable, 
we need a way to map a target reported pid to an internal inferior).
Even current_inferior() does that.

> The reason I didn't suggest that is that we don't have an
> address space object --- yet.  My multi-exec patches add one, and
> there's a similar mechanism to per-objfile data where we can
> do things like these.  More than one inferior can share the
> same address space, 

Forgot to be explicit on a real, current, example of
this: debugging parent/child vforks.

> so we could share a cache between them, but,  
> if the cache if write-through, it won't be a big problem if we
> don't ---  it would be if it was left as writeback.
> 
> I don't have a problem with adding this to struct inferior for now,
> it just seemed premature.
> 

Actually, I think that if you make it per-inferior, and *don't flush it
when switching inferiors*, making it per-inferior introduces a bug
visible when debugging vforks.  Say: stay attached to both parent/child,
get a backtrace when selecting the child; switch to the parent, change
some piece of memory that would be in the parent's stack dcache; switch
back to parent, and observe that the stack cache is now stale.

-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Jacob Potter]

Adding to struct inferior makes sense to me. Would we then just call
current_inferior() in memory_xfer_partial()?

On Wed, Jul 8, 2009 at 1:51 PM, Daniel Jacobowitz<drow@false.org> wrote:
> On Wed, Jul 08, 2009 at 09:46:40PM +0100, Pedro Alves wrote:
>> On Wednesday 08 July 2009 21:08:00, Jacob Potter write:
>> > --- a/gdb/thread.c
>> > +++ b/gdb/thread.c
>> > @@ -848,6 +848,9 @@ switch_to_thread (ptid_t ptid)
>> >    if (ptid_equal (ptid, inferior_ptid))
>> >      return;
>> >
>> > +  if (ptid_get_pid (ptid) != ptid_get_pid (inferior_ptid))
>> > +    dcache_invalidate (target_dcache);
>> > +
>>
>> I'm not sure this would be 100% multi-address space safe.
>>
>> Do we not have places where we switch inferior_ptid temporarily
>> before calling reading memory, with save_inferior_ptid, without
>> going through the high level switch_to_thread ?
>>
>> What if we do this within dcache itself, similarly
>> to get_thread_regcache?  That would be probably in memory_xfer_partial.
>
> Or could we store a dcache per-inferior?  Jacob's right - I thought
> there was an 'inferior_data' to store arbitrary data per-inferior,
> but there isn't.  I don't like baking knowledge into other modules
> of GDB that they can extract the PID and use it to key per-inferior
> data.
>
> Or just add it to struct inferior?
>
> --
> Daniel Jacobowitz
> CodeSourcery
>

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Friday 10 July 2009 00:59:30, Jacob Potter wrote:
> Adding to struct inferior makes sense to me. Would we then just call
> current_inferior() in memory_xfer_partial()?

You can't do that unconditionaly.  struct inferior's are mostly a
process_stratum and inferior control entity: it is an internal error to
call it when there's no current inferior (but note that it is still valid
to read memory from the executable).

> 
> On Wed, Jul 8, 2009 at 1:51 PM, Daniel Jacobowitz<drow@false.org> wrote:
> > On Wed, Jul 08, 2009 at 09:46:40PM +0100, Pedro Alves wrote:
> >> On Wednesday 08 July 2009 21:08:00, Jacob Potter write:
> >> > --- a/gdb/thread.c
> >> > +++ b/gdb/thread.c
> >> > @@ -848,6 +848,9 @@ switch_to_thread (ptid_t ptid)
> >> >    if (ptid_equal (ptid, inferior_ptid))
> >> >      return;
> >> >
> >> > +  if (ptid_get_pid (ptid) != ptid_get_pid (inferior_ptid))
> >> > +    dcache_invalidate (target_dcache);
> >> > +
> >>
> >> I'm not sure this would be 100% multi-address space safe.
> >>
> >> Do we not have places where we switch inferior_ptid temporarily
> >> before calling reading memory, with save_inferior_ptid, without
> >> going through the high level switch_to_thread ?
> >>
> >> What if we do this within dcache itself, similarly
> >> to get_thread_regcache?  That would be probably in memory_xfer_partial.
> >
> > Or could we store a dcache per-inferior?  Jacob's right - I thought
> > there was an 'inferior_data' to store arbitrary data per-inferior,
> > but there isn't.  I don't like baking knowledge into other modules
> > of GDB that they can extract the PID and use it to key per-inferior
> > data.
> >
> > Or just add it to struct inferior?
> >
> > --
> > Daniel Jacobowitz
> > CodeSourcery
> >
> 



-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Jacob Potter]

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

On Fri, Jul 10, 2009 at 2:33 AM, Pedro Alves<pedro@codesourcery.com> wrote:
> You can't do that unconditionaly.  struct inferior's are mostly a
> process_stratum and inferior control entity: it is an internal error to
> call it when there's no current inferior (but note that it is still valid
> to read memory from the executable).

OK, how's this look? I've:
- moved target_dcache to be in struct inferior
- added documentation on the flag changes to gdb.texinfo

- Jacob

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

diff --git a/gdb/NEWS b/gdb/NEWS
index b444f74..b70bcc9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -345,6 +345,12 @@ show schedule-multiple
   Allow GDB to resume all threads of all processes or only threads of
   the current process.
 
+set stackcache
+show stackcache
+  Use more aggressive caching for accesses to the stack. This improves
+  performance of remote debugging (particularly tracebacks) without
+  affecting correctness.
+
 * Removed commands
 
 info forks
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 6de0772..e70688e 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/dcache.c b/gdb/dcache.c
index 06956cb..cf531df 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -116,15 +116,23 @@ static void dcache_info (char *exp, int tty);
 
 void _initialize_dcache (void);
 
-static int dcache_enabled_p = 0;
+int dcache_stack_enabled_p = 1;
+
+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 */
 
@@ -135,6 +143,8 @@ dcache_invalidate (DCACHE *dcache)
 {
   struct dcache_block *block, *next;
 
+  if (!dcache) return;
+
   block = dcache->oldest;
 
   while (block)
@@ -159,13 +169,17 @@ dcache_invalidate (DCACHE *dcache)
 static struct dcache_block *
 dcache_hit (DCACHE *dcache, CORE_ADDR addr)
 {
+  struct dcache_block *db;
+
   splay_tree_node node = splay_tree_lookup (dcache->tree,
 					    (splay_tree_key) MASK(addr));
 
-  if (node)
-    return (struct dcache_block *) node->value;
-  else
+  if (!node)
     return NULL;
+
+  db = (struct dcache_block *) node->value;
+  db->refs++;
+  return db;
 }
 
 
@@ -506,20 +520,33 @@ dcache_info (char *exp, int tty)
 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 on."),
 			   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."));
+Print information on the dcache performance.\n\
+With no arguments, this command prints the cache configuration and a\n\
+summary of each line in the cache.  Use \"info dcache <lineno> to dump\"\n\
+the contents of a given line."));
 }
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fc5e60f..6280e97 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8397,32 +8397,47 @@ character.
 @section Caching Data of Remote Targets
 @cindex caching data of remote targets
 
-@value{GDBN} can cache data exchanged between the debugger and a
+@value{GDBN} caches data exchanged between the debugger and a
 remote target (@pxref{Remote Debugging}).  Such caching generally improves
 performance, because it reduces the overhead of the remote protocol by
-bundling memory reads and writes into large chunks.  Unfortunately,
-@value{GDBN} does not currently know anything about volatile
-registers, and thus data caching will produce incorrect results when
-volatile registers are in use.
+bundling memory reads and writes into large chunks.  Unfortunately, simply
+caching everything would lead to incorrect results, since @value{GDBN} 
+does not necessarily know anything about volatile values, memory-mapped I/O
+addresses, etc.  Therefore, by default, @value{GDBN} only caches data
+known to be on the stack.  Other regions of memory can be explicitly marked
+cacheable; see @pxref{Memory Region Attributes}.
 
 @table @code
 @kindex set remotecache
 @item set remotecache on
 @itemx set remotecache off
-Set caching state for remote targets.  When @code{ON}, use data
-caching.  By default, this option is @code{OFF}.
+This option no longer does anything; it exists for compatibility
+with old scripts.
 
 @kindex show remotecache
 @item show remotecache
-Show the current state of data caching for remote targets.
+Show the current state of the obsolete remotecache flag.
+
+@kindex set stackcache
+@item set stackcache on
+@itemx set stackcache off
+Enable or diable caching of stack accesses.  When @code{ON}, use
+caching.  By default, this option is @code{ON}.
+
+@kindex show remotecache
+@item show remotecache
+Show the current state of data caching for stack accesses.
 
 @kindex info dcache
-@item info dcache
+@item info dcache @r{[}line@r{]}
 Print the information about the data cache performance.  The
-information displayed includes: the dcache width and depth; and for
-each cache line, how many times it was referenced, and its data and
-state (invalid, dirty, valid).  This command is useful for debugging
-the data cache operation.
+information displayed includes the dcache width and depth, and for
+each cache line, its number, address, and how many times it was
+referenced.  This command is useful for debugging the data cache
+operation.
+
+If a line number is specified, the contents of that line will be
+printed in hex.
 @end table
 
 @node Searching Memory
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index b163231..da8f53c 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -271,6 +271,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 238c6a1..f0c2a64 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -153,8 +153,10 @@ struct value *
 frame_unwind_got_memory (struct frame_info *frame, int regnum, CORE_ADDR addr)
 {
   struct gdbarch *gdbarch = frame_unwind_arch (frame);
+  struct value *v = value_at_lazy (register_type (gdbarch, regnum), addr);
 
-  return value_at_lazy (register_type (gdbarch, regnum), addr);
+  set_value_stack (v, 1);
+  return v;
 }
 
 /* Return a value which indicates that FRAME's saved version of
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index e339c0b..7a7dcb2 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/inferior.c b/gdb/inferior.c
index 43eacda..8b0964c 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -49,6 +49,7 @@ free_inferior (struct inferior *inf)
 {
   discard_all_inferior_continuations (inf);
   xfree (inf->private);
+  dcache_free (inf->dcache);
   xfree (inf);
 }
 
@@ -85,6 +86,8 @@ add_inferior_silent (int pid)
   inf->next = inferior_list;
   inferior_list = inf;
 
+  inf->dcache = dcache_init ();
+
   observer_notify_new_inferior (pid);
 
   return inf;
@@ -285,6 +288,14 @@ have_inferiors (void)
   return inferior_list != NULL;
 }
 
+void
+invalidate_inferior_dcache (void)
+{
+  struct inferior *inf = find_inferior_pid (ptid_get_pid (inferior_ptid));
+  if (!inf) return;
+  dcache_invalidate (inf->dcache);
+}
+
 int
 have_live_inferiors (void)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 7312e51..f3690b6 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -423,6 +423,9 @@ struct inferior
   /* Terminal info and state managed by inflow.c.  */
   struct terminal_info *terminal_info;
 
+  /* Data cache used by dcache.c.  */
+  DCACHE *dcache;
+
   /* Private data used by the target vector implementation.  */
   struct private_inferior *private;
 };
@@ -494,6 +497,10 @@ extern void print_inferior (struct ui_out *uiout, int requested_inferior);
 /* Returns true if the inferior list is not empty.  */
 extern int have_inferiors (void);
 
+/* If there is an active inferior, invalidate its dcache.  If not,
+   do nothing.  */
+extern void invalidate_inferior_dcache (void);
+
 /* Returns true if there are any live inferiors in the inferior list
    (not cores, not executables, real live processes).  */
 extern int have_live_inferiors (void);
diff --git a/gdb/memattr.c b/gdb/memattr.c
index 356b4d6..5030e06 100644
--- a/gdb/memattr.c
+++ b/gdb/memattr.c
@@ -27,6 +27,7 @@
 #include "language.h"
 #include "vec.h"
 #include "gdb_string.h"
+#include "inferior.h"
 
 const struct mem_attrib default_mem_attrib =
 {
@@ -571,7 +572,7 @@ mem_enable_command (char *args, int from_tty)
 
   require_user_regions (from_tty);
 
-  dcache_invalidate (target_dcache);
+  invalidate_inferior_dcache ();
 
   if (p == 0)
     {
@@ -625,7 +626,7 @@ mem_disable_command (char *args, int from_tty)
 
   require_user_regions (from_tty);
 
-  dcache_invalidate (target_dcache);
+  invalidate_inferior_dcache ();
 
   if (p == 0)
     {
@@ -686,7 +687,7 @@ mem_delete_command (char *args, int from_tty)
 
   require_user_regions (from_tty);
 
-  dcache_invalidate (target_dcache);
+  invalidate_inferior_dcache ();
 
   if (p == 0)
     {
diff --git a/gdb/target.c b/gdb/target.c
index 0623bc3..ab73936 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -210,8 +210,6 @@ show_targetdebug (struct ui_file *file, int from_tty,
 
 static void setup_target_debug (void);
 
-DCACHE *target_dcache;
-
 /* The user just typed 'target' without the name of a target.  */
 
 static void
@@ -413,7 +411,7 @@ target_kill (void)
 void
 target_load (char *arg, int from_tty)
 {
-  dcache_invalidate (target_dcache);
+  invalidate_inferior_dcache ();
   (*current_target.to_load) (arg, from_tty);
 }
 
@@ -1143,12 +1141,14 @@ 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;
   struct mem_region *region;
+  struct inferior *inf;
 
   /* Zero length requests are ok and require no work.  */
   if (len == 0)
@@ -1223,16 +1223,20 @@ memory_xfer_partial (struct target_ops *ops, void *readbuf, const void *writebuf
       return -1;
     }
 
-  if (region->attrib.cache)
+  inf = find_inferior_pid (ptid_get_pid (inferior_ptid));
+
+  if ((region->attrib.cache
+       || (dcache_stack_enabled_p && object == TARGET_OBJECT_STACK_MEMORY))
+      && inf)
     {
       if (readbuf != NULL)
-	res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
+	res = dcache_xfer_memory (ops, inf->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 (ops, target_dcache, memaddr,
+	res = dcache_xfer_memory (ops, inf->dcache, memaddr,
 				  (void *) writebuf,
 				  reg_len, 1);
       if (res <= 0)
@@ -1245,6 +1249,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 (inf && readbuf == NULL && !region->attrib.cache &&
+      dcache_stack_enabled_p && object != TARGET_OBJECT_STACK_MEMORY)
+    {
+      dcache_update (inf->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
@@ -1308,8 +1321,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;
@@ -1391,6 +1405,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)
 {
@@ -2031,7 +2062,7 @@ target_resume (ptid_t ptid, int step, enum target_signal signal)
 {
   struct target_ops *t;
 
-  dcache_invalidate (target_dcache);
+  invalidate_inferior_dcache ();
 
   for (t = current_target.beneath; t != NULL; t = t->beneath)
     {
@@ -3453,6 +3484,4 @@ Tells gdb whether to control the inferior in asynchronous mode."),
 			   show_maintenance_target_async_permitted,
 			   &setlist,
 			   &showlist);
-
-  target_dcache = dcache_init ();
 }
diff --git a/gdb/target.h b/gdb/target.h
index c425a5b..351cc32 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.  */
@@ -676,6 +680,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/thread.c b/gdb/thread.c
index a7ac3c8..ca36349 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -849,6 +849,7 @@ switch_to_thread (ptid_t ptid)
     return;
 
   inferior_ptid = ptid;
+  invalidate_inferior_dcache ();
   reinit_frame_cache ();
   registers_changed ();
 
diff --git a/gdb/valops.c b/gdb/valops.c
index 2d20b41..c64598c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -565,6 +565,32 @@ 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)
+{
+  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);
+      read_memory (addr, value_contents_all_raw (val), TYPE_LENGTH (type));
+    }
+
+  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;
@@ -580,19 +606,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);
 }
 
 /* Return a lazy value with type TYPE located at ADDR (cf. value_at).  */
@@ -600,17 +614,7 @@ 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."));
-
-  val = allocate_value_lazy (type);
-
-  VALUE_LVAL (val) = lval_memory;
-  set_value_address (val, addr);
-
-  return val;
+  return get_value_at (type, addr, 1);
 }
 
 /* Called only from the value_contents and value_contents_all()
@@ -637,8 +641,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 fffc183..fd0df02 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -191,6 +191,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;
@@ -427,6 +431,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 d816156..9259768 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

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

On Wed, Jul 8, 2009 at 1:46 PM, Pedro Alves<pedro@codesourcery.com> wrote:
> On Wednesday 08 July 2009 21:08:00, Jacob Potter write:
>> --- a/gdb/thread.c
>> +++ b/gdb/thread.c
>> @@ -848,6 +848,9 @@ switch_to_thread (ptid_t ptid)
>>    if (ptid_equal (ptid, inferior_ptid))
>>      return;
>>
>> +  if (ptid_get_pid (ptid) != ptid_get_pid (inferior_ptid))
>> +    dcache_invalidate (target_dcache);
>> +
>
> I'm not sure this would be 100% multi-address space safe.
>
> Do we not have places where we switch inferior_ptid temporarily
> before calling reading memory, with save_inferior_ptid, without
> going through the high level switch_to_thread ?

Yeah.

> What if we do this within dcache itself, similarly
> to get_thread_regcache?  That would be probably in [dcache_xfer_partial].

It seems that given that we can temporarily change inferiors without
giving subsystems notice of the change, and given vfork, then we need
to have intelligence in dcache to handle this (and then it's not clear
if we should keep one dcache per inferior).

How about having memory_xfer_partial notify dcache of every
write/read, and then dcache could keep just one copy of the cache and
flush it appropriately?

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

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

On Thu, Aug 20, 2009 at 11:00 PM, Doug Evans<dje@google.com> wrote:
> On Wed, Jul 8, 2009 at 1:46 PM, Pedro Alves<pedro@codesourcery.com> wrote:
>> On Wednesday 08 July 2009 21:08:00, Jacob Potter write:
>>> --- a/gdb/thread.c
>>> +++ b/gdb/thread.c
>>> @@ -848,6 +848,9 @@ switch_to_thread (ptid_t ptid)
>>>    if (ptid_equal (ptid, inferior_ptid))
>>>      return;
>>>
>>> +  if (ptid_get_pid (ptid) != ptid_get_pid (inferior_ptid))
>>> +    dcache_invalidate (target_dcache);
>>> +
>>
>> I'm not sure this would be 100% multi-address space safe.
>>
>> Do we not have places where we switch inferior_ptid temporarily
>> before calling reading memory, with save_inferior_ptid, without
>> going through the high level switch_to_thread ?
>
> Yeah.
>
>> What if we do this within dcache itself, similarly
>> to get_thread_regcache?  That would be probably in [dcache_xfer_partial].
>
> It seems that given that we can temporarily change inferiors without
> giving subsystems notice of the change, and given vfork, then we need
> to have intelligence in dcache to handle this (and then it's not clear
> if we should keep one dcache per inferior).
>
> How about having memory_xfer_partial notify dcache of every
> write/read, and then dcache could keep just one copy of the cache and
> flush it appropriately?
>

Something like this?

2009-08-24  Jacob Potter  <jdpotter@google.com>
            Doug Evans  <dje@google.com>

        Implement TARGET_OBJECT_STACK_MEMORY.
        * NEWS: Add note on new "set stack-cache" option.
        * corefile.c (read_stack): New function.
        * dcache.c (dcache_struct): New member ptid.
        (dcache_enable_p): Mark as obsolete.
        (stack_cache_enabled_p): New static global.
        (show_dcache_enabled_p): Flag option as deprecated.
        (show_stack_cache_enabled_p): New function.
        (dcache_invalidate): Update ptid.
        (dcache_read_line): No longer check cacheable attribute, stack
        accesses get cached despite attribute.
        (dcache_init): Set ptid.
        (dcache_xfer_memory): Flush cache if from different ptid than before.
        (dcache_update): New function.
        (dcache_info): Report ptid.
        (_initialize_dcache): Update text for `remotecache' to indicate it
        is obsolete.  Install new option `stack-cache'.
        * dcache.h (dcache_update): Declare.
        (stack_cache_enabled_p): Declare.
        * dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
        set_value_stack.
        * frame-unwind.c (frame_unwind_got_memory): Ditto.
        * gdbcore.h (read_stack): Declare.
        * target.c (memory_xfer_partial): New arg object, all callers updated.
        Check for existing inferior before calling dcache routines.
        When writing non-TARGET_OBJECT_STACK_MEMORY, notify dcache.
        (target_xfer_partial): Call memory_xfer_partial for
        TARGET_OBJECT_STACK_MEMORY.
        (target_read_stack): New function.
        * target.h (enum target_object): New value TARGET_OBJECT_STACK_MEMORY.
        (target_read_stack): Declare.
        * valops.c (get_value_at): New function.
        (value_at): Guts moved to get_value_at.
        (value_at_lazy): Similarly.
        (value_fetch_lazy): Call read_stack for stack values.
        * value.c (struct value): New member `stack'.
        (value_stack, set_value_stack): New functions.
        * value.h (value_stack, set_value_stack): Declare.

        * doc/gdb.texinfo (Caching Data of Remote Targets): Update text.
        (set remotecache): Mark option as obsolete.
        (set stack-cache): Document new option.
        (info dcache): Update text.

[-- Attachment #2: 090824-jdpotter-stack-cache-3.patch.txt --]
[-- Type: text/plain, Size: 23331 bytes --]

2009-08-24  Jacob Potter  <jdpotter@google.com>
	    Doug Evans  <dje@google.com>

	Implement TARGET_OBJECT_STACK_MEMORY.
	* NEWS: Add note on new "set stack-cache" option.
	* corefile.c (read_stack): New function.
	* dcache.c (dcache_struct): New member ptid.
	(dcache_enable_p): Mark as obsolete.
	(stack_cache_enabled_p): New static global.
	(show_dcache_enabled_p): Flag option as deprecated.
	(show_stack_cache_enabled_p): New function.
	(dcache_invalidate): Update ptid.
	(dcache_read_line): No longer check cacheable attribute, stack
	accesses get cached despite attribute.
	(dcache_init): Set ptid.
	(dcache_xfer_memory): Flush cache if from different ptid than before.
	(dcache_update): New function.
	(dcache_info): Report ptid.
	(_initialize_dcache): Update text for `remotecache' to indicate it
	is obsolete.  Install new option `stack-cache'.
	* dcache.h (dcache_update): Declare.
	(stack_cache_enabled_p): Declare.
	* dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
	set_value_stack.
	* frame-unwind.c (frame_unwind_got_memory): Ditto.
	* gdbcore.h (read_stack): Declare.
	* target.c (memory_xfer_partial): New arg object, all callers updated.
	Check for existing inferior before calling dcache routines.
	When writing non-TARGET_OBJECT_STACK_MEMORY, notify dcache.
	(target_xfer_partial): Call memory_xfer_partial for
	TARGET_OBJECT_STACK_MEMORY.
	(target_read_stack): New function.
	* target.h (enum target_object): New value TARGET_OBJECT_STACK_MEMORY.
	(target_read_stack): Declare.
	* valops.c (get_value_at): New function.
	(value_at): Guts moved to get_value_at.
	(value_at_lazy): Similarly.
	(value_fetch_lazy): Call read_stack for stack values.
	* value.c (struct value): New member `stack'.
	(value_stack, set_value_stack): New functions.
	* value.h (value_stack, set_value_stack): Declare.

	* doc/gdb.texinfo (Caching Data of Remote Targets): Update text.
	(set remotecache): Mark option as obsolete.
	(set stack-cache): Document new option.
	(info dcache): Update text.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.326
diff -u -p -r1.326 NEWS
--- NEWS	20 Aug 2009 18:02:47 -0000	1.326
+++ NEWS	25 Aug 2009 00:23:18 -0000
@@ -395,6 +395,12 @@ show schedule-multiple
   Allow GDB to resume all threads of all processes or only threads of
   the current process.
 
+set stack-cache
+show stack-cache
+  Use more aggressive caching for accesses to the stack. This improves
+  performance of remote debugging (particularly backtraces) without
+  affecting correctness.
+
 * Removed commands
 
 info forks
Index: corefile.c
===================================================================
RCS file: /cvs/src/src/gdb/corefile.c,v
retrieving revision 1.54
diff -u -p -r1.54 corefile.c
--- corefile.c	2 Jul 2009 17:25:53 -0000	1.54
+++ corefile.c	25 Aug 2009 00:23:18 -0000
@@ -228,6 +228,7 @@ memory_error (int status, CORE_ADDR mema
 }
 
 /* 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
     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
Index: dcache.c
===================================================================
RCS file: /cvs/src/src/gdb/dcache.c,v
retrieving revision 1.35
diff -u -p -r1.35 dcache.c
--- dcache.c	20 Aug 2009 23:30:15 -0000	1.35
+++ dcache.c	25 Aug 2009 00:23:18 -0000
@@ -24,6 +24,7 @@
 #include "gdb_string.h"
 #include "gdbcore.h"
 #include "target.h"
+#include "inferior.h"
 #include "splay-tree.h"
 
 /* The data cache could lead to incorrect results because it doesn't
@@ -103,6 +104,9 @@ struct dcache_struct
 
   /* The number of in-use lines in the cache.  */
   int size;
+
+  /* The ptid of last inferior to use cache or null_ptid.  */
+  ptid_t ptid;
 };
 
 static struct dcache_block *dcache_hit (DCACHE *dcache, CORE_ADDR addr);
@@ -117,15 +121,23 @@ static void dcache_info (char *exp, int 
 
 void _initialize_dcache (void);
 
-static int dcache_enabled_p = 0;
+static int dcache_enabled_p = 0; /* OBSOLETE */
+
+int stack_cache_enabled_p = 1;
 
 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, _("Deprecated remotecache flag is %s.\n"), value);
 }
 
+static void
+show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
+			    struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Cache use for stack accesses is %s.\n"), value);
+}
 
 static DCACHE *last_cache; /* Used by info dcache */
 
@@ -152,6 +164,7 @@ dcache_invalidate (DCACHE *dcache)
   dcache->oldest = NULL;
   dcache->newest = NULL;
   dcache->size = 0;
+  dcache->ptid = null_ptid;
 }
 
 /* If addr is present in the dcache, return the address of the block
@@ -198,8 +211,9 @@ dcache_read_line (DCACHE *dcache, struct
       else
 	reg_len = region->hi - memaddr;
 
-      /* Skip non-cacheable/non-readable regions.  */
-      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;
@@ -338,6 +352,7 @@ dcache_init (void)
   dcache->newest = NULL;
   dcache->freelist = NULL;
   dcache->size = 0;
+  dcache->ptid = null_ptid;
   last_cache = dcache;
 
   return dcache;
@@ -378,6 +393,15 @@ dcache_xfer_memory (struct target_ops *o
   int (*xfunc) (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr);
   xfunc = should_write ? dcache_poke_byte : dcache_peek_byte;
 
+  /* If this is a different inferior from what we've recorded,
+     flush the cache.  */
+
+  if (! ptid_equal (inferior_ptid, dcache->ptid))
+    {
+      dcache_invalidate (dcache);
+      dcache->ptid = inferior_ptid;
+    }
+
   /* Do write-through first, so that if it fails, we don't write to
      the cache at all.  */
 
@@ -407,6 +431,18 @@ dcache_xfer_memory (struct target_ops *o
    "logically" connected but not actually a single call to one of the
    memory transfer functions.  */
 
+/* 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 index)
 {
@@ -474,12 +510,15 @@ dcache_info (char *exp, int tty)
   printf_filtered (_("Dcache line width %d, maximum size %d\n"),
 		   LINE_SIZE, DCACHE_SIZE);
 
-  if (!last_cache)
+  if (!last_cache || ptid_equal (last_cache->ptid, null_ptid))
     {
       printf_filtered (_("No data cache available.\n"));
       return;
     }
 
+  printf_filtered (_("Contains data for %s\n"),
+		   target_pid_to_str (last_cache->ptid));
+
   refcount = 0;
 
   n = splay_tree_min (last_cache->tree);
@@ -507,15 +546,25 @@ _initialize_dcache (void)
 			   &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."),
+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\
+\"stack-cache\" flag; \"remotecache\" now does nothing and\n\
+exists only for compatibility reasons."),
 			   NULL,
 			   show_dcache_enabled_p,
 			   &setlist, &showlist);
 
+  add_setshow_boolean_cmd ("stack-cache", class_support,
+			   &stack_cache_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 on."),
+			   NULL,
+			   show_stack_cache_enabled_p,
+			   &setlist, &showlist);
+
   add_info ("dcache", dcache_info,
 	    _("\
 Print information on the dcache performance.\n\
Index: dcache.h
===================================================================
RCS file: /cvs/src/src/gdb/dcache.h,v
retrieving revision 1.15
diff -u -p -r1.15 dcache.h
--- dcache.h	20 Aug 2009 22:30:12 -0000	1.15
+++ dcache.h	25 Aug 2009 00:23:18 -0000
@@ -38,4 +38,10 @@ void dcache_free (DCACHE *);
 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 stack_cache_enabled_p;
+
 #endif /* DCACHE_H */
Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.63
diff -u -p -r1.63 dwarf2loc.c
--- dwarf2loc.c	11 Aug 2009 20:36:49 -0000	1.63
+++ dwarf2loc.c	25 Aug 2009 00:23:18 -0000
@@ -280,6 +280,7 @@ dwarf2_evaluate_loc_desc (struct symbol 
       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);
     }
 
Index: frame-unwind.c
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.c,v
retrieving revision 1.26
diff -u -p -r1.26 frame-unwind.c
--- frame-unwind.c	2 Jul 2009 17:25:53 -0000	1.26
+++ frame-unwind.c	25 Aug 2009 00:23:18 -0000
@@ -153,8 +153,10 @@ struct value *
 frame_unwind_got_memory (struct frame_info *frame, int regnum, CORE_ADDR addr)
 {
   struct gdbarch *gdbarch = frame_unwind_arch (frame);
+  struct value *v = value_at_lazy (register_type (gdbarch, regnum), addr);
 
-  return value_at_lazy (register_type (gdbarch, regnum), addr);
+  set_value_stack (v, 1);
+  return v;
 }
 
 /* Return a value which indicates that FRAME's saved version of
Index: gdbcore.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbcore.h,v
retrieving revision 1.35
diff -u -p -r1.35 gdbcore.h
--- gdbcore.h	2 Jul 2009 17:25:53 -0000	1.35
+++ gdbcore.h	25 Aug 2009 00:23:18 -0000
@@ -47,6 +47,10 @@ extern void memory_error (int status, CO
 
 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.  */
 
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.221
diff -u -p -r1.221 target.c
--- target.c	20 Aug 2009 22:30:12 -0000	1.221
+++ target.c	25 Aug 2009 00:23:18 -0000
@@ -210,6 +210,7 @@ show_targetdebug (struct ui_file *file, 
 
 static void setup_target_debug (void);
 
+/* Cache of memory operations, to speed up remote access.  */
 DCACHE *target_dcache;
 
 /* The user just typed 'target' without the name of a target.  */
@@ -1143,12 +1144,14 @@ target_section_by_addr (struct target_op
    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;
   struct mem_region *region;
+  struct inferior *inf;
 
   /* Zero length requests are ok and require no work.  */
   if (len == 0)
@@ -1223,7 +1226,11 @@ memory_xfer_partial (struct target_ops *
       return -1;
     }
 
-  if (region->attrib.cache)
+  inf = find_inferior_pid (ptid_get_pid (inferior_ptid));
+
+  if (inf
+      && (region->attrib.cache
+	  || (stack_cache_enabled_p && object == TARGET_OBJECT_STACK_MEMORY)))
     {
       if (readbuf != NULL)
 	res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
@@ -1245,6 +1252,19 @@ memory_xfer_partial (struct target_ops *
 	}
     }
 
+  /* 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 (inf
+      && readbuf == NULL
+      && !region->attrib.cache
+      && stack_cache_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
@@ -1308,8 +1328,9 @@ target_xfer_partial (struct target_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;
@@ -1391,6 +1412,23 @@ target_read_memory (CORE_ADDR memaddr, g
     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)
 {
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.162
diff -u -p -r1.162 target.h
--- target.h	31 Jul 2009 15:28:27 -0000	1.162
+++ target.h	25 Aug 2009 00:23:18 -0000
@@ -203,6 +203,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.  */
@@ -677,6 +681,8 @@ extern int target_read_string (CORE_ADDR
 
 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);
 
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.224
diff -u -p -r1.224 valops.c
--- valops.c	21 Jul 2009 18:15:32 -0000	1.224
+++ valops.c	25 Aug 2009 00:23:18 -0000
@@ -565,6 +565,32 @@ value_one (struct type *type, enum lval_
   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)
+{
+  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);
+      read_memory (addr, value_contents_all_raw (val), TYPE_LENGTH (type));
+    }
+
+  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;
@@ -580,19 +606,7 @@ value_one (struct type *type, enum lval_
 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);
 }
 
 /* Return a lazy value with type TYPE located at ADDR (cf. value_at).  */
@@ -600,17 +614,7 @@ value_at (struct type *type, CORE_ADDR a
 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."));
-
-  val = allocate_value_lazy (type);
-
-  VALUE_LVAL (val) = lval_memory;
-  set_value_address (val, addr);
-
-  return val;
+  return get_value_at (type, addr, 1);
 }
 
 /* Called only from the value_contents and value_contents_all()
@@ -656,7 +660,12 @@ value_fetch_lazy (struct value *val)
       int length = TYPE_LENGTH (check_typedef (value_enclosing_type (val)));
 
       if (length)
-	read_memory (addr, value_contents_all_raw (val), 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)
     {
Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.93
diff -u -p -r1.93 value.c
--- value.c	19 Aug 2009 13:00:28 -0000	1.93
+++ value.c	25 Aug 2009 00:23:18 -0000
@@ -196,6 +196,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;
@@ -424,6 +428,18 @@ set_value_lazy (struct value *value, int
   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)
 {
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.148
diff -u -p -r1.148 value.h
--- value.h	13 Aug 2009 18:39:20 -0000	1.148
+++ value.h	25 Aug 2009 00:23:18 -0000
@@ -215,6 +215,9 @@ extern void *value_computed_closure (str
 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
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.616
diff -u -p -r1.616 gdb.texinfo
--- doc/gdb.texinfo	20 Aug 2009 18:02:48 -0000	1.616
+++ doc/gdb.texinfo	25 Aug 2009 00:23:19 -0000
@@ -8398,32 +8398,47 @@ character.
 @section Caching Data of Remote Targets
 @cindex caching data of remote targets
 
-@value{GDBN} can cache data exchanged between the debugger and a
+@value{GDBN} caches data exchanged between the debugger and a
 remote target (@pxref{Remote Debugging}).  Such caching generally improves
 performance, because it reduces the overhead of the remote protocol by
-bundling memory reads and writes into large chunks.  Unfortunately,
-@value{GDBN} does not currently know anything about volatile
-registers, and thus data caching will produce incorrect results when
-volatile registers are in use.
+bundling memory reads and writes into large chunks.  Unfortunately, simply
+caching everything would lead to incorrect results, since @value{GDBN} 
+does not necessarily know anything about volatile values, memory-mapped I/O
+addresses, etc.  Therefore, by default, @value{GDBN} only caches data
+known to be on the stack.  Other regions of memory can be explicitly marked
+cacheable; see @pxref{Memory Region Attributes}.
 
 @table @code
 @kindex set remotecache
 @item set remotecache on
 @itemx set remotecache off
-Set caching state for remote targets.  When @code{ON}, use data
-caching.  By default, this option is @code{OFF}.
+This option no longer does anything; it exists for compatibility
+with old scripts.
 
 @kindex show remotecache
 @item show remotecache
-Show the current state of data caching for remote targets.
+Show the current state of the obsolete remotecache flag.
+
+@kindex set stack-cache
+@item set stack-cache on
+@itemx set stack-cache off
+Enable or disable caching of stack accesses.  When @code{ON}, use
+caching.  By default, this option is @code{ON}.
+
+@kindex show stack-cache
+@item show stack-cache
+Show the current state of data caching for memory accesses.
 
 @kindex info dcache
-@item info dcache
+@item info dcache @r{[}line@r{]}
 Print the information about the data cache performance.  The
-information displayed includes: the dcache width and depth; and for
-each cache line, how many times it was referenced, and its data and
-state (invalid, dirty, valid).  This command is useful for debugging
-the data cache operation.
+information displayed includes the dcache width and depth, and for
+each cache line, its number, address, and how many times it was
+referenced.  This command is useful for debugging the data cache
+operation.
+
+If a line number is specified, the contents of that line will be
+printed in hex.
 @end table
 
 @node Searching Memory

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Tuesday 25 August 2009 01:48:30, Doug Evans wrote:
> On Thu, Aug 20, 2009 at 11:00 PM, Doug Evans<dje@google.com> wrote:
> > On Wed, Jul 8, 2009 at 1:46 PM, Pedro Alves<pedro@codesourcery.com> wrote:
> >> On Wednesday 08 July 2009 21:08:00, Jacob Potter write:

> >> What if we do this within dcache itself, similarly
> >> to get_thread_regcache?  That would be probably in [dcache_xfer_partial].
> >
> > It seems that given that we can temporarily change inferiors without
> > giving subsystems notice of the change, and given vfork, then we need
> > to have intelligence in dcache to handle this (and then it's not clear
> > if we should keep one dcache per inferior).
> >
> > How about having memory_xfer_partial notify dcache of every
> > write/read, and then dcache could keep just one copy of the cache and
> > flush it appropriately?

> Something like this?

Eh, that's exactly what I meant by 'similarly to get_thread_regcache'.
(Ulrich rewritten it since somewhat to keep more than one more than
one regcache live at once).

A few small nits: Please fix up a few missing
double-space-after-period instances, and here,

> +  if (inf
> +      && readbuf == NULL

both inf and readbuf are pointers, so please either make both
subpredicates compare with NULL or neither.  Hmm, actually, I'm not
sure  why you still need to check the inferior for nullness
in this version?

I think that the cache should be flushed with
"set stack-cache off" -> "set stack-cache on", you never
know what happened between these two commands, so you end up
with a stale cache.

If write_memory|stack tries to write to [foo,bar), and that
operation fails for some reason somewhere between foo and bar, I
think that the cache between somewhere and bar shouldn't be
updated with the new values.  Is it?

I worry about new stale cache issues in non-stop mode.  E.g.,
take this test case:

#include <pthread.h>

volatile int *foop;

void *
thread_function0 (void *arg)
{
  while (1)
    {
      if (foop)
	(*foop)++;
      usleep (1);
    }
}

void *
thread_function1 (void *arg)
{
  volatile int foo = 0;

  foop = &foo;

  while (1)
    {
      usleep (1);
    }
}

int
main ()
{
  pthread_t threads[2];
  void *result;

  pthread_create(&threads[0], NULL,
		 thread_function0, NULL);
  pthread_create(&threads[1], NULL,
		 thread_function1, NULL);
  pthread_join (threads[0], &result);
  pthread_join (threads[1], &result);
}

If you set a breakpoint on thread_function1 e.g, at
the usleep line, while letting thread_function0 run free,
you'll see that if you issue multiple "(gdb) print foo"
commands, with the cache on, you'll get the same stale
result.  

 (gdb) set stack-cache on
 (gdb) p foo
 $19 = 7482901
 (gdb) p foo
 $20 = 7482901
 (gdb) p foo
 $21 = 7482901

 (gdb) set stack-cache off
 (gdb) p foo
 $22 = 155394461
 (gdb) p foo
 $23 = 155541672
 (gdb) p foo
 $24 = 155642546

(note that the cache isn't flushed with the stack-cache command:)

 (gdb) set stack-cache on
 (gdb) p foo
 $25 = 7482901

It appears that (at least in non-stop or if any thread is running)
the cache should only be live for the duration of an "high level
operation" --- that is, for a "backtrace", or a "print", etc.
Did you consider this?


Did you post number showing off the improvements from
having the cache on?  E.g., when doing foo, with cache off,
I get NNN memory reads, while with cache off, we get only
nnn reads.  I'd be curious to have some backing behind
"This improves remote performance significantly".

-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

On Tue, Aug 25, 2009 at 11:44 AM, Pedro Alves<pedro@codesourcery.com> wrote:
> On Tuesday 25 August 2009 01:48:30, Doug Evans wrote:
>> On Thu, Aug 20, 2009 at 11:00 PM, Doug Evans<dje@google.com> wrote:
>> > On Wed, Jul 8, 2009 at 1:46 PM, Pedro Alves<pedro@codesourcery.com> wrote:
>> >> On Wednesday 08 July 2009 21:08:00, Jacob Potter write:
>
>> >> What if we do this within dcache itself, similarly
>> >> to get_thread_regcache?  That would be probably in [dcache_xfer_partial].
>> >
>> > It seems that given that we can temporarily change inferiors without
>> > giving subsystems notice of the change, and given vfork, then we need
>> > to have intelligence in dcache to handle this (and then it's not clear
>> > if we should keep one dcache per inferior).
>> >
>> > How about having memory_xfer_partial notify dcache of every
>> > write/read, and then dcache could keep just one copy of the cache and
>> > flush it appropriately?
>
>> Something like this?
>
> Eh, that's exactly what I meant by 'similarly to get_thread_regcache'.
> (Ulrich rewritten it since somewhat to keep more than one more than
> one regcache live at once).
>
> A few small nits: Please fix up a few missing
> double-space-after-period instances, and here,
>
>> +  if (inf
>> +      && readbuf == NULL
>
> both inf and readbuf are pointers, so please either make both
> subpredicates compare with NULL or neither.  Hmm, actually, I'm not
> sure  why you still need to check the inferior for nullness
> in this version?

It wasn't clear to me whether TRTTD was check inf for NULL either, but
the code can only be used if there is an inferior so I left it in.

> I think that the cache should be flushed with
> "set stack-cache off" -> "set stack-cache on", you never
> know what happened between these two commands, so you end up
> with a stale cache.

Righto.

> If write_memory|stack tries to write to [foo,bar), and that
> operation fails for some reason somewhere between foo and bar, I
> think that the cache between somewhere and bar shouldn't be
> updated with the new values.  Is it?

Indeed.  I recall looking at this but I'll go back and check.

> I worry about new stale cache issues in non-stop mode.
> [...]
> It appears that (at least in non-stop or if any thread is running)
> the cache should only be live for the duration of an "high level
> operation" --- that is, for a "backtrace", or a "print", etc.
> Did you consider this?

It wasn't clear how to handle non-stop/etc. mode so I left that for
the next iteration.
If only having the data live across a high level operation works for
you, it works for me.

> Did you post number showing off the improvements from
> having the cache on?  E.g., when doing foo, with cache off,
> I get NNN memory reads, while with cache off, we get only
> nnn reads.  I'd be curious to have some backing behind
> "This improves remote performance significantly".

For a typical gdb/gdbserver connection here a backtrace of 256 levels
went from 48 seconds (average over 6 tries) to 4 seconds (average over
6 tries).

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Wednesday 26 August 2009 17:29:40, Doug Evans wrote:
> On Tue, Aug 25, 2009 at 11:44 AM, Pedro Alves<pedro@codesourcery.com> wrote:
>
> > I worry about new stale cache issues in non-stop mode.
> > [...]
> > It appears that (at least in non-stop or if any thread is running)
> > the cache should only be live for the duration of an "high level
> > operation" --- that is, for a "backtrace", or a "print", etc.
> > Did you consider this?
> 
> It wasn't clear how to handle non-stop/etc. mode so I left that for
> the next iteration.
> If only having the data live across a high level operation works for
> you, it works for me.

Well, I'm not sure either, but much better to discuss it
upfront than to introduce subtle, hard to reproduce bugs
induced by GDB itself.

Reading things from memory while threads are running is
always racy --- if we want to get an accurate snapshot of
the inferior's memory, we *have* to stop all threads
temporarily.  If we don't (stop all threads), then, even if
the dcache gets stale while we do a series of micro
memory reads (that logically are part of a bigger higher level
operation --- extracting a backtrace, reading a structure from
memory, etc.), it's OK, we can just pretend the memory had
changed only after we did the reads instead of before.  If
we restart the higher level operation, we shouldn't not
hit the stale cache, and that seems good enough.

Writes are more dangerous though, since the dcache writes
back cache lines in chunks, meaning, there's the risk that
the dcache undoes changes the inferior had done meanwhile
to other parts of the cache line higher layers of gdb code
were not trying to write to (it's effectively a read-modify-write,
hence, racy).  This is a more general non-stop mode problem, not
strictly related to the dcache, e.g., ptrace writes to memory
do a word-sized read-modify-write, clearly a smaller risk than
a 64-byte wide cache line, which is itself small, but still.
The only cure for this is to stop all threads momentarily...


We're shifting the caching decision to the intent of the
transfer, but, we still cache whole lines (larger than the
read_stack request) --- Do we still have rare border cases
where the cache line can cover more than stack memory, hence
still leading to incorrect results?  Probably a very rare
problem in practice.  Even less problematic if the cache
is only live across high level operations (essentially
getting rid of most of the volatile memory problem).  Which
brings me to what amounts to chunking vs caching improvements
you are seeing:

> > Did you post number showing off the improvements from
> > having the cache on?  E.g., when doing foo, with cache off,
> > I get NNN memory reads, while with cache off, we get only
> > nnn reads.  I'd be curious to have some backing behind
> > "This improves remote performance significantly".
> 
> For a typical gdb/gdbserver connection here a backtrace of 256 levels
> went from 48 seconds (average over 6 tries) to 4 seconds (average over
> 6 tries).

Nice!  Were all those single runs started from cold cache, or
are you starting from a cold cache and issuing 6 backtraces in
a row?  I mean, how sparse were those 6 tries?  Shall one
read that as 48,48,48,48,48,48 vs 20,1,1,1,1,1 (some improvement
due to chunking, and large improvement due to caching in following
repeats of the command); or 48,48,48,48,48,48 vs 4,4,4,4,4,4 (large
improvement due to chunking --- caching not actually measured)?

-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

On Wed, Aug 26, 2009 at 1:08 PM, Pedro Alves<pedro@codesourcery.com> wrote:
>> > Did you post number showing off the improvements from
>> > having the cache on?  E.g., when doing foo, with cache off,
>> > I get NNN memory reads, while with cache off, we get only
>> > nnn reads.  I'd be curious to have some backing behind
>> > "This improves remote performance significantly".
>>
>> For a typical gdb/gdbserver connection here a backtrace of 256 levels
>> went from 48 seconds (average over 6 tries) to 4 seconds (average over
>> 6 tries).
>
> Nice!  Were all those single runs started from cold cache, or
> are you starting from a cold cache and issuing 6 backtraces in
> a row?  I mean, how sparse were those 6 tries?  Shall one
> read that as 48,48,48,48,48,48 vs 20,1,1,1,1,1 (some improvement
> due to chunking, and large improvement due to caching in following
> repeats of the command); or 48,48,48,48,48,48 vs 4,4,4,4,4,4 (large
> improvement due to chunking --- caching not actually measured)?

The cache was always flushed between backtraces, so that's
48, 48. ..., 48 vs 4, 4, ..., 4.

Backtraces win from both chunking and caching.
Even in one backtrace gdb will often fetch the same value multiple times.
I haven't computed the relative win.

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

On Wed, Aug 26, 2009 at 5:32 PM, Doug Evans<dje@google.com> wrote:
> On Wed, Aug 26, 2009 at 1:08 PM, Pedro Alves<pedro@codesourcery.com> wrote:
>>> > Did you post number showing off the improvements from
>>> > having the cache on?  E.g., when doing foo, with cache off,
>>> > I get NNN memory reads, while with cache off, we get only
>>> > nnn reads.  I'd be curious to have some backing behind
>>> > "This improves remote performance significantly".
>>>
>>> For a typical gdb/gdbserver connection here a backtrace of 256 levels
>>> went from 48 seconds (average over 6 tries) to 4 seconds (average over
>>> 6 tries).
>>
>> Nice!  Were all those single runs started from cold cache, or
>> are you starting from a cold cache and issuing 6 backtraces in
>> a row?  I mean, how sparse were those 6 tries?  Shall one
>> read that as 48,48,48,48,48,48 vs 20,1,1,1,1,1 (some improvement
>> due to chunking, and large improvement due to caching in following
>> repeats of the command); or 48,48,48,48,48,48 vs 4,4,4,4,4,4 (large
>> improvement due to chunking --- caching not actually measured)?
>
> The cache was always flushed between backtraces, so that's
> 48, 48. ..., 48 vs 4, 4, ..., 4.
>
> Backtraces win from both chunking and caching.
> Even in one backtrace gdb will often fetch the same value multiple times.
> I haven't computed the relative win.

Besides, the chunking doesn't really work without the caching. :-)

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

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

Here's is a revised version of the patch.
Thanks for the review.

Responses to comments:

> Flush cache between turning stack-cache on/off.

Done.

> If write_memory|stack tries to write to [foo,bar), and that
> operation fails for some reason somewhere between foo and bar, I
> think that the cache between somewhere and bar shouldn't be
> updated with the new values.  Is it?

The write is done first so that if it fails the cache isn't updated at all.
It's problem was that if it partially succeeded, the part that succeeded
didn't get updated.

> dcache writes back lines in chunks.

Actually, the dcache only writes to the target exactly what
target_xfer_partial would do.
Data point: For writes, if the line isn't already in cache, it is not
brought in.

> Do we still have rare border cases
> where the cache line can cover more than stack memory, hence
> still leading to incorrect results?

Yeah, if, for example, .data abuts the stack in just the right way,
it'll get cached when "set stack-cache on".
Pretty rare that it would be a problem.

> Cache becoming stale in non-stop mode.

The cache is now flushed between commands in non-stop mode.

---

2009-08-28  Jacob Potter  <jdpotter@google.com>
	    Doug Evans  <dje@google.com>

	Implement TARGET_OBJECT_STACK_MEMORY.
	* NEWS: Add note on new "set stack-cache" option.
	* corefile.c (read_stack): New function.
	* dcache.c (dcache_struct): New member ptid.
	(dcache_enable_p): Mark as obsolete.
	(show_dcache_enabled_p): Flag option as deprecated.
	(dcache_invalidate): Update ptid.
	(dcache_invalidate_line): New function.
	(dcache_read_line): No longer check cacheable attribute, stack
	accesses get cached despite attribute.
	(dcache_init): Set ptid.
	(dcache_xfer_memory): Flush cache if from different ptid than before.
	Update cache after write.
	(dcache_update): New function.
	(dcache_info): Report ptid.
	(_initialize_dcache): Update text for `remotecache' to indicate it
	is obsolete.
	* dcache.h (dcache_update): Declare.
	* dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
	set_value_stack.
	* frame-unwind.c (frame_unwind_got_memory): Ditto.
	* gdbcore.h (read_stack): Declare.
	* memattr.c (mem_enable_command): Call target_dcache_invalidate
	instead of dcache_invalidate.
	(mem_disable_command, mem_delete_command): Ditto.
	* target.c (stack_cache_enabled_p_1): New static global.
	(stack_cache_enabled_p): New static global.
	(set_stack_cache_enabled_p): New function.
	(show_stack_cache_enabled_p): New function.
	(target_dcache): Make static.
	(target_dcache_invalidate): New function.
	(target_load, target_resume): Call target_dcache_invalidate
	instead of dcache_invalidate.
	(memory_xfer_partial): New arg object, all callers updated.
	Check for existing inferior before calling dcache routines.
	When writing non-TARGET_OBJECT_STACK_MEMORY, notify dcache.
	(target_xfer_partial): Call memory_xfer_partial for
	TARGET_OBJECT_STACK_MEMORY.
	(target_read_stack): New function.
	(initialize_targets): Install new option `stack-cache'.
	* target.h: Remove #include of dcache.h.
	(enum target_object): New value TARGET_OBJECT_STACK_MEMORY.
	(target_dcache): Delete.
	(target_dcache_invalidate): Declare.
	(target_read_stack): Declare.
	* top.c (prepare_execute_command): New function.
	(execute_command): Call prepare_execute_command
	instead of free_all_values.
	* top.h (prepare_execute_command): Declare.
	* valops.c (get_value_at): New function.
	(value_at): Guts moved to get_value_at.
	(value_at_lazy): Similarly.
	(value_fetch_lazy): Call read_stack for stack values.
	* value.c (struct value): New member `stack'.
	(value_stack, set_value_stack): New functions.
	* value.h (value_stack, set_value_stack): Declare.
	* mi/mi-main.c (mi_cmd_execute): Call prepare_execute_command
	instead of free_all_values.

	* doc/gdb.texinfo (Caching Data of Remote Targets): Update text.
	Mark `set/show remotecache' options as obsolete.
	Document new `set/show stack-cache' option.
	Update text for `info dcache'.

[-- Attachment #2: gdb-090828-jdpotter-stack-cache-4.patch.txt --]
[-- Type: text/plain, Size: 30883 bytes --]

2009-08-28  Jacob Potter  <jdpotter@google.com>
	    Doug Evans  <dje@google.com>

	Implement TARGET_OBJECT_STACK_MEMORY.
	* NEWS: Add note on new "set stack-cache" option.
	* corefile.c (read_stack): New function.
	* dcache.c (dcache_struct): New member ptid.
	(dcache_enable_p): Mark as obsolete.
	(show_dcache_enabled_p): Flag option as deprecated.
	(dcache_invalidate): Update ptid.
	(dcache_invalidate_line): New function.
	(dcache_read_line): No longer check cacheable attribute, stack
	accesses get cached despite attribute.
	(dcache_init): Set ptid.
	(dcache_xfer_memory): Flush cache if from different ptid than before.
	Update cache after write.
	(dcache_update): New function.
	(dcache_info): Report ptid.
	(_initialize_dcache): Update text for `remotecache' to indicate it
	is obsolete.
	* dcache.h (dcache_update): Declare.
	* dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
	set_value_stack.
	* frame-unwind.c (frame_unwind_got_memory): Ditto.
	* gdbcore.h (read_stack): Declare.
	* memattr.c (mem_enable_command): Call target_dcache_invalidate
	instead of dcache_invalidate.
	(mem_disable_command, mem_delete_command): Ditto.
	* target.c (stack_cache_enabled_p_1): New static global.
	(stack_cache_enabled_p): New static global.
	(set_stack_cache_enabled_p): New function.
	(show_stack_cache_enabled_p): New function.
	(target_dcache): Make static.
	(target_dcache_invalidate): New function.
	(target_load, target_resume): Call target_dcache_invalidate
	instead of dcache_invalidate.
	(memory_xfer_partial): New arg object, all callers updated.
	Check for existing inferior before calling dcache routines.
	When writing non-TARGET_OBJECT_STACK_MEMORY, notify dcache.
	(target_xfer_partial): Call memory_xfer_partial for
	TARGET_OBJECT_STACK_MEMORY.
	(target_read_stack): New function.
	(initialize_targets): Install new option `stack-cache'.
	* target.h: Remove #include of dcache.h.
	(enum target_object): New value TARGET_OBJECT_STACK_MEMORY.
	(target_dcache): Delete.
	(target_dcache_invalidate): Declare.
	(target_read_stack): Declare.
	* top.c (prepare_execute_command): New function.
	(execute_command): Call prepare_execute_command
	instead of free_all_values.
	* top.h (prepare_execute_command): Declare.
	* valops.c (get_value_at): New function.
	(value_at): Guts moved to get_value_at.
	(value_at_lazy): Similarly.
	(value_fetch_lazy): Call read_stack for stack values.
	* value.c (struct value): New member `stack'.
	(value_stack, set_value_stack): New functions.
	* value.h (value_stack, set_value_stack): Declare.
	* mi/mi-main.c (mi_cmd_execute): Call prepare_execute_command
	instead of free_all_values.

	* doc/gdb.texinfo (Caching Data of Remote Targets): Update text.
	Mark `set/show remotecache' options as obsolete.
	Document new `set/show stack-cache' option.
	Update text for `info dcache'.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.327
diff -u -p -r1.327 NEWS
--- NEWS	27 Aug 2009 21:56:38 -0000	1.327
+++ NEWS	29 Aug 2009 00:36:12 -0000
@@ -394,6 +394,12 @@ show schedule-multiple
   Allow GDB to resume all threads of all processes or only threads of
   the current process.
 
+set stack-cache
+show stack-cache
+  Use more aggressive caching for accesses to the stack.  This improves
+  performance of remote debugging (particularly backtraces) without
+  affecting correctness.
+
 * Removed commands
 
 info forks
Index: corefile.c
===================================================================
RCS file: /cvs/src/src/gdb/corefile.c,v
retrieving revision 1.54
diff -u -p -r1.54 corefile.c
--- corefile.c	2 Jul 2009 17:25:53 -0000	1.54
+++ corefile.c	29 Aug 2009 00:36:12 -0000
@@ -228,6 +228,7 @@ memory_error (int status, CORE_ADDR mema
 }
 
 /* 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
     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
Index: dcache.c
===================================================================
RCS file: /cvs/src/src/gdb/dcache.c,v
retrieving revision 1.35
diff -u -p -r1.35 dcache.c
--- dcache.c	20 Aug 2009 23:30:15 -0000	1.35
+++ dcache.c	29 Aug 2009 00:36:12 -0000
@@ -24,6 +24,7 @@
 #include "gdb_string.h"
 #include "gdbcore.h"
 #include "target.h"
+#include "inferior.h"
 #include "splay-tree.h"
 
 /* The data cache could lead to incorrect results because it doesn't
@@ -103,6 +104,9 @@ struct dcache_struct
 
   /* The number of in-use lines in the cache.  */
   int size;
+
+  /* The ptid of last inferior to use cache or null_ptid.  */
+  ptid_t ptid;
 };
 
 static struct dcache_block *dcache_hit (DCACHE *dcache, CORE_ADDR addr);
@@ -117,16 +121,15 @@ static void dcache_info (char *exp, int 
 
 void _initialize_dcache (void);
 
-static int dcache_enabled_p = 0;
+static int dcache_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, _("Deprecated remotecache flag is %s.\n"), value);
 }
 
-
 static DCACHE *last_cache; /* Used by info dcache */
 
 /* Free all the data cache blocks, thus discarding all cached data.  */
@@ -152,6 +155,23 @@ dcache_invalidate (DCACHE *dcache)
   dcache->oldest = NULL;
   dcache->newest = NULL;
   dcache->size = 0;
+  dcache->ptid = null_ptid;
+}
+
+/* Invalidate the line associated with ADDR.  */
+
+static void
+dcache_invalidate_line (DCACHE *dcache, CORE_ADDR addr)
+{
+  struct dcache_block *db = dcache_hit (dcache, addr);
+
+  if (db)
+    {
+      splay_tree_remove (dcache->tree, (splay_tree_key) db->addr);
+      db->newer = dcache->freelist;
+      dcache->freelist = db;
+      --dcache->size;
+    }
 }
 
 /* If addr is present in the dcache, return the address of the block
@@ -198,8 +218,9 @@ dcache_read_line (DCACHE *dcache, struct
       else
 	reg_len = region->hi - memaddr;
 
-      /* Skip non-cacheable/non-readable regions.  */
-      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;
@@ -296,7 +317,7 @@ dcache_peek_byte (DCACHE *dcache, CORE_A
    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.  */
+   Always return 1 (meaning success) to simplify dcache_xfer_memory.  */
 
 static int
 dcache_poke_byte (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr)
@@ -338,6 +359,7 @@ dcache_init (void)
   dcache->newest = NULL;
   dcache->freelist = NULL;
   dcache->size = 0;
+  dcache->ptid = null_ptid;
   last_cache = dcache;
 
   return dcache;
@@ -366,7 +388,7 @@ 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.  */
+   The meaning of the result is the same as for target_write.  */
 
 int
 dcache_xfer_memory (struct target_ops *ops, DCACHE *dcache,
@@ -378,6 +400,15 @@ dcache_xfer_memory (struct target_ops *o
   int (*xfunc) (DCACHE *dcache, CORE_ADDR addr, gdb_byte *ptr);
   xfunc = should_write ? dcache_poke_byte : dcache_peek_byte;
 
+  /* If this is a different inferior from what we've recorded,
+     flush the cache.  */
+
+  if (! ptid_equal (inferior_ptid, dcache->ptid))
+    {
+      dcache_invalidate (dcache);
+      dcache->ptid = inferior_ptid;
+    }
+
   /* Do write-through first, so that if it fails, we don't write to
      the cache at all.  */
 
@@ -385,14 +416,25 @@ dcache_xfer_memory (struct target_ops *o
     {
       res = target_write (ops, TARGET_OBJECT_RAW_MEMORY,
 			  NULL, myaddr, memaddr, len);
-      if (res < len)
-	return 0;
+      if (res <= 0)
+	return res;
+      /* Update LEN to what was actually written.  */
+      len = res;
     }
       
   for (i = 0; i < len; i++)
     {
       if (!xfunc (dcache, memaddr + i, myaddr + i))
-	return 0;
+	{
+	  /* That failed.  Discard its cache line so we don't have a
+	     partially read line.  */
+	  dcache_invalidate_line (dcache, memaddr + i);
+	  /* If we're writing, we still wrote LEN bytes.  */
+	  if (should_write)
+	    return len;
+	  else
+	    return i;
+	}
     }
     
   return len;
@@ -407,6 +449,18 @@ dcache_xfer_memory (struct target_ops *o
    "logically" connected but not actually a single call to one of the
    memory transfer functions.  */
 
+/* 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 index)
 {
@@ -474,12 +528,15 @@ dcache_info (char *exp, int tty)
   printf_filtered (_("Dcache line width %d, maximum size %d\n"),
 		   LINE_SIZE, DCACHE_SIZE);
 
-  if (!last_cache)
+  if (!last_cache || ptid_equal (last_cache->ptid, null_ptid))
     {
       printf_filtered (_("No data cache available.\n"));
       return;
     }
 
+  printf_filtered (_("Contains data for %s\n"),
+		   target_pid_to_str (last_cache->ptid));
+
   refcount = 0;
 
   n = splay_tree_min (last_cache->tree);
@@ -507,11 +564,10 @@ _initialize_dcache (void)
 			   &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."),
+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\
+\"stack-cache\" flag; \"remotecache\" now does nothing and\n\
+exists only for compatibility reasons."),
 			   NULL,
 			   show_dcache_enabled_p,
 			   &setlist, &showlist);
Index: dcache.h
===================================================================
RCS file: /cvs/src/src/gdb/dcache.h,v
retrieving revision 1.15
diff -u -p -r1.15 dcache.h
--- dcache.h	20 Aug 2009 22:30:12 -0000	1.15
+++ dcache.h	29 Aug 2009 00:36:12 -0000
@@ -38,4 +38,7 @@ void dcache_free (DCACHE *);
 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);
+
 #endif /* DCACHE_H */
Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.63
diff -u -p -r1.63 dwarf2loc.c
--- dwarf2loc.c	11 Aug 2009 20:36:49 -0000	1.63
+++ dwarf2loc.c	29 Aug 2009 00:36:12 -0000
@@ -280,6 +280,7 @@ dwarf2_evaluate_loc_desc (struct symbol 
       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);
     }
 
Index: frame-unwind.c
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.c,v
retrieving revision 1.26
diff -u -p -r1.26 frame-unwind.c
--- frame-unwind.c	2 Jul 2009 17:25:53 -0000	1.26
+++ frame-unwind.c	29 Aug 2009 00:36:12 -0000
@@ -153,8 +153,10 @@ struct value *
 frame_unwind_got_memory (struct frame_info *frame, int regnum, CORE_ADDR addr)
 {
   struct gdbarch *gdbarch = frame_unwind_arch (frame);
+  struct value *v = value_at_lazy (register_type (gdbarch, regnum), addr);
 
-  return value_at_lazy (register_type (gdbarch, regnum), addr);
+  set_value_stack (v, 1);
+  return v;
 }
 
 /* Return a value which indicates that FRAME's saved version of
Index: gdbcore.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbcore.h,v
retrieving revision 1.35
diff -u -p -r1.35 gdbcore.h
--- gdbcore.h	2 Jul 2009 17:25:53 -0000	1.35
+++ gdbcore.h	29 Aug 2009 00:36:12 -0000
@@ -47,6 +47,10 @@ extern void memory_error (int status, CO
 
 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.  */
 
Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.34
diff -u -p -r1.34 memattr.c
--- memattr.c	17 Jun 2009 18:44:23 -0000	1.34
+++ memattr.c	29 Aug 2009 00:36:12 -0000
@@ -571,7 +571,7 @@ mem_enable_command (char *args, int from
 
   require_user_regions (from_tty);
 
-  dcache_invalidate (target_dcache);
+  target_dcache_invalidate ();
 
   if (p == 0)
     {
@@ -625,7 +625,7 @@ mem_disable_command (char *args, int fro
 
   require_user_regions (from_tty);
 
-  dcache_invalidate (target_dcache);
+  target_dcache_invalidate ();
 
   if (p == 0)
     {
@@ -686,7 +686,7 @@ mem_delete_command (char *args, int from
 
   require_user_regions (from_tty);
 
-  dcache_invalidate (target_dcache);
+  target_dcache_invalidate ();
 
   if (p == 0)
     {
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.221
diff -u -p -r1.221 target.c
--- target.c	20 Aug 2009 22:30:12 -0000	1.221
+++ target.c	29 Aug 2009 00:36:13 -0000
@@ -210,7 +210,45 @@ show_targetdebug (struct ui_file *file, 
 
 static void setup_target_debug (void);
 
-DCACHE *target_dcache;
+/* The option sets this.  */
+static int stack_cache_enabled_p_1 = 1;
+/* And set_stack_cache_enabled_p updates this.
+   The reason for the separation is so that we don't flush the cache for
+   on->on transitions.  */
+static int stack_cache_enabled_p = 1;
+
+/* This is called *after* the stack-cache has been set.
+   Flush the cache for off->on and on->off transitions.
+   There's no real need to flush the cache for on->off transitions,
+   except cleanliness.  */
+
+static void
+set_stack_cache_enabled_p (char *args, int from_tty,
+			   struct cmd_list_element *c)
+{
+  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
+    target_dcache_invalidate ();
+
+  stack_cache_enabled_p = stack_cache_enabled_p_1;
+}
+
+static void
+show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
+			    struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Cache use for stack accesses is %s.\n"), value);
+}
+
+/* Cache of memory operations, to speed up remote access.  */
+static DCACHE *target_dcache;
+
+/* Invalidate the target dcache.  */
+
+void
+target_dcache_invalidate (void)
+{
+  dcache_invalidate (target_dcache);
+}
 
 /* The user just typed 'target' without the name of a target.  */
 
@@ -413,7 +451,7 @@ target_kill (void)
 void
 target_load (char *arg, int from_tty)
 {
-  dcache_invalidate (target_dcache);
+  target_dcache_invalidate ();
   (*current_target.to_load) (arg, from_tty);
 }
 
@@ -1143,12 +1181,14 @@ target_section_by_addr (struct target_op
    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;
   struct mem_region *region;
+  struct inferior *inf;
 
   /* Zero length requests are ok and require no work.  */
   if (len == 0)
@@ -1223,7 +1263,11 @@ memory_xfer_partial (struct target_ops *
       return -1;
     }
 
-  if (region->attrib.cache)
+  inf = find_inferior_pid (ptid_get_pid (inferior_ptid));
+
+  if (inf != NULL
+      && (region->attrib.cache
+	  || (stack_cache_enabled_p && object == TARGET_OBJECT_STACK_MEMORY)))
     {
       if (readbuf != NULL)
 	res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
@@ -1245,6 +1289,19 @@ memory_xfer_partial (struct target_ops *
 	}
     }
 
+  /* 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 (inf != NULL
+      && readbuf == NULL
+      && !region->attrib.cache
+      && stack_cache_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
@@ -1308,8 +1365,9 @@ target_xfer_partial (struct target_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;
@@ -1391,6 +1449,23 @@ target_read_memory (CORE_ADDR memaddr, g
     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)
 {
@@ -2055,7 +2130,7 @@ target_resume (ptid_t ptid, int step, en
 {
   struct target_ops *t;
 
-  dcache_invalidate (target_dcache);
+  target_dcache_invalidate ();
 
   for (t = current_target.beneath; t != NULL; t = t->beneath)
     {
@@ -3479,5 +3554,16 @@ Tells gdb whether to control the inferio
 			   &setlist,
 			   &showlist);
 
+  add_setshow_boolean_cmd ("stack-cache", class_support,
+			   &stack_cache_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 on."),
+			   set_stack_cache_enabled_p,
+			   show_stack_cache_enabled_p,
+			   &setlist, &showlist);
+
   target_dcache = dcache_init ();
 }
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.162
diff -u -p -r1.162 target.h
--- target.h	31 Jul 2009 15:28:27 -0000	1.162
+++ target.h	29 Aug 2009 00:36:13 -0000
@@ -53,7 +53,6 @@ struct target_section_table;
 
 #include "bfd.h"
 #include "symtab.h"
-#include "dcache.h"
 #include "memattr.h"
 #include "vec.h"
 #include "gdb_signals.h"
@@ -203,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.  */
@@ -671,12 +674,15 @@ extern void target_store_registers (stru
 #define	target_supports_multi_process()	\
      (*current_target.to_supports_multi_process) ()
 
-extern DCACHE *target_dcache;
+/* Invalidate all target dcaches.  */
+extern void target_dcache_invalidate (void);
 
 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);
 
Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.169
diff -u -p -r1.169 top.c
--- top.c	28 Aug 2009 23:26:54 -0000	1.169
+++ top.c	29 Aug 2009 00:36:13 -0000
@@ -345,6 +345,19 @@ do_chdir_cleanup (void *old_dir)
 }
 #endif
 
+void
+prepare_execute_command (void)
+{
+  free_all_values ();
+
+  /* With multiple threads running while the one we're examining is stopped,
+     the dcache can get stale without us being able to detect it.
+     For the duration of the command, though, use the dcache to help
+     things like backtrace.  */
+  if (non_stop)
+    target_dcache_invalidate ();
+}
+
 /* Execute the line P as a command, in the current user context.
    Pass FROM_TTY as second argument to the defining function.  */
 
@@ -374,8 +387,8 @@ execute_command (char *p, int from_tty)
 #endif
 	}
     }
-  
-  free_all_values ();
+
+  prepare_execute_command ();
 
   /* Force cleanup of any alloca areas if using C alloca instead of
      a builtin alloca.  */
Index: top.h
===================================================================
RCS file: /cvs/src/src/gdb/top.h,v
retrieving revision 1.18
diff -u -p -r1.18 top.h
--- top.h	3 Jan 2009 05:57:53 -0000	1.18
+++ top.h	29 Aug 2009 00:36:13 -0000
@@ -49,6 +49,10 @@ extern void quit_command (char *, int);
 extern int quit_cover (void *);
 extern void execute_command (char *, int);
 
+/* Prepare for execution of a command.
+   Call this before every command, CLI or MI.  */
+extern void prepare_execute_command (void);
+
 /* This function returns a pointer to the string that is used
    by gdb for its command prompt. */
 extern char *get_prompt (void);
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.224
diff -u -p -r1.224 valops.c
--- valops.c	21 Jul 2009 18:15:32 -0000	1.224
+++ valops.c	29 Aug 2009 00:36:13 -0000
@@ -565,6 +565,32 @@ value_one (struct type *type, enum lval_
   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)
+{
+  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);
+      read_memory (addr, value_contents_all_raw (val), TYPE_LENGTH (type));
+    }
+
+  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;
@@ -580,19 +606,7 @@ value_one (struct type *type, enum lval_
 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);
 }
 
 /* Return a lazy value with type TYPE located at ADDR (cf. value_at).  */
@@ -600,17 +614,7 @@ value_at (struct type *type, CORE_ADDR a
 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."));
-
-  val = allocate_value_lazy (type);
-
-  VALUE_LVAL (val) = lval_memory;
-  set_value_address (val, addr);
-
-  return val;
+  return get_value_at (type, addr, 1);
 }
 
 /* Called only from the value_contents and value_contents_all()
@@ -656,7 +660,12 @@ value_fetch_lazy (struct value *val)
       int length = TYPE_LENGTH (check_typedef (value_enclosing_type (val)));
 
       if (length)
-	read_memory (addr, value_contents_all_raw (val), 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)
     {
Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.95
diff -u -p -r1.95 value.c
--- value.c	28 Aug 2009 18:50:49 -0000	1.95
+++ value.c	29 Aug 2009 00:36:13 -0000
@@ -196,6 +196,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;
@@ -424,6 +428,18 @@ set_value_lazy (struct value *value, int
   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)
 {
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.148
diff -u -p -r1.148 value.h
--- value.h	13 Aug 2009 18:39:20 -0000	1.148
+++ value.h	29 Aug 2009 00:36:13 -0000
@@ -215,6 +215,9 @@ extern void *value_computed_closure (str
 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
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.618
diff -u -p -r1.618 gdb.texinfo
--- doc/gdb.texinfo	27 Aug 2009 21:56:38 -0000	1.618
+++ doc/gdb.texinfo	29 Aug 2009 00:36:13 -0000
@@ -8421,32 +8421,47 @@ character.
 @section Caching Data of Remote Targets
 @cindex caching data of remote targets
 
-@value{GDBN} can cache data exchanged between the debugger and a
+@value{GDBN} caches data exchanged between the debugger and a
 remote target (@pxref{Remote Debugging}).  Such caching generally improves
 performance, because it reduces the overhead of the remote protocol by
-bundling memory reads and writes into large chunks.  Unfortunately,
-@value{GDBN} does not currently know anything about volatile
-registers, and thus data caching will produce incorrect results when
-volatile registers are in use.
+bundling memory reads and writes into large chunks.  Unfortunately, simply
+caching everything would lead to incorrect results, since @value{GDBN} 
+does not necessarily know anything about volatile values, memory-mapped I/O
+addresses, etc.  Therefore, by default, @value{GDBN} only caches data
+known to be on the stack.  Other regions of memory can be explicitly marked
+cacheable; see @pxref{Memory Region Attributes}.
 
 @table @code
 @kindex set remotecache
 @item set remotecache on
 @itemx set remotecache off
-Set caching state for remote targets.  When @code{ON}, use data
-caching.  By default, this option is @code{OFF}.
+This option no longer does anything; it exists for compatibility
+with old scripts.
 
 @kindex show remotecache
 @item show remotecache
-Show the current state of data caching for remote targets.
+Show the current state of the obsolete remotecache flag.
+
+@kindex set stack-cache
+@item set stack-cache on
+@itemx set stack-cache off
+Enable or disable caching of stack accesses.  When @code{ON}, use
+caching.  By default, this option is @code{ON}.
+
+@kindex show stack-cache
+@item show stack-cache
+Show the current state of data caching for memory accesses.
 
 @kindex info dcache
-@item info dcache
+@item info dcache @r{[}line@r{]}
 Print the information about the data cache performance.  The
-information displayed includes: the dcache width and depth; and for
-each cache line, how many times it was referenced, and its data and
-state (invalid, dirty, valid).  This command is useful for debugging
-the data cache operation.
+information displayed includes the dcache width and depth, and for
+each cache line, its number, address, and how many times it was
+referenced.  This command is useful for debugging the data cache
+operation.
+
+If a line number is specified, the contents of that line will be
+printed in hex.
 @end table
 
 @node Searching Memory
Index: mi/mi-main.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
retrieving revision 1.156
diff -u -p -r1.156 mi-main.c
--- mi/mi-main.c	2 Jul 2009 17:25:59 -0000	1.156
+++ mi/mi-main.c	29 Aug 2009 00:36:13 -0000
@@ -1353,7 +1353,8 @@ mi_cmd_execute (struct mi_parse *parse)
   struct cleanup *cleanup;
   int i;
 
-  free_all_values ();
+  prepare_execute_command ();
+
   cleanup = make_cleanup (null_cleanup, NULL);
 
   if (parse->frame != -1 && parse->thread == -1)

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

On Fri, Aug 28, 2009 at 5:55 PM, Doug Evans<dje@google.com> wrote:
>> Cache becoming stale in non-stop mode.
>
> The cache is now flushed between commands in non-stop mode.

I need to mention this in the docs.
I'll fix that in a separate patch.

Re: [RFA] Use data cache for stack accesses

[Pedro Alves]

On Saturday 29 August 2009 01:55:30, Doug Evans wrote:

> > dcache writes back lines in chunks.
> 
> Actually, the dcache only writes to the target exactly what
> target_xfer_partial would do.
> Data point: For writes, if the line isn't already in cache, it is not
> brought in.

Oh, dcache_write_line has been deleted!  I was looking at an
older tree...  Sorry about that.  :-/  Great then!

> > Do we still have rare border cases
> > where the cache line can cover more than stack memory, hence
> > still leading to incorrect results?
> 
> Yeah, if, for example, .data abuts the stack in just the right way,
> it'll get cached when "set stack-cache on".
> Pretty rare that it would be a problem.

Yeah.  It would even be a rarer problem if we flushed the
cache between high level operations in all-stop as well.  In
fact, with that, wouldn't we be safe to enable the dcache
for all operations?

> > Cache becoming stale in non-stop mode.
> 
> The cache is now flushed between commands in non-stop mode.

Thanks.  I wonder how much performance we lose here, but I don't
have one of those large apps to test handy ...  :-)


> +  /* 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. */

EMISDBLSPC.                 ^

I'm fine with the patch now.  Thanks.

> 2009-08-28  Jacob Potter  <jdpotter@google.com>
> 	    Doug Evans  <dje@google.com>
> (...)
> 
> 	* doc/gdb.texinfo (Caching Data of Remote Targets): Update text.
> 	Mark `set/show remotecache' options as obsolete.
> 	Document new `set/show stack-cache' option.
> 	Update text for `info dcache'.

Another convention is to put the ChangeLog path line like so:

        doc/
	* gdb.texinfo (Caching Data of Remote Targets): Update text.
	Mark `set/show remotecache' options as obsolete.
	Document new `set/show stack-cache' option.
	Update text for `info dcache'.

This has the advantange that you only have to remove that line
when pasting the entry to the correct log file, instead of editing
every possible changed-file entry, and it's clearer to everyone else
where the log is supposed to go, IMHO.  This is the convention we
use at CS, FWIW.

-- 
Pedro Alves

Re: [RFA] Use data cache for stack accesses

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> * dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
Doug> set_value_stack.

I ran across this while merging the DW_OP_*_value patch.

Do we really know that such values always come from the stack?  It seems
plausible to me that this is the case in practice, but aren't compilers
free to refer to any memory at all from a DWARF expression?

Tom

Re: [RFA] Use data cache for stack accesses

[Doug Evans]

On Wed, Sep 2, 2009 at 1:42 PM, Tom Tromey<tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> * dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
> Doug> set_value_stack.
>
> I ran across this while merging the DW_OP_*_value patch.
>
> Do we really know that such values always come from the stack?  It seems
> plausible to me that this is the case in practice, but aren't compilers
> free to refer to any memory at all from a DWARF expression?

Blech.  It's a bit confusing.

I don't honestly know.
I'll do some research.
[We can certainly pull the patch or default stack-cache to off if you like.]

Re: [RFA] Use data cache for stack accesses

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> [We can certainly pull the patch or default stack-cache to off if
Doug> you like.]

I don't know if that is warranted.  I think it probably depends on the
consequences of this code, and I haven't looked into that.

Tom

Re: [RFA] Use data cache for stack accesses

[Daniel Jacobowitz]

On Thu, Sep 03, 2009 at 08:38:30AM -0700, Doug Evans wrote:
> On Wed, Sep 2, 2009 at 1:42 PM, Tom Tromey<tromey@redhat.com> wrote:
> >>>>>> "Doug" == Doug Evans <dje@google.com> writes:
> >
> > Doug> * dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
> > Doug> set_value_stack.
> >
> > I ran across this while merging the DW_OP_*_value patch.
> >
> > Do we really know that such values always come from the stack?  It seems
> > plausible to me that this is the case in practice, but aren't compilers
> > free to refer to any memory at all from a DWARF expression?
> 
> Blech.  It's a bit confusing.
> 
> I don't honestly know.
> I'll do some research.

They can come from the stack, or elsewhere.  You might be able to
track based-on (similar to how prologue-value.h does it) to identify
CFA-relative accesses.  DW_OP_deref has to clear it, though.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Use data cache for stack accesses

[Eli Zaretskii]

> Date: Wed, 8 Jul 2009 13:08:00 -0700
> From: Jacob Potter <jdpotter@google.com>
> 
> This is the second half of the pair of patches I first submitted last
> week. Differences from the first pass are:
> 
> - Added a NEWS entry for the new option
> - Changed the stackcache option to default to on, rather than off
> - Got rid of the unnecessary new value_at_lazy_stack() function
> - Flush the cache when switching inferiors.

Thanks.

> I haven't changed the new read_stack function to take a target_ops,
> since it's intended to be consistent with read_memory(); converting
> read_memory() to take target_ops would be out of the scope of this
> patch (it and target_read_memory have a *lot* of callers). Other than
> that, I think I've addressed all the issues with the first pass; is
> there anything else to fix?

The NEWS entry is approved.  But we need a patch for the manual that
describes the new command and removes the description of remotecache.