From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9426 invoked by alias); 7 May 2004 20:18:12 -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 9412 invoked from network); 7 May 2004 20:18:11 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 7 May 2004 20:18:11 -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 i47KIB0m002407 for ; Fri, 7 May 2004 16:18:11 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i47KIBv07502 for ; Fri, 7 May 2004 16:18:11 -0400 Received: from localhost.localdomain (vpn50-64.rdu.redhat.com [172.16.50.64]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id i47KIAOq022539; Fri, 7 May 2004 16:18:10 -0400 Received: from saguaro (saguaro.lan [192.168.64.2]) by localhost.localdomain (8.12.10/8.12.10) with SMTP id i47KI4OR004806; Fri, 7 May 2004 13:18:05 -0700 Date: Fri, 07 May 2004 20:18:00 -0000 From: Kevin Buettner To: Jim Blandy Cc: gdb-patches@sources.redhat.com Subject: Re: RFA: handle missing fpregs Message-Id: <20040507131804.7c9325d6@saguaro> In-Reply-To: References: Organization: Red Hat Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SW-Source: 2004-05/txt/msg00212.txt.bz2 On 06 May 2004 17:24:11 -0500 Jim Blandy wrote: > 2004-05-06 Jim Blandy > > * ppc-tdep.h (struct gdbarch_tdep): Change definition of > ppc_fp0_regnum and ppc_fpscr_regnum: if they are -1, then this > processor variant lacks those registers. > (ppc_floating_point_unit_p): Change description to make it clear > that this returns info about the ISA, not the ABI. > * rs6000-tdep.c (ppc_floating_point_unit_p): Decide whether to > return true or false by checking tdep->ppc_fp0_regnum and > tdep->ppc_fpscr_regnum. The original code replicated the BFD > arch/mach switching done in rs6000_gdbarch_init; it's better to > keep that logic there, and just check the results here. > (rs6000_gdbarch_init): On the E500, set tdep->ppc_fp0_regnum and > tdep->ppc_fpscr_regnum to -1 to indicate that we have no > floating-point registers. > (ppc_supply_fpregset, ppc_collect_fpregset, > rs6000_push_dummy_call, rs6000_extract_return_value, > rs6000_store_return_value): Assert that we have floating-point > registers. > (rs6000_dwarf2_stab_reg_to_regnum): Add FIXME. > (rs6000_frame_cache): Don't note the locations at which > floating-point registers were saved if we have no fprs. > * aix-thread.c (supply_fprs, fill_fprs): Assert that we have FP > registers. > (fetch_regs_user_thread, fetch_regs_kernel_thread, > store_regs_user_thread, store_regs_kernel_thread): Only call > supply_fprs / fill_fprs if we actually have floating-point > registers. > (special_register_p): Check ppc_fpscr_regnum before matching > against it. > (supply_sprs64, supply_sprs32, fill_sprs64, fill_sprs32): Don't > supply / collect fpscr if we don't have it. > * ppc-bdm.c: #include "gdb_assert.h". > (bdm_ppc_fetch_registers, bdm_ppc_store_registers): Assert that we > have floating-point registers, since I can't test this code on > FP-free systems to adapt it. > * ppc-linux-nat.c (ppc_register_u_addr): Don't match against the > fpscr and floating point register numbers if they don't exist. > (fetch_register): Assert that we have > floating-point registers before we reach the code that handles > them. > (store_register): Same. And use tdep instead of calling > gdbarch_tdep again. > (fill_fpregset): Don't try to collect FP registers and fpscr if we > don't have them. > (ppc_linux_sigtramp_cache): Don't record the saved locations of > fprs and fpscr if we don't have them. > (ppc_linux_supply_fpregset): Don't supply fp regs and fpscr if we > don't have them. > * ppcnbsd-nat.c: #include "gdb_assert.h". > (getfpregs_supplies): Assert that we have floating-point registers. > * ppcnbsd-tdep.c (ppcnbsd_supply_fpreg, ppcnbsd_fill_fpreg): Same. > * ppcobsd-tdep.c: #include "gdb_assert.h". > (ppcobsd_supply_gregset, ppcobsd_collect_gregset): Assert that we > have floating-point registers. > * rs6000-nat.c (regmap): Don't match against the > fpscr and floating point register numbers if they don't exist. > (fetch_inferior_registers, store_inferior_registers, > fetch_core_registers): Only fetch / store / supply the > floating-point registers and the fpscr if we have them. > * Makefile.in (ppc-bdm.o, ppc-linux-nat.o, ppcnbsd-nat.o, > ppcobsd-tdep.o): Update dependencies. Okay except for a few nits. This portion of the above ChangeLog entry... > (ppc_supply_fpregset, ppc_collect_fpregset, > rs6000_push_dummy_call, rs6000_extract_return_value, > rs6000_store_return_value): Assert that we have floating-point ...should be formated differently: (ppc_supply_fpregset, ppc_collect_fpregset) (rs6000_push_dummy_call, rs6000_extract_return_value) (rs6000_store_return_value): Assert that we have floating-point Personally, I prefer the way you did it, but I've been told that emacs likes the other form better. Regarding: > *** gdb/ppc-tdep.h 2004-05-06 14:18:00.000000000 -0500 > --- gdb/ppc-tdep.h 2004-05-06 14:35:11.000000000 -0500 [...] > *************** struct gdbarch_tdep > *** 150,158 **** > --- 150,162 ---- > int ppc_lr_regnum; /* Link register */ > int ppc_ctr_regnum; /* Count register */ > int ppc_xer_regnum; /* Integer exception register */ > + > + /* On RS6000 variants that have no floating-point registers, the > + next two members will be -1. */ I'm not comfortable with the term "RS6000 variants" here. I'd be happier with "PPC variants", though that's probably not strictly correct either. I suppose you could just say "On cores that have no floating-point registers...". Regarding: > *** gdb/rs6000-nat.c 2004-05-06 14:18:00.000000000 -0500 > --- gdb/rs6000-nat.c 2004-05-06 14:35:11.000000000 -0500 [...] > *************** fetch_core_registers (char *core_reg_sec > *** 583,591 **** > for (regi = 0; regi < 32; regi++) > supply_register (regi, (char *) ®s->r64.gpr[regi]); > > ! for (regi = 0; regi < 32; regi++) > ! supply_register (tdep->ppc_fp0_regnum + regi, > ! (char *) ®s->r64.fpr[regi]); > > supply_register (PC_REGNUM, (char *) ®s->r64.iar); > supply_register (tdep->ppc_ps_regnum, (char *) ®s->r64.msr); > --- 589,598 ---- > for (regi = 0; regi < 32; regi++) > supply_register (regi, (char *) ®s->r64.gpr[regi]); > > ! if (tdep->ppc_fp0_regnum >= 0) > ! for (regi = 0; regi < 32; regi++) > ! supply_register (tdep->ppc_fp0_regnum + regi, > ! (char *) ®s->r64.fpr[regi]); > > supply_register (PC_REGNUM, (char *) ®s->r64.iar); > supply_register (tdep->ppc_ps_regnum, (char *) ®s->r64.msr); I know it's not really related to this patch, but I happened to notice that constant 32 in the above (and elsewhere too). If you get a chance, could you change these to use either ppc_num_fprs or ppc_num_gprs? (I mention this because I noticed that you had made this kind of change at several points elsewhere in the current patch. A separate patch which addressed these remaining occurrences would certainly be welcome.) Kevin