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

Yao Qi
2013-11-21T02:56:42Z

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