[PATCH 1/3] Renaming in target-dcache.c

[PATCH 1/3] Renaming in target-dcache.c

[Yao Qi]

Hi,
This patch does some renamings on "stack-cache" related functions and
variables.

In the review to "code cache" series v2, we have some discussions on the
name of predicate function 'stack_cache_enabled', and have some options,

 1 keep it unchanged, as it is already a predicate clearly,
 2 rename it to stack_cache_enabled_p,
 3 rename it to enable_stack_cache_p,

I choose #2, because 'stack_cache_enabled' is a predicate, but
it's better to add "_p" suffix to stress this.  There are some other
similar patterns used in GDB source, such as unop_user_defined_p
and agent_loaded_p.

Then, I have to rename variable stack_cache_enabled_p to something
else.  The option is "stack-cache", so I'd like to name the variable
associated with this command as "stack_cache".  Similarly, the commands
associated with this command should be renamed to "set_stack_cache"
and "show_stack_cache" respectively.

gdb:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* target-dcache.c (stack_cache_enabled_p_1): Rename to ...
	(stack_cache_1): ... this.  New variable.
	(stack_cache_enabled_p): Rename to ...
	(stack_cache): ... this.  New variable.
	(set_stack_cache_enabled_p): Rename to ...
	(set_stack_cache): ... this.  Update caller.
	(show_stack_cache_enabled_p): Rename to ...
	(show_stack_cache): ... this.  Update caller.
	(stack_cache_enabled): Rename to ...
	(stack_cache_enabled_p): ... this.  Update caller.
	(_initialize_target_dcache): Replace "data cache" with
	"target memory cache".
	* target-dcache.h (stack_cache_enabled): Remove declaration.
	(stack_cache_enabled_p): Add declaration.
---
 gdb/target-dcache.c |   29 ++++++++++++++---------------
 gdb/target-dcache.h |    2 +-
 gdb/target.c        |    5 ++---
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index 76160ef..26f6f5d 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -89,11 +89,11 @@ target_dcache_get_or_init (void)
 }
 
 /* The option sets this.  */
-static int stack_cache_enabled_p_1 = 1;
-/* And set_stack_cache_enabled_p updates this.
+static int stack_cache_1 = 1;
+/* And set_stack_cache 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;
+static int stack_cache = 1;
 
 /* This is called *after* the stack-cache has been set.
    Flush the cache for off->on and on->off transitions.
@@ -101,18 +101,17 @@ static int stack_cache_enabled_p = 1;
    except cleanliness.  */
 
 static void
-set_stack_cache_enabled_p (char *args, int from_tty,
-			   struct cmd_list_element *c)
+set_stack_cache (char *args, int from_tty, struct cmd_list_element *c)
 {
-  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
+  if (stack_cache != stack_cache_1)
     target_dcache_invalidate ();
 
-  stack_cache_enabled_p = stack_cache_enabled_p_1;
+  stack_cache = stack_cache_1;
 }
 
 static void
-show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
-			    struct cmd_list_element *c, const char *value)
+show_stack_cache (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);
 }
@@ -120,9 +119,9 @@ show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
 /* Return true if "stack cache" is enabled, otherwise, return false.  */
 
 int
-stack_cache_enabled (void)
+stack_cache_enabled_p (void)
 {
-  return stack_cache_enabled_p;
+  return stack_cache;
 }
 
 /* -Wmissing-prototypes */
@@ -132,14 +131,14 @@ void
 _initialize_target_dcache (void)
 {
   add_setshow_boolean_cmd ("stack-cache", class_support,
-			   &stack_cache_enabled_p_1, _("\
+			   &stack_cache_1, _("\
 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\
+When on, use the target memory 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,
+			   set_stack_cache,
+			   show_stack_cache,
 			   &setlist, &showlist);
 
   target_dcache_aspace_key
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
index a5e1556..3200dd9 100644
--- a/gdb/target-dcache.h
+++ b/gdb/target-dcache.h
@@ -28,6 +28,6 @@ extern DCACHE *target_dcache_get_or_init (void);
 
 extern int target_dcache_init_p (void);
 
-extern int stack_cache_enabled (void);
+extern int stack_cache_enabled_p (void);
 
 #endif /* TARGET_DCACHE_H */
diff --git a/gdb/target.c b/gdb/target.c
index 5001643..29f06b6 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1547,7 +1547,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	 the collected memory range fails.  */
       && get_traceframe_number () == -1
       && (region->attrib.cache
-	  || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)))
+	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get_or_init ();
 
@@ -1600,8 +1600,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && stack_cache_enabled ()
-      && object != TARGET_OBJECT_STACK_MEMORY)
+      && stack_cache_enabled_p () && object != TARGET_OBJECT_STACK_MEMORY)
     {
       DCACHE *dcache = target_dcache_get ();
 
-- 
1.7.7.6

[PATCH 3/3] Use target_read_code in disassemble.

[Yao Qi]

It was reviewed by Doug and nothing is changed in V3.

gdb:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* disasm.c (dis_asm_read_memory): Call target_read_code
	instead of target_read_memory.
---
 gdb/disasm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index 1690395..09ae465 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -47,7 +47,7 @@ static int
 dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len,
 		     struct disassemble_info *info)
 {
-  return target_read_memory (memaddr, myaddr, len);
+  return target_read_code (memaddr, myaddr, len);
 }
 
 /* Like memory_error with slightly different parameters.  */
-- 
1.7.7.6

[PATCH 2/3] set/show code-cache

[Yao Qi]

Similar to stack cache, in this patch, we add
TARGET_OBJECT_CODE_MEMORY to read code from target and add a new
option "set code-cache on|off" to optimize code accesses by
using the target memory cache.

In V3:
 - Rename functions and variables.
 - Update command help, doc and NEWS entry.
 - Invalidate cache on option transitions, to align with
   the behaviour of "stack-cache".  Since cache invalidation is
   transparent to users, users don't know option "stack-cache"
   transitions cause code cache invalidation.

V2 was reviewed by Doug.  There are some changes in V3, so I post it
here.

gdb:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* NEWS: Add note on new "set code-cache" option.
	* target-dcache.c (code_cache_1, code_cache): New variables.
	(show_code_cache, set_code_cache): New function.
	(code_cache_enabled_p): New function.
	(_initialize_target_dcache): Register command.
	* target-dcache.h (code_cache_enabled_p): Declare.
	* target.c (memory_xfer_partial_1):Handle
	TARGET_OBJECT_CODE_MEMORY and code_cache_enabled.
	(target_read_code): New function.
	* target.h (enum target_object) <TARGET_OBJECT_CODE_MEMORY>:
	New.
	(target_read_code): Declare.

