Patchwork [RFC,v2,1/3] Target FP printing: Simplify and fix print_floating

login
register
mail settings
Submitter Ulrich Weigand
Date Oct. 5, 2017, 5:31 p.m.
Message ID <20171005173126.C1CBBD835F0@oc3748833570.ibm.com>
Download mbox | patch
Permalink /patch/23360/
State New
Headers show

Comments

Ulrich Weigand - Oct. 5, 2017, 5:31 p.m.
[RFC v2][1/3] Target FP printing: Simplify and fix print_floating

The print_floating routine currently makes a lot of assumptions about host
and target floating point formats.  This patch cleans up many of those.

One problem is that print_floating may currently be called with types
that are not actually floating-point types, and it tries hard to output
those as floating-point values anyway.  However, there is only one single
caller of print_floating where this can ever happen: print_scalar_formatted.
And in fact, it is much simpler to handle the case where the value to be
printed is not already of floating-point type right there.

So this patch changes print_scalar_formatted to handle the 'f' format
as follows:

- If the value to be printed is already of floating-point type, just
  call print_floating on it.

- Otherwise, if there is a standard target floating-point type of
  the same size as the value, call print_floating using that type.

- Otherwise, just print the value as if the 'f' format had not been
  specified at all.

This has the overall effect to printing everything the same way as
the old code did, but is overall a lot simpler.  (Also, it would
allow us to change the above strategy more easily, if that might
be a more intuitive user interface.  For example, in the third
case above, maybe an error would be more appropriate?)

Given that change, print_floating can become much simpler.  In particular,
we now always have a floating-point format that we can consult.  This
means we can use the floating-point format to programmatically determine
the number of digits necessary to print the value.

The current code uses a hard-coded value of 9, 17, or 35 digits.  Note
that this matches the DECIMAL_DIG values for IEEE-32, IEEE-64, and
IEEE-128.  (Actually, for IEEE-128 the correct value is 36 -- the 35
seems to be an oversight.)  The DECIMAL_DIG value is defined to be
the smallest number so that any number in the target format, when
printed to this number of digits and then scanned back into a binary
floating-point number, will result in the original value.

Now that we always have a FP format, we can just compute the DECIMAL_DIG
value using the formula from the C standard.  This will be correct for
*all* FP formats, not just the above list, and it will be correct (as
opposed to current code) if the target formats differ from the host ones.

The patch moves the new logic to a new floatformat_to_string routine
(analogous to the existing decimal_to_string).  The print_floating
routine now calls floatformat_to_string or decimal_to_string, making
the separate print_decimal_floating and generic_val_print_decfloat routines
unnecessary.

Bye,
Ulrich


ChangeLog:

	* doublest.c (floatformat_precision): New routine.
	(floatformat_to_string): Likewise.
	* doublest.c (floatformat_to_string): Add prototype.

	* printcmd.c (print_scalar_formatted): Only call print_floating
	on floating-point types.
	* valprint.c: Do not include "floatformat.h".
	(generic_val_print_decfloat): Remove.
	(generic_val_print): Call generic_val_print_float for both
	TYPE_CODE_FLT and TYPE_CODE_DECFLOAT.
	(print_floating): Use floatformat_to_string.  Handle decimal float.
	(print_decimal_floating): Remove, merge into floatformat_to_string.
	* value.h (print_decimal_floating): Remove.

	* Makefile.in: Do not build doublest.c with -Wformat-nonliteral.

Patch

Index: binutils-gdb/gdb/doublest.c
===================================================================
--- binutils-gdb.orig/gdb/doublest.c
+++ binutils-gdb/gdb/doublest.c
@@ -680,6 +680,25 @@  floatformat_mantissa (const struct float
   return res;
 }
 
+/* Return the precision of the floating point format FMT.  */
+
+static int
+floatformat_precision (const struct floatformat *fmt)
+{
+  /* Assume the precision of and IBM long double is twice the precision
+     of the underlying double.  This matches what GCC does.  */
+  if (fmt->split_half)
+    return 2 * floatformat_precision (fmt->split_half);
+
+  /* Otherwise, the precision is the size of mantissa in bits,
+     including the implicit bit if present.  */
+  int prec = fmt->man_len;
+  if (fmt->intbit == floatformat_intbit_no)
+    prec++;
+
+  return prec;
+}
+
 
 /* Convert TO/FROM target to the hosts DOUBLEST floating-point format.
 
@@ -769,6 +788,51 @@  floatformat_from_doublest (const struct
     convert_doublest_to_floatformat (fmt, in, out);
 }
 
+/* Convert the byte-stream ADDR, interpreted as floating-point format FMT,
+   to a string.  */
+std::string
+floatformat_to_string (const struct floatformat *fmt,
+		       const gdb_byte *in)
+{
+  /* Detect invalid representations.  */
+  if (!floatformat_is_valid (fmt, in))
+    return "<invalid float value>";
+
+  /* Handle NaN and Inf.  */
+  enum float_kind kind = floatformat_classify (fmt, in);
+  if (kind == float_nan)
+    {
+      const char *sign = floatformat_is_negative (fmt, in)? "-" : "";
+      const char *mantissa = floatformat_mantissa (fmt, in);
+      return string_printf ("%snan(0x%s)", sign, mantissa);
+    }
+  else if (kind == float_infinite)
+    {
+      const char *sign = floatformat_is_negative (fmt, in)? "-" : "";
+      return string_printf ("%sinf", sign);
+    }
+
+  /* Print the number using a format string where the precision is set
+     to the DECIMAL_DIG value for the given floating-point format.
+     This value is computed as
+
+		ceil(1 + p * log10(b)),
+
+     where p is the precision of the floating-point format in bits, and
+     b is the base (which is always 2 for the formats we support).  */
+  const double log10_2 = .30102999566398119521;
+  double d_decimal_dig = 1 + floatformat_precision (fmt) * log10_2;
+  int decimal_dig = d_decimal_dig;
+  if (decimal_dig < d_decimal_dig)
+    decimal_dig++;
+
+  std::string host_format
+    = string_printf ("%%.%d" DOUBLEST_PRINT_FORMAT, decimal_dig);
+
+  DOUBLEST doub;
+  floatformat_to_doublest (fmt, in, &doub);
+  return string_printf (host_format.c_str (), doub);
+}
 
 /* Extract a floating-point number of type TYPE from a target-order
    byte-stream at ADDR.  Returns the value as type DOUBLEST.  */
Index: binutils-gdb/gdb/doublest.h
===================================================================
--- binutils-gdb.orig/gdb/doublest.h
+++ binutils-gdb/gdb/doublest.h
@@ -71,6 +71,9 @@  extern enum float_kind floatformat_class
 extern const char *floatformat_mantissa (const struct floatformat *,
 					 const bfd_byte *);
 
+extern std::string floatformat_to_string (const struct floatformat *fmt,
+					  const gdb_byte *in);
+
 /* Return the floatformat's total size in host bytes.  */
 
 extern size_t floatformat_totalsize_bytes (const struct floatformat *fmt);
Index: binutils-gdb/gdb/printcmd.c
===================================================================
--- binutils-gdb.orig/gdb/printcmd.c
+++ binutils-gdb/gdb/printcmd.c
@@ -420,7 +420,18 @@  print_scalar_formatted (const gdb_byte *
       valaddr = converted_float_bytes.data ();
     }
 
-  switch (options->format)
+  /* Printing a non-float type as 'f' will interpret the data as if it were
+     of a floating-point type of the same length, if that exists.  Otherwise,
+     the data is printed as integer.  */
+  char format = options->format;
+  if (format == 'f' && TYPE_CODE (type) != TYPE_CODE_FLT)
+    {
+      type = float_type_from_length (type);
+      if (TYPE_CODE (type) != TYPE_CODE_FLT)
+        format = 0;
+    }
+
+  switch (format)
     {
     case 'o':
       print_octal_chars (stream, valaddr, len, byte_order);
@@ -440,7 +451,6 @@  print_scalar_formatted (const gdb_byte *
 	}
       /* FALLTHROUGH */
     case 'f':
-      type = float_type_from_length (type);
       print_floating (valaddr, type, stream);
       break;
 
@@ -478,7 +488,7 @@  print_scalar_formatted (const gdb_byte *
       break;
 
     default:
-      error (_("Undefined output format \"%c\"."), options->format);
+      error (_("Undefined output format \"%c\"."), format);
     }
 }
 
Index: binutils-gdb/gdb/valprint.c
===================================================================
--- binutils-gdb.orig/gdb/valprint.c
+++ binutils-gdb/gdb/valprint.c
@@ -27,7 +27,6 @@ 
 #include "language.h"
 #include "annotate.h"
 #include "valprint.h"
-#include "floatformat.h"
 #include "doublest.h"
 #include "dfp.h"
 #include "extension.h"
@@ -820,7 +819,7 @@  generic_val_print_char (struct type *typ
     }
 }
 
-/* generic_val_print helper for TYPE_CODE_FLT.  */
+/* generic_val_print helper for TYPE_CODE_FLT and TYPE_CODE_DECFLOAT.  */
 
 static void
 generic_val_print_float (struct type *type,
@@ -844,29 +843,6 @@  generic_val_print_float (struct type *ty
     }
 }
 
-/* generic_val_print helper for TYPE_CODE_DECFLOAT.  */
-
-static void
-generic_val_print_decfloat (struct type *type,
-			    int embedded_offset, struct ui_file *stream,
-			    struct value *original_value,
-			    const struct value_print_options *options)
-{
-  struct gdbarch *gdbarch = get_type_arch (type);
-  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
-
-  if (options->format)
-    val_print_scalar_formatted (type, embedded_offset, original_value,
-				options, 0, stream);
-  else
-    {
-      const gdb_byte *valaddr = value_contents_for_printing (original_value);
-
-      print_decimal_floating (valaddr + embedded_offset * unit_size, type,
-			      stream);
-    }
-}
-
 /* generic_val_print helper for TYPE_CODE_COMPLEX.  */
 
 static void
@@ -990,15 +966,11 @@  generic_val_print (struct type *type,
       break;
 
     case TYPE_CODE_FLT:
+    case TYPE_CODE_DECFLOAT:
       generic_val_print_float (type, embedded_offset, stream,
 			       original_value, options);
       break;
 
-    case TYPE_CODE_DECFLOAT:
-      generic_val_print_decfloat (type, embedded_offset, stream,
-				  original_value, options);
-      break;
-
     case TYPE_CODE_VOID:
       fputs_filtered (decorations->void_name, stream);
       break;
@@ -1391,90 +1363,25 @@  longest_to_int (LONGEST arg)
   return (rtnval);
 }
 
-/* Print a floating point value of type TYPE (not always a
-   TYPE_CODE_FLT), pointed to in GDB by VALADDR, on STREAM.  */
+/* Print a floating point value of floating-point type TYPE,
+   pointed to in GDB by VALADDR, on STREAM.  */
 
 void
 print_floating (const gdb_byte *valaddr, struct type *type,
 		struct ui_file *stream)
 {
-  DOUBLEST doub;
-  int inv;
-  const struct floatformat *fmt = NULL;
-  unsigned len = TYPE_LENGTH (type);
-  enum float_kind kind;
-
-  /* If it is a floating-point, check for obvious problems.  */
+  std::string str;
   if (TYPE_CODE (type) == TYPE_CODE_FLT)
-    fmt = floatformat_from_type (type);
-  if (fmt != NULL)
     {
-      kind = floatformat_classify (fmt, valaddr);
-      if (kind == float_nan)
-	{
-	  if (floatformat_is_negative (fmt, valaddr))
-	    fprintf_filtered (stream, "-");
-	  fprintf_filtered (stream, "nan(");
-	  fputs_filtered ("0x", stream);
-	  fputs_filtered (floatformat_mantissa (fmt, valaddr), stream);
-	  fprintf_filtered (stream, ")");
-	  return;
-	}
-      else if (kind == float_infinite)
-	{
-	  if (floatformat_is_negative (fmt, valaddr))
-	    fputs_filtered ("-", stream);
-	  fputs_filtered ("inf", stream);
-	  return;
-	}
+      const struct floatformat *fmt = floatformat_from_type (type);
+      str = floatformat_to_string (fmt, valaddr);
     }
-
-  /* NOTE: cagney/2002-01-15: The TYPE passed into print_floating()
-     isn't necessarily a TYPE_CODE_FLT.  Consequently, unpack_double
-     needs to be used as that takes care of any necessary type
-     conversions.  Such conversions are of course direct to DOUBLEST
-     and disregard any possible target floating point limitations.
-     For instance, a u64 would be converted and displayed exactly on a
-     host with 80 bit DOUBLEST but with loss of information on a host
-     with 64 bit DOUBLEST.  */
-
-  doub = unpack_double (type, valaddr, &inv);
-  if (inv)
+  else
     {
-      fprintf_filtered (stream, "<invalid float value>");
-      return;
+      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+      unsigned len = TYPE_LENGTH (type);
+      str = decimal_to_string (valaddr, len, byte_order);
     }
-
-  /* FIXME: kettenis/2001-01-20: The following code makes too much
-     assumptions about the host and target floating point format.  */
-
-  /* NOTE: cagney/2002-02-03: Since the TYPE of what was passed in may
-     not necessarily be a TYPE_CODE_FLT, the below ignores that and
-     instead uses the type's length to determine the precision of the
-     floating-point value being printed.  */
-
-  if (len < sizeof (double))
-      fprintf_filtered (stream, "%.9g", (double) doub);
-  else if (len == sizeof (double))
-      fprintf_filtered (stream, "%.17g", (double) doub);
-  else
-#ifdef PRINTF_HAS_LONG_DOUBLE
-    fprintf_filtered (stream, "%.35Lg", doub);
-#else
-    /* This at least wins with values that are representable as
-       doubles.  */
-    fprintf_filtered (stream, "%.17g", (double) doub);
-#endif
-}
-
-void
-print_decimal_floating (const gdb_byte *valaddr, struct type *type,
-			struct ui_file *stream)
-{
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
-  unsigned len = TYPE_LENGTH (type);
-
-  std::string str = decimal_to_string (valaddr, len, byte_order);
   fputs_filtered (str.c_str (), stream);
 }
 
Index: binutils-gdb/gdb/value.h
===================================================================
--- binutils-gdb.orig/gdb/value.h
+++ binutils-gdb/gdb/value.h
@@ -1046,9 +1046,6 @@  extern void print_longest (struct ui_fil
 extern void print_floating (const gdb_byte *valaddr, struct type *type,
 			    struct ui_file *stream);
 
-extern void print_decimal_floating (const gdb_byte *valaddr, struct type *type,
-				    struct ui_file *stream);
-
 extern void value_print (struct value *val, struct ui_file *stream,
 			 const struct value_print_options *options);
 
Index: binutils-gdb/gdb/Makefile.in
===================================================================
--- binutils-gdb.orig/gdb/Makefile.in
+++ binutils-gdb/gdb/Makefile.in
@@ -2724,6 +2724,11 @@  printcmd.o: $(srcdir)/printcmd.c
 		$(COMPILE.post) $(srcdir)/printcmd.c
 	$(POSTCOMPILE)
 
+# Same for "doublest.c".
+doublest.o: $(srcdir)/doublest.c
+	$(COMPILE.pre) $(INTERNAL_CFLAGS) $(GDB_WARN_CFLAGS_NO_FORMAT) \
+		$(COMPILE.post) $(srcdir)/doublest.c
+
 # ada-exp.c can appear in srcdir, for releases; or in ., for
 # development builds.
 ADA_EXP_C = `if test -f ada-exp.c; then echo ada-exp.c; else echo $(srcdir)/ada-exp.c; fi`