From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22834 invoked by alias); 21 Feb 2002 15:33:02 -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 22636 invoked from network); 21 Feb 2002 15:32:53 -0000 Received: from unknown (HELO cygnus.com) (205.180.230.5) by sources.redhat.com with SMTP; 21 Feb 2002 15:32:53 -0000 Received: from localhost.redhat.com (cse.cygnus.com [205.180.230.236]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id HAA03877; Thu, 21 Feb 2002 07:32:41 -0800 (PST) Received: by localhost.redhat.com (Postfix, from userid 469) id 75EB111403; Thu, 21 Feb 2002 10:32:32 -0500 (EST) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15477.4880.329875.377925@localhost.redhat.com> Date: Thu, 21 Feb 2002 07:33:00 -0000 To: Kevin Buettner Cc: Elena Zannoni , gdb-patches@sources.redhat.com Subject: Re: [RFA] ppc-linux-nat.c AltiVec regs ptrace In-Reply-To: <1020221050423.ZM829@localhost.localdomain> References: <15476.1308.919907.110811@localhost.redhat.com> <20020220153946.A24439@nevyn.them.org> <15476.4080.303671.894065@localhost.redhat.com> <20020220171519.A28726@nevyn.them.org> <15476.11279.326712.932158@localhost.redhat.com> <20020220184649.B7963@nevyn.them.org> <15476.16171.455269.862123@localhost.redhat.com> <20020220193446.A9812@nevyn.them.org> <15476.22212.358982.6179@localhost.redhat.com> <1020221050423.ZM829@localhost.localdomain> X-Mailer: VM 7.00 under Emacs 20.7.1 X-SW-Source: 2002-02/txt/msg00591.txt.bz2 Kevin Buettner writes: > Elena, > > It looks pretty good. I noticed a typo in a comment. After your > earlier remarks, I'm beginning to think that you put these in > deliberately to see if I'd notice. ;-) No, that was not intentional! :-) > > There are a few things that I'm confused about though... > > On Feb 20, 9:09pm, Elena Zannoni wrote: > > > + time throuhg, and we have comfirmed that there is kernel > ^^^^^^^ > Here's the typo. > > Here's where I'm confused... > > > +static void > > +supply_vrregset (gdb_vrregset_t *vrregsetp) > > +{ > > + int i; > > + struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch); > > + int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum; > > Are you off by one here? I.e, shouldn't the above statement be > > int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum + 1; > > ? > Yes, you are right. Thanks for catching this. I was using a different algorithm, and I changed things at the last minute. I shuld have gone to sleep instead. > > + int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum); > > + int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum); > > + > > + for (i = 0; i < num_of_vrregs - 1; i++) > > Why ``num_of_vrregs - 1'' ? It seems to me that the combination of the > fencepost error (above) combined with this one means that the last two > Altivec registers won't get processed. > Yes, right again. I was processing those two separately in an earlier version of this patch. That's why the loop is off. Will fix. > > + { > > + /* The last 2 registers of this set are only 32 bit long, not > > + 128. However an offset is necessary only for VSCR because it > > + occupies a whole vector, while VRSAVE occupies a full 4 bytes > > + slot. */ > > + if (i == (tdep->ppc_vrsave_regnum - 1)) > > + supply_register (tdep->ppc_vr0_regnum + i, > > + *vrregsetp + i * vrregsize + offset); > > + else > > + supply_register (tdep->ppc_vr0_regnum + i, *vrregsetp + i * vrregsize); > > + } > > +} > > [...] > > > static void > > +fill_vrregset (gdb_vrregset_t *vrregsetp) > > +{ > > + int i; > > + struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch); > > + int num_of_vrregs = tdep->ppc_vrsave_regnum - tdep->ppc_vr0_regnum; > > I think this is another fencepost error. > Yes, as above. > > + int vrregsize = REGISTER_RAW_SIZE (tdep->ppc_vr0_regnum); > > + int offset = vrregsize - REGISTER_RAW_SIZE (tdep->ppc_vrsave_regnum); > > + > > + for (i = 0; i < num_of_vrregs; i++) > > This one looks right to me though, so long as you fix the computation of > num_of_vrregs. > Thanks. I had a last minute change of hart before posting the patch. That will teach me. I'll repost with fixes. Elena > Kevin