From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7563 invoked by alias); 10 May 2003 20:30:13 -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 6088 invoked from network); 10 May 2003 20:29:49 -0000 Received: from unknown (HELO localhost.redhat.com) (24.157.166.107) by sources.redhat.com with SMTP; 10 May 2003 20:29:49 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 6E6EF2B56; Sat, 10 May 2003 16:29:37 -0400 (EDT) Message-ID: <3EBD6131.30209@redhat.com> Date: Sat, 10 May 2003 20:30:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030223 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kevin Buettner Cc: gdb-patches@sources.redhat.com Subject: Re: [WIP/RFC] MIPS registers overhaul References: <1030510002453.ZM3880@localhost.localdomain> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-05/txt/msg00168.txt.bz2 > The patch below represents the results of trying to fix two problems > related to floating point registers, stumbling at various points along > the way, and realizing that I needed to fix even more... > > The two problems related to floating point registers that I started out > trying to fix are the following: > > 1) On mips64, it's not possible to set a floating point register to > a single precision value. Hmm, ``oops''. > 2) Again, on mips64, when running a program using the o32 ABI, floating > point doubles were not being fetched from GDB's regcache correctly. > For o32, floating point doubles are represented using register pairs, > but, for mips64, each register is stored in a 64-bit container. When > attempting to retrieve the value of a 64-bit double that should > be retrieved by taking 32-bits from a pair, the single 64-bit container > was being considered to be the value of the double. For a long long time people were trying to solve this using the convertible methods. It never really worked. > Problem 1 was solved by introducing a union type for floating point > registers. When attempting to display a value using ``print'', you'll > see (with my patch) something like this: > > (gdb) p $f20 > $1 = {i = 4621199872640208077, f = -107374184, d = 8.9000000000000004} > > (If someone can think of more meaningful, but still terse field names for > the above, please let me know.) I'd try to be consistent with the other register unions, uint64 for instance. As for the d/f, I don't know. > Problem 2 was solved by introducing pseudo-registers for the floating point > registers. Along the way (last Friday, I think), I ran into an assertion > failure which caused me to make even more extensive changes to the MIPS code. Ya! Finally! > BTW, the raw floating point registers are still accessible. Doing > "info registers raw" will display all of the raw registers. Or, if > you know the names of the registers you want to display, you can do, > e.g, "info registers raw_f20". Hmm, is this necessary? Confusing? ``maint print {raw-,cooked-,}registers'' are already available and provide access to the underlying values. ``info registers'' should always display the target's underlying register set. In the case of o32 running on a 64 bit ISA, the 64 bit registers should be displayed. > Which reminds me... I've introduced two sets of register numbers. I > called these the "cooked" and "raw" regnums. In many cases, the > cooked and raw numbers are the same. (They'll usually be the same > when the raw and cooked types are the same.) In the cases where the > numbers are different, the cooked number is a pseudo register number > and the corresponding raw register is (usually) used to determine the > value of cooked value. Raw registers numbers should be used by the > lower layers of GDB which communicate with the target and need to > populate GDB's regcache. The cooked registers *should* be used almost > everywhere else. Except they aren't at the moment. In particular, > most of mips-tdep.c wants to still operate on "raw" register data. > (Anywhere you see a byte order check, that's a clue that raw register > numbers should still be used.) So, in order to prevent this patch from > becoming even larger than it already is, much of mips-tdep.c still > uses raw register numbers. These should (probably) be cleaned up some > day to use cooked numbers instead. (I need to check to make sure I > have this as a comment in the code somewhere...) > > For the o32 case that I was originally trying to fix, we end up > creating 16 64-bit cooked floating point registers of the union type > mentioned above. The raw floating point registers (in pairs) are used > to construct the cooked value. It's also possible to end up with no > cooked floating point registers or 32-floating floating point > registers, either 32- or 64-bit. (The values of FP_REGISTER_DOUBLE > and MIPS_FPU_TYPE influence what exactly gets done.) Ok. > Index: mdebugread.c The below isn't right. RA_REGNUM is defined by both Alpha and MIPS. There are a number of long standing problems with mdebug read vis: tm-alpha.h:64:#define mips_extra_func_info alpha_extra_func_info and hard-wiring the reference to RA_REGNUM just wouldn't work. I'd leave RA_REGNUM as is (or put the definition in tm-mips*.h so that it is clear that it still needs to be fixed). > =================================================================== > RCS file: /cvs/src/src/gdb/mdebugread.c,v > retrieving revision 1.44 > diff -u -p -r1.44 mdebugread.c > --- mdebugread.c 19 Mar 2003 19:45:49 -0000 1.44 > +++ mdebugread.c 9 May 2003 23:36:32 -0000 > @@ -55,6 +55,9 @@ > #include "gdb_assert.h" > #include "block.h" > > +#include "mips-tdep.h" > +#define RA_REGNUM (mips_cooked_regnums (current_gdbarch)->ra_regnum) > + > /* These are needed if the tm.h file does not contain the necessary > mips specific definitions. */ > > @@ -69,9 +72,6 @@ typedef struct mips_extra_func_info > PDR pdr; > } > *mips_extra_func_info_t; > -#ifndef RA_REGNUM > -#define RA_REGNUM 0 > -#endif > #endif > > #ifdef USG > Index: mips-tdep.h Should there be separate raw and cooked num structures? > +struct mips_regnums > + { > + int fp0_regnum; /* First floating point register. */ > + int fplast_regnum; /* Last floating point register. */ > + int last_arg_regnum; /* Last general purpose register used for > + passing arguments. (a0_regnum is the > + first.) */ > + int first_fp_arg_regnum; /* First floating point register used for > + passing floating point arguments. */ > + int last_fp_arg_regnum; /* Last floating point register used for > + passing floating point arguments. */ > + int zero_regnum; /* The zero register; read-only, always 0. */ > + int v0_regnum; /* Function return value. */ > + int a0_regnum; /* First GPR used for passing arguments. */ > + int t9_regnum; /* Contains address of callee in PIC code. */ > + int sp_regnum; /* Stack pointer. */ > + int ra_regnum; /* Return address. */ > + int ps_regnum; /* Processor status. */ > + int hi_regnum; /* High portion of internal multiply/divide > + register. */ > + int lo_regnum; /* Low portion of internal multiply/divide > + register. */ > + int badvaddr_regnum; /* Address associated with > + addressing exception. */ > + int cause_regnum; /* Describes last exception. */ > + int pc_regnum; /* Program counter. */ > + int fcrcs_regnum; /* FP control/status. */ > + int fcrir_regnum; /* FP implementation/revision. */ > + int first_embed_regnum; /* First CP0 register for embedded use. */ > + int last_embed_regnum; /* Last CP0 register for embedded use. */ > + int prid_regnum; /* Processor ID. */ > + }; > + > +const struct mips_regnums *mips_raw_regnums (struct gdbarch *gdbarch); > +const struct mips_regnums *mips_cooked_regnums (struct gdbarch *gdbarch); or at least keep things like "last_arg_regnum" out this space (they only belong in one of the two spaces). Having them appear in both makes it too easy to do the wrong thing. Can I suggest for this code: > mips_linux_cannot_fetch_register (int regno) > { > + const struct mips_regnums *regnums = mips_raw_regnums (current_gdbarch); use a consistent nameing schema that makes the register's space clear vis: rawnums = mips_raw_regnums (...); ... if (regno == rawnums->ps_regnum) it should be possible to apply separate patches that: - add mips_raw_regnums() and their initialization - roll out the mechanical change s/BADVADDR_REGNUM/rawnums->badvaddr_regnum/ How do you know that the raw register numbers were computed correctly? -- -#define REGISTER_PTRACE_ADDR(regno) \ +register_ptrace_addr (int regno) is obvious (and separate also). -- Make certain you use "mips-tdep.h" and not . -- Delete the lsi33k support as Stan noted, it likely never worked (separate also). -- This should greatly reduce the diff to just the change adding the cooked<->raw map, and code using it. Andrew