[RFA] Use data cache for stack accesses
Jacob Potter
jdpotter@google.com
Mon Jul 13 19:25:00 GMT 2009
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
-------------- next part --------------
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
More information about the Gdb-patches
mailing list