[PATCH] Remove some hardwired assumptions about register sets

[PATCH] Remove some hardwired assumptions about register sets

[Fred Fish]

This patch removes some hard coded assumptions about the sizes of the
various processor specific register sets, and also allows them to be
of different sizes if necessary.

Comments?


2003-11-19  Fred Fish  <fnf@redhat.com>

	* mips-tdep.c (NUM_MIPS_PROCESSOR_REGS): Remove enum and replace
	with num_mips_processor_regs variable.
	(mips_generic_reg_names): Let defined strings determine size.
	(mips_tx39_reg_names): Ditto.
	(mips_irix_reg_names): Ditto.
	(mips_register_name): Test num_mips_processor_regs instead of
	NUM_MIPS_PROCESSOR_REGS.
	(mips_gdbarch_init): Remove num_regs and uses.
	Set num_mips_processor_regs when setting mips_processor_reg_names.

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.262
diff -c -r1.262 mips-tdep.c
*** mips-tdep.c	25 Nov 2003 22:15:24 -0000	1.262
--- mips-tdep.c	17 Dec 2003 05:44:04 -0000
***************
*** 416,426 ****
     mips_register_name().  Processor specific registers 32 and above
     are listed in the followign tables.  */
  
! enum { NUM_MIPS_PROCESSOR_REGS = (90 - 32) };
  
  /* Generic MIPS.  */
  
! static const char *mips_generic_reg_names[NUM_MIPS_PROCESSOR_REGS] = {
    "sr",	"lo",	"hi",	"bad",	"cause","pc",
    "f0",   "f1",   "f2",   "f3",   "f4",   "f5",   "f6",   "f7",
    "f8",   "f9",   "f10",  "f11",  "f12",  "f13",  "f14",  "f15",
--- 416,426 ----
     mips_register_name().  Processor specific registers 32 and above
     are listed in the followign tables.  */
  
! static int num_mips_processor_regs;
  
  /* Generic MIPS.  */
  
! static const char *mips_generic_reg_names[] = {
    "sr",	"lo",	"hi",	"bad",	"cause","pc",
    "f0",   "f1",   "f2",   "f3",   "f4",   "f5",   "f6",   "f7",
    "f8",   "f9",   "f10",  "f11",  "f12",  "f13",  "f14",  "f15",
***************
*** 428,434 ****
    "f24",  "f25",  "f26",  "f27",  "f28",  "f29",  "f30",  "f31",
    "fsr",  "fir",  ""/*"fp"*/,	"",
    "",	"",	"",	"",	"",	"",	"",	"",
!   "",	"",	"",	"",	"",	"",	"",	"",
  };
  
  /* Names of IDT R3041 registers.  */
--- 428,434 ----
    "f24",  "f25",  "f26",  "f27",  "f28",  "f29",  "f30",  "f31",
    "fsr",  "fir",  ""/*"fp"*/,	"",
    "",	"",	"",	"",	"",	"",	"",	"",
!   "",	"",	"",	"",	"",	"",	"",	""
  };
  
  /* Names of IDT R3041 registers.  */
***************
*** 441,452 ****
    "f24",  "f25",  "f26",  "f27",  "f28",  "f29",  "f30",  "f31",
    "fsr",  "fir",  "",/*"fp"*/	"",
    "",	"",	"bus",	"ccfg",	"",	"",	"",	"",
!   "",	"",	"port",	"cmp",	"",	"",	"epc",	"prid",
  };
  
  /* Names of tx39 registers.  */
  
! static const char *mips_tx39_reg_names[NUM_MIPS_PROCESSOR_REGS] = {
    "sr",	"lo",	"hi",	"bad",	"cause","pc",
    "",   "",   	"",   	"",   	"",   	"",   	"",   	"",
    "",   "",   	"",  	"",  	"",  	"",  	"",  	"",
--- 441,452 ----
    "f24",  "f25",  "f26",  "f27",  "f28",  "f29",  "f30",  "f31",
    "fsr",  "fir",  "",/*"fp"*/	"",
    "",	"",	"bus",	"ccfg",	"",	"",	"",	"",
!   "",	"",	"port",	"cmp",	"",	"",	"epc",	"prid"
  };
  
  /* Names of tx39 registers.  */
  