gdb/doc:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Remote Data): Document new
	`set/show stack-cache' option.
---
 gdb/NEWS            |    6 +++++
 gdb/doc/gdb.texinfo |   16 ++++++++++++++-
 gdb/target-dcache.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target-dcache.h |    2 +
 gdb/target.c        |   22 ++++++++++++++++++--
 gdb/target.h        |    5 ++++
 6 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 9fc3638..86a9d63 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -117,6 +117,12 @@ show startup-with-shell
   Specifies whether Unix child processes are started via a shell or
   directly.
 
+set code-cache
+show code-cache
+  Use the target memory cache for accesses to the code segment.  This
+  improves performance of remote debugging (particularly disassembly)
+  without affecting correctness.
+
 * You can now use a literal value 'unlimited' for options that
   interpret 0 or -1 as meaning "unlimited".  E.g., "set
   trace-buffer-size unlimited" is now an alias for "set
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d1fa6c8..9e64dd2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10842,7 +10842,8 @@ Therefore, by default, @value{GDBN} only caches data
 known to be on the stack@footnote{In non-stop mode, it is moderately
 rare for a running thread to modify the stack of a stopped thread
 in a way that would interfere with a backtrace, and caching of
-stack reads provides a significant speed up of remote backtraces.}.
+stack reads provides a significant speed up of remote backtraces.} or
+in the code segment.
 Other regions of memory can be explicitly marked as
 cacheable; @pxref{Memory Region Attributes}.
 
@@ -10867,6 +10868,19 @@ caching.  By default, this option is @code{ON}.
 @item show stack-cache
 Show the current state of data caching for memory accesses.
 
+@kindex set code-cache
+@item set code-cache on
+@itemx set code-cache off
+Enable or disable caching of code segment accesses.  When @code{ON},
+use caching.  By default, this option is @code{ON}.  This improves
+performance of disassembly in remote debugging without affecting
+correctness.
+
+@kindex show code-cache
+@item show code-cache
+Show the current state of target memory cache for code segment
+accesses.
+
 @kindex info dcache
 @item info dcache @r{[}line@r{]}
 Print the information about the performance of data cache of the
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index 26f6f5d..58be6cd 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -124,6 +124,46 @@ stack_cache_enabled_p (void)
   return stack_cache;
 }
 
+/* The option sets this.  */
+
+static int code_cache_1 = 1;
+
+/* And set_code_cache updates this.
+   The reason for the separation is so that we don't flush the cache for
+   on->on transitions.  */
+static int code_cache = 1;
+
+/* This is called *after* the code-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_code_cache (char *args, int from_tty, struct cmd_list_element *c)
+{
+  if (code_cache != code_cache_1)
+    target_dcache_invalidate ();
+
+  code_cache = code_cache_1;
+}
+
+/* Show option "code-cache".  */
+
+static void
+show_code_cache (struct ui_file *file, int from_tty,
+		 struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Cache use for code accesses is %s.\n"), value);
+}
+
+/* Return true if "code cache" is enabled, otherwise, return false.  */
+
+int
+code_cache_enabled_p (void)
+{
+  return code_cache;
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_target_dcache;
 
@@ -141,6 +181,18 @@ By default, caching for stack access is on."),
 			   show_stack_cache,
 			   &setlist, &showlist);
 
+  add_setshow_boolean_cmd ("code-cache", class_support,
+			   &code_cache, _("\
+Set cache use for code segment access."), _("\
+Show cache use for code segment access."), _("\
+When on, use the target memory cache for all code segment access,\n\
+regardless of any configured memory regions.  This improves remote\n\
+performance significantly.  By default, caching for code segment\n\
+access is on."),
+			   set_code_cache,
+			   show_code_cache,
+			   &setlist, &showlist);
+
   target_dcache_aspace_key
     = register_address_space_data_with_cleanup (NULL,
 						target_dcache_cleanup);
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
index 3200dd9..59e0b3d 100644
--- a/gdb/target-dcache.h
+++ b/gdb/target-dcache.h
@@ -30,4 +30,6 @@ extern int target_dcache_init_p (void);
 
 extern int stack_cache_enabled_p (void);
 
+extern int code_cache_enabled_p (void);
+
 #endif /* TARGET_DCACHE_H */
diff --git a/gdb/target.c b/gdb/target.c
index 29f06b6..cc6194b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1547,7 +1547,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	 the collected memory range fails.  */
       && get_traceframe_number () == -1
       && (region->attrib.cache
-	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)))
+	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)
+	  || (code_cache_enabled_p () && object == TARGET_OBJECT_CODE_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get_or_init ();
 
@@ -1600,7 +1601,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && stack_cache_enabled_p () && object != TARGET_OBJECT_STACK_MEMORY)
+      && ((stack_cache_enabled_p () && object != TARGET_OBJECT_STACK_MEMORY)
+	  || (code_cache_enabled_p () && object != TARGET_OBJECT_CODE_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get ();
 
@@ -1696,7 +1698,8 @@ 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 || object == TARGET_OBJECT_STACK_MEMORY)
+  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY
+      || object == TARGET_OBJECT_CODE_MEMORY)
     retval = memory_xfer_partial (ops, object, readbuf,
 				  writebuf, offset, len);
   else
@@ -1798,6 +1801,19 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     return TARGET_XFER_E_IO;
 }
 
+/* Like target_read_memory, but specify explicitly that this is a read from
+   the target's code.  This may trigger different cache behavior.  */
+
+int
+target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
+{
+  if (target_read (current_target.beneath, TARGET_OBJECT_CODE_MEMORY, NULL,
+		   myaddr, memaddr, len) == len)
+    return 0;
+  else
+    return TARGET_XFER_E_IO;
+}
+
 /* Write LEN bytes from MYADDR to target memory at address MEMADDR.
    Returns either 0 for success or a target_xfer_error value if any
    error occurs.  If an error occurs, no guarantee is made about how
diff --git a/gdb/target.h b/gdb/target.h
index 646907a..890171d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -145,6 +145,9 @@ enum target_object
      if it is not in a region marked as such, since it is known to be
      "normal" RAM.  */
   TARGET_OBJECT_STACK_MEMORY,
+  /* Memory known to be part of the target code.   This is cached even
+     if it is not in a region marked as such.  */
+  TARGET_OBJECT_CODE_MEMORY,
   /* Kernel Unwind Table.  See "ia64-tdep.c".  */
   TARGET_OBJECT_UNWIND_TABLE,
   /* Transfer auxilliary vector.  */
@@ -1050,6 +1053,8 @@ extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
 
 extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
 
+extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
+
 extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 				ssize_t len);
 
-- 
1.7.7.6

[PATCH 0/3 V3] Cache code access for disassemble

[Yao Qi]

Hi,
This is the V3 of this patch series.  Most of the patches were
committed because they are cleanups, and V3 only has three patches.

Patch 1/3 is to rename some functions and variables related to
"stack-cache", to address review comments in V2 that we find they are
improper.  Patch 2/3 is to add commands "set/show code-cache", and we
choose a way that "both stack cache and code cache use the single global
dcache, and transitions of either options will invalidate the dcache".
Patch 3/3 is unchanged.

The performance improvement is measured by gdb.perf/disassemble.exp,
posted https://sourceware.org/ml/gdb-patches/2013-11/msg00574.html
(note that gdb.perf/disassemble.py is modified to flush code cache after
each run)

			      Original Patched
disassemble cpu_time	0	0.49	0.16
disassemble cpu_time	1	0.93	0.21
disassemble cpu_time	2	1.35	0.33
disassemble wall_time	0	0.722337007523	0.167083024979
disassemble wall_time	1	1.37996888161	0.215560913086
disassemble wall_time	2	2.07086896896	0.323045969009
disassemble vmsize	0	151000	150880
disassemble vmsize	1	151692	151740
disassemble vmsize	2	151692	151740

Regression tested x86_64-linux.

*** BLURB HERE ***

Yao Qi (3):
  Renaming in target-dcache.c
  set/show code-cache
  Use target_read_code in disassemble.

 gdb/NEWS            |    6 ++++
 gdb/disasm.c        |    2 +-
 gdb/doc/gdb.texinfo |   16 +++++++++-
 gdb/target-dcache.c |   81 +++++++++++++++++++++++++++++++++++++++++---------
 gdb/target-dcache.h |    4 ++-
 gdb/target.c        |   23 ++++++++++++--
 gdb/target.h        |    5 +++
 7 files changed, 115 insertions(+), 22 deletions(-)

-- 
1.7.7.6

Re: [PATCH 1/3] Renaming in target-dcache.c

[Doug Evans]

On Wed, Nov 20, 2013 at 5:16 PM, Yao Qi <yao@codesourcery.com> wrote:
> Hi,
> This patch does some renamings on "stack-cache" related functions and
> variables.
>
> In the review to "code cache" series v2, we have some discussions on the
> name of predicate function 'stack_cache_enabled', and have some options,
>
>  1 keep it unchanged, as it is already a predicate clearly,
>  2 rename it to stack_cache_enabled_p,
>  3 rename it to enable_stack_cache_p,
>
> I choose #2, because 'stack_cache_enabled' is a predicate, but
> it's better to add "_p" suffix to stress this.  There are some other
> similar patterns used in GDB source, such as unop_user_defined_p
> and agent_loaded_p.
>
> Then, I have to rename variable stack_cache_enabled_p to something
> else.  The option is "stack-cache", so I'd like to name the variable
> associated with this command as "stack_cache".  Similarly, the commands
> associated with this command should be renamed to "set_stack_cache"
> and "show_stack_cache" respectively.

I like the name "stack_cache_optimization_enabled".
I think it could help avoid future confusion.
But then the predicate should probably be stack_cache_optimization_enabled_p.
I don't have a problem with it, sometimes long and *really* clear
names are just what's called for.
I can imagine others wanting to keep it at stack_cache_enabled_p,
thus I could settle for stack_cache_enabled for the variable name.
"stack_cache" feels too nondescript.

[I think(!) we *could* also change the option name to
stack-cache-optimization, but I don't see as much a need to at the
moment.
I wouldn't do it for this patch series, just mentioning it for reference sake.]

>
> gdb:
>
> 2013-11-21  Yao Qi  <yao@codesourcery.com>
>
>         * target-dcache.c (stack_cache_enabled_p_1): Rename to ...
>         (stack_cache_1): ... this.  New variable.
>         (stack_cache_enabled_p): Rename to ...
>         (stack_cache): ... this.  New variable.
>         (set_stack_cache_enabled_p): Rename to ...
>         (set_stack_cache): ... this.  Update caller.
>         (show_stack_cache_enabled_p): Rename to ...
>         (show_stack_cache): ... this.  Update caller.
>         (stack_cache_enabled): Rename to ...
>         (stack_cache_enabled_p): ... this.  Update caller.
>         (_initialize_target_dcache): Replace "data cache" with
>         "target memory cache".
>         * target-dcache.h (stack_cache_enabled): Remove declaration.
>         (stack_cache_enabled_p): Add declaration.
> ---
>  gdb/target-dcache.c |   29 ++++++++++++++---------------
>  gdb/target-dcache.h |    2 +-
>  gdb/target.c        |    5 ++---
>  3 files changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
> index 76160ef..26f6f5d 100644
> --- a/gdb/target-dcache.c
> +++ b/gdb/target-dcache.c
> @@ -89,11 +89,11 @@ target_dcache_get_or_init (void)
>  }
>
>  /* The option sets this.  */
> -static int stack_cache_enabled_p_1 = 1;
> -/* And set_stack_cache_enabled_p updates this.
> +static int stack_cache_1 = 1;
> +/* And set_stack_cache 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;
> +static int stack_cache = 1;
>
>  /* This is called *after* the stack-cache has been set.
>     Flush the cache for off->on and on->off transitions.
> @@ -101,18 +101,17 @@ static int stack_cache_enabled_p = 1;
>     except cleanliness.  */
>
>  static void
> -set_stack_cache_enabled_p (char *args, int from_tty,
> -                          struct cmd_list_element *c)
> +set_stack_cache (char *args, int from_tty, struct cmd_list_element *c)
>  {
> -  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
> +  if (stack_cache != stack_cache_1)
>      target_dcache_invalidate ();
>
> -  stack_cache_enabled_p = stack_cache_enabled_p_1;
> +  stack_cache = stack_cache_1;
>  }
>
>  static void
> -show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
> -                           struct cmd_list_element *c, const char *value)
> +show_stack_cache (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);
>  }
> @@ -120,9 +119,9 @@ show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
>  /* Return true if "stack cache" is enabled, otherwise, return false.  */
>
>  int
> -stack_cache_enabled (void)
> +stack_cache_enabled_p (void)
>  {
> -  return stack_cache_enabled_p;
> +  return stack_cache;
>  }
>
>  /* -Wmissing-prototypes */
> @@ -132,14 +131,14 @@ void
>  _initialize_target_dcache (void)
>  {
>    add_setshow_boolean_cmd ("stack-cache", class_support,
> -                          &stack_cache_enabled_p_1, _("\
> +                          &stack_cache_1, _("\
>  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\
> +When on, use the target memory 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,
> +                          set_stack_cache,
> +                          show_stack_cache,
>                            &setlist, &showlist);
>
>    target_dcache_aspace_key
> diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
> index a5e1556..3200dd9 100644
> --- a/gdb/target-dcache.h
> +++ b/gdb/target-dcache.h
> @@ -28,6 +28,6 @@ extern DCACHE *target_dcache_get_or_init (void);
>
>  extern int target_dcache_init_p (void);
>
> -extern int stack_cache_enabled (void);
> +extern int stack_cache_enabled_p (void);
>
>  #endif /* TARGET_DCACHE_H */
> diff --git a/gdb/target.c b/gdb/target.c
> index 5001643..29f06b6 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1547,7 +1547,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
>          the collected memory range fails.  */
>        && get_traceframe_number () == -1
>        && (region->attrib.cache
> -         || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)))
> +         || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)))
>      {
>        DCACHE *dcache = target_dcache_get_or_init ();
>
> @@ -1600,8 +1600,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
>        && writebuf != NULL
>        && target_dcache_init_p ()
>        && !region->attrib.cache
> -      && stack_cache_enabled ()
> -      && object != TARGET_OBJECT_STACK_MEMORY)
> +      && stack_cache_enabled_p () && object != TARGET_OBJECT_STACK_MEMORY)
>      {
>        DCACHE *dcache = target_dcache_get ();

I'd prefer to keep each && on a line by itself.

Ok with those changes.

Re: [PATCH 3/3] Use target_read_code in disassemble.

[Doug Evans]

On Wed, Nov 20, 2013 at 5:16 PM, Yao Qi <yao@codesourcery.com> wrote:
> It was reviewed by Doug and nothing is changed in V3.

I think you mean unchanged.
Still ok by me. :-)

>
> gdb:
>
> 2013-11-21  Yao Qi  <yao@codesourcery.com>
>
>         * disasm.c (dis_asm_read_memory): Call target_read_code
>         instead of target_read_memory.
> ---
>  gdb/disasm.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index 1690395..09ae465 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -47,7 +47,7 @@ static int
>  dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len,
>                      struct disassemble_info *info)
>  {
> -  return target_read_memory (memaddr, myaddr, len);
> +  return target_read_code (memaddr, myaddr, len);
>  }
>
>  /* Like memory_error with slightly different parameters.  */
> --
> 1.7.7.6
>

Re: [PATCH 1/3] Renaming in target-dcache.c

[Yao Qi]

On 11/21/2013 09:45 AM, Doug Evans wrote:
> I like the name "stack_cache_optimization_enabled".
> I think it could help avoid future confusion.
> But then the predicate should probably be stack_cache_optimization_enabled_p.
> I don't have a problem with it, sometimes long and*really*  clear
> names are just what's called for.
> I can imagine others wanting to keep it at stack_cache_enabled_p,
> thus I could settle for stack_cache_enabled for the variable name.
> "stack_cache" feels too nondescript.
> 

I prefer to name the variable "stack_cache_enabled".

> [I think(!) we*could*  also change the option name to
> stack-cache-optimization, but I don't see as much a need to at the
> moment.
> I wouldn't do it for this patch series, just mentioning it for reference sake.]
> 

We could add "optimization" to the name of command, variables, and
functions all together, if "stack-cache" is still confusing in the
future.

>> >@@ -1600,8 +1600,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
>> >        && writebuf != NULL
>> >        && target_dcache_init_p ()
>> >        && !region->attrib.cache
>> >-      && stack_cache_enabled ()
>> >-      && object != TARGET_OBJECT_STACK_MEMORY)
>> >+      && stack_cache_enabled_p () && object != TARGET_OBJECT_STACK_MEMORY)
>> >      {
>> >        DCACHE *dcache = target_dcache_get ();
> I'd prefer to keep each && on a line by itself.

Fixed.  Patch is updated, and I'll commit it today.

-- 
Yao (齐尧)

gdb:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* target-dcache.c (stack_cache_enabled_p_1): Rename to ...
	(stack_cache_enabled_1): ... this.  New variable.
	(stack_cache_enabled_p): Rename to ...
	(stack_cache_enabled): ... this.  New variable.
	(set_stack_cache_enabled_p): Rename to ...
	(set_stack_cache): ... this.  Update caller.
	(show_stack_cache_enabled_p): Rename to ...
	(show_stack_cache): ... this.  Update caller.
	(stack_cache_enabled): Rename to ...
	(stack_cache_enabled_p): ... this.  Update caller.
	(_initialize_target_dcache): Replace "data cache" with
	"target memory cache".
	* target-dcache.h (stack_cache_enabled): Remove declaration.
	(stack_cache_enabled_p): Add declaration.
---
 gdb/target-dcache.c |   29 ++++++++++++++---------------
 gdb/target-dcache.h |    2 +-
 gdb/target.c        |    4 ++--
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index 76160ef..6b7986a 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -89,11 +89,11 @@ target_dcache_get_or_init (void)
 }
 
 /* The option sets this.  */
-static int stack_cache_enabled_p_1 = 1;
-/* And set_stack_cache_enabled_p updates this.
+static int stack_cache_enabled_1 = 1;
+/* And set_stack_cache 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;
+static int stack_cache_enabled = 1;
 
 /* This is called *after* the stack-cache has been set.
    Flush the cache for off->on and on->off transitions.
@@ -101,18 +101,17 @@ static int stack_cache_enabled_p = 1;
    except cleanliness.  */
 
 static void
-set_stack_cache_enabled_p (char *args, int from_tty,
-			   struct cmd_list_element *c)
+set_stack_cache (char *args, int from_tty, struct cmd_list_element *c)
 {
-  if (stack_cache_enabled_p != stack_cache_enabled_p_1)
+  if (stack_cache_enabled != stack_cache_enabled_1)
     target_dcache_invalidate ();
 
-  stack_cache_enabled_p = stack_cache_enabled_p_1;
+  stack_cache_enabled = stack_cache_enabled_1;
 }
 
 static void
-show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
-			    struct cmd_list_element *c, const char *value)
+show_stack_cache (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);
 }
@@ -120,9 +119,9 @@ show_stack_cache_enabled_p (struct ui_file *file, int from_tty,
 /* Return true if "stack cache" is enabled, otherwise, return false.  */
 
 int
-stack_cache_enabled (void)
+stack_cache_enabled_p (void)
 {
-  return stack_cache_enabled_p;
+  return stack_cache_enabled;
 }
 
 /* -Wmissing-prototypes */
@@ -132,14 +131,14 @@ void
 _initialize_target_dcache (void)
 {
   add_setshow_boolean_cmd ("stack-cache", class_support,
-			   &stack_cache_enabled_p_1, _("\
+			   &stack_cache_enabled_1, _("\
 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\
+When on, use the target memory 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,
+			   set_stack_cache,
+			   show_stack_cache,
 			   &setlist, &showlist);
 
   target_dcache_aspace_key
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
index a5e1556..3200dd9 100644
--- a/gdb/target-dcache.h
+++ b/gdb/target-dcache.h
@@ -28,6 +28,6 @@ extern DCACHE *target_dcache_get_or_init (void);
 
 extern int target_dcache_init_p (void);
 
-extern int stack_cache_enabled (void);
+extern int stack_cache_enabled_p (void);
 
 #endif /* TARGET_DCACHE_H */
diff --git a/gdb/target.c b/gdb/target.c
index 5001643..f402a18 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1547,7 +1547,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	 the collected memory range fails.  */
       && get_traceframe_number () == -1
       && (region->attrib.cache
-	  || (stack_cache_enabled () && object == TARGET_OBJECT_STACK_MEMORY)))
+	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get_or_init ();
 
@@ -1600,7 +1600,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && stack_cache_enabled ()
+      && stack_cache_enabled_p ()
       && object != TARGET_OBJECT_STACK_MEMORY)
     {
       DCACHE *dcache = target_dcache_get ();
-- 
1.7.7.6

Re: [PATCH 2/3] set/show code-cache

[Yao Qi]

I update this patch for the name of some variables, after we discussed
in patch 1/3.  I'll commit it in two days.

-- 
Yao (齐尧)

gdb:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* NEWS: Add note on new "set code-cache" option.
	* target-dcache.c (code_cache_enabled_1): New variable.
	(code_cache_enabled): New variable.
	(show_code_cache, set_code_cache): New function.
	(code_cache_enabled_p): New function.
	(_initialize_target_dcache): Register command.
	* target-dcache.h (code_cache_enabled_p): Declare.
	* target.c (memory_xfer_partial_1):Handle
	TARGET_OBJECT_CODE_MEMORY and code_cache_enabled.
	(target_read_code): New function.
	* target.h (enum target_object) <TARGET_OBJECT_CODE_MEMORY>:
	New.
	(target_read_code): Declare.

gdb/doc:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Remote Data): Document new
	`set/show stack-cache' option.
