From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2893 invoked by alias); 4 Aug 2004 21:26:31 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 2875 invoked from network); 4 Aug 2004 21:26:27 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 4 Aug 2004 21:26:27 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i74LQMe1024119 for ; Wed, 4 Aug 2004 17:26:22 -0400 Received: from zenia.home.redhat.com (porkchop.devel.redhat.com [172.16.58.2]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i74LQIa15815; Wed, 4 Aug 2004 17:26:19 -0400 To: Andrew Cagney Cc: gdb-patches@sources.redhat.com Subject: Re: RFA: PowerPC sim & GDB: use fixed register numbering References: <410F8C3E.3060502@gnu.org> <4110EA27.7020904@gnu.org> <41111DEC.7080102@gnu.org> <411125CF.1040102@gnu.org> From: Jim Blandy Date: Wed, 04 Aug 2004 21:26:00 -0000 In-Reply-To: <411125CF.1040102@gnu.org> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2004-08/txt/msg00092.txt.bz2 Andrew Cagney writes: > > Andrew Cagney 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 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",