! static const char *mips_tx39_reg_names[] = {
    "sr",	"lo",	"hi",	"bad",	"cause","pc",
    "",   "",   	"",   	"",   	"",   	"",   	"",   	"",
    "",   "",   	"",  	"",  	"",  	"",  	"",  	"",
***************
*** 458,464 ****
  };
  
  /* Names of IRIX registers.  */
! static const char *mips_irix_reg_names[NUM_MIPS_PROCESSOR_REGS] = {
    "f0",   "f1",   "f2",   "f3",   "f4",   "f5",   "f6",   "f7",
    "f8",   "f9",   "f10",  "f11",  "f12",  "f13",  "f14",  "f15",
    "f16",  "f17",  "f18",  "f19",  "f20",  "f21",  "f22",  "f23",
--- 458,464 ----
  };
  
  /* Names of IRIX registers.  */
! static const char *mips_irix_reg_names[] = {
    "f0",   "f1",   "f2",   "f3",   "f4",   "f5",   "f6",   "f7",
    "f8",   "f9",   "f10",  "f11",  "f12",  "f13",  "f14",  "f15",
    "f16",  "f17",  "f18",  "f19",  "f20",  "f21",  "f22",  "f23",
***************
*** 508,514 ****
      }
    else if (32 <= rawnum && rawnum < NUM_REGS)
      {
!       gdb_assert (rawnum - 32 < NUM_MIPS_PROCESSOR_REGS);
        return tdep->mips_processor_reg_names[rawnum - 32];
      }
    else
--- 508,514 ----
      }
    else if (32 <= rawnum && rawnum < NUM_REGS)
      {
!       gdb_assert (rawnum - 32 < num_mips_processor_regs);
        return tdep->mips_processor_reg_names[rawnum - 32];
      }
    else
***************
*** 5643,5649 ****
    struct gdbarch_tdep *tdep;
    int elf_flags;
    enum mips_abi mips_abi, found_abi, wanted_abi;
-   int num_regs;
  
    elf_flags = 0;
  
--- 5643,5648 ----
***************
*** 5792,5798 ****
  	regnum->lo = 68;
  	regnum->fp_control_status = 69;
  	regnum->fp_implementation_revision = 70;
- 	num_regs = 71;
        }
      else
        {
--- 5791,5796 ----
***************
*** 5804,5817 ****
  	regnum->fp0 = MIPS_EMBED_FP0_REGNUM;
  	regnum->fp_control_status = 70;
  	regnum->fp_implementation_revision = 71;
- 	num_regs = 90;
        }
      /* FIXME: cagney/2003-11-15: For MIPS, hasn't PC_REGNUM been
         replaced by read_pc?  */
      set_gdbarch_pc_regnum (gdbarch, regnum->pc);
      set_gdbarch_fp0_regnum (gdbarch, regnum->fp0);
-     set_gdbarch_num_regs (gdbarch, num_regs);
-     set_gdbarch_num_pseudo_regs (gdbarch, num_regs);
    }
  
    switch (mips_abi)
--- 5802,5812 ----
***************
*** 5965,5975 ****
    /* MIPS version of register names.  */
    set_gdbarch_register_name (gdbarch, mips_register_name);
    if (info.osabi == GDB_OSABI_IRIX)
!     tdep->mips_processor_reg_names = mips_irix_reg_names;
    else if (info.bfd_arch_info != NULL && info.bfd_arch_info->mach == bfd_mach_mips3900)
!     tdep->mips_processor_reg_names = mips_tx39_reg_names;
    else
!     tdep->mips_processor_reg_names = mips_generic_reg_names;
    set_gdbarch_read_pc (gdbarch, mips_read_pc);
    set_gdbarch_write_pc (gdbarch, generic_target_write_pc);
    set_gdbarch_deprecated_target_read_fp (gdbarch, mips_read_sp); /* Draft FRAME base.  */
--- 5960,5981 ----
    /* MIPS version of register names.  */
    set_gdbarch_register_name (gdbarch, mips_register_name);
    if (info.osabi == GDB_OSABI_IRIX)