---
 gdb/NEWS            |    6 +++++
 gdb/doc/gdb.texinfo |   16 ++++++++++++++-
 gdb/target-dcache.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target-dcache.h |    2 +
 gdb/target.c        |   23 ++++++++++++++++++---
 gdb/target.h        |    5 ++++
 6 files changed, 99 insertions(+), 5 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 9fc3638..86a9d63 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -117,6 +117,12 @@ show startup-with-shell
   Specifies whether Unix child processes are started via a shell or
   directly.
 
+set code-cache
+show code-cache
+  Use the target memory cache for accesses to the code segment.  This
+  improves performance of remote debugging (particularly disassembly)
+  without affecting correctness.
+
 * You can now use a literal value 'unlimited' for options that
   interpret 0 or -1 as meaning "unlimited".  E.g., "set
   trace-buffer-size unlimited" is now an alias for "set
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d1fa6c8..9e64dd2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10842,7 +10842,8 @@ Therefore, by default, @value{GDBN} only caches data
 known to be on the stack@footnote{In non-stop mode, it is moderately
 rare for a running thread to modify the stack of a stopped thread
 in a way that would interfere with a backtrace, and caching of
-stack reads provides a significant speed up of remote backtraces.}.
+stack reads provides a significant speed up of remote backtraces.} or
+in the code segment.
 Other regions of memory can be explicitly marked as
 cacheable; @pxref{Memory Region Attributes}.
 
