diff mbox

[RFC,06/19] Target FP: Use print_floating in tdep code

Message ID 20170905182100.37B12D8086F@oc3748833570.ibm.com
State New, archived
Headers

Commit Message

Ulrich Weigand Sept. 5, 2017, 6:21 p.m. UTC 
  [RFC][06/19] Target FP: Use print_floating in tdep code

A few tdep files use target-specific printing routines to output values in
the floating-point registers.  Now that print_floating should do the right
thing for all target FP formats, this shouldn't be necesssary any more
(also, this get rid of more host-FP code).

Specifically, the i387-tdep file uses a special routine to print the
80-bit extended format using 19 digits.  The common print_floating now
uses the correct DECIMAL_DIG value of 21 digits.  This is two digits
more, but those may in some cases make a difference when converting
back to the original value, so it seems better to just use the default.

In addition, both mips-tdep and sh64-tdep use host FP to print
floating-point values.  This patch changes them to print_floating as well.
Again, this may change the output format a bit, but in general it would
appear preferable to use the default print_floating now (this will also
now match how all other FP values are printed).

Bye,
Ulrich


ChangeLog:

	* i387-tdep.c: Do not include "doublest.h".
	(print_i387_value): Remove.
	(print_i387_ext): Use print_floating instead of print_i387_value.

	* mips-tdep.c (mips_print_fp_register): Use print_floating.
	* sh64-tdep.c (sh64_do_fp_register): Likewise.

testsuite/ChangeLog:

	* gdb.arch/i386-float.exp: Update for larger number of digits printed.

Comments

Maciej W. Rozycki Oct. 4, 2017, 10:26 p.m. UTC | #1 
Hi Ulrich,

 Thank you for your effort in getting this area right, the limitations in 
our FP arithmetic handling really troubled me.

> In addition, both mips-tdep and sh64-tdep use host FP to print
> floating-point values.  This patch changes them to print_floating as well.
> Again, this may change the output format a bit, but in general it would
> appear preferable to use the default print_floating now (this will also
> now match how all other FP values are printed).

 I pushed your changes 01-06 combined through testing with a couple of 
MIPS/Linux targets, native and crossed (from x86-64), and all have 
consistently regressed in gdb.base/dfp-exprs.exp, e.g.:

 (gdb) p 1.2df
-$1 = 1.2
-(gdb) PASS: gdb.base/dfp-exprs.exp: p 1.2df
+$1 =
+(gdb) FAIL: gdb.base/dfp-exprs.exp: p 1.2df
 p -1.2df
-$2 = -1.2
-(gdb) PASS: gdb.base/dfp-exprs.exp: p -1.2df
+$2 =
+(gdb) FAIL: gdb.base/dfp-exprs.exp: p -1.2df

i.e. values to be output are missing, numerous across this test case.  No 
other regressions though.

 Also I find the new formatting of `info all-registers' output 
functionally regressed (rather than merely "changed a bit"), e.g.:

- f0:  0x00000000 flt: 0                 dbl: 0
- f1:  0x00000000 flt: 0
- f2:  0xd2f1a9fc flt: -5.18969491e+11   dbl: -0.001
- f3:  0xbf50624d flt: -0.813999951
+ f0:  0x00000000 flt: 0 dbl: 0
+ f1:  0x00000000 flt: 0
+ f2:  0xd2f1a9fc flt: -5.18969491e+11 dbl: -0.001
+ f3:  0xbf50624d flt: -0.813999951

or taking a complete listing what used to look like:

(gdb) info all-registers
          zero       at       v0       v1       a0       a1       a2       a3
 R0   00000000 00000001 00000001 d2f1a9fc 00000000 00000000 00000fff 00000000
            t0       t1       t2       t3       t4       t5       t6       t7
 R8   00000000 00021000 00000000 00000417 00000000 8144f6e0 8122d6b8 00000000
            s0       s1       s2       s3       s4       s5       s6       s7
 R16  00000000 00000000 00000000 00500000 0052de28 0052e008 00000000 00000000
            t8       t9       k0       k1       gp       sp       s8       ra
 R24  77db4000 77e3f390 00000000 00000000 0041e220 7fff6090 7fff6090 00405c2c
        status       lo       hi badvaddr    cause       pc
      00009cf3 0046790f 00000005 00417004 00800024 00405c30
 f0:  0x00000000 flt: 0                 dbl: 0
 f1:  0x00000000 flt: 0
 f2:  0xd2f1a9fc flt: -5.18969491e+11   dbl: -0.001
 f3:  0xbf50624d flt: -0.813999951
 f4:  0x7ff80000 flt: nan               dbl: nan
 f5:  0x7ff80000 flt: nan
 f6:  0x7ff80000 flt: nan               dbl: nan
 f7:  0x7ff80000 flt: nan
 f8:  0x7ff80000 flt: nan               dbl: nan
 f9:  0x7ff80000 flt: nan
 f10: 0x7ff80000 flt: nan               dbl: nan
 f11: 0x7ff80000 flt: nan
 f12: 0x45a1cac1 flt: 5177.34424        dbl: 45.654000000000003
 f13: 0x4046d3b6 flt: 3.10667181
 f14: 0x70a3d70a flt: 4.05648183e+29    dbl: -67.659999999999997
 f15: 0xc050ea3d flt: -3.26429677
 f16: 0x7ff80000 flt: nan               dbl: nan
 f17: 0x7ff80000 flt: nan
 f18: 0x7ff80000 flt: nan               dbl: nan
 f19: 0x7ff80000 flt: nan
 f20: 0x7ff80000 flt: nan               dbl: nan
 f21: 0x7ff80000 flt: nan
 f22: 0x7ff80000 flt: nan               dbl: nan
 f23: 0x7ff80000 flt: nan
 f24: 0x7ff80000 flt: nan               dbl: nan
 f25: 0x7ff80000 flt: nan
 f26: 0x7ff80000 flt: nan               dbl: nan
 f27: 0x7ff80000 flt: nan
 f28: 0x7ff80000 flt: nan               dbl: nan
 f29: 0x7ff80000 flt: nan
 f30: 0x7ff80000 flt: nan               dbl: nan
 f31: 0x7ff80000 flt: nan
          fcsr      fir  restart
      00800000 00f30000 00000000
(gdb) 

is now:

(gdb) info all-registers
          zero       at       v0       v1       a0       a1       a2       a3
 R0   00000000 00000001 00000001 d2f1a9fc 00000000 00000000 00000fff 00000000
            t0       t1       t2       t3       t4       t5       t6       t7
 R8   00000000 00021000 00000000 00000417 00000000 8144f6e0 8122d6b8 00000000
            s0       s1       s2       s3       s4       s5       s6       s7
 R16  00000000 00000000 00000000 00500000 0052de28 0052e008 00000000 00000000
            t8       t9       k0       k1       gp       sp       s8       ra
 R24  77db4000 77e3f390 00000000 00000000 0041e220 7fff6090 7fff6090 00405c2c
        status       lo       hi badvaddr    cause       pc
      00009cf3 0046790f 00000005 00417004 00800024 00405c30
 f0:  0x00000000 flt: 0 dbl: 0
 f1:  0x00000000 flt: 0
 f2:  0xd2f1a9fc flt: -5.18969491e+11 dbl: -0.001
 f3:  0xbf50624d flt: -0.813999951
 f4:  0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f5:  0x7ff80000 flt: nan(0x780000)
 f6:  0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f7:  0x7ff80000 flt: nan(0x780000)
 f8:  0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f9:  0x7ff80000 flt: nan(0x780000)
 f10: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f11: 0x7ff80000 flt: nan(0x780000)
 f12: 0x45a1cac1 flt: 5177.34424 dbl: 45.654000000000003
 f13: 0x4046d3b6 flt: 3.10667181
 f14: 0x70a3d70a flt: 4.05648183e+29 dbl: -67.659999999999997
 f15: 0xc050ea3d flt: -3.26429677
 f16: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f17: 0x7ff80000 flt: nan(0x780000)
 f18: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f19: 0x7ff80000 flt: nan(0x780000)
 f20: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f21: 0x7ff80000 flt: nan(0x780000)
 f22: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f23: 0x7ff80000 flt: nan(0x780000)
 f24: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f25: 0x7ff80000 flt: nan(0x780000)
 f26: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f27: 0x7ff80000 flt: nan(0x780000)
 f28: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f29: 0x7ff80000 flt: nan(0x780000)
 f30: 0x7ff80000 flt: nan(0x780000) dbl: nan(0x800007ff80000)
 f31: 0x7ff80000 flt: nan(0x780000)
          fcsr      fir  restart
      00800000 00f30000 00000000
(gdb) 

I think we need to get the ability to align output (previously achieved 
with formatters, i.e. "%-17.9g" and "%-24.17g") restored; frankly I find 
the new format unreadable, which makes me consider it unacceptable.

 Let me know if I could help with either issue anyhow.

 NB while striving for FP arithmetic to match target hardware accurately 
we'll have to handle the two opposite qNaN vs sNaN encodings that the MIPS 
architecture now supports, i.e. the original IEEE 754-1985 format with the 
quiet bit being 0 and the recent IEEE 754-2008 format with the quiet bit 
being 1.  This obviously has to be taken into account in calculations, 
according to the hardware mode being active (or the relevant ELF file 
header flag in the binary chosen to debug if not connected to any target), 
and I do hope MPFR can be switched at the run time to do the right thing.

 This can of course be the next step, once we've settled on the base 
implementation -- and I'll take care of the MIPS architecture bits.

  Maciej
diff mbox

Patch

Index: binutils-gdb/gdb/i387-tdep.c
===================================================================
--- binutils-gdb.orig/gdb/i387-tdep.c
+++ binutils-gdb/gdb/i387-tdep.c
@@ -18,7 +18,6 @@ 
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
-#include "doublest.h"
 #include "frame.h"
 #include "gdbcore.h"
 #include "inferior.h"
@@ -30,31 +29,6 @@ 
 #include "i387-tdep.h"
 #include "x86-xstate.h"
 
-/* Print the floating point number specified by RAW.  */
-
-static void
-print_i387_value (struct gdbarch *gdbarch,
-		  const gdb_byte *raw, struct ui_file *file)
-{
-  DOUBLEST value;
-
-  /* Using extract_typed_floating here might affect the representation
-     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));
-
-  /* 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
-     to print the value, 1 position for the sign, 1 for the decimal
-     point, 19 for the digits and 6 for the exponent adds up to 27.  */
-#ifdef PRINTF_HAS_LONG_DOUBLE
-  fprintf_filtered (file, " %-+27.19Lg", (long double) value);
-#else
-  fprintf_filtered (file, " %-+27.19g", (double) value);
-#endif
-}
-
 /* Print the classification for the register contents RAW.  */
 
 static void
@@ -89,12 +63,16 @@  print_i387_ext (struct gdbarch *gdbarch,
 	fputs_filtered (" SNaN", file);
     }
   else if (exponent < 0x7fff && exponent > 0x0000 && integer)
-    /* Normal.  */
-    print_i387_value (gdbarch, raw, file);
+    {
+      /* Normal.  */
+      fputs_filtered (" ", file);
+      print_floating (raw, i387_ext_type (gdbarch), file);
+    }
   else if (exponent == 0x0000)
     {
       /* Denormal or zero.  */
-      print_i387_value (gdbarch, raw, file);
+      fputs_filtered (" ", file);
+      print_floating (raw, i387_ext_type (gdbarch), file);
       
       if (integer)
 	/* Pseudo-denormal.  */
Index: binutils-gdb/gdb/testsuite/gdb.arch/i386-float.exp
===================================================================
--- binutils-gdb.orig/gdb/testsuite/gdb.arch/i386-float.exp
+++ binutils-gdb/gdb/testsuite/gdb.arch/i386-float.exp
@@ -47,9 +47,9 @@  with_test_prefix "val" {
 }
 with_test_prefix "smallval" {
     gdb_test "stepi" ".*fldt.*"
-    gdb_test "info float" "=>R6: Valid   0x03e5c6f8c103dc90456a \\+3.500000000000000007e-4632\r\n.*"
+    gdb_test "info float" "=>R6: Valid   0x03e5c6f8c103dc90456a 3.50000000000000000\[0-9\]*e-4632\r\n.*"
 }
 with_test_prefix "bigval" {
     gdb_test "stepi" ".*ret.*"
-    gdb_test "info float" "=>R5: Valid   0x6a5cc643b78165d7d0e9 \\+7.250000000000000005e\\+3264\r\n.*"
+    gdb_test "info float" "=>R5: Valid   0x6a5cc643b78165d7d0e9 7.25000000000000000\[0-9\]*e\\+3264\r\n.*"
 }
Index: binutils-gdb/gdb/mips-tdep.c
===================================================================
--- binutils-gdb.orig/gdb/mips-tdep.c
+++ binutils-gdb/gdb/mips-tdep.c
@@ -6256,8 +6256,6 @@  mips_print_fp_register (struct ui_file *
 {				/* Do values for FP (float) regs.  */
   struct gdbarch *gdbarch = get_frame_arch (frame);
   gdb_byte *raw_buffer;
-  double doub, flt1;	/* Doubles extracted from raw hex data.  */
-  int inv1, inv2;
 
   raw_buffer
     = ((gdb_byte *)
@@ -6275,8 +6273,6 @@  mips_print_fp_register (struct ui_file *
       /* 4-byte registers: Print hex and floating.  Also print even
          numbered registers as doubles.  */
       mips_read_fp_register_single (frame, regnum, raw_buffer);
-      flt1 = unpack_double (builtin_type (gdbarch)->builtin_float,
-			    raw_buffer, &inv1);
 
       get_formatted_print_options (&opts, 'x');
       print_scalar_formatted (raw_buffer,
@@ -6284,22 +6280,16 @@  mips_print_fp_register (struct ui_file *
 			      &opts, 'w', file);
 
       fprintf_filtered (file, " flt: ");
-      if (inv1)
-	fprintf_filtered (file, " <invalid float> ");
-      else
-	fprintf_filtered (file, "%-17.9g", flt1);
+      print_floating (raw_buffer,
+                      builtin_type (gdbarch)->builtin_float, file);
 
       if ((regnum - gdbarch_num_regs (gdbarch)) % 2 == 0)
 	{
 	  mips_read_fp_register_double (frame, regnum, raw_buffer);
-	  doub = unpack_double (builtin_type (gdbarch)->builtin_double,
-				raw_buffer, &inv2);
 
 	  fprintf_filtered (file, " dbl: ");
-	  if (inv2)
-	    fprintf_filtered (file, "<invalid double>");
-	  else
-	    fprintf_filtered (file, "%-24.17g", doub);
+          print_floating (raw_buffer,
+                          builtin_type (gdbarch)->builtin_double, file);
 	}
     }
   else
@@ -6307,30 +6297,24 @@  mips_print_fp_register (struct ui_file *
       struct value_print_options opts;
 
       /* Eight byte registers: print each one as hex, float and double.  */
-      mips_read_fp_register_single (frame, regnum, raw_buffer);
-      flt1 = unpack_double (builtin_type (gdbarch)->builtin_float,
-			    raw_buffer, &inv1);
-
       mips_read_fp_register_double (frame, regnum, raw_buffer);
-      doub = unpack_double (builtin_type (gdbarch)->builtin_double,
-			    raw_buffer, &inv2);
 
       get_formatted_print_options (&opts, 'x');
       print_scalar_formatted (raw_buffer,
 			      builtin_type (gdbarch)->builtin_uint64,
 			      &opts, 'g', file);
 
+      mips_read_fp_register_single (frame, regnum, raw_buffer);
+
       fprintf_filtered (file, " flt: ");
-      if (inv1)
-	fprintf_filtered (file, "<invalid float>");
-      else
-	fprintf_filtered (file, "%-17.9g", flt1);
+      print_floating (raw_buffer,
+                      builtin_type (gdbarch)->builtin_float, file);
+
+      mips_read_fp_register_double (frame, regnum, raw_buffer);
 
       fprintf_filtered (file, " dbl: ");
-      if (inv2)
-	fprintf_filtered (file, "<invalid double>");
-      else
-	fprintf_filtered (file, "%-24.17g", doub);
+      print_floating (raw_buffer,
+                      builtin_type (gdbarch)->builtin_double, file);
     }
 }
 
Index: binutils-gdb/gdb/sh64-tdep.c
===================================================================
--- binutils-gdb.orig/gdb/sh64-tdep.c
+++ binutils-gdb/gdb/sh64-tdep.c
@@ -1915,8 +1915,6 @@  sh64_do_fp_register (struct gdbarch *gdb
 		     struct frame_info *frame, int regnum)
 {				/* Do values for FP (float) regs.  */
   unsigned char *raw_buffer;
-  double flt;	/* Double extracted from raw hex data.  */
-  int inv;
 
   /* Allocate space for the float.  */
   raw_buffer = (unsigned char *)
@@ -1927,20 +1925,13 @@  sh64_do_fp_register (struct gdbarch *gdb
     error (_("can't read register %d (%s)"),
 	   regnum, gdbarch_register_name (gdbarch, regnum));
 
-  /* Get the register as a number.  */ 
-  flt = unpack_double (builtin_type (gdbarch)->builtin_float,
-		       raw_buffer, &inv);
-
   /* Print the name and some spaces.  */
   fputs_filtered (gdbarch_register_name (gdbarch, regnum), file);
   print_spaces_filtered (15 - strlen (gdbarch_register_name
 					(gdbarch, regnum)), file);
 
   /* Print the value.  */
-  if (inv)
-    fprintf_filtered (file, "<invalid float>");
-  else
-    fprintf_filtered (file, "%-10.9g", flt);
+  print_floating (raw_buffer, builtin_type (gdbarch)->builtin_float, file);
 
   /* Print the fp register as hex.  */
   fprintf_filtered (file, "\t(raw ");