Re: [remote] Make registers network byteordered?

[jtc@redback.com (J.T. Conklin)]

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
Andrew> At present the target sends back registers in target byte
Andrew> order format vis:
Andrew> [...]
Andrew> General comments?

As the proposal only goes as far as any register values returned by
the 'T' response (and perhaps those sent by the the 'P' command), I
wonder if there is enough value there to implement it?

You mentioned that with the registers being represented in network
byte order, you would be able to determine when a bi-endian target has
switched byte order.  But to accomplish this, you'd need to be able to
ensure that all registers fetches/stores used network order otherwise
pure chaos would erupt.  

For example, the 'p' command to fetch a single register doesn't exist,
either in the code or the spec, so there is no way to fetch the
processor's status register to know what the current endianess is.  I
once experimented adding a 'p' command just to parallel the existing
'P' command, and found that performance was greatly impacted, since 
many register writes were used instead of one 'g'.

I think this is yet another thing to put on the list of deficiencies
in the remote protocol's handling of registers, and that all should be
addressed with a new set of fetch/store register protocol commands. 

The list as remember them:

        * The spec does not nail down the offset, length, and
          representation of the registers in the g/G command.

        * With the g/G command you fetch/store all registers, with P
          you store just one.  In a lot of cases, all you care about
          is a subset of registers (integers, fpu, pc+fp+sp, etc).

        * fetching and storing registers is one of the most common 
          protocol operations, but there is no way to transfer them
          in binary (as there is for memory write).

Any more?

        --jtc

-- 
J.T. Conklin
RedBack Networks
From jtc@redback.com Thu Dec 07 14:17:00 2000
From: jtc@redback.com (J.T. Conklin)
To: gdb@sourceware.cygnus.com
Subject: additional vector function to improve  register fetch performance
Date: Thu, 07 Dec 2000 14:17:00 -0000
Message-id: <5m3dfzj2qs.fsf@jtc.redback.com>
X-SW-Source: 2000-12/msg00042.html
Content-length: 2711

While we're talking about register fetch and stores, I had an idea the
outher night about improving performance by adding some new functions
to the target vector.

* target_prefetch_register()

  With the register cache and targets that always fetch the entire
  register set, fetch performance is as good as can be expected.  But
  with a target that can fetch one register at a time, GDB will issue
  multiple single register fetches.  Due to command/response latency,
  this has a significant performance impact.

  One way this could be addressed is to always fetch the entire
  register set.  The remote protocol is like this, while it can set a
  single register, there is no command to fetch one.  This approach
  may lose when the register set is large and the number of registers
  to be fetched is small; it may be possible to issue several single-
  register fetches in the time for one for the entire register set.

  Another is the proposed target_prefetch_register() vector function.
  All this does is do a hint that we'll need the value of a register
  sometime "soon".  A sequence like:

             sp = read_sp ();
             fp = read_fp ();
             pc = read_pc ();
             r0 = read_register (R0_REGNUM);

  Might be changed to:

             prefetch_sp ();
             prefetch_fp ();
             prefetch_pc ();
             prefetch_register (R0_REGNUM);
             sp = read_sp ();
             fp = read_fp ();
             pc = read_pc ();
             r0 = read_register (R0_REGNUM);

  (I'm assuming the prefetch* functions are added to regcache.c to do
  whatever housekeeping is required and call the target vector
  function).

  In a trival target, prefetch would do nothing.  In one that has a
  async protocol, it might start fetching those registers (a callback
  would install the value in the cache when the values were received).
  In one that could do single, or full register set fetches, it might
  defer fetching anything until the first "real" read was received.
  At that time it would decide whether what type of fetch is the most
  optimum to perform.

  The disadvantage of this is that there is no benefit if the prefetch
  hints aren't added.  The good thing is that it keeps the interface
  between the target independent and target specific parts of GDB
  reasonably clean.  For contrast, imagine of a target vector function
  that took a list of registers to read.  This (IMO) would be much
  more difficult to use effectively.

Thoughts?  I have some partially thought ideas on how to do the same
for register stores, but I'm going to wait until they've firmed up a
bit before sharing.

        --jtc
  
