Re: PATCH: PATCH: Add builtin floating point types

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Thu, 4 Feb 2010 17:14:47 -0800
> From: "H.J. Lu" <hongjiu.lu@intel.com>
> 
> 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  <hongjiu.lu@intel.com>
> 
> 	* 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",
>