@@ -10867,6 +10868,19 @@ caching.  By default, this option is @code{ON}.
 @item show stack-cache
 Show the current state of data caching for memory accesses.
 
+@kindex set code-cache
+@item set code-cache on
+@itemx set code-cache off
+Enable or disable caching of code segment accesses.  When @code{ON},
+use caching.  By default, this option is @code{ON}.  This improves
+performance of disassembly in remote debugging without affecting
+correctness.
+
+@kindex show code-cache
+@item show code-cache
+Show the current state of target memory cache for code segment
+accesses.
+
 @kindex info dcache
 @item info dcache @r{[}line@r{]}
 Print the information about the performance of data cache of the
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index 6b7986a..25b2a85 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -124,6 +124,46 @@ stack_cache_enabled_p (void)
   return stack_cache_enabled;
 }
 
+/* The option sets this.  */
+
+static int code_cache_enabled_1 = 1;
+
+/* And set_code_cache updates this.
+   The reason for the separation is so that we don't flush the cache for
+   on->on transitions.  */
+static int code_cache_enabled = 1;
+
+/* This is called *after* the code-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_code_cache (char *args, int from_tty, struct cmd_list_element *c)
+{
+  if (code_cache_enabled != code_cache_enabled_1)
+    target_dcache_invalidate ();
+
+  code_cache_enabled = code_cache_enabled_1;
+}
+
+/* Show option "code-cache".  */
+
+static void
+show_code_cache (struct ui_file *file, int from_tty,
+		 struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Cache use for code accesses is %s.\n"), value);
+}
+
+/* Return true if "code cache" is enabled, otherwise, return false.  */
+
+int
+code_cache_enabled_p (void)
+{
+  return code_cache_enabled;
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_target_dcache;
 
@@ -141,6 +181,18 @@ By default, caching for stack access is on."),
 			   show_stack_cache,
 			   &setlist, &showlist);
 