!     {
!       tdep->mips_processor_reg_names = mips_irix_reg_names;
!       num_mips_processor_regs = sizeof (mips_irix_reg_names) / sizeof (char *);
!     }
    else if (info.bfd_arch_info != NULL && info.bfd_arch_info->mach == bfd_mach_mips3900)
!     {
!       tdep->mips_processor_reg_names = mips_tx39_reg_names;
!       num_mips_processor_regs = sizeof (mips_tx39_reg_names) / sizeof (char *);
!     }
    else
!     {
!       tdep->mips_processor_reg_names = mips_generic_reg_names;
!       num_mips_processor_regs = sizeof (mips_generic_reg_names) / sizeof (char *);
!     }
!   set_gdbarch_num_regs (gdbarch, 32 + num_mips_processor_regs);
!   set_gdbarch_num_pseudo_regs (gdbarch, 32 + num_mips_processor_regs);
    set_gdbarch_read_pc (gdbarch, mips_read_pc);
    set_gdbarch_write_pc (gdbarch, generic_target_write_pc);
    set_gdbarch_deprecated_target_read_fp (gdbarch, mips_read_sp); /* Draft FRAME base.  */

Re: [PATCH] Remove some hardwired assumptions about register sets

[Fred Fish]

> Comments?

OK, I get to offer the first comment.  :-)

> ! num_mips_processor_regs = sizeof (mips_irix_reg_names) / sizeof (char *);

These should use ARRAY_SIZE().

-Fred

Re: [PATCH] Remove some hardwired assumptions about register sets

[Fred Fish]

On Tuesday 16 December 2003 23:14, Fred Fish wrote:
> This patch removes some hard coded assumptions about the sizes of the
> various processor specific register sets, and also allows them to be
> of different sizes if necessary.
>
> Comments?

Except for my own followup, there haven't been any comments about this
patch.

I propose that if there are no objections by this weekend that the
patch should be checked in, after the previously noted fix is made of
course.

-Fred

Re: [PATCH] Remove some hardwired assumptions about register sets

[Daniel Jacobowitz]

On Mon, Dec 22, 2003 at 06:51:10PM -0700, Fred Fish wrote:
> On Tuesday 16 December 2003 23:14, Fred Fish wrote:
> > This patch removes some hard coded assumptions about the sizes of the
> > various processor specific register sets, and also allows them to be
> > of different sizes if necessary.
> >
> > Comments?
> 
> Except for my own followup, there haven't been any comments about this
> patch.
> 
> I propose that if there are no objections by this weekend that the
> patch should be checked in, after the previously noted fix is made of
> course.

Let's hold off on that.  Andrew is the listed maintainer of this code, and
he's said that he will be traveling until the end of the month.

I like the patch, for what that's worth.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Remove some hardwired assumptions about register sets

[Mark Kettenis]

   From: Fred Fish <fnf@ninemoons.com>
   Date: Mon, 22 Dec 2003 18:51:10 -0700

   On Tuesday 16 December 2003 23:14, Fred Fish wrote:
   > This patch removes some hard coded assumptions about the sizes of the
   > various processor specific register sets, and also allows them to be
   > of different sizes if necessary.
   >
   > Comments?

   Except for my own followup, there haven't been any comments about this
   patch.

   I propose that if there are no objections by this weekend that the
   patch should be checked in, after the previously noted fix is made of
   course.

Not a real objection, but instead of sizeof()/sizeof() it's probably
better to use the ARRAY_SIZE() macro.

Mark

Re: [PATCH] Remove some hardwired assumptions about register sets

[Andrew Cagney]

> On Mon, Dec 22, 2003 at 06:51:10PM -0700, Fred Fish wrote:
> 
>> On Tuesday 16 December 2003 23:14, Fred Fish wrote:
> 
>> > This patch removes some hard coded assumptions about the sizes of the
>> > various processor specific register sets, and also allows them to be
>> > of different sizes if necessary.
>> >
>> > Comments?
> 
>> 
>> Except for my own followup, there haven't been any comments about this
>> patch.
>> 
>> I propose that if there are no objections by this weekend that the
>> patch should be checked in, after the previously noted fix is made of
>> course.

> I like the patch, for what that's worth.

Daniel, the patch adds a global variable "num_mips_processor_regs" vis:
!       tdep->mips_processor_reg_names = mips_irix_reg_names;
!       num_mips_processor_regs = sizeof (mips_irix_reg_names) / sizeof 
(char *);
and that can't be right :-(

