This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.
- From: Doug Evans <dje at google dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Sun, 17 Nov 2013 12:34:34 -0800
- Subject: Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.
- Authentication-results: sourceware.org; auth=none
- References: <1383458049-20893-1-git-send-email-yao at codesourcery dot com> <1383458049-20893-6-git-send-email-yao at codesourcery dot com>
On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> Nowadays, when cache size or line size is changed by command,
> 'target_dcache' is invalidated. It is too conservative. We can
> optimize in the following ways,
>
> - Don't have to invalidate dcache immediately after cache size or
> line size is changed. We can postpone the invalidation to the moment
> using 'target_dcache'.
> - Don't have to invalidate dcache if the cache size is changed. If
> cache size is changed to the value which is still greater than
> dcache's size, nothing should be done. If change to the value
> which is less than dcache's size, just evict cache lines.
>
> This is what this patch does.
>
> gdb:
>
> 2013-11-02 Yao Qi <yao@codesourcery.com>
>
> * dcache.c (dcache_evict): New function.
> (dcache_shrink): New function.
> (dcache_invalidate_p): New function.
> (dcache_alloc): Call dcache_evict.
> (set_dcache_size): Remove call target_dcache_invalidate.
> (set_dcache_line_size): Likewise.
> * dcache.h (dcache_shrink): Declare.
> (dcache_invalidate_p): Declare.
> * target-dcache.c (target_dcache_get): Invalidate and shrink
> cache if necessary.
> (target_dcache_get_or_init): Likewise.
> [...]
> diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
> index bf04679..28f1aa6 100644
> --- a/gdb/target-dcache.c
> +++ b/gdb/target-dcache.c
> @@ -45,6 +45,14 @@ target_dcache_invalidate (void)
> DCACHE *
> target_dcache_get (void)
> {
> + if (target_dcache_init_p ())
> + {
> + if (dcache_invalidate_p (target_dcache))
> + dcache_invalidate (target_dcache);
> +
> + dcache_shrink (target_dcache);
> + }
> +
> return target_dcache;
> }
It feels simpler if this were:
if (target_dcache_init_p ())
dcache_check_resize (target_dcache);
return target_dcache;
There may be a better name than dcache_check_resize, one may want to
call it for situations other than resizings, but here target-dcache.c
is using 3 API routines from dcache.c to achieve what is essentially
one operation: check if the cache needs to be reconfigured and update
if so.
> @@ -54,7 +62,14 @@ target_dcache_get (void)
> DCACHE *
> target_dcache_get_or_init (void)
> {
> - if (!target_dcache_init_p ())
> + if (target_dcache_init_p ())
> + {
> + if (dcache_invalidate_p (target_dcache))
> + dcache_invalidate (target_dcache);
> +
> + dcache_shrink (target_dcache);
> + }
> + else
> target_dcache = dcache_init ();
>
> return target_dcache;
I think it would be better to remove the duplication and have:
if (target_dcache_init_p ())
return target_dcache_get ();
...