From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27305 invoked by alias); 7 Feb 2010 21:52:29 -0000 Received: (qmail 27296 invoked by uid 22791); 7 Feb 2010 21:52:28 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 07 Feb 2010 21:52:23 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.3/8.14.3) with ESMTP id o17LqGaL003277; Sun, 7 Feb 2010 22:52:16 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.3/8.14.3/Submit) id o17LqEm7030005; Sun, 7 Feb 2010 22:52:14 +0100 (CET) Date: Sun, 07 Feb 2010 21:52:00 -0000 Message-Id: <201002072152.o17LqEm7030005@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: hjl.tools@gmail.com CC: gdb-patches@sourceware.org, mark.kettenis@xs4all.nl In-reply-to: <20100205011447.GA28263@lucon.org> (hongjiu.lu@intel.com) Subject: Re: PATCH: PATCH: Add builtin floating point types References: <20100205011447.GA28263@lucon.org> 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: 2010-02/txt/msg00187.txt.bz2 > Date: Thu, 4 Feb 2010 17:14:47 -0800 > From: "H.J. Lu" > > I am resending this patch. The motivation is I am working on x86 xml > target descriptions. x86 has i387_ext type. I added > > case TDESC_TYPE_I387_EXT: > return arch_float_type (gdbarch, -1, "builtin_type_i387_ext", > floatformats_i387_ext); > > to tdesc_gdb_type. I would up 8 i387_ext types at 8 different addresses > with the same bits. x86 does > > if (i386_fp_regnum_p (gdbarch, regnum)) > { > /* Floating point registers must be converted unless we are > accessing them in their hardware type. */ > if (type == i387_ext_type (gdbarch)) > return 0; > else > return 1; > } > > It expects 2 i387_ext types should have the same address. This > patch caches ieee_single, ieee_double and i387_ext. OK to install? Sorry, but I don't like this diff. At the very least, keep the i387_ext_type() function around. It makes the diff far less invasive. Adding architecture-specific types the list of builtin types feels wrong as well. > H.J. > --- > 2010-02-04 H.J. Lu > > * amd64-tdep.c (amd64_register_type): Replace i387_ext_type > with builtin_i387_ext. > * i387-tdep.c (print_i387_value): Likewise. > (i387_convert_register_p): Likewise. > (i387_register_to_value): Likewise. > (i387_value_to_register): Likewise. > > * gdbtypes.c (gdbtypes_post_init): Initialize > builtin_ieee_single, builtin_ieee_double and builtin_i387_ext. > > * gdbtypes.h (builtin_type): Add builtin_ieee_single, > builtin_ieee_double and builtin_i387_ext. > > * i386-tdep.c (i386_extract_return_value): Replace i387_ext_type > with builtin_i387_ext. > (i386_store_return_value): Likewise. > (i386_register_type): Likewise. > (i387_ext_type): Removed. > > * i386-tdep.h (gdbarch_tdep): Remove i387_ext_type. > (i387_ext_type): Removed. > > * target-descriptions.c (tdesc_gdb_type): Use > builtin_ieee_single and builtin_ieee_double. > > diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c > index 2b15141..51d6824 100644 > --- a/gdb/amd64-tdep.c > +++ b/gdb/amd64-tdep.c > @@ -113,7 +113,7 @@ amd64_register_type (struct gdbarch *gdbarch, int regnum) > if (regnum >= AMD64_CS_REGNUM && regnum <= AMD64_GS_REGNUM) > return builtin_type (gdbarch)->builtin_int32; > if (regnum >= AMD64_ST0_REGNUM && regnum <= AMD64_ST0_REGNUM + 7) > - return i387_ext_type (gdbarch); > + return builtin_type (gdbarch)->builtin_i387_ext; > if (regnum >= AMD64_FCTRL_REGNUM && regnum <= AMD64_FCTRL_REGNUM + 7) > return builtin_type (gdbarch)->builtin_int32; > if (regnum >= AMD64_XMM0_REGNUM && regnum <= AMD64_XMM0_REGNUM + 15) > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c > index 46846c4..397adba 100644 > --- a/gdb/gdbtypes.c > +++ b/gdb/gdbtypes.c > @@ -3470,6 +3470,16 @@ gdbtypes_post_init (struct gdbarch *gdbarch) > TYPE_NOTTEXT (builtin_type->builtin_int8) = 1; > TYPE_NOTTEXT (builtin_type->builtin_uint8) = 1; > > + builtin_type->builtin_ieee_single > + = arch_float_type (gdbarch, -1, "builtin_type_ieee_single", > + floatformats_ieee_single); > + builtin_type->builtin_ieee_double > + = arch_float_type (gdbarch, -1, "builtin_type_ieee_double", > + floatformats_ieee_double); > + builtin_type->builtin_i387_ext > + = arch_float_type (gdbarch, -1, "builtin_type_i387_ext", > + floatformats_i387_ext); > + > /* Default data/code pointer types. */ > builtin_type->builtin_data_ptr > = lookup_pointer_type (builtin_type->builtin_void); > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h > index 643fa03..ad8153f 100644 > --- a/gdb/gdbtypes.h > +++ b/gdb/gdbtypes.h > @@ -1122,6 +1122,9 @@ struct builtin_type > struct type *builtin_int128; > struct type *builtin_uint128; > > + struct type *builtin_ieee_single; > + struct type *builtin_ieee_double; > + struct type *builtin_i387_ext; > > /* Pointer types. */ > > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index 83aa81f..3888799 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -1905,7 +1905,9 @@ i386_extract_return_value (struct gdbarch *gdbarch, struct type *type, > exactly how it would happen on the target itself, but it is > the best we can do. */ > regcache_raw_read (regcache, I386_ST0_REGNUM, buf); > - convert_typed_floating (buf, i387_ext_type (gdbarch), valbuf, type); > + convert_typed_floating (buf, > + builtin_type (gdbarch)->builtin_i387_ext, > + valbuf, type); > } > else > { > @@ -1959,7 +1961,8 @@ i386_store_return_value (struct gdbarch *gdbarch, struct type *type, > floating-point format used by the FPU. This is probably > not exactly how it would happen on the target itself, but > it is the best we can do. */ > - convert_typed_floating (valbuf, type, buf, i387_ext_type (gdbarch)); > + convert_typed_floating (valbuf, type, buf, > + builtin_type (gdbarch)->builtin_i387_ext); > regcache_raw_write (regcache, I386_ST0_REGNUM, buf); > > /* Set the top of the floating-point register stack to 7. The > @@ -2178,19 +2181,6 @@ i386_mxcsr_type (struct gdbarch *gdbarch) > return tdep->i386_mxcsr_type; > } > > -struct type * > -i387_ext_type (struct gdbarch *gdbarch) > -{ > - struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > - > - if (!tdep->i387_ext_type) > - tdep->i387_ext_type > - = arch_float_type (gdbarch, -1, "builtin_type_i387_ext", > - floatformats_i387_ext); > - > - return tdep->i387_ext_type; > -} > - > /* Construct vector type for MMX registers. */ > struct type * > i386_mmx_type (struct gdbarch *gdbarch) > @@ -2299,7 +2289,7 @@ i386_register_type (struct gdbarch *gdbarch, int regnum) > return builtin_type (gdbarch)->builtin_data_ptr; > > if (i386_fp_regnum_p (gdbarch, regnum)) > - return i387_ext_type (gdbarch); > + return builtin_type (gdbarch)->builtin_i387_ext; > > if (i386_mmx_regnum_p (gdbarch, regnum)) > return i386_mmx_type (gdbarch); > diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h > index 5915eb9..bb3cc14 100644 > --- a/gdb/i386-tdep.h > +++ b/gdb/i386-tdep.h > @@ -151,7 +151,6 @@ struct gdbarch_tdep > struct type *i386_mxcsr_type; > struct type *i386_mmx_type; > struct type *i386_sse_type; > - struct type *i387_ext_type; > > /* Process record/replay target. */ > /* The map for registers because the AMD64's registers order > @@ -244,7 +243,6 @@ extern struct type *i386_eflags_type (struct gdbarch *gdbarch); > extern struct type *i386_mxcsr_type (struct gdbarch *gdbarch); > extern struct type *i386_mmx_type (struct gdbarch *gdbarch); > extern struct type *i386_sse_type (struct gdbarch *gdbarch); > -extern struct type *i387_ext_type (struct gdbarch *gdbarch); > > /* Segment selectors. */ > #define I386_SEL_RPL 0x0003 /* Requester's Privilege Level mask. */ > diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c > index 3fb5b56..25dcae5 100644 > --- a/gdb/i387-tdep.c > +++ b/gdb/i387-tdep.c > @@ -47,7 +47,8 @@ print_i387_value (struct gdbarch *gdbarch, > of certain numbers such as NaNs, even if GDB is running natively. > This is fine since our caller already detects such special > numbers and we print the hexadecimal representation anyway. */ > - value = extract_typed_floating (raw, i387_ext_type (gdbarch)); > + value = extract_typed_floating (raw, > + builtin_type (gdbarch)->builtin_i387_ext); > > /* We try to print 19 digits. The last digit may or may not contain > garbage, but we'd better print one too many. We need enough room > @@ -293,7 +294,7 @@ i387_convert_register_p (struct gdbarch *gdbarch, int regnum, struct type *type) > { > /* Floating point registers must be converted unless we are > accessing them in their hardware type. */ > - if (type == i387_ext_type (gdbarch)) > + if (type == builtin_type (gdbarch)->builtin_i387_ext) > return 0; > else > return 1; > @@ -324,7 +325,8 @@ i387_register_to_value (struct frame_info *frame, int regnum, > > /* Convert to TYPE. */ > get_frame_register (frame, regnum, from); > - convert_typed_floating (from, i387_ext_type (gdbarch), to, type); > + convert_typed_floating (from, builtin_type (gdbarch)->builtin_i387_ext, > + to, type); > } > > /* Write the contents FROM of a value of type TYPE into register > @@ -348,7 +350,8 @@ i387_value_to_register (struct frame_info *frame, int regnum, > } > > /* Convert from TYPE. */ > - convert_typed_floating (from, type, to, i387_ext_type (gdbarch)); > + convert_typed_floating (from, type, to, > + builtin_type (gdbarch)->builtin_i387_ext); > put_frame_register (frame, regnum, to); > } > > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index 4fbc72c..cef91e5 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -532,12 +532,10 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type) > return builtin_type (gdbarch)->builtin_data_ptr; > > case TDESC_TYPE_IEEE_SINGLE: > - return arch_float_type (gdbarch, -1, "builtin_type_ieee_single", > - floatformats_ieee_single); > + return builtin_type (gdbarch)->builtin_ieee_single; > > case TDESC_TYPE_IEEE_DOUBLE: > - return arch_float_type (gdbarch, -1, "builtin_type_ieee_double", > - floatformats_ieee_double); > + return builtin_type (gdbarch)->builtin_ieee_double; > > case TDESC_TYPE_ARM_FPA_EXT: > return arch_float_type (gdbarch, -1, "builtin_type_arm_ext", >