-- 
J.T. Conklin
RedBack Networks
From tromey@redhat.com Thu Dec 07 15:00:00 2000
From: Tom Tromey <tromey@redhat.com>
To: Gdb List <gdb@sources.redhat.com>
Subject: Renaming insight to `insight'
Date: Thu, 07 Dec 2000 15:00:00 -0000
Message-id: <873dfzddwd.fsf@creche.redhat.com>
X-SW-Source: 2000-12/msg00043.html
Content-length: 1481

I recently sent a patch to gdb-patches.

It contains one deceptively simple change to top.c:

Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.22
diff -u -r1.22 top.c
--- top.c	2000/11/27 02:18:44	1.22
+++ top.c	2000/12/07 00:03:02
@@ -173,7 +173,7 @@
 /* If nonzero, and GDB has been configured to be able to use windows,
    attempt to open them upon startup.  */
 
-int use_windows = 1;
+int use_windows = 0;
 
 extern char lang_frame_mismatch_warn[];		/* language.c */
 


This part of change has gone without public comment so far.  (And
without any apparent review by the maintainers.)

The idea here is that `gdb' should just be gdb, and if you want the
GUI you should have to use some other command (eg in this case,
"insight").

I personally like this.  That is why I implemented it.  When I use gdb
from the command line I always use `gdb -nw', which gets annoying
after a while.

Also, I think this patches emphasizes that insight isn't really gdb.
I think if users type "gdb" then they should get gdb -- not some GUI
that isn't an official part of the gdb distribution.  I think this
should happen regardless of how system integrators choose to build gdb
(for instance, Debian or Red Hat might choose to build insight, to
offer more features to their uses.  But "gdb" should still be the
trusty CLI, even though that particular gdb executable happens to have
the GUI in it).

Tom
From shebs@apple.com Thu Dec 07 15:38:00 2000
From: Stan Shebs <shebs@apple.com>
To: tromey@redhat.com
Cc: Gdb List <gdb@sources.redhat.com>
Subject: Re: Renaming insight to `insight'
Date: Thu, 07 Dec 2000 15:38:00 -0000
Message-id: <3A301F35.B273D447@apple.com>
References: <873dfzddwd.fsf@creche.redhat.com>
X-SW-Source: 2000-12/msg00044.html
Content-length: 1856

Tom Tromey wrote:
> 
> -int use_windows = 1;
> +int use_windows = 0;

> This part of change has gone without public comment so far.  (And
> without any apparent review by the maintainers.)

Heh, I didn't notice it either.

> The idea here is that `gdb' should just be gdb, and if you want the
> GUI you should have to use some other command (eg in this case,
> "insight").
> 
> I personally like this.  That is why I implemented it.  When I use gdb
> from the command line I always use `gdb -nw', which gets annoying
> after a while.

The original theory was that GDB's behavior in this respect ought be
similar to Emacs; if you have a window system available, open up
windows for it, but if you don't have windows, or want to use the
console version, use -nw.  Curiously, although the analogy is
sensible, the behavior seems never to have really caught on for GDB.
(If I had to hazard a guess, I would say that this is because windowed
Emacs tends to be launched once and live in the background, but GDB
tends to be brought up only as needed.)

> Also, I think this patches emphasizes that insight isn't really gdb.
> I think if users type "gdb" then they should get gdb -- not some GUI
> that isn't an official part of the gdb distribution.  I think this
> should happen regardless of how system integrators choose to build gdb
> (for instance, Debian or Red Hat might choose to build insight, to
> offer more features to their uses.  But "gdb" should still be the
> trusty CLI, even though that particular gdb executable happens to have
> the GUI in it).

The official long-term GNU goal is still to have a Guile+Gtk interface
that would be available by default and come up by default.  However,
until this interface exists,  I don't see any problem with turning off
use_windows.  It's only a 1-char change anyway, easy to change back if
desired.

