From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10990 invoked by alias); 5 Jul 2009 10:15:49 -0000 Received: (qmail 10979 invoked by uid 22791); 5 Jul 2009 10:15:47 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from wf-out-1314.google.com (HELO wf-out-1314.google.com) (209.85.200.174) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 05 Jul 2009 10:15:41 +0000 Received: by wf-out-1314.google.com with SMTP id 23so1342852wfg.24 for ; Sun, 05 Jul 2009 03:15:39 -0700 (PDT) MIME-Version: 1.0 Received: by 10.143.5.20 with SMTP id h20mr1042922wfi.279.1246788939573; Sun, 05 Jul 2009 03:15:39 -0700 (PDT) In-Reply-To: <210466.48533.qm@web112501.mail.gq1.yahoo.com> References: <210466.48533.qm@web112501.mail.gq1.yahoo.com> Date: Sun, 05 Jul 2009 10:15:00 -0000 Message-ID: Subject: Re: i386.record.floating.point.patch : with more testing and assurity From: Hui Zhu To: paawan oza Cc: Michael Snyder , Mark Kettenis , "pedro@codesourcery.com" , "gdb-patches@sourceware.org" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-07/txt/msg00107.txt.bz2 Hi Paawan, 1. gcc -g -O2 -I. -I../../src/gdb -I../../src/gdb/common -I../../src/gdb/config -DLOCALEDIR=3D"\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include -I../libdecnumber -I../../src/gdb/../libdecnumber -I../../src/gdb/gnulib -Ignulib -DMI_OUT=3D1 -DTUI=3D1 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused -Wno-switch -Wno-char-subscripts -Werror -c -o i386-tdep.o -MT i386-tdep.o -MMD -MP -MF .deps/i386-tdep.Tpo ../../src/gdb/i386-tdep.c cc1: warnings being treated as errors ../../src/gdb/i386-tdep.c: In function 'i386_process_record': ../../src/gdb/i386-tdep.c:4985: warning: implicit declaration of function 'paddr_nz' ../../src/gdb/i386-tdep.c:4985: warning: format '%s' expects type 'char *', but argument 2 has type 'int' make[2]: *** [i386-tdep.o] Error 1 make[2]: Leaving directory `/media/disk/gdb/bgdb/gdb' make[1]: *** [all-gdb] Error 2 make[1]: Leaving directory `/media/disk/gdb/bgdb' make: *** [all] Error 2 paddr_nz was removed. Please update your patch follow cvs-head. 2. +#define I386_SAVE_FPU_REGS 0xFFFD +#define I386_SAVE_FPU_ENV 0xFFFE +#define I386_SAVE_FPU_ENV_REG_STACK 0xFFFF They just used in prec right? Maybe you can move them close to record code in i386-tedp.c. 3. +static int i386_record_floats(struct i386_record_s *ir, uint32_t iregnu= m) +{ + int i; + + /* Oza : push/pop of fpu stack is going to happen + currently we store st0-st7 registers, but we need not store all registers all the time. + using fstatus, we use 11-13 bits which gives us stack top and hence we optimize our storage. + alternatively we can use ftag register too */ + if (I386_SAVE_FPU_REGS =3D=3D iregnum) + { + for (i=3DI386_ST0_REGNUM;i<=3DI386_ST0_REGNUM+7;i++) + { + if (record_arch_list_add_reg (ir->regcache,i)) + return -1; + } + } About the number of fp regs. Please use the code: #define I387_ST0_REGNUM(tdep) ((tdep)->st0_regnum) #define I387_NUM_XMM_REGS(tdep) ((tdep)->num_xmm_regs) #define I387_MM0_REGNUM(tdep) ((tdep)->mm0_regnum) #define I387_FCTRL_REGNUM(tdep) (I387_ST0_REGNUM (tdep) + 8) #define I387_FSTAT_REGNUM(tdep) (I387_FCTRL_REGNUM (tdep) + 1) #define I387_FTAG_REGNUM(tdep) (I387_FCTRL_REGNUM (tdep) + 2) #define I387_FISEG_REGNUM(tdep) (I387_FCTRL_REGNUM (tdep) + 3) #define I387_FIOFF_REGNUM(tdep) (I387_FCTRL_REGNUM (tdep) + 4) #define I387_FOSEG_REGNUM(tdep) (I387_FCTRL_REGNUM (tdep) + 5) #define I387_FOOFF_REGNUM(tdep) (I387_FCTRL_REGNUM (tdep) + 6) #define I387_FOP_REGNUM(tdep) (I387_FCTRL_REGNUM (tdep) + 7) #define I387_XMM0_REGNUM(tdep) (I387_ST0_REGNUM (tdep) + 16) #define I387_MXCSR_REGNUM(tdep) \ (I387_XMM0_REGNUM (tdep) + I387_NUM_XMM_REGS (tdep)) They are in i387-tdep.h. And maybe you can try function i387_supply_fsave and i387_collect_fsave. 4. Your code's format is not very well. Please make it like the code in c= vs. Thanks, Hui On Sat, Jul 4, 2009 at 13:19, paawan oza wrote: > > Hi, > > Actually, the initial patch which I submitted were using them. > but as I have incorporated Hui's comments I have removed those constants = completely. > in the sense I have no longer extended the enumration. > > but of course, those registers are recorded as and when required. > e.g. (ffree insn changes FTAG register, so we record it) > > Regards, > Oza. > > --- On Sat, 7/4/09, Michael Snyder wrote: > >> From: Michael Snyder >> Subject: Re: i386.record.floating.point.patch : with more testing and as= surity >> To: "paawan oza" >> Cc: "Mark Kettenis" , "pedro@codesourcery.com" = , "teawater@gmail.com" , "gdb-p= atches@sourceware.org" >> Date: Saturday, July 4, 2009, 3:19 AM >> paawan oza wrote: >> > Hi, >> > >> > In My understanding the point was like below. >> > in the patch there were following register extended in >> enumeration in i386-tdep.h >> > >> > I386_FSTAT, >> > I386_FTAG,=A0 =A0 =A0=A0=A0I386_FISEG, >> > I386_FIOFF, >> > I386_FOSEG, >> > I386_FOOFF, >> > I386_FOP >> > >> > >> > According to Hui in some of his previous mails...his >> idea was >> >> FCTRL, FOP and so on are the fp reg of >> amd64.=A0 For now, prec is still >> >> not support amd64 And amd64's support are in >> amd64-tedp.... files.=A0 >Change i386_regnum is not a >> good idea. I suggest you divide fp patch to 2 >parts. One >> is for i386, the other for amd64. For now, just send i386 >> patch >for review.=A0 And send amd64 patch when prec >> support amd64" >> > >> > >> > while, my idea/understanding is: >> > FCTRL, FOP registers are not only a part of amd64, but >> also part of i386 (x87 FPU unit) also. >> > so according to me these registers are part of i386 >> also and it needed to be also in i386-tdep.h. >> > >> > Regards, >> > Oza. >> >> I'm not sure why you want to add those constants to >> i386-tdep.h, >> when the rest of your patch does not seem to use them. >> >> > > > >