Re: [PATCH] Remove frame_observer_target_changed

[Yao Qi <yao at codesourcery dot com>]

On 09/12/2012 10:41 PM, Pedro Alves wrote:
> But, is there a general direction this is following?  I ask because
> reinit_frame_cache will still be called twice, through
> regcache_observer_target_changed, which is installed as target_changed observer
> too.  Extra calls to reinit_frame_cache should be extremely cheap (given
> the caches are built on demand).
> 
> The observer was introduced from this discussion:
> 
>   http://sourceware.org/ml/gdb-patches/2004-04/msg00520.html
> 
> And looking at:
> 
>    /* Assigning to the stack pointer, frame pointer, and other
>       (architecture and calling convention specific) registers may
>       cause the frame cache to be out of date.  Assigning to memory
>       also can.  We just do this on all assignments to registers or
>       memory, for simplicity's sake; I doubt the slowdown matters.  */
>    switch (VALUE_LVAL (toval))
>      {
>      case lval_memory:
>      case lval_register:
>      case lval_computed:
> 
>        reinit_frame_cache ();
> 
> 
> makes me wonder ... shouldn't we also be invalidating the register
> cache when we change memory, considering writes to memory-mapped
> registers?  Andrew even mentioned something of the sort in that

Right, we have to be conservative on modifying any memory or register
unless we have an enhanced target description to describe 1) memory-
mapped register, 2) bank register 3) write-able read-only register etc.

> email - it seems like it was overlooked.  With that in mind, we'd go the
> other way around, and replace that reinit_frame_cache call with a
> observer_notify_target_changed, and remove the call from
> the earlier switch.

Here is the patch.  Regression tested on x86_64-linux.

-- 
Yao

gdb:

2012-09-13  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* valops.c (value_assign): Move observer_notify_target_changed
	below to replace reinit_frame_cache.
---
 gdb/valops.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/gdb/valops.c b/gdb/valops.c
index 17696ee..3a5ac0f 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1374,7 +1374,6 @@ value_assign (struct value *toval, struct value *fromval)
 
 	if (deprecated_register_changed_hook)
 	  deprecated_register_changed_hook (-1);
-	observer_notify_target_changed (&current_target);
 	break;
       }
 
@@ -1396,7 +1395,7 @@ value_assign (struct value *toval, struct value *fromval)
 
   /* Assigning to the stack pointer, frame pointer, and other
      (architecture and calling convention specific) registers may
-     cause the frame cache to be out of date.  Assigning to memory
+     cause the frame cache and regcache to be out of date.  Assigning to memory
      also can.  We just do this on all assignments to registers or
      memory, for simplicity's sake; I doubt the slowdown matters.  */
   switch (VALUE_LVAL (toval))
@@ -1405,7 +1404,7 @@ value_assign (struct value *toval, struct value *fromval)
     case lval_register:
     case lval_computed:
 
-      reinit_frame_cache ();
+      observer_notify_target_changed (&current_target);
 
       /* Having destroyed the frame cache, restore the selected
 	 frame.  */
-- 
1.7.7.6