Anyway, Fred, I'm wondering what motivated the change?  During one of my 
recent cleanups I considered adding register name descriptors like:
	const struct name_desc mips_generic_reg_name_desc = {
	  mips_generic_reg_names, ARRAY_SIZE (mips_generic_reg_names)
	};
(I guess that's valid C) but I eventually decided to sidestep the issue 
(well at least for the moment) and made all the arrays the same size.

Andrew

Re: [PATCH] Remove some hardwired assumptions about register sets

[Daniel Jacobowitz]

On Fri, Jan 02, 2004 at 02:28:02PM -0500, Andrew Cagney wrote:
> >On Mon, Dec 22, 2003 at 06:51:10PM -0700, Fred Fish wrote:
> >
> >>On Tuesday 16 December 2003 23:14, Fred Fish wrote:
> >
> >>> This patch removes some hard coded assumptions about the sizes of the
> >>> various processor specific register sets, and also allows them to be
> >>> of different sizes if necessary.
> >>>
> >>> Comments?
> >
> >>
> >>Except for my own followup, there haven't been any comments about this
> >>patch.
> >>
> >>I propose that if there are no objections by this weekend that the
> >>patch should be checked in, after the previously noted fix is made of
> >>course.
> 
> >I like the patch, for what that's worth.
> 
> Daniel, the patch adds a global variable "num_mips_processor_regs" vis:
> !       tdep->mips_processor_reg_names = mips_irix_reg_names;
> !       num_mips_processor_regs = sizeof (mips_irix_reg_names) / sizeof 
> (char *);
> and that can't be right :-(

Good lord, I'm not paying enough attention.  I'm glad you caught that!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Remove some hardwired assumptions about register sets

[Fred Fish]

> Daniel, the patch adds a global variable "num_mips_processor_regs" 

We could use a new "tdep->num_mips_processor_regs" variable instead.
That is probably cleaner anyway.

> I'm wondering what motivated the change?
> ... sidestep the issue
> ... made all the arrays the same size.

The problem is the assumption that all the arrays are the same size,
or perhaps more fundamentally, that "all mips architectures will have
registers sets of similar size'.

I'm working on port, which will soon be contributed, that currently
adds 384 new register names (reg numbers 32-415) for a matrix
coprocessor (cop 2).

 When contributed though the names will be reduced to just 69 new
names (32-100).  The original port assigned separate register names to
each of the individual 256 matrix elements (16 vectors of 16 elements)
instead of handling each 16 element vector as a single named vector
register.

Still, even with 69 new names, it gets really ugly having all the
other *_reg_name[] arrays have to be full of mostly null strings.

-Fred

Re: [PATCH] Remove some hardwired assumptions about register sets

[Andrew Cagney]

>> Daniel, the patch adds a global variable "num_mips_processor_regs" 
> 
> 
> We could use a new "tdep->num_mips_processor_regs" variable instead.
> That is probably cleaner anyway.
> 
> 
>> I'm wondering what motivated the change?
>> ... sidestep the issue
>> ... made all the arrays the same size.
> 
> 
> The problem is the assumption that all the arrays are the same size,
> or perhaps more fundamentally, that "all mips architectures will have
> registers sets of similar size'.

The number of registers is determined by "num_regs".  Thinking about it, 
wouldn't ->num_mips_processor_regs be redundant information?

> I'm working on port, which will soon be contributed, that currently
> adds 384 new register names (reg numbers 32-415) for a matrix
> coprocessor (cop 2).
> 
>  When contributed though the names will be reduced to just 69 new
> names (32-100).  The original port assigned separate register names to
> each of the individual 256 matrix elements (16 vectors of 16 elements)
> instead of handling each 16 element vector as a single named vector
> register.
> 
> Still, even with 69 new names, it gets really ugly having all the
> other *_reg_name[] arrays have to be full of mostly null strings.

True, but it isn't that much memory.

Does setting "num_regs" to ARRAY_SIZE (relevant register name array) + 
magic:32 work?  What of the function 
deprecated_mips_set_processor_regs_hack which would need an array size 
sanity check.

Andrew