This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]