Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Multiplexed registers and invalidating the register cache
@ 2004-04-14 11:44 Orjan Friberg
  2004-04-14 14:46 ` Daniel Jacobowitz
  0 siblings, 1 reply; 27+ messages in thread
From: Orjan Friberg @ 2004-04-14 11:44 UTC (permalink / raw)
  To: gdb-patches

Hi all,

I'm working on support for a CPU where some of the registers are in a 
bank selected by another register, meaning that changing the bank select 
register changes the contents (and meaning) for a whole set of other 
registers.

The target is a remote stub, and the way I've pictured this in my head a 
change to the bank select register (via a 'P' packet) invalidates the 
register cache, causing the whole register contents to be fetched again 
(with a 'g' packet).  (The remote stub needs to re-read the affected 
registers upon the write of the bank select register, of course.)

Is there some sort of "write register" hook I could use to indicate that 
the registers should be fetched again if the bank select register is 
written to?  I followed what happens when doing a "set $register", but I 
couldn't find any such hook in that path.

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-14 11:44 Multiplexed registers and invalidating the register cache Orjan Friberg
@ 2004-04-14 14:46 ` Daniel Jacobowitz
  2004-04-14 16:01   ` Andrew Cagney
  2004-04-15 10:46   ` Orjan Friberg
  0 siblings, 2 replies; 27+ messages in thread
From: Daniel Jacobowitz @ 2004-04-14 14:46 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: gdb-patches

On Wed, Apr 14, 2004 at 01:44:43PM +0200, Orjan Friberg wrote:
> Hi all,
> 
> I'm working on support for a CPU where some of the registers are in a 
> bank selected by another register, meaning that changing the bank select 
> register changes the contents (and meaning) for a whole set of other 
> registers.
> 
> The target is a remote stub, and the way I've pictured this in my head a 
> change to the bank select register (via a 'P' packet) invalidates the 
> register cache, causing the whole register contents to be fetched again 
> (with a 'g' packet).  (The remote stub needs to re-read the affected 
> registers upon the write of the bank select register, of course.)
> 
> Is there some sort of "write register" hook I could use to indicate that 
> the registers should be fetched again if the bank select register is 
> written to?  I followed what happens when doing a "set $register", but I 
> couldn't find any such hook in that path.

I think you should make this change unconditionally - and flush the
entire frame cache.  I'm not sure whether it should be in the generic
code that writes a register or the user-level code triggered by set
$reg = val, though.

I've been meaning to do this for a long time.  For instance, there is a
writeable register on PowerPC targets which has some read-only bits. 
Right now, if you set it to an arbitrary value and then print it you'll
get the value GDB wrote - not the value that was actually accepted into
the register.

Andrew convinced me that the performance cost associated with this
would be small in practice.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-14 14:46 ` Daniel Jacobowitz
@ 2004-04-14 16:01   ` Andrew Cagney
  2004-04-15 10:46   ` Orjan Friberg
  1 sibling, 0 replies; 27+ messages in thread
From: Andrew Cagney @ 2004-04-14 16:01 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Orjan Friberg, gdb-patches


> I think you should make this change unconditionally - and flush the
> entire frame cache.  I'm not sure whether it should be in the generic
> code that writes a register or the user-level code triggered by set
> $reg = val, though.
> 
> I've been meaning to do this for a long time.  For instance, there is a
> writeable register on PowerPC targets which has some read-only bits. 
> Right now, if you set it to an arbitrary value and then print it you'll
> get the value GDB wrote - not the value that was actually accepted into
> the register.
> 
> Andrew convinced me that the performance cost associated with this
> would be small in practice.

Right.

Andrew



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-14 14:46 ` Daniel Jacobowitz
  2004-04-14 16:01   ` Andrew Cagney
@ 2004-04-15 10:46   ` Orjan Friberg
  2004-04-15 11:25     ` Orjan Friberg
  1 sibling, 1 reply; 27+ messages in thread
From: Orjan Friberg @ 2004-04-15 10:46 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

Daniel Jacobowitz wrote:
> 
> I think you should make this change unconditionally - and flush the
> entire frame cache.

Pardon my ignorance, but why should the entire frame cache be flushed, 
instead of just invalidating the current set of registers?  Is it 
because frame pointer/return address registers (or something else 
affecting other frames) might change?

> I'm not sure whether it should be in the generic
> code that writes a register or the user-level code triggered by set
> $reg = val, though.

So either something like

   target_register_write (regno)

where the target-specific code can do whatever it wants (presumably 
calling flush_cached_frames), or a more specialized function

   target_flush_register_cache_on_register_write (regno)

and then have whatever calls that function do the actual frame cache 
flushing.

> Andrew convinced me that the performance cost associated with this
> would be small in practice.

So, it's not worth the trouble implementing a more fine-grained solution 
like "refetch only the modified register".

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-15 10:46   ` Orjan Friberg
@ 2004-04-15 11:25     ` Orjan Friberg
  2004-04-15 15:29       ` Andrew Cagney
  0 siblings, 1 reply; 27+ messages in thread
From: Orjan Friberg @ 2004-04-15 11:25 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

Orjan Friberg wrote:
> Daniel Jacobowitz wrote:
> 
>>
>> I think you should make this change unconditionally - and flush the
>> entire frame cache.
> 
> 
> Pardon my ignorance, but why should the entire frame cache be flushed, 
> instead of just invalidating the current set of registers?  Is it 
> because frame pointer/return address registers (or something else 
> affecting other frames) might change?

Too quick on the send button... just found the flushregs command, which 
does what I want for the time being (flushing the register cache).

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-15 11:25     ` Orjan Friberg
@ 2004-04-15 15:29       ` Andrew Cagney
  2004-04-16 12:51         ` Orjan Friberg
  0 siblings, 1 reply; 27+ messages in thread
From: Andrew Cagney @ 2004-04-15 15:29 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: Daniel Jacobowitz, gdb-patches

> Orjan Friberg wrote:
> 
>> Daniel Jacobowitz wrote:
>>
>>>
>>> I think you should make this change unconditionally - and flush the
>>> entire frame cache.
>>
>>
>>
>> Pardon my ignorance, but why should the entire frame cache be flushed, instead of just invalidating the current set of registers?  Is it because frame pointer/return address registers (or something else affecting other frames) might change?

Consider the effect of modifying the $sp.

While it might in theory be possible to implement some sort of 
complicated look-aside cache schema, in reality there is zero return on 
investment.  Since recovery from the flush can't be slower than recovery 
from single-step, and single step is way way more critical, we should 
focus on single step.

Andrew



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-15 15:29       ` Andrew Cagney
@ 2004-04-16 12:51         ` Orjan Friberg
  2004-04-16 14:13           ` Daniel Jacobowitz
  2004-04-16 19:15           ` Andrew Cagney
  0 siblings, 2 replies; 27+ messages in thread
From: Orjan Friberg @ 2004-04-16 12:51 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Daniel Jacobowitz, gdb-patches

Andrew Cagney wrote:
> 
> Consider the effect of modifying the $sp.
> 
> While it might in theory be possible to implement some sort of 
> complicated look-aside cache schema, in reality there is zero return on 
> investment.  Since recovery from the flush can't be slower than recovery 
> from single-step, and single step is way way more critical, we should 
> focus on single step.

Ok, I'm convinced that the whole frame cache should be discarded, except 
I must be missing something since calling flush_cached_frames doesn't 
refetch the current set of registers (like calling registers_changed does).

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-16 12:51         ` Orjan Friberg
@ 2004-04-16 14:13           ` Daniel Jacobowitz
  2004-04-16 14:54             ` Orjan Friberg
  2004-04-16 19:15           ` Andrew Cagney
  1 sibling, 1 reply; 27+ messages in thread
From: Daniel Jacobowitz @ 2004-04-16 14:13 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: Andrew Cagney, gdb-patches

On Fri, Apr 16, 2004 at 02:50:27PM +0200, Orjan Friberg wrote:
> Andrew Cagney wrote:
> >
> >Consider the effect of modifying the $sp.
> >
> >While it might in theory be possible to implement some sort of 
> >complicated look-aside cache schema, in reality there is zero return on 
> >investment.  Since recovery from the flush can't be slower than recovery 
> >from single-step, and single step is way way more critical, we should 
> >focus on single step.
> 
> Ok, I'm convinced that the whole frame cache should be discarded, except 
> I must be missing something since calling flush_cached_frames doesn't 
> refetch the current set of registers (like calling registers_changed does).

Yes, I think you need to do both.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-16 14:13           ` Daniel Jacobowitz
@ 2004-04-16 14:54             ` Orjan Friberg
  0 siblings, 0 replies; 27+ messages in thread
From: Orjan Friberg @ 2004-04-16 14:54 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Andrew Cagney, gdb-patches

Daniel Jacobowitz wrote:
> On Fri, Apr 16, 2004 at 02:50:27PM +0200, Orjan Friberg wrote:
>>
>>Ok, I'm convinced that the whole frame cache should be discarded, except 
>>I must be missing something since calling flush_cached_frames doesn't 
>>refetch the current set of registers (like calling registers_changed does).
> 
> 
> Yes, I think you need to do both.

Ok, I'll put together a patch on Monday.  It probably won't be correct, 
   but at least it will serve as a basis for discussion.  Thanks.

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-16 12:51         ` Orjan Friberg
  2004-04-16 14:13           ` Daniel Jacobowitz
@ 2004-04-16 19:15           ` Andrew Cagney
  2004-04-19 14:13             ` Orjan Friberg
  1 sibling, 1 reply; 27+ messages in thread
From: Andrew Cagney @ 2004-04-16 19:15 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: Daniel Jacobowitz, gdb-patches

> Andrew Cagney wrote:
> 
>>
>> Consider the effect of modifying the $sp.
>>
>> While it might in theory be possible to implement some sort of complicated look-aside cache schema, in reality there is zero return on investment.  Since recovery from the flush can't be slower than recovery from single-step, and single step is way way more critical, we should focus on single step.
> 
> 
> Ok, I'm convinced that the whole frame cache should be discarded, except I must be missing something since calling flush_cached_frames doesn't refetch the current set of registers (like calling registers_changed does). 

(mumble something about doco) two fyis:

- the frame cache is built on-demand, hence the absence of any explict 
rebuild call (one characteristic of a frame ID is that it survives frame 
cache flushes)
- there shouldn't be separate register and frame flush calls, combining 
the two into a single observer call is a thing-to-do-today

enjoy,
Andrew




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-16 19:15           ` Andrew Cagney
@ 2004-04-19 14:13             ` Orjan Friberg
  2004-04-21 16:48               ` Andrew Cagney
  0 siblings, 1 reply; 27+ messages in thread
From: Orjan Friberg @ 2004-04-19 14:13 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

Andrew Cagney wrote:
> 
> - the frame cache is built on-demand, hence the absence of any explict 
> rebuild call (one characteristic of a frame ID is that it survives frame 
> cache flushes)

In other words, I shouldn't expect to see a whole lot of communication 
with the remote target simply because I flush the frame cache.

> - there shouldn't be separate register and frame flush calls, combining 
> the two into a single observer call is a thing-to-do-today

Ok, let's see if I understand this correctly.  Sorry if I'm asking what 
is (or should be) obvious (I only see observer.exp using this code as of 
now, so it's not clear to me how this is supposed to be used within GDB).

A new event needs to be defined (user_changed_registers(?), taking the 
register number as an argument).  This will give us attach/detach/notify 
functions related to that event.  So, someone needs to attach a 
callback: is this something that should be done in target-specific files?

If so, should targets provide their own callback implementation (which 
flushes the register and frame cache when deemed appropriate), or should 
there be a predefined function that the target-specific file simply 
references when attaching the callback?

In addition, someone needs to notify that the event has happened, but I 
assume these notifications should be inserted at the same places that 
register_changed_hook is called for GUI purposes.

Anything I missed or misunderstood?

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-19 14:13             ` Orjan Friberg
@ 2004-04-21 16:48               ` Andrew Cagney
  2004-04-22 13:58                 ` Orjan Friberg
  0 siblings, 1 reply; 27+ messages in thread
From: Andrew Cagney @ 2004-04-21 16:48 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: gdb-patches

> Andrew Cagney wrote:
> 
>>
>> - the frame cache is built on-demand, hence the absence of any explict rebuild call (one characteristic of a frame ID is that it survives frame cache flushes)
> 
> 
> In other words, I shouldn't expect to see a whole lot of communication with the remote target simply because I flush the frame cache.

Yes.

>> - there shouldn't be separate register and frame flush calls, combining the two into a single observer call is a thing-to-do-today
> 
> 
> Ok, let's see if I understand this correctly.  Sorry if I'm asking what is (or should be) obvious (I only see observer.exp using this code as of now, so it's not clear to me how this is supposed to be used within GDB).
> 
> A new event needs to be defined (user_changed_registers(?), taking the register number as an argument).  This will give us attach/detach/notify functions related to that event.  So, someone needs to attach a callback: is this something that should be done in target-specific files?

Simpler, "target_changed" - no parameters.

> If so, should targets provide their own callback implementation (which flushes the register and frame cache when deemed appropriate), or should there be a predefined function that the target-specific file simply references when attaching the callback?

The core code, after doing the write, should trigger this event ...

> In addition, someone needs to notify that the event has happened, but I assume these notifications should be inserted at the same places that register_changed_hook is called for GUI purposes.

... [yes] replacing registers changed.

> Anything I missed or misunderstood?

No, you've got the theory.

Andrew



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-21 16:48               ` Andrew Cagney
@ 2004-04-22 13:58                 ` Orjan Friberg
  2004-04-22 14:33                   ` Andrew Cagney
  0 siblings, 1 reply; 27+ messages in thread
From: Orjan Friberg @ 2004-04-22 13:58 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

Andrew Cagney wrote:
> 
> Simpler, "target_changed" - no parameters.

Hm. observer.texi says "all events must take at least one parameter", 
but having an event without a parameter worked fine.  I assumed 
observer.sh would yell at me, but it didn't ;) .

But wouldn't it make sense to have the register number parameter, and 
based on that determine (in target-specific code) whether to flush the 
frame cache and registers?  Or was that part of the cost discussion 
between you and Daniel, and what you're saying is that "whenever *any* 
register is changed in *any* target, flush the frame and register cache"?

> The core code, after doing the write, should trigger this event ...
> 
>> In addition, someone needs to notify that the event has happened, but 
>> I assume these notifications should be inserted at the same places 
>> that register_changed_hook is called for GUI purposes.
> 
> 
> ... [yes] replacing registers changed.

... meaning all calls to registers_changed should be changed into calls 
to observer_notify_target_changed?

Also, if the observer should a generic

   void
   observer_target_changed (void)
   {
     registers_changed ();
     flush_cached_frames ();
   }

instead of target-specific one (I couldn't infer that from your answer), 
where should it go?

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-22 13:58                 ` Orjan Friberg
@ 2004-04-22 14:33                   ` Andrew Cagney
  2004-04-23 11:25                     ` Orjan Friberg
  0 siblings, 1 reply; 27+ messages in thread
From: Andrew Cagney @ 2004-04-22 14:33 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: gdb-patches

> Andrew Cagney wrote:
> 
>>
>> Simpler, "target_changed" - no parameters.
> 
> 
> Hm. observer.texi says "all events must take at least one parameter", but having an event without a parameter worked fine.  I assumed observer.sh would yell at me, but it didn't ;) .

Oops, yes, pass the current_target (I ment that it shouldn't have a 
parameter trying to tell the observers anything beyond the fact that the 
target has changed).  (the sed script generates () instead of (void)).

> But wouldn't it make sense to have the register number parameter, and based on that determine (in target-specific code) whether to flush the frame cache and registers?  Or was that part of the cost discussion between you and Daniel, and what you're saying is that "whenever *any* register is changed in *any* target, flush the frame and register cache"?

There are architectures where registers live in memory so the 
information is simply misleading.

>> The core code, after doing the write, should trigger this event ...
>>
>>> In addition, someone needs to notify that the event has happened, but I assume these notifications should be inserted at the same places that register_changed_hook is called for GUI purposes.
>>
>>
>>
>> ... [yes] replacing registers changed.
> 
> 
> ... meaning all calls to registers_changed should be changed into calls to observer_notify_target_changed?

Eventually, yes.  The two can co-habitate for a bit, first just:

- add the observer
- add code to frame.c and regcache.c to register themselves
- add code to the problem area to trigger the observer

> Also, if the observer should a generic
> 
>   void
>   observer_target_changed (void)
>   {
>     registers_changed ();
>     flush_cached_frames ();
>   }
> 
> instead of target-specific one (I couldn't infer that from your answer), where should it go?

sorry, I'm lost

Andrew



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-22 14:33                   ` Andrew Cagney
@ 2004-04-23 11:25                     ` Orjan Friberg
  2004-04-24  0:03                       ` Andrew Cagney
  0 siblings, 1 reply; 27+ messages in thread
From: Orjan Friberg @ 2004-04-23 11:25 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

Andrew Cagney wrote:
> 
> Oops, yes, pass the current_target (I ment that it shouldn't have a 
> parameter trying to tell the observers anything beyond the fact that the 
> target has changed).  (the sed script generates () instead of (void)).

Ok.

> There are architectures where registers live in memory so the 
> information is simply misleading.

Ok, I'm convinced.  (Thanks for clarifying.)

> Eventually, yes.  The two can co-habitate for a bit, first just:
> 
> - add the observer

Ok.  observer.texi gets the following lines added:

   @deftypefun void target_changed (struct target_ops *@var{current_target})
   The target's register contents has changed.
   @end deftypefun

(Proper patch delayed until I've got all the pieces in place.)

> - add code to frame.c and regcache.c to register themselves

I'm sorry; this is the part I don't get - my skull must be getting really thick. 
  Are you saying that frame.c and regcache.c should register an observer each 
for the target_changed event, like this (frame.c):

   void
   frame_observer_target_changed (struct target_ops *current_target)
   {
     flush_cached_frames ();
   }
   observer_attach_target_changed (frame_observer_target_changed);

and (regcache.c):

   void
   regcache_observer_target_changed (struct target_ops *current_target)
   {
     registers_changed ();
   }
   observer_attach_target_changed (regcache_observer_target_changed);

> - add code to the problem area to trigger the observer

Ok; something like (example from valops.c):

         if (deprecated_register_changed_hook)
           deprecated_register_changed_hook (-1);
         target_changed_event ();
+       observer_notify_target_changed (current_target);
         break;
        }

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-24  0:03                       ` Andrew Cagney
@ 2004-04-23 15:20                         ` Orjan Friberg
  2004-04-23 17:58                           ` Eli Zaretskii
  2004-04-24  0:03                           ` Andrew Cagney
  0 siblings, 2 replies; 27+ messages in thread
From: Orjan Friberg @ 2004-04-23 15:20 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

Andrew Cagney wrote:
> Yes, on all counts!

Wohoo!  Patches below:

2004-04-23  Orjan Friberg  <orjanf@axis.com>

	* observer.texi (GDB Observers): Add target_changed event.

Index: doc/observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.3
diff -u -p -r1.3 observer.texi
--- doc/observer.texi   15 Apr 2004 14:29:21 -0000      1.3
+++ doc/observer.texi   23 Apr 2004 15:02:02 -0000
@@ -73,3 +73,7 @@ The following observable events are defi
  @deftypefun void normal_stop (struct bpstats *@var{bs})
  The inferior has stopped for real.
  @end deftypefun
+
+@deftypefun void target_changed (struct target_ops *@var{current_target})
+The target's register contents has changed.
+@end deftypefun


2004-04-23  Orjan Friberg <orjanf@axis.com>

	* frame.c: Include "observer.h".
	(frame_observer_target_changed): New function.
	(_initialize_frame): Attach target_changed observer.
	* regcache.c: Include "observer.h".
	(regcache_observer_target_changed): New function.
	(_initialize_regcache): Attach target_changed observer.
	* valops.c: Include "observer.h".
	(value_assign): Notify target_changed event when modifying register.


Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.172
diff -u -p -r1.172 frame.c
--- frame.c     21 Apr 2004 23:52:20 -0000      1.172
+++ frame.c     23 Apr 2004 15:17:59 -0000
@@ -39,6 +39,7 @@
  #include "frame-base.h"
  #include "command.h"
  #include "gdbcmd.h"
+#include "observer.h"

  static struct frame_info *get_prev_frame_1 (struct frame_info *this_frame);

@@ -1237,6 +1238,14 @@ get_next_frame (struct frame_info *this_
      return NULL;
  }

+/* Observer for the target_changed event.  */
+
+void
+frame_observer_target_changed (struct target_ops *current_target)
+{
+  flush_cached_frames ();
+}
+
  /* Flush the entire frame cache.  */

  void
@@ -2355,6 +2364,8 @@ void
  _initialize_frame (void)
  {
    obstack_init (&frame_cache_obstack);
+
+  observer_attach_target_changed (frame_observer_target_changed);

    add_prefix_cmd ("backtrace", class_maintenance, set_backtrace_cmd, "\
  Set backtrace specific variables.\n\
Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.112
diff -u -p -r1.112 regcache.c
--- regcache.c  21 Apr 2004 23:52:20 -0000      1.112
+++ regcache.c  23 Apr 2004 15:17:59 -0000
@@ -30,6 +30,7 @@
  #include "gdb_assert.h"
  #include "gdb_string.h"
  #include "gdbcmd.h"            /* For maintenanceprintlist.  */
+#include "observer.h"

  /*
   * DATA STRUCTURE
@@ -566,6 +567,14 @@ real_register (int regnum)
    return regnum >= 0 && regnum < NUM_REGS;
  }

+/* Observer for the target_changed event.  */
+
+void
+regcache_observer_target_changed (struct target_ops *current_target)
+{
+  registers_changed ();
+}
+
  /* Low level examining and depositing of registers.

     The caller is responsible for making sure that the inferior is
@@ -1696,6 +1705,8 @@ _initialize_regcache (void)
    DEPRECATED_REGISTER_GDBARCH_SWAP (deprecated_registers);
    DEPRECATED_REGISTER_GDBARCH_SWAP (deprecated_register_valid);
    deprecated_register_gdbarch_swap (NULL, 0, build_regcache);
+
+  observer_attach_target_changed (regcache_observer_target_changed);

    add_com ("flushregs", class_maintenance, reg_flush_command,
            "Force gdb to flush its register cache (maintainer command)");
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.123
diff -u -p -r1.123 valops.c
--- valops.c    21 Apr 2004 23:52:21 -0000      1.123
+++ valops.c    23 Apr 2004 15:18:00 -0000
@@ -42,6 +42,7 @@
  #include "gdb_string.h"
  #include "gdb_assert.h"
  #include "cp-support.h"
+#include "observer.h"

  extern int overload_debug;
  /* Local functions.  */
@@ -701,6 +702,7 @@ value_assign (struct value *toval, struc
         if (deprecated_register_changed_hook)
           deprecated_register_changed_hook (-1);
         target_changed_event ();
+       observer_notify_target_changed (current_target);
         break;
        }


-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-23 15:20                         ` Orjan Friberg
@ 2004-04-23 17:58                           ` Eli Zaretskii
  2004-04-26  9:41                             ` Orjan Friberg
  2004-04-24  0:03                           ` Andrew Cagney
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2004-04-23 17:58 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: cagney, gdb-patches

> Date: Fri, 23 Apr 2004 17:20:23 +0200
> From: Orjan Friberg <orjan.friberg@axis.com>
> 
> Andrew Cagney wrote:
> > Yes, on all counts!
> 
> Wohoo!  Patches below:
> 
> 2004-04-23  Orjan Friberg  <orjanf@axis.com>
> 
> 	* observer.texi (GDB Observers): Add target_changed event.

This part of your patch is approved.  Thanks.


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-23 15:20                         ` Orjan Friberg
  2004-04-23 17:58                           ` Eli Zaretskii
@ 2004-04-24  0:03                           ` Andrew Cagney
  2004-04-24  8:30                             ` Eli Zaretskii
  2004-04-26  9:50                             ` Orjan Friberg
  1 sibling, 2 replies; 27+ messages in thread
From: Andrew Cagney @ 2004-04-24  0:03 UTC (permalink / raw)
  To: Orjan Friberg, Eli Zaretskii; +Cc: gdb-patches

> +The target's register contents has changed.

FYI, this should probably read:
	The target's memory or register contents have [has?] changed.
eli?

> +@deftypefun void target_changed (struct target_ops *@var{current_target})

- As a parameter, I'd just use "target" (you've current_target as a 
parameter in several places) (one day there will be more than one target 
...).

> +#include "observer.h"

- you'll want to update Makefile.in

Otherwize assuming, eli's ok with the fixed wording, its ok to commit.

Andrew



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-23 11:25                     ` Orjan Friberg
@ 2004-04-24  0:03                       ` Andrew Cagney
  2004-04-23 15:20                         ` Orjan Friberg
  0 siblings, 1 reply; 27+ messages in thread
From: Andrew Cagney @ 2004-04-24  0:03 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: gdb-patches

Yes, on all counts!


> Andrew Cagney wrote:
> 
>>
>> Oops, yes, pass the current_target (I ment that it shouldn't have a parameter trying to tell the observers anything beyond the fact that the target has changed).  (the sed script generates () instead of (void)).
> 
> 
> Ok.
> 
>> There are architectures where registers live in memory so the information is simply misleading.
> 
> 
> Ok, I'm convinced.  (Thanks for clarifying.)
> 
>> Eventually, yes.  The two can co-habitate for a bit, first just:
>>
>> - add the observer
> 
> 
> Ok.  observer.texi gets the following lines added:
> 
>   @deftypefun void target_changed (struct target_ops *@var{current_target})
>   The target's register contents has changed.
>   @end deftypefun
> 
> (Proper patch delayed until I've got all the pieces in place.)
> 
>> - add code to frame.c and regcache.c to register themselves
> 
> 
> I'm sorry; this is the part I don't get - my skull must be getting really thick.  Are you saying that frame.c and regcache.c should register an observer each for the target_changed event, like this (frame.c):
> 
>   void
>   frame_observer_target_changed (struct target_ops *current_target)
>   {
>     flush_cached_frames ();
>   }
>   observer_attach_target_changed (frame_observer_target_changed);
> 
> and (regcache.c):
> 
>   void
>   regcache_observer_target_changed (struct target_ops *current_target)
>   {
>     registers_changed ();
>   }
>   observer_attach_target_changed (regcache_observer_target_changed);
> 
>> - add code to the problem area to trigger the observer
> 
> 
> Ok; something like (example from valops.c):
> 
>         if (deprecated_register_changed_hook)
>           deprecated_register_changed_hook (-1);
>         target_changed_event ();
> +       observer_notify_target_changed (current_target);
>         break;
>        }
> 
> -- 
> Orjan Friberg
> Axis Communications
> 
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-24  0:03                           ` Andrew Cagney
@ 2004-04-24  8:30                             ` Eli Zaretskii
  2004-04-28 16:43                               ` Andrew Cagney
  2004-04-26  9:50                             ` Orjan Friberg
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2004-04-24  8:30 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: orjan.friberg, gdb-patches

> Date: Fri, 23 Apr 2004 14:41:49 -0400
> From: Andrew Cagney <cagney@gnu.org>
> 
> > +The target's register contents has changed.
> 
> FYI, this should probably read:
> 	The target's memory or register contents have [has?] changed.
> eli?

I'm not sure; what is the difference between the two wordings?


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-23 17:58                           ` Eli Zaretskii
@ 2004-04-26  9:41                             ` Orjan Friberg
  0 siblings, 0 replies; 27+ messages in thread
From: Orjan Friberg @ 2004-04-26  9:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cagney, gdb-patches

Eli Zaretskii wrote:
> 
> This part of your patch is approved.  Thanks.

Committed, with Andrew's suggested name change of the current_target 
argument.  (If you object to that change, please let me know.)

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-24  0:03                           ` Andrew Cagney
  2004-04-24  8:30                             ` Eli Zaretskii
@ 2004-04-26  9:50                             ` Orjan Friberg
  1 sibling, 0 replies; 27+ messages in thread
From: Orjan Friberg @ 2004-04-26  9:50 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Eli Zaretskii, gdb-patches

Andrew Cagney wrote:
> 
> - you'll want to update Makefile.in
> 
> Otherwize assuming, eli's ok with the fixed wording, its ok to commit.

Ok, committed. (Makefile.in updated, plus the passing of the 
current_target argument was wrong in the original patch).  Thanks.

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-24  8:30                             ` Eli Zaretskii
@ 2004-04-28 16:43                               ` Andrew Cagney
  2004-04-28 17:40                                 ` Eli Zaretskii
  0 siblings, 1 reply; 27+ messages in thread
From: Andrew Cagney @ 2004-04-28 16:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: orjan.friberg, gdb-patches

>>> Date: Fri, 23 Apr 2004 14:41:49 -0400
>>> From: Andrew Cagney <cagney@gnu.org>
>>> 
>>
>>>> > +The target's register contents has changed.
>>
>>> 
>>> FYI, this should probably read:
>>> 	The target's memory or register contents have [has?] changed.
>>> eli?
> 
> 
> I'm not sure; what is the difference between the two wordings?

"have" sounds right (...), hmm.  Check dictionary ``/has/ 3rd person 
_singular_, present of /have/'' [canadian oxford] so "have" is correct.

Andrew



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-28 16:43                               ` Andrew Cagney
@ 2004-04-28 17:40                                 ` Eli Zaretskii
  2004-04-29  8:09                                   ` Orjan Friberg
  0 siblings, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2004-04-28 17:40 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: orjan.friberg, gdb-patches

> Date: Wed, 28 Apr 2004 12:43:12 -0400
> From: Andrew Cagney <cagney@gnu.org>
> >>
> >>>> > +The target's register contents has changed.
> >>
> >>> 
> >>> FYI, this should probably read:
> >>> 	The target's memory or register contents have [has?] changed.
> >>> eli?
> > 
> > 
> > I'm not sure; what is the difference between the two wordings?
> 
> "have" sounds right (...), hmm.  Check dictionary ``/has/ 3rd person 
> _singular_, present of /have/'' [canadian oxford] so "have" is correct.

I didn't realize that you were talking only about "has" vs "have"
(your alternative wording was different in other ways).  I agree that
"have" is correct here.


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-28 17:40                                 ` Eli Zaretskii
@ 2004-04-29  8:09                                   ` Orjan Friberg
  2004-04-29 17:56                                     ` Eli Zaretskii
  0 siblings, 1 reply; 27+ messages in thread
From: Orjan Friberg @ 2004-04-29  8:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Andrew Cagney, gdb-patches

Eli Zaretskii wrote:
>>Date: Wed, 28 Apr 2004 12:43:12 -0400
>>From: Andrew Cagney <cagney@gnu.org>
>>
>>>>>>>+The target's register contents has changed.
>>>>
>>>>>FYI, this should probably read:
>>>>>	The target's memory or register contents have [has?] changed.
>>>>>eli?
>>>
>>>
>>>I'm not sure; what is the difference between the two wordings?
>>
>>"have" sounds right (...), hmm.  Check dictionary ``/has/ 3rd person 
>>_singular_, present of /have/'' [canadian oxford] so "have" is correct.
> 
> 
> I didn't realize that you were talking only about "has" vs "have"
> (your alternative wording was different in other ways).  I agree that
> "have" is correct here.

Ok to commit the below, then?  (Or should the event description include 
the "memory" part of Andrew's suggestion as well?)

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
retrieving revision 1.406
diff -u -p -r1.406 ChangeLog
--- ChangeLog   26 Apr 2004 09:36:56 -0000      1.406
+++ ChangeLog   29 Apr 2004 08:03:19 -0000
@@ -1,3 +1,7 @@
+2004-04-29  Orjan Friberg  <orjanf@axis.com>
+
+       * observer.texi (GDB Observers): Correct spelling.
+
  2004-04-26  Orjan Friberg  <orjanf@axis.com>

         * observer.texi (GDB Observers): Add target_changed event.
Index: observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.4
diff -u -p -r1.4 observer.texi
--- observer.texi       26 Apr 2004 09:36:56 -0000      1.4
+++ observer.texi       29 Apr 2004 08:03:19 -0000
@@ -75,5 +75,5 @@ The inferior has stopped for real.
  @end deftypefun

  @deftypefun void target_changed (struct target_ops *@var{target})
-The target's register contents has changed.
+The target's register contents have changed.
  @end deftypefun


-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-29  8:09                                   ` Orjan Friberg
@ 2004-04-29 17:56                                     ` Eli Zaretskii
  2004-04-30  7:39                                       ` Orjan Friberg
  0 siblings, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2004-04-29 17:56 UTC (permalink / raw)
  To: Orjan Friberg; +Cc: cagney, gdb-patches

> Date: Thu, 29 Apr 2004 10:09:13 +0200
> From: Orjan Friberg <orjan.friberg@axis.com>
> 
> Ok to commit the below, then?

Fine with me.


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Multiplexed registers and invalidating the register cache
  2004-04-29 17:56                                     ` Eli Zaretskii
@ 2004-04-30  7:39                                       ` Orjan Friberg
  0 siblings, 0 replies; 27+ messages in thread
From: Orjan Friberg @ 2004-04-30  7:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cagney, gdb-patches

Eli Zaretskii wrote:
>>Date: Thu, 29 Apr 2004 10:09:13 +0200
>>From: Orjan Friberg <orjan.friberg@axis.com>
>>
>>Ok to commit the below, then?
> 
> 
> Fine with me.

Committed.

-- 
Orjan Friberg
Axis Communications


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2004-04-30  7:39 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-04-14 11:44 Multiplexed registers and invalidating the register cache Orjan Friberg
2004-04-14 14:46 ` Daniel Jacobowitz
2004-04-14 16:01   ` Andrew Cagney
2004-04-15 10:46   ` Orjan Friberg
2004-04-15 11:25     ` Orjan Friberg
2004-04-15 15:29       ` Andrew Cagney
2004-04-16 12:51         ` Orjan Friberg
2004-04-16 14:13           ` Daniel Jacobowitz
2004-04-16 14:54             ` Orjan Friberg
2004-04-16 19:15           ` Andrew Cagney
2004-04-19 14:13             ` Orjan Friberg
2004-04-21 16:48               ` Andrew Cagney
2004-04-22 13:58                 ` Orjan Friberg
2004-04-22 14:33                   ` Andrew Cagney
2004-04-23 11:25                     ` Orjan Friberg
2004-04-24  0:03                       ` Andrew Cagney
2004-04-23 15:20                         ` Orjan Friberg
2004-04-23 17:58                           ` Eli Zaretskii
2004-04-26  9:41                             ` Orjan Friberg
2004-04-24  0:03                           ` Andrew Cagney
2004-04-24  8:30                             ` Eli Zaretskii
2004-04-28 16:43                               ` Andrew Cagney
2004-04-28 17:40                                 ` Eli Zaretskii
2004-04-29  8:09                                   ` Orjan Friberg
2004-04-29 17:56                                     ` Eli Zaretskii
2004-04-30  7:39                                       ` Orjan Friberg
2004-04-26  9:50                             ` Orjan Friberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox