[rfc][4/13] Eliminate read_register: read_register in deprecated_mips_set_processor_regs_hack

[rfc][4/13] Eliminate read_register: read_register in deprecated_mips_set_processor_regs_hack

[Ulrich Weigand]

Hello,

one more weird global register access in a tdep file:
deprecated_mips_set_processor_regs_hack in mips-tdep.c.

(Not sure if this is still required, or if it could be replaced by
the new XML register mechanism ...)


To be able to continue elimination of read_register, this patch simply
inlines the definition and reads from current_regcache.  This should
obviously have no change in behaviour.


Bye,
Ulrich


ChangeLog:

	* mips-tdep.c (deprecated_mips_set_processor_regs_hack): Read from
	current regcache instead of calling read_register.


diff -urNp gdb-orig/gdb/mips-tdep.c gdb-head/gdb/mips-tdep.c
--- gdb-orig/gdb/mips-tdep.c	2007-06-06 18:57:02.982185081 +0200
+++ gdb-head/gdb/mips-tdep.c	2007-06-06 18:56:16.508824426 +0200
@@ -4314,10 +4314,10 @@ void
 deprecated_mips_set_processor_regs_hack (void)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
-  CORE_ADDR prid;
-
-  prid = read_register (MIPS_PRID_REGNUM);
+  ULONGEST prid;
 
+  regcache_cooked_read_unsigned (current_regcache,
+				 MIPS_PRID_REGNUM, &prid);
   if ((prid & ~0xf) == 0x700)
     tdep->mips_processor_reg_names = mips_r3041_reg_names;
 }
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc][4/13] Eliminate read_register: read_register in deprecated_mips_set_processor_regs_hack

[Maciej W. Rozycki]

On Thu, 7 Jun 2007, Ulrich Weigand wrote:

> one more weird global register access in a tdep file:
> deprecated_mips_set_processor_regs_hack in mips-tdep.c.

 This register is read-only, so it does not matter which copy you read. 
;-)

> (Not sure if this is still required, or if it could be replaced by
> the new XML register mechanism ...)

 Well, if the latter provides similar means.  I have extensive changes 
pending for deprecated_mips_set_processor_regs_hack() and this access 
definitely has to stay.

  Maciej

Re: [rfc][4/13] Eliminate read_register: read_register in deprecated_mips_set_processor_regs_hack

[Daniel Jacobowitz]

On Fri, Jun 08, 2007 at 01:27:56PM +0100, Maciej W. Rozycki wrote:
> > (Not sure if this is still required, or if it could be replaced by
> > the new XML register mechanism ...)
> 
>  Well, if the latter provides similar means.  I have extensive changes 
> pending for deprecated_mips_set_processor_regs_hack() and this access 
> definitely has to stay.

In the new GDB scheme for such things, a processor with different
register names should be a different gdbarch.  The XML descriptions
are just one way of specifying a new architecture variant and which
variant to use - there are other ways, too.  I would recommend a look
at remote_read_description, which provides another way that is closer
to what you need here.

It's not a perfect fit, though.  What
deprecated_mips_set_processor_regs_hack could be doing is waiting
until the point where we know the architecture well enough to find the
PRID register contents, and then selecting a variant of the
architecture based on that PRID value.  But we do not know the
register layout yet at the point of target_read_description; we don't
know which registers are provided or how big they are.

I don't have an easy answer for this.  Simplest would be to keep it
local to remote-mips.c (as it currently is), but change how it works;
move it from common_open to a new remote_mips_read_description, fetch
the PRID without going through GDB's register cache at all, and then
create an appropriate target description which specifies the processor
based on the PRID.  It would be nicer if we could make it work for
remote.c too though.

Of course you can also make any remote MIPS targets respond with XML
<architecture> tags :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc][4/13] Eliminate read_register: read_register in deprecated_mips_set_processor_regs_hack

[Maciej W. Rozycki]

On Tue, 12 Jun 2007, Daniel Jacobowitz wrote:

> >  Well, if the latter provides similar means.  I have extensive changes 
> > pending for deprecated_mips_set_processor_regs_hack() and this access 
> > definitely has to stay.
> 
> In the new GDB scheme for such things, a processor with different
> register names should be a different gdbarch.  The XML descriptions
> are just one way of specifying a new architecture variant and which
> variant to use - there are other ways, too.  I would recommend a look
> at remote_read_description, which provides another way that is closer
> to what you need here.

 I will.

> It's not a perfect fit, though.  What
> deprecated_mips_set_processor_regs_hack could be doing is waiting
> until the point where we know the architecture well enough to find the
> PRID register contents, and then selecting a variant of the
> architecture based on that PRID value.  But we do not know the
> register layout yet at the point of target_read_description; we don't
> know which registers are provided or how big they are.

 Worse yet -- the approach that we have taken is generic.  We can handle 
arbitrary MIPS32/MIPS64 processors as conforming to the current revisions 
of the base architecture specs and the application-specific extensions 
(ASEs) by decoding feature bits defined in cp0 config registers -- there 
four config registers defined so far; additional registers may need to be 
read for variable length register subsets (e.g. watch and performance 
counter registers).

 These in turn may differ between members of the same processor family 
based on configuration of a given system which might be hardwired and/or 
configured from the bootstrap bits.  Further changes may be obtained by 
fiddling with some processor-specific configuration bits, for example in 
some implementations the MMU may be switched between the TLB and the fixed 
mode and this affects some cp0 registers.

 There are only two or three variations that care of specific processor 
IDs from PRId -- all the rest is generic using PRId to determine whether a 
chip is a MIPS architecture or a legacy implementation only.  Using PRId 
as a selector here would be a maintenance nightmare -- there are too many 
of them.

> I don't have an easy answer for this.  Simplest would be to keep it
> local to remote-mips.c (as it currently is), but change how it works;
> move it from common_open to a new remote_mips_read_description, fetch
> the PRID without going through GDB's register cache at all, and then
> create an appropriate target description which specifies the processor
> based on the PRID.  It would be nicer if we could make it work for
> remote.c too though.

 Well, it's actually in mips-tdep.c, so it should work for any MIPS 
target.

> Of course you can also make any remote MIPS targets respond with XML
> <architecture> tags :-)

 I guess this is unfeasible -- there are too many possibilites which are 
neither fixed nor easy to predict as you can see from the above.  Unless 
the XML tags provide means for subsetting the architecture.  Please note 
that to make the matter more exciting the subsets do overlap.

 I shall see how the change fits into gdb in its current state and submit 
the proposal as soon as feasible.

  Maciej

Re: [rfc][4/13] Eliminate read_register: read_register in deprecated_mips_set_processor_regs_hack

[Daniel Jacobowitz]

On Tue, Jun 12, 2007 at 03:49:57PM +0100, Maciej W. Rozycki wrote:
>  Worse yet -- the approach that we have taken is generic.  We can handle 
> arbitrary MIPS32/MIPS64 processors as conforming to the current revisions 
> of the base architecture specs and the application-specific extensions 
> (ASEs) by decoding feature bits defined in cp0 config registers -- there 
> four config registers defined so far; additional registers may need to be 
> read for variable length register subsets (e.g. watch and performance 
> counter registers).

I see - that sounds pretty nice; we just need to come up with a way to
support it in the rest of GDB.  Perhaps we should simply bite the
bullet and have a late target description hook in addition to the
early one.  We can't do without the early one; I added it e.g. for
existing MIPS remote targets, where the stub might send back 32-bit or
64-bit registers.

Or maybe I'm barking up the wrong tree entirely and we need more than
just a target description.  The description could report which
registers are available and how big they are, and then GDB could
gather other details from the config registers.

> > I don't have an easy answer for this.  Simplest would be to keep it
> > local to remote-mips.c (as it currently is), but change how it works;
> > move it from common_open to a new remote_mips_read_description, fetch
> > the PRID without going through GDB's register cache at all, and then
> > create an appropriate target description which specifies the processor
> > based on the PRID.  It would be nicer if we could make it work for
> > remote.c too though.
> 
>  Well, it's actually in mips-tdep.c, so it should work for any MIPS 
> target.

Have you moved the call?  Right now, that function is only reached
through remote-mips.c.

>  I guess this is unfeasible -- there are too many possibilites which are 
> neither fixed nor easy to predict as you can see from the above.  Unless 
> the XML tags provide means for subsetting the architecture.  Please note 
> that to make the matter more exciting the subsets do overlap.

The XML tags provide whatever you want them to - it's very easy to add
new ones :-)  We could provide the values of the config registers
directly in a description, for instance, and let mips-tdep.c interpret
them there.  But it might be better to have GDB query them using
normal register methods.

-- 
Daniel Jacobowitz
CodeSourcery