diff mbox

[RFC,v2,3/3] Target FP printing: Use floatformat_to_string in tdep code

Message ID 20171017090843.390EBD807C2@oc3748833570.ibm.com
State New
Headers show

Commit Message

Ulrich Weigand Oct. 17, 2017, 9:08 a.m. UTC 
Maciej W. Rozycki wrote:
> > A few tdep files use target-specific printing routines to output values in
> > the floating-point registers.  To get rid of host floating-point code,
> > this patch changes them to use floatformat_to_string instead.
> > 
> > No functional change intended, the resulting output should look the same.
> 
>  I have now quickly checked it with a single target only and unfortunately 
> the layout still has changed, e.g. in gdb.base/callfuncs.exp it used to 
> be:
> 
> [...]
>  f0:  0xd2f1a9fc flt: -5.18969491e+11   dbl: -0.001
>  f1:  0xbf50624d flt: -0.813999951
>  f2:  0x00000000 flt: 0                 dbl: 0
>  f3:  0x00000000 flt: 0
>  f4:  0xffffffff flt: -nan              dbl: -nan
>  f5:  0xffffffff flt: -nan
>  f6:  0xffffffff flt: -nan              dbl: -nan
>  f7:  0xffffffff flt: -nan
> [...]
> 
> while now it is:
> 
> [...]
>  f0:  0xd2f1a9fc flt: -5.18969491e+11   dbl: -0.001
>  f1:  0xbf50624d flt: -0.813999951
>  f2:  0x00000000 flt: 0                 dbl: 0
>  f3:  0x00000000 flt: 0
>  f4:  0xffffffff flt: -nan(0x7fffff) dbl: -nan(0xfffffffffffff)
>  f5:  0xffffffff flt: -nan(0x7fffff)
>  f6:  0xffffffff flt: -nan(0x7fffff) dbl: -nan(0xfffffffffffff)
>  f7:  0xffffffff flt: -nan(0x7fffff)
> [...]
> 
> so it looks to me that the NaN formatter gets the character count wrong.

I see.  I guess we shouldn't use the special output strings at all
when using formatted printing, but rely on the underlying printer
to handle all cases.  (This actually matches today's behavior of
printf_command, which my patches inadvertently changed.)

Can you verify using this additional patch on top of the series?

Thanks again,
Ulrich

Comments

Maciej W. Rozycki Oct. 17, 2017, 11:40 a.m. UTC | #1 
On Tue, 17 Oct 2017, Ulrich Weigand wrote:

> >  I have now quickly checked it with a single target only and unfortunately 
> > the layout still has changed, e.g. in gdb.base/callfuncs.exp it used to 
> > be:
> > 
> > [...]
> >  f0:  0xd2f1a9fc flt: -5.18969491e+11   dbl: -0.001
> >  f1:  0xbf50624d flt: -0.813999951
> >  f2:  0x00000000 flt: 0                 dbl: 0
> >  f3:  0x00000000 flt: 0
> >  f4:  0xffffffff flt: -nan              dbl: -nan
> >  f5:  0xffffffff flt: -nan
> >  f6:  0xffffffff flt: -nan              dbl: -nan
> >  f7:  0xffffffff flt: -nan
> > [...]
> > 
> > while now it is:
> > 
> > [...]
> >  f0:  0xd2f1a9fc flt: -5.18969491e+11   dbl: -0.001
> >  f1:  0xbf50624d flt: -0.813999951
> >  f2:  0x00000000 flt: 0                 dbl: 0
> >  f3:  0x00000000 flt: 0
> >  f4:  0xffffffff flt: -nan(0x7fffff) dbl: -nan(0xfffffffffffff)
> >  f5:  0xffffffff flt: -nan(0x7fffff)
> >  f6:  0xffffffff flt: -nan(0x7fffff) dbl: -nan(0xfffffffffffff)
> >  f7:  0xffffffff flt: -nan(0x7fffff)
> > [...]
> > 
> > so it looks to me that the NaN formatter gets the character count wrong.
> 
> I see.  I guess we shouldn't use the special output strings at all
> when using formatted printing, but rely on the underlying printer
> to handle all cases.  (This actually matches today's behavior of
> printf_command, which my patches inadvertently changed.)
> 
> Can you verify using this additional patch on top of the series?

 This indeed brings the original output back, although I think printing 
the NaN payload does have a value.

 Would it be a tough problem to have it printed according to the width 
requested in the format specifier?  We know it's always going to take less 
than the maximum required for a floating-point number of the same 
floating-point format.

 As an independent change I think it would best be made with a separate 
patch though.

  Maciej
diff mbox

Patch

Index: binutils-gdb/gdb/doublest.c
===================================================================
--- binutils-gdb.orig/gdb/doublest.c
+++ binutils-gdb/gdb/doublest.c
@@ -794,22 +794,27 @@  std::string
 floatformat_to_string (const struct floatformat *fmt,
 		       const gdb_byte *in, const char *format)
 {
-  /* 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)
+  /* Unless we need to adhere to a specific format, provide special
+     output for certain cases.  */
+  if (format == nullptr)
     {
-      const char *sign = floatformat_is_negative (fmt, in)? "-" : "";
-      return string_printf ("%sinf", sign);
+      /* 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);
+	}
     }
 
   /* Determine the format string to use on the host side.  */