Re: RFA: PowerPC sim & GDB: use fixed register numbering

Jim Blandy
2004-08-04T21:26:31Z

Andrew Cagney <cagney@gnu.org> writes:

> > Andrew Cagney <cagney@gnu.org> writes:
> >
> >>>> > The old code, which did an under-the-covers call into gdb to map number -> name, was bad and definitly needed to be replaced. The above though could still do with some work - too much on the magic of those numbers in gdb/sim-ppc.h :-(
> >>>> > Anyway, lets first get the enum in place.
> >>
> >>> Since we've nailed the enum, what should we do about the sim/ppc?
> > What do you have in mind?
>
> Something that uses the enum directly. Have a look at the other
> simulators that use the enum - they have a big switch doing a map.
>
> Andrew
>
> PS: I see old sim changes were accidently committed, I've reverted them.

It wasn't an accident; I thought you had approved them. You replied
to a message that included the entire patch I committed, saying:

>>> Yes, that should go in.

No big deal, but do try to be clearer about this sort of stuff.

Something like this?

2004-08-04 Jim Blandy <jimb@redhat.com>

Use a fixed register numbering when communicating with the PowerPC
simulator.
* sim_calls.c: #include "registers.h" and "gdb/sim-ppc.h"; do not
include GDB's "defs.h".
(gdb_register_name): New function.
(sim_fetch_register, sim_store_register): Use gdb_register_name,
instead of calling gdbarch_register_name.
* Makefile.in (GDB_SIM_PPC_H): New variable.
(DEFS_H): Delete variable.
(sim_calls.o): Update dependencies.

Index: sim/ppc/Makefile.in
===================================================================
RCS file: /cvs/src/src/sim/ppc/Makefile.in,v
retrieving revision 1.15
diff -c -p -r1.15 Makefile.in
*** sim/ppc/Makefile.in 4 Aug 2004 18:09:34 -0000 1.15
--- sim/ppc/Makefile.in 4 Aug 2004 21:22:34 -0000
*************** sim/ppc/Makefile.in run $(TARGETLIB) $(GDB_OBJ)
*** 172,180 ****
# Headers outside sim/ppc.
ANSIDECL_H = $(srcroot)/include/ansidecl.h
BFD_H = ../../bfd/bfd.h
- DEFS_H = $(srcroot)/gdb/defs.h
GDB_CALLBACK_H = $(srcroot)/include/gdb/callback.h
GDB_REMOTE_SIM_H = $(srcroot)/include/gdb/remote-sim.h
COMMON_SIM_BASE_H = $(srcroot)/sim/common/sim-base.h
COMMON_SIM_BASICS_H = $(srcroot)/sim/common/sim-basics.h
COMMON_SIM_FPU_H = $(srcroot)/sim/common/sim-fpu.h
--- 172,180 ----
# Headers outside sim/ppc.
ANSIDECL_H = $(srcroot)/include/ansidecl.h
BFD_H = ../../bfd/bfd.h
GDB_CALLBACK_H = $(srcroot)/include/gdb/callback.h
GDB_REMOTE_SIM_H = $(srcroot)/include/gdb/remote-sim.h
+ GDB_SIM_PPC_H = $(srcroot)/include/gdb/sim-ppc.h
COMMON_SIM_BASE_H = $(srcroot)/sim/common/sim-base.h
COMMON_SIM_BASICS_H = $(srcroot)/sim/common/sim-basics.h
COMMON_SIM_FPU_H = $(srcroot)/sim/common/sim-fpu.h
*************** model.o: model.c $(CPU_H) $(MON_H)
*** 597,603 ****

events.o: events.c $(BASICS_H) $(EVENTS_H)

! sim_calls.o: sim_calls.c $(PSIM_H) $(OPTIONS_H) $(DEFS_H) $(BFD_H) $(GDB_CALLBACK_H) $(GDB_REMOTE_SIM_H)

spreg.o: spreg.c $(BASICS_H) $(SPREG_H)

--- 597,603 ----

events.o: events.c $(BASICS_H) $(EVENTS_H)

! sim_calls.o: sim_calls.c $(PSIM_H) $(OPTIONS_H) $(REGISTERS_H) $(BFD_H) $(GDB_CALLBACK_H) $(GDB_REMOTE_SIM_H) $(GDB_SIM_PPC_H)

spreg.o: spreg.c $(BASICS_H) $(SPREG_H)

Index: sim/ppc/sim_calls.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/sim_calls.c,v
retrieving revision 1.9
diff -c -p -r1.9 sim_calls.c
*** sim/ppc/sim_calls.c 4 Aug 2004 18:09:34 -0000 1.9
--- sim/ppc/sim_calls.c 4 Aug 2004 21:22:34 -0000
***************
*** 25,30 ****
--- 25,31 ----

#include "psim.h"
#include "options.h"
+ #include "registers.h"

#undef printf_filtered /* blow away the mapping */

***************
*** 40,49 ****
#endif
#endif

- #include "defs.h"
#include "bfd.h"
#include "gdb/callback.h"
#include "gdb/remote-sim.h"

/* Define the rate at which the simulator should poll the host
for a quit. */
--- 41,50 ----
#endif
#endif

#include "bfd.h"
#include "gdb/callback.h"
#include "gdb/remote-sim.h"
+ #include "gdb/sim-ppc.h"

/* Define the rate at which the simulator should poll the host
for a quit. */
*************** static psim *simulator;
*** 59,87 ****
static device *root_device;
static host_callback *callbacks;

- /* We use GDB's gdbarch_register_name function to map GDB register
- numbers onto names, which we can then look up in the register
- table. Since the `set architecture' command can select a new
- processor variant at run-time, the meanings of the register numbers
- can change, so we need to make sure the sim uses the same
- name/number mapping that GDB uses.
-
- (We don't use the REGISTER_NAME macro, which is a wrapper for
- gdbarch_register_name. We #include GDB's "defs.h", which tries to
- #include GDB's "config.h", but gets ours instead, and REGISTER_NAME
- ends up not getting defined. Simpler to just use
- gdbarch_register_name directly.)
-
- We used to just use the REGISTER_NAMES macro from GDB's
- target-dependent header files, which expanded into an initializer
- for an array of strings. That was kind of nice, because it meant
- that libsim.a had only a compile-time dependency on GDB; using
- gdbarch_register_name directly means that there are now link-time
- and run-time dependencies too.
-
- Perhaps the host_callback structure could provide a function for
- retrieving register names; that would be cleaner. */
-
SIM_DESC
sim_open (SIM_OPEN_KIND kind,
host_callback *callback,
--- 60,65 ----
*************** sim_write (SIM_DESC sd, SIM_ADDR mem, un
*** 177,182 ****
--- 155,368 ----
}

+ /* Return the name of the register whose number is REGNUM, or zero if
+ REGNUM is an invalid register number. */
+ static const char *
+ gdb_register_name (int regnum)
+ {
+ /* Is it a special-purpose register? */
+ if (sim_ppc_spr0_regnum <= regnum
+ && regnum < sim_ppc_spr0_regnum + sim_ppc_num_sprs)
+ {
+ int spr = regnum - sim_ppc_spr0_regnum;
+ if (spr_is_valid (spr))
+ return spr_name (spr);
+ else
+ return 0;
+ }
+ else
+ switch (regnum)
+ {
+ case sim_ppc_r0_regnum: return "r0";
+ case sim_ppc_r1_regnum: return "r1";
+ case sim_ppc_r2_regnum: return "r2";
+ case sim_ppc_r3_regnum: return "r3";
+ case sim_ppc_r4_regnum: return "r4";
+ case sim_ppc_r5_regnum: return "r5";
+ case sim_ppc_r6_regnum: return "r6";
+ case sim_ppc_r7_regnum: return "r7";
+ case sim_ppc_r8_regnum: return "r8";
+ case sim_ppc_r9_regnum: return "r9";
+ case sim_ppc_r10_regnum: return "r10";
+ case sim_ppc_r11_regnum: return "r11";
+ case sim_ppc_r12_regnum: return "r12";
+ case sim_ppc_r13_regnum: return "r13";
+ case sim_ppc_r14_regnum: return "r14";
+ case sim_ppc_r15_regnum: return "r15";
+ case sim_ppc_r16_regnum: return "r16";
+ case sim_ppc_r17_regnum: return "r17";
+ case sim_ppc_r18_regnum: return "r18";
+ case sim_ppc_r19_regnum: return "r19";
+ case sim_ppc_r20_regnum: return "r20";
+ case sim_ppc_r21_regnum: return "r21";
+ case sim_ppc_r22_regnum: return "r22";
+ case sim_ppc_r23_regnum: return "r23";
+ case sim_ppc_r24_regnum: return "r24";
+ case sim_ppc_r25_regnum: return "r25";
+ case sim_ppc_r26_regnum: return "r26";
+ case sim_ppc_r27_regnum: return "r27";
+ case sim_ppc_r28_regnum: return "r28";
+ case sim_ppc_r29_regnum: return "r29";
+ case sim_ppc_r30_regnum: return "r30";
+ case sim_ppc_r31_regnum: return "r31";
+ case sim_ppc_f0_regnum: return "f0";
+ case sim_ppc_f1_regnum: return "f1";
+ case sim_ppc_f2_regnum: return "f2";
+ case sim_ppc_f3_regnum: return "f3";
+ case sim_ppc_f4_regnum: return "f4";
+ case sim_ppc_f5_regnum: return "f5";
+ case sim_ppc_f6_regnum: return "f6";
+ case sim_ppc_f7_regnum: return "f7";
+ case sim_ppc_f8_regnum: return "f8";
+ case sim_ppc_f9_regnum: return "f9";
+ case sim_ppc_f10_regnum: return "f10";
+ case sim_ppc_f11_regnum: return "f11";
+ case sim_ppc_f12_regnum: return "f12";
+ case sim_ppc_f13_regnum: return "f13";
+ case sim_ppc_f14_regnum: return "f14";
+ case sim_ppc_f15_regnum: return "f15";
+ case sim_ppc_f16_regnum: return "f16";
+ case sim_ppc_f17_regnum: return "f17";
+ case sim_ppc_f18_regnum: return "f18";
+ case sim_ppc_f19_regnum: return "f19";
+ case sim_ppc_f20_regnum: return "f20";
+ case sim_ppc_f21_regnum: return "f21";
+ case sim_ppc_f22_regnum: return "f22";
+ case sim_ppc_f23_regnum: return "f23";
+ case sim_ppc_f24_regnum: return "f24";
+ case sim_ppc_f25_regnum: return "f25";
+ case sim_ppc_f26_regnum: return "f26";
+ case sim_ppc_f27_regnum: return "f27";
+ case sim_ppc_f28_regnum: return "f28";
+ case sim_ppc_f29_regnum: return "f29";
+ case sim_ppc_f30_regnum: return "f30";
+ case sim_ppc_f31_regnum: return "f31";
+ case sim_ppc_vr0_regnum: return "vr0";
+ case sim_ppc_vr1_regnum: return "vr1";
+ case sim_ppc_vr2_regnum: return "vr2";
+ case sim_ppc_vr3_regnum: return "vr3";
+ case sim_ppc_vr4_regnum: return "vr4";
+ case sim_ppc_vr5_regnum: return "vr5";
+ case sim_ppc_vr6_regnum: return "vr6";
+ case sim_ppc_vr7_regnum: return "vr7";
+ case sim_ppc_vr8_regnum: return "vr8";
+ case sim_ppc_vr9_regnum: return "vr9";
+ case sim_ppc_vr10_regnum: return "vr10";
+ case sim_ppc_vr11_regnum: return "vr11";
+ case sim_ppc_vr12_regnum: return "vr12";
+ case sim_ppc_vr13_regnum: return "vr13";
+ case sim_ppc_vr14_regnum: return "vr14";
+ case sim_ppc_vr15_regnum: return "vr15";
+ case sim_ppc_vr16_regnum: return "vr16";
+ case sim_ppc_vr17_regnum: return "vr17";
+ case sim_ppc_vr18_regnum: return "vr18";
+ case sim_ppc_vr19_regnum: return "vr19";
+ case sim_ppc_vr20_regnum: return "vr20";
+ case sim_ppc_vr21_regnum: return "vr21";
+ case sim_ppc_vr22_regnum: return "vr22";
+ case sim_ppc_vr23_regnum: return "vr23";
+ case sim_ppc_vr24_regnum: return "vr24";
+ case sim_ppc_vr25_regnum: return "vr25";
+ case sim_ppc_vr26_regnum: return "vr26";
+ case sim_ppc_vr27_regnum: return "vr27";
+ case sim_ppc_vr28_regnum: return "vr28";
+ case sim_ppc_vr29_regnum: return "vr29";
+ case sim_ppc_vr30_regnum: return "vr30";
+ case sim_ppc_vr31_regnum: return "vr31";
+ case sim_ppc_rh0_regnum: return "rh0";
+ case sim_ppc_rh1_regnum: return "rh1";
+ case sim_ppc_rh2_regnum: return "rh2";
+ case sim_ppc_rh3_regnum: return "rh3";
+ case sim_ppc_rh4_regnum: return "rh4";
+ case sim_ppc_rh5_regnum: return "rh5";
+ case sim_ppc_rh6_regnum: return "rh6";
+ case sim_ppc_rh7_regnum: return "rh7";
+ case sim_ppc_rh8_regnum: return "rh8";
+ case sim_ppc_rh9_regnum: return "rh9";
+ case sim_ppc_rh10_regnum: return "rh10";
+ case sim_ppc_rh11_regnum: return "rh11";
+ case sim_ppc_rh12_regnum: return "rh12";
+ case sim_ppc_rh13_regnum: return "rh13";
+ case sim_ppc_rh14_regnum: return "rh14";
+ case sim_ppc_rh15_regnum: return "rh15";
+ case sim_ppc_rh16_regnum: return "rh16";
+ case sim_ppc_rh17_regnum: return "rh17";
+ case sim_ppc_rh18_regnum: return "rh18";
+ case sim_ppc_rh19_regnum: return "rh19";
+ case sim_ppc_rh20_regnum: return "rh20";
+ case sim_ppc_rh21_regnum: return "rh21";
+ case sim_ppc_rh22_regnum: return "rh22";
+ case sim_ppc_rh23_regnum: return "rh23";
+ case sim_ppc_rh24_regnum: return "rh24";
+ case sim_ppc_rh25_regnum: return "rh25";
+ case sim_ppc_rh26_regnum: return "rh26";
+ case sim_ppc_rh27_regnum: return "rh27";
+ case sim_ppc_rh28_regnum: return "rh28";
+ case sim_ppc_rh29_regnum: return "rh29";
+ case sim_ppc_rh30_regnum: return "rh30";
+ case sim_ppc_rh31_regnum: return "rh31";
+ case sim_ppc_ev0_regnum: return "ev0";
+ case sim_ppc_ev1_regnum: return "ev1";
+ case sim_ppc_ev2_regnum: return "ev2";
+ case sim_ppc_ev3_regnum: return "ev3";
+ case sim_ppc_ev4_regnum: return "ev4";
+ case sim_ppc_ev5_regnum: return "ev5";
+ case sim_ppc_ev6_regnum: return "ev6";
+ case sim_ppc_ev7_regnum: return "ev7";
+ case sim_ppc_ev8_regnum: return "ev8";
+ case sim_ppc_ev9_regnum: return "ev9";
+ case sim_ppc_ev10_regnum: return "ev10";
+ case sim_ppc_ev11_regnum: return "ev11";
+ case sim_ppc_ev12_regnum: return "ev12";
+ case sim_ppc_ev13_regnum: return "ev13";
+ case sim_ppc_ev14_regnum: return "ev14";
+ case sim_ppc_ev15_regnum: return "ev15";
+ case sim_ppc_ev16_regnum: return "ev16";
+ case sim_ppc_ev17_regnum: return "ev17";
+ case sim_ppc_ev18_regnum: return "ev18";
+ case sim_ppc_ev19_regnum: return "ev19";
+ case sim_ppc_ev20_regnum: return "ev20";
+ case sim_ppc_ev21_regnum: return "ev21";
+ case sim_ppc_ev22_regnum: return "ev22";
+ case sim_ppc_ev23_regnum: return "ev23";
+ case sim_ppc_ev24_regnum: return "ev24";
+ case sim_ppc_ev25_regnum: return "ev25";
+ case sim_ppc_ev26_regnum: return "ev26";
+ case sim_ppc_ev27_regnum: return "ev27";
+ case sim_ppc_ev28_regnum: return "ev28";
+ case sim_ppc_ev29_regnum: return "ev29";
+ case sim_ppc_ev30_regnum: return "ev30";
+ case sim_ppc_ev31_regnum: return "ev31";
+ case sim_ppc_sr0_regnum: return "sr0";
+ case sim_ppc_sr1_regnum: return "sr1";
+ case sim_ppc_sr2_regnum: return "sr2";
+ case sim_ppc_sr3_regnum: return "sr3";
+ case sim_ppc_sr4_regnum: return "sr4";
+ case sim_ppc_sr5_regnum: return "sr5";
+ case sim_ppc_sr6_regnum: return "sr6";
+ case sim_ppc_sr7_regnum: return "sr7";
+ case sim_ppc_sr8_regnum: return "sr8";
+ case sim_ppc_sr9_regnum: return "sr9";
+ case sim_ppc_sr10_regnum: return "sr10";
+ case sim_ppc_sr11_regnum: return "sr11";
+ case sim_ppc_sr12_regnum: return "sr12";
+ case sim_ppc_sr13_regnum: return "sr13";
+ case sim_ppc_sr14_regnum: return "sr14";
+ case sim_ppc_sr15_regnum: return "sr15";
+ case sim_ppc_pc_regnum: return "pc";
+ case sim_ppc_ps_regnum: return "ps";
+ case sim_ppc_cr_regnum: return "cr";
+ case sim_ppc_fpscr_regnum: return "fpscr";
+ case sim_ppc_acc_regnum: return "acc";
+ case sim_ppc_vscr_regnum: return "vscr";
+
+ default:
+ /* Not a valid register number at all. */
+ return 0;
+ }
+ }
+
+
int
sim_fetch_register (SIM_DESC sd, int regno, unsigned char *buf, int length)
{
*************** sim_fetch_register (SIM_DESC sd, int reg
*** 186,200 ****
return 0;
}

! /* GDB will sometimes ask for the contents of a register named "";
! we ignore such requests, and leave garbage in *BUF. In GDB
! terms, the empty string means "the register with this number is
! not present in the currently selected architecture variant."
! That's following the kludge we're using for the MIPS processors.
! But there are loops that just walk through the entire list of
! names and try to get everything. */
! regname = gdbarch_register_name (current_gdbarch, regno);
! if (! regname || regname[0] == '\0')
return -1;

TRACE(trace_gdb, ("sim_fetch_register(regno=%d(%s), buf=0x%lx)\n",
--- 372,382 ----
return 0;
}

! regname = gdb_register_name (regno);
!
! /* Occasionally, GDB will pass invalid register numbers to us; it
! wants us to ignore them. */
! if (! regname)
return -1;

TRACE(trace_gdb, ("sim_fetch_register(regno=%d(%s), buf=0x%lx)\n",
*************** sim_store_register (SIM_DESC sd, int reg
*** 212,220 ****
if (simulator == NULL)
return 0;

! /* See comments in sim_fetch_register, above. */
! regname = gdbarch_register_name (current_gdbarch, regno);
! if (! regname || regname[0] == '\0')
return -1;

TRACE(trace_gdb, ("sim_store_register(regno=%d(%s), buf=0x%lx)\n",
--- 394,404 ----
if (simulator == NULL)
return 0;

! regname = gdb_register_name (regno);
!
! /* Occasionally, GDB will pass invalid register numbers to us; it
! wants us to ignore them. */
! if (! regname)
return -1;

TRACE(trace_gdb, ("sim_store_register(regno=%d(%s), buf=0x%lx)\n",