Stan
From eliz@is.elta.co.il Thu Dec 07 22:25:00 2000
From: "Eli Zaretskii" <eliz@is.elta.co.il>
To: tromey@redhat.com
Cc: gdb@sources.redhat.com
Subject: Re: Renaming insight to `insight'
Date: Thu, 07 Dec 2000 22:25:00 -0000
Message-id: <9003-Fri08Dec2000082500+0200-eliz@is.elta.co.il>
References: <873dfzddwd.fsf@creche.redhat.com>
X-SW-Source: 2000-12/msg00045.html
Content-length: 625

> From: Tom Tromey <tromey@redhat.com>
> Date: 07 Dec 2000 16:12:34 -0700
>  
> -int use_windows = 1;
> +int use_windows = 0;
>  
>  extern char lang_frame_mismatch_warn[];		/* language.c */
> 
> This part of change has gone without public comment so far.  (And
> without any apparent review by the maintainers.)

I did see it, but didn't comment because I have no objections to this
change.

> I personally like this.  That is why I implemented it.  When I use gdb
> from the command line I always use `gdb -nw', which gets annoying
> after a while.

If the patch is accepted, we will need a suitable change for the
manual.
From tromey@redhat.com Thu Dec 07 23:40:00 2000
From: Tom Tromey <tromey@redhat.com>
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: gdb@sources.redhat.com
Subject: Re: Renaming insight to `insight'
Date: Thu, 07 Dec 2000 23:40:00 -0000
Message-id: <87hf4f8i3t.fsf@creche.redhat.com>
References: <873dfzddwd.fsf@creche.redhat.com> <9003-Fri08Dec2000082500+0200-eliz@is.elta.co.il>
X-SW-Source: 2000-12/msg00046.html
Content-length: 555

>>>>> "Eli" == Eli Zaretskii <eliz@is.elta.co.il> writes:

>> I personally like this.  That is why I implemented it.  When I use
>> gdb from the command line I always use `gdb -nw', which gets
>> annoying after a while.

Eli> If the patch is accepted, we will need a suitable change for the
Eli> manual.

I looked at the manual but to me it doesn't even say that `-w' is the
default.  Also, insight isn't part of gdb, so additions there aren't
necessary.

Changes might be required for the insight help files.  I'm looking at
updating them tomorrow.

Tom
From eliz@is.elta.co.il Thu Dec 07 23:57:00 2000
From: "Eli Zaretskii" <eliz@is.elta.co.il>
To: tromey@redhat.com
Cc: gdb@sources.redhat.com
Subject: Re: Renaming insight to `insight'
Date: Thu, 07 Dec 2000 23:57:00 -0000
Message-id: <1438-Fri08Dec2000095645+0200-eliz@is.elta.co.il>
References: <873dfzddwd.fsf@creche.redhat.com> <9003-Fri08Dec2000082500+0200-eliz@is.elta.co.il> <87hf4f8i3t.fsf@creche.redhat.com>
X-SW-Source: 2000-12/msg00047.html
Content-length: 563

> From: Tom Tromey <tromey@redhat.com>
> Date: 08 Dec 2000 00:52:54 -0700
> 
> Eli> If the patch is accepted, we will need a suitable change for the
> Eli> manual.
> 
> I looked at the manual but to me it doesn't even say that `-w' is the
> default.

I consider that a bug in the current version of the manual.  I think
it should say which one of -w and -nw is the default.

> Also, insight isn't part of gdb, so additions there aren't
> necessary.

The change we are talking about is for top.c in the GDB distribution,
no?  So it _is_ relevant for GDB, I think.
From ciceron@sunchorus.France.Sun.COM Fri Dec 08 02:53:00 2000
From: Stephane Carrez <ciceron@sunchorus.France.Sun.COM>
To: gdb@sourceware.cygnus.com, jtc@redback.com
Subject: Re: additional vector function to improve  register fetch performance
Date: Fri, 08 Dec 2000 02:53:00 -0000
Message-id: <200012081053.LAA22829@sunchorus.France.Sun.COM>
X-SW-Source: 2000-12/msg00048.html
Content-length: 4792

Hi!

> While we're talking about register fetch and stores, I had an idea the
> outher night about improving performance by adding some new functions
> to the target vector.
> 

For gdb for ChorusOS (system debug), I'm doing some prefetch to
improve performance. For each processor, I've defined a list of
"gdb important registers". I retreive the complete list when gdb asks
for one of them.

For example for Sparc, I've defined the following list:

 	sp, pc, rp, fp

So, when gdb asks for either sp or pc, I retreive these 4 registers.

In general, I've observed this is enought. For most processors
Gdb needs the sp, pc and fp (in general after each stop). I'm using this
for sparc, x86 and ppc.


> * target_prefetch_register()
> 
>   With the register cache and targets that always fetch the entire
>   register set, fetch performance is as good as can be expected.  But
>   with a target that can fetch one register at a time, GDB will issue
>   multiple single register fetches.  Due to command/response latency,
>   this has a significant performance impact.
> 
>   One way this could be addressed is to always fetch the entire
>   register set.  The remote protocol is like this, while it can set a
>   single register, there is no command to fetch one.  This approach
>   may lose when the register set is large and the number of registers
>   to be fetched is small; it may be possible to issue several single-
>   register fetches in the time for one for the entire register set.

Fetching all registers is a killer for PPC... In general, only
the pc, lr and sp are used by Gdb (ok, except for arguments/locals).

> 
>   Another is the proposed target_prefetch_register() vector function.
>   All this does is do a hint that we'll need the value of a register
>   sometime "soon".  A sequence like:
> 
>              sp = read_sp ();
>              fp = read_fp ();
>              pc = read_pc ();
>              r0 = read_register (R0_REGNUM);
> 
>   Might be changed to:
> 
>              prefetch_sp ();
>              prefetch_fp ();
>              prefetch_pc ();
>              prefetch_register (R0_REGNUM);
>              sp = read_sp ();
>              fp = read_fp ();
>              pc = read_pc ();
>              r0 = read_register (R0_REGNUM);
> 
>   (I'm assuming the prefetch* functions are added to regcache.c to do
>   whatever housekeeping is required and call the target vector
>   function).
> 
>   In a trival target, prefetch would do nothing.  In one that has a
>   async protocol, it might start fetching those registers (a callback
>   would install the value in the cache when the values were received).
>   In one that could do single, or full register set fetches, it might
>   defer fetching anything until the first "real" read was received.
>   At that time it would decide whether what type of fetch is the most
>   optimum to perform.
> 
>   The disadvantage of this is that there is no benefit if the prefetch
>   hints aren't added.  The good thing is that it keeps the interface
>   between the target independent and target specific parts of GDB
>   reasonably clean.  For contrast, imagine of a target vector function
>   that took a list of registers to read.  This (IMO) would be much
>   more difficult to use effectively.
> 
> Thoughts?  I have some partially thought ideas on how to do the same
> for register stores, but I'm going to wait until they've firmed up a
> bit before sharing.
> 
>         --jtc
>   
> -- 
> J.T. Conklin
> RedBack Networks

I like the idea of pre-fetching but I wouldn't introduce a new target
vector for that. The 'prefetch_reg' is somewhat generic. We just have
to keep a list of registers that will, soon, be required.

Then, when we really need a register, the target_fetch_register() can
look at the prefetch list that was built. It can then retrieve all of
them depending on the remote protocol.


Adding prefetch hints might be difficult and sometimes you will win nothing.
This is because when you are in the Gdb-generic code, you don't know
in advance which registers you will need. For example, when the frame is
computed, the processor specific code is called. Since this is processor
specific, you don't know which register to prefetch. Adding the pre-fetch 
in the *-tdep.c files will not help you because in general you need the 
register rigth now (ex: sparc_saved_pc_after_call, rs6000_saved_pc_after_call).


The "gdb important registers" approach is interesting in that it gives
good performance win and does not need to add the pre-fetch hints.

	Stephane


-	-	-	-	-	-	-	-	-	-
Stephane |Sun Microsystems			|
 Carrez	 |Network Service Provider Division	| http://www.sun.com
	 |6 avenue Gustave Eiffel		|
	 |F-78182, St-Quentin-en-Yvelines-Cedex |

email: Stephane.Carrez@France.Sun.COM