This is the mail archive of the
mailing list for the GDB project.
Re: i386.record.floating.point.patch : with more testing and assurity
- From: Hui Zhu <teawater at gmail dot com>
- To: paawan oza <paawan1982 at yahoo dot com>
- Cc: Michael Snyder <msnyder at vmware dot com>, Mark Kettenis <mark dot kettenis at xs4all dot nl>, "pedro at codesourcery dot com" <pedro at codesourcery dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Sun, 5 Jul 2009 18:15:39 +0800
- Subject: Re: i386.record.floating.point.patch : with more testing and assurity
- References: <email@example.com>
1. gcc -g -O2 -I. -I../../src/gdb -I../../src/gdb/common
-I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd
-I../../src/gdb/../libdecnumber -I../../src/gdb/gnulib -Ignulib
-DMI_OUT=1 -DTUI=1 -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
../../src/gdb/i386-tdep.c:4985: warning: format '%s' expects type
'char *', but argument 2 has type 'int'
make: *** [i386-tdep.o] Error 1
make: Leaving directory `/media/disk/gdb/bgdb/gdb'
make: *** [all-gdb] Error 2
make: 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 iregnum)
+ 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 == iregnum)
+ for (i=I386_ST0_REGNUM;i<=I386_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 cvs.
On Sat, Jul 4, 2009 at 13:19, paawan oza<firstname.lastname@example.org> wrote:
> 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)
> --- On Sat, 7/4/09, Michael Snyder <email@example.com> wrote:
>> From: Michael Snyder <firstname.lastname@example.org>
>> Subject: Re: i386.record.floating.point.patch : with more testing and assurity
>> To: "paawan oza" <email@example.com>
>> Cc: "Mark Kettenis" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.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,? ? ???I386_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.? For now, prec is still
>> >> not support amd64 And amd64's support are in
>> amd64-tedp.... files.? >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.? 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
>> when the rest of your patch does not seem to use them.