+  add_setshow_boolean_cmd ("code-cache", class_support,
+			   &code_cache_enabled_1, _("\
+Set cache use for code segment access."), _("\
+Show cache use for code segment access."), _("\
+When on, use the target memory cache for all code segment access,\n\
+regardless of any configured memory regions.  This improves remote\n\
+performance significantly.  By default, caching for code segment\n\
+access is on."),
+			   set_code_cache,
+			   show_code_cache,
+			   &setlist, &showlist);
+
   target_dcache_aspace_key
     = register_address_space_data_with_cleanup (NULL,
 						target_dcache_cleanup);
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
index 3200dd9..59e0b3d 100644
--- a/gdb/target-dcache.h
+++ b/gdb/target-dcache.h
@@ -30,4 +30,6 @@ extern int target_dcache_init_p (void);
 
 extern int stack_cache_enabled_p (void);
 
+extern int code_cache_enabled_p (void);
+
 #endif /* TARGET_DCACHE_H */
diff --git a/gdb/target.c b/gdb/target.c
index f402a18..cc6194b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1547,7 +1547,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	 the collected memory range fails.  */
       && get_traceframe_number () == -1
       && (region->attrib.cache
-	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)))
+	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)
+	  || (code_cache_enabled_p () && object == TARGET_OBJECT_CODE_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get_or_init ();
 
@@ -1600,8 +1601,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && stack_cache_enabled_p ()
-      && object != TARGET_OBJECT_STACK_MEMORY)
+      && ((stack_cache_enabled_p () && object != TARGET_OBJECT_STACK_MEMORY)
+	  || (code_cache_enabled_p () && object != TARGET_OBJECT_CODE_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get ();
 
@@ -1697,7 +1698,8 @@ 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 || object == TARGET_OBJECT_STACK_MEMORY)
+  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY
+      || object == TARGET_OBJECT_CODE_MEMORY)
     retval = memory_xfer_partial (ops, object, readbuf,
 				  writebuf, offset, len);
   else
