| Message ID | 20170905182100.37B12D8086F@oc3748833570.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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 ");