@@ -1799,6 +1801,19 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     return TARGET_XFER_E_IO;
 }
 
+/* Like target_read_memory, but specify explicitly that this is a read from
+   the target's code.  This may trigger different cache behavior.  */
+
+int
+target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
+{
+  if (target_read (current_target.beneath, TARGET_OBJECT_CODE_MEMORY, NULL,
+		   myaddr, memaddr, len) == len)
+    return 0;
+  else
+    return TARGET_XFER_E_IO;
+}
+
 /* Write LEN bytes from MYADDR to target memory at address MEMADDR.
    Returns either 0 for success or a target_xfer_error value if any
    error occurs.  If an error occurs, no guarantee is made about how
diff --git a/gdb/target.h b/gdb/target.h
index 646907a..890171d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -145,6 +145,9 @@ enum target_object
      if it is not in a region marked as such, since it is known to be
      "normal" RAM.  */
   TARGET_OBJECT_STACK_MEMORY,
+  /* Memory known to be part of the target code.   This is cached even
+     if it is not in a region marked as such.  */
+  TARGET_OBJECT_CODE_MEMORY,
   /* Kernel Unwind Table.  See "ia64-tdep.c".  */
   TARGET_OBJECT_UNWIND_TABLE,
   /* Transfer auxilliary vector.  */
@@ -1050,6 +1053,8 @@ extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
 
 extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
 
+extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
+
 extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 				ssize_t len);
 
-- 
1.7.7.6

Re: [PATCH 2/3] set/show code-cache

[Eli Zaretskii]

> Date: Thu, 21 Nov 2013 11:03:54 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> I update this patch for the name of some variables, after we discussed
> in patch 1/3.  I'll commit it in two days.

Thanks.

>  gdb/NEWS            |    6 +++++
>  gdb/doc/gdb.texinfo |   16 ++++++++++++++-

These parts are OK.

> +  add_setshow_boolean_cmd ("code-cache", class_support,
> +			   &code_cache_enabled_1, _("\
> +Set cache use for code segment access."), _("\
> +Show cache use for code segment access."), _("\
> +When on, use the target memory cache for all code segment access,\n\
                                                             ^^^^^^
"accesses"

> +regardless of any configured memory regions.  This improves remote\n\
> +performance significantly.  By default, caching for code segment\n\
> +access is on."),

Re: [PATCH 2/3] set/show code-cache

[Pedro Alves]

On 11/21/2013 03:03 AM, Yao Qi wrote:
> +@kindex set code-cache
> +@item set code-cache on
> +@itemx set code-cache off
> +Enable or disable caching of code segment accesses.  When @code{ON},
> +use caching.  By default, this option is @code{ON}.  This improves
> +performance of disassembly in remote debugging without affecting
> +correctness.
> +

Putting a user hat on, I'd be left wondering what "affecting
correctness" means here.

-- 
Pedro Alves

Re: [PATCH 2/3] set/show code-cache

[Yao Qi]

On 11/21/2013 07:47 PM, Pedro Alves wrote:
> Putting a user hat on, I'd be left wondering what "affecting
> correctness" means here.

I don't have any troubles understanding "without affecting correctness".
It means GDB still behaves correctly when code-cache option is on.

-- 
Yao (齐尧)

Re: [PATCH 2/3] set/show code-cache

[Eli Zaretskii]

> Date: Thu, 21 Nov 2013 20:19:35 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 11/21/2013 07:47 PM, Pedro Alves wrote:
> > Putting a user hat on, I'd be left wondering what "affecting
> > correctness" means here.
> 
> I don't have any troubles understanding "without affecting correctness".
> It means GDB still behaves correctly when code-cache option is on.

Right, but will the description lose something if you just omit that
part?  I mean, if we never say anything about correctness, users will
have no reason to suspect that correctness might suffer in this case.

Re: [PATCH 2/3] set/show code-cache

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
 
Yao> +@kindex set code-cache
Yao> +@item set code-cache on
Yao> +@itemx set code-cache off
Yao> +Enable or disable caching of code segment accesses.  When @code{ON},
Yao> +use caching.  By default, this option is @code{ON}.  This improves
Yao> +performance of disassembly in remote debugging without affecting
Yao> +correctness.

It seems unusual to have "ON" in upper case.

Tom

Re: [PATCH 2/3] set/show code-cache

[Doug Evans]

On Thu, Nov 21, 2013 at 3:47 AM, Pedro Alves <palves@redhat.com> wrote:
> On 11/21/2013 03:03 AM, Yao Qi wrote:
>> +@kindex set code-cache
>> +@item set code-cache on
>> +@itemx set code-cache off
>> +Enable or disable caching of code segment accesses.  When @code{ON},
>> +use caching.  By default, this option is @code{ON}.  This improves
>> +performance of disassembly in remote debugging without affecting
>> +correctness.
>> +
>
> Putting a user hat on, I'd be left wondering what "affecting
> correctness" means here.

I wondered about this and left it as is, given that's what's written
for the stack-cache case.

Re: [PATCH 2/3] set/show code-cache

[Doug Evans]

On Thu, Nov 21, 2013 at 11:32 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
>
> Yao> +@kindex set code-cache
> Yao> +@item set code-cache on
> Yao> +@itemx set code-cache off
> Yao> +Enable or disable caching of code segment accesses.  When @code{ON},
> Yao> +use caching.  By default, this option is @code{ON}.  This improves
> Yao> +performance of disassembly in remote debugging without affecting
> Yao> +correctness.
>
> It seems unusual to have "ON" in upper case.

cut-n-paste from the stack-cache case.

I forget what the rules are here.
Do we require fixing the stack-cache case first?

Re: [PATCH 2/3] set/show code-cache

[Tom Tromey]

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

Tom> It seems unusual to have "ON" in upper case.

Doug> cut-n-paste from the stack-cache case.

Doug> I forget what the rules are here.
Doug> Do we require fixing the stack-cache case first?

I don't know if we have a rule.

I generally try not to require people to fix existing problems.
My view is that patch submitters aren't responsible for the context; and
that if it matters that much to me, I can fix it myself.  (On occasion I
may ask nicely though...)

But, I think it's ok to require that new bad cases not go in, as a
general rule.

Tom

Re: [PATCH 2/3] set/show code-cache

[Doug Evans]

On Thu, Nov 21, 2013 at 1:11 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Tom> It seems unusual to have "ON" in upper case.
>
> Doug> cut-n-paste from the stack-cache case.
>
> Doug> I forget what the rules are here.
> Doug> Do we require fixing the stack-cache case first?
>
> I don't know if we have a rule.
>
> I generally try not to require people to fix existing problems.
> My view is that patch submitters aren't responsible for the context; and
> that if it matters that much to me, I can fix it myself.  (On occasion I
> may ask nicely though...)
>
> But, I think it's ok to require that new bad cases not go in, as a
> general rule.

"works for me"

Re: [PATCH 2/3] set/show code-cache

[Yao Qi]

On 11/22/2013 12:13 AM, Eli Zaretskii wrote:
>> Date: Thu, 21 Nov 2013 20:19:35 +0800
>> From: Yao Qi <yao@codesourcery.com>
>> CC: <gdb-patches@sourceware.org>
>>
>> On 11/21/2013 07:47 PM, Pedro Alves wrote:
>>> Putting a user hat on, I'd be left wondering what "affecting
>>> correctness" means here.
>>
>> I don't have any troubles understanding "without affecting correctness".
>> It means GDB still behaves correctly when code-cache option is on.
> 
> Right, but will the description lose something if you just omit that
> part?  I mean, if we never say anything about correctness, users will
> have no reason to suspect that correctness might suffer in this case.
> 

OK, let us remove "without affecting correctness".  This patch below is
updated to remove "without affecting correctness" from NEWS and doc,
, write "ON" in lower case in doc. and replace "access" with "accesses"
in command help.

-- 
Yao (齐尧)

gdb:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* NEWS: Add note on new "set code-cache" option.
	* target-dcache.c (code_cache_enabled_1): New variable.
	(code_cache_enabled): New variable.
	(show_code_cache, set_code_cache): New function.
	(code_cache_enabled_p): New function.
	(_initialize_target_dcache): Register command.
	* target-dcache.h (code_cache_enabled_p): Declare.
	* target.c (memory_xfer_partial_1):Handle
	TARGET_OBJECT_CODE_MEMORY and code_cache_enabled.
	(target_read_code): New function.
	* target.h (enum target_object) <TARGET_OBJECT_CODE_MEMORY>:
	New.
	(target_read_code): Declare.

gdb/doc:

2013-11-21  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Remote Data): Document new
	"set/show stack-cache" option.
---
 gdb/NEWS            |    5 ++++
 gdb/doc/gdb.texinfo |   15 +++++++++++++-
 gdb/target-dcache.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target-dcache.h |    2 +
 gdb/target.c        |   23 ++++++++++++++++++---
 gdb/target.h        |    5 ++++
 6 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 9fc3638..5110b27 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -117,6 +117,11 @@ show startup-with-shell
   Specifies whether Unix child processes are started via a shell or
   directly.
 
+set code-cache
+show code-cache
+  Use the target memory cache for accesses to the code segment.  This
+  improves performance of remote debugging (particularly disassembly).
+
 * You can now use a literal value 'unlimited' for options that
   interpret 0 or -1 as meaning "unlimited".  E.g., "set
   trace-buffer-size unlimited" is now an alias for "set
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ec9d901..3371835 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10842,7 +10842,8 @@ Therefore, by default, @value{GDBN} only caches data
 known to be on the stack@footnote{In non-stop mode, it is moderately
 rare for a running thread to modify the stack of a stopped thread
 in a way that would interfere with a backtrace, and caching of
-stack reads provides a significant speed up of remote backtraces.}.
+stack reads provides a significant speed up of remote backtraces.} or
+in the code segment.
 Other regions of memory can be explicitly marked as
 cacheable; @pxref{Memory Region Attributes}.
 
@@ -10867,6 +10868,18 @@ caching.  By default, this option is @code{ON}.
 @item show stack-cache
 Show the current state of data caching for memory accesses.
 
+@kindex set code-cache
+@item set code-cache on
+@itemx set code-cache off
+Enable or disable caching of code segment accesses.  When @code{on},
+use caching.  By default, this option is @code{on}.  This improves
+performance of disassembly in remote debugging.
+
+@kindex show code-cache
+@item show code-cache
+Show the current state of target memory cache for code segment
+accesses.
+
 @kindex info dcache
 @item info dcache @r{[}line@r{]}
 Print the information about the performance of data cache of the
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index 6b7986a..7f68ddc 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -124,6 +124,46 @@ stack_cache_enabled_p (void)
   return stack_cache_enabled;
 }
 
+/* The option sets this.  */
+
+static int code_cache_enabled_1 = 1;
+
+/* And set_code_cache updates this.
+   The reason for the separation is so that we don't flush the cache for
+   on->on transitions.  */
+static int code_cache_enabled = 1;
+
+/* This is called *after* the code-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_code_cache (char *args, int from_tty, struct cmd_list_element *c)
+{
+  if (code_cache_enabled != code_cache_enabled_1)
+    target_dcache_invalidate ();
+
+  code_cache_enabled = code_cache_enabled_1;
+}
+
+/* Show option "code-cache".  */
+
+static void
+show_code_cache (struct ui_file *file, int from_tty,
+		 struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Cache use for code accesses is %s.\n"), value);
+}
+
+/* Return true if "code cache" is enabled, otherwise, return false.  */
+
+int
+code_cache_enabled_p (void)
+{
+  return code_cache_enabled;
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_target_dcache;
 
@@ -141,6 +181,18 @@ By default, caching for stack access is on."),
 			   show_stack_cache,
 			   &setlist, &showlist);
 
+  add_setshow_boolean_cmd ("code-cache", class_support,
+			   &code_cache_enabled_1, _("\
+Set cache use for code segment access."), _("\
+Show cache use for code segment access."), _("\
+When on, use the target memory cache for all code segment accesses,\n\
+regardless of any configured memory regions.  This improves remote\n\
+performance significantly.  By default, caching for code segment\n\
+access is on."),
+			   set_code_cache,
+			   show_code_cache,
+			   &setlist, &showlist);
+
   target_dcache_aspace_key
     = register_address_space_data_with_cleanup (NULL,
 						target_dcache_cleanup);
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
index 3200dd9..59e0b3d 100644
--- a/gdb/target-dcache.h
+++ b/gdb/target-dcache.h
@@ -30,4 +30,6 @@ extern int target_dcache_init_p (void);
 
 extern int stack_cache_enabled_p (void);
 
+extern int code_cache_enabled_p (void);
+
 #endif /* TARGET_DCACHE_H */
diff --git a/gdb/target.c b/gdb/target.c
index f402a18..cc6194b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1547,7 +1547,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	 the collected memory range fails.  */
       && get_traceframe_number () == -1
       && (region->attrib.cache
-	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)))
+	  || (stack_cache_enabled_p () && object == TARGET_OBJECT_STACK_MEMORY)
+	  || (code_cache_enabled_p () && object == TARGET_OBJECT_CODE_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get_or_init ();
 
@@ -1600,8 +1601,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
       && writebuf != NULL
       && target_dcache_init_p ()
       && !region->attrib.cache
-      && stack_cache_enabled_p ()
-      && object != TARGET_OBJECT_STACK_MEMORY)
+      && ((stack_cache_enabled_p () && object != TARGET_OBJECT_STACK_MEMORY)
+	  || (code_cache_enabled_p () && object != TARGET_OBJECT_CODE_MEMORY)))
     {
       DCACHE *dcache = target_dcache_get ();
 
@@ -1697,7 +1698,8 @@ 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 || object == TARGET_OBJECT_STACK_MEMORY)
+  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY
+      || object == TARGET_OBJECT_CODE_MEMORY)
     retval = memory_xfer_partial (ops, object, readbuf,
 				  writebuf, offset, len);
   else
@@ -1799,6 +1801,19 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     return TARGET_XFER_E_IO;
 }
 
+/* Like target_read_memory, but specify explicitly that this is a read from
+   the target's code.  This may trigger different cache behavior.  */
+
+int
+target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
+{
+  if (target_read (current_target.beneath, TARGET_OBJECT_CODE_MEMORY, NULL,
+		   myaddr, memaddr, len) == len)
+    return 0;
+  else
+    return TARGET_XFER_E_IO;
+}
+
 /* Write LEN bytes from MYADDR to target memory at address MEMADDR.
    Returns either 0 for success or a target_xfer_error value if any
    error occurs.  If an error occurs, no guarantee is made about how
diff --git a/gdb/target.h b/gdb/target.h
index 646907a..890171d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -145,6 +145,9 @@ enum target_object
      if it is not in a region marked as such, since it is known to be
      "normal" RAM.  */
   TARGET_OBJECT_STACK_MEMORY,
+  /* Memory known to be part of the target code.   This is cached even
+     if it is not in a region marked as such.  */
+  TARGET_OBJECT_CODE_MEMORY,
   /* Kernel Unwind Table.  See "ia64-tdep.c".  */
   TARGET_OBJECT_UNWIND_TABLE,
   /* Transfer auxilliary vector.  */
@@ -1050,6 +1053,8 @@ extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
 
 extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
 
+extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
+
 extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 				ssize_t len);
 
-- 
1.7.7.6

Re: [PATCH 2/3] set/show code-cache

[Yao Qi]

On 11/22/2013 05:11 AM, Tom Tromey wrote:
> I generally try not to require people to fix existing problems.
> My view is that patch submitters aren't responsible for the context; and
> that if it matters that much to me, I can fix it myself.  (On occasion I
> may ask nicely though...)
> 

I'd like to fix existing problems related to my submissions.  Patch
below updates "ON" and "OFF" in doc in lower case.

> But, I think it's ok to require that new bad cases not go in, as a
> general rule.

Agreed.

-- 
Yao (齐尧)

gdb/doc:

2013-11-22  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Caching Target Data): Replace "ON" with "on".
	(Maintenance Commands): Replace "ON" and "OFF" with "on" and
	"off" respectively.
---
 gdb/doc/gdb.texinfo |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 3371835..0359570 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10861,8 +10861,8 @@ 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}.
+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
@@ -37545,7 +37545,7 @@ compiled with the @samp{-pg} compiler option.
 @item maint set show-debug-regs
 @itemx maint show show-debug-regs
 Control whether to show variables that mirror the hardware debug
-registers.  Use @code{ON} to enable, @code{OFF} to disable.  If
+registers.  Use @code{on} to enable, @code{off} to disable.  If
 enabled, the debug registers values are shown when @value{GDBN} inserts or
 removes a hardware breakpoint or watchpoint, and when the inferior
 triggers a hardware-assisted breakpoint or watchpoint.
-- 
1.7.7.6

Re: [PATCH 2/3] set/show code-cache

[Eli Zaretskii]

> Date: Fri, 22 Nov 2013 09:49:40 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: Doug Evans <dje@google.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> On 11/22/2013 05:11 AM, Tom Tromey wrote:
> > I generally try not to require people to fix existing problems.
> > My view is that patch submitters aren't responsible for the context; and
> > that if it matters that much to me, I can fix it myself.  (On occasion I
> > may ask nicely though...)
> > 
> 
> I'd like to fix existing problems related to my submissions.  Patch
> below updates "ON" and "OFF" in doc in lower case.

OK, thanks.

Re: [PATCH 0/3 V3] Cache code access for disassemble

[Yao Qi]

On 11/21/2013 09:16 AM, Yao Qi wrote:
> This is the V3 of this patch series.  Most of the patches were
> committed because they are cleanups, and V3 only has three patches.
>
> Patch 1/3 is to rename some functions and variables related to
> "stack-cache", to address review comments in V2 that we find they are
> improper.  Patch 2/3 is to add commands "set/show code-cache", and we
> choose a way that "both stack cache and code cache use the single global
> dcache, and transitions of either options will invalidate the dcache".
> Patch 3/3 is unchanged.

This patch series is pushed!

-- 
Yao (齐尧)