* [RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code
@ 2017-10-05 17:31 Ulrich Weigand
2017-10-11 18:35 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2017-10-05 17:31 UTC (permalink / raw)
To: gdb-patches
[RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code
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.
Bye,
Ulrich
ChangeLog:
* i387-tdep.c (print_i387_value): Use floatformat_to_string.
* sh64-tdep.c (sh64_do_fp_register): Likewise.
* mips-tdep.c (mips_print_fp_register): Likewise.
Index: binutils-gdb/gdb/i387-tdep.c
===================================================================
--- binutils-gdb.orig/gdb/i387-tdep.c
+++ binutils-gdb/gdb/i387-tdep.c
@@ -36,23 +36,14 @@ 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
+ const struct floatformat *fmt
+ = floatformat_from_type (i387_ext_type (gdbarch));
+ std::string str = floatformat_to_string (fmt, raw, " %-+27.19g");
+ fprintf_filtered (file, "%s", str.c_str ());
}
/* Print the classification for the register contents RAW. */
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,16 @@ 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);
+ const struct floatformat *fmt
+ = floatformat_from_type (builtin_type (gdbarch)->builtin_float);
+ std::string str = floatformat_to_string (fmt, raw_buffer, "%-10.9g");
+ fprintf_filtered (file, "%s", str.c_str ());
/* Print the fp register as hex. */
fprintf_filtered (file, "\t(raw ");
Index: binutils-gdb/gdb/mips-tdep.c
===================================================================
--- binutils-gdb.orig/gdb/mips-tdep.c
+++ binutils-gdb/gdb/mips-tdep.c
@@ -6256,8 +6256,12 @@ 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;
+ std::string flt_str, dbl_str;
+
+ const struct floatformat *flt_fmt
+ = floatformat_from_type (builtin_type (gdbarch)->builtin_float);
+ const struct floatformat *dbl_fmt
+ = floatformat_from_type (builtin_type (gdbarch)->builtin_double);
raw_buffer
= ((gdb_byte *)
@@ -6275,31 +6279,21 @@ 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);
+ flt_str = floatformat_to_string (flt_fmt, raw_buffer, "%-17.9g");
get_formatted_print_options (&opts, 'x');
print_scalar_formatted (raw_buffer,
builtin_type (gdbarch)->builtin_uint32,
&opts, 'w', file);
- fprintf_filtered (file, " flt: ");
- if (inv1)
- fprintf_filtered (file, " <invalid float> ");
- else
- fprintf_filtered (file, "%-17.9g", flt1);
+ fprintf_filtered (file, " flt: %s", flt_str.c_str ());
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);
+ dbl_str = floatformat_to_string (dbl_fmt, raw_buffer, "%-24.17g");
- fprintf_filtered (file, " dbl: ");
- if (inv2)
- fprintf_filtered (file, "<invalid double>");
- else
- fprintf_filtered (file, "%-24.17g", doub);
+ fprintf_filtered (file, " dbl: %s", dbl_str.c_str ());
}
}
else
@@ -6308,29 +6302,18 @@ mips_print_fp_register (struct ui_file *
/* 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);
+ flt_str = floatformat_to_string (flt_fmt, raw_buffer, "%-17.9g");
mips_read_fp_register_double (frame, regnum, raw_buffer);
- doub = unpack_double (builtin_type (gdbarch)->builtin_double,
- raw_buffer, &inv2);
+ dbl_str = floatformat_to_string (dbl_fmt, raw_buffer, "%-24.17g");
get_formatted_print_options (&opts, 'x');
print_scalar_formatted (raw_buffer,
builtin_type (gdbarch)->builtin_uint64,
&opts, 'g', file);
- fprintf_filtered (file, " flt: ");
- if (inv1)
- fprintf_filtered (file, "<invalid float>");
- else
- fprintf_filtered (file, "%-17.9g", flt1);
-
- fprintf_filtered (file, " dbl: ");
- if (inv2)
- fprintf_filtered (file, "<invalid double>");
- else
- fprintf_filtered (file, "%-24.17g", doub);
+ fprintf_filtered (file, " flt: %s", flt_str.c_str ());
+ fprintf_filtered (file, " dbl: %s", dbl_str.c_str ());
}
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code
2017-10-05 17:31 [RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code Ulrich Weigand
@ 2017-10-11 18:35 ` Maciej W. Rozycki
2017-10-17 9:08 ` Ulrich Weigand
0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2017-10-11 18:35 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
Hi Ulrich,
> 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.
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code
2017-10-11 18:35 ` Maciej W. Rozycki
@ 2017-10-17 9:08 ` Ulrich Weigand
2017-10-17 11:40 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2017-10-17 9:08 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: gdb-patches
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
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. */
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code
2017-10-17 9:08 ` Ulrich Weigand
@ 2017-10-17 11:40 ` Maciej W. Rozycki
2017-10-17 13:19 ` Ulrich Weigand
0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2017-10-17 11:40 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code
2017-10-17 11:40 ` Maciej W. Rozycki
@ 2017-10-17 13:19 ` Ulrich Weigand
2017-10-24 16:06 ` [pushed][RFC " Ulrich Weigand
0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2017-10-17 13:19 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: gdb-patches
> On Tue, 17 Oct 2017, Ulrich Weigand wrote:
> > 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.
OK, thanks for confirming.
> 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.
I'm not sure I like adding (more of) a format parser here, that would
need deciding which parts of the format to act on and which not ...
If you'd like to see this in the mips output, maybe this can be done
in the tdep file: go back to using the print routine *without* format
string, and then print the resulting string using %s with a width
specifier.
> As an independent change I think it would best be made with a separate
> patch though.
Agreed.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pushed][RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code
2017-10-17 13:19 ` Ulrich Weigand
@ 2017-10-24 16:06 ` Ulrich Weigand
0 siblings, 0 replies; 6+ messages in thread
From: Ulrich Weigand @ 2017-10-24 16:06 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: Maciej W. Rozycki, gdb-patches
> > On Tue, 17 Oct 2017, Ulrich Weigand wrote:
> > > 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.
>
> OK, thanks for confirming.
I've now pushed the three "Target FP printing" patches in their
latest form.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-24 16:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 17:31 [RFC v2][3/3] Target FP printing: Use floatformat_to_string in tdep code Ulrich Weigand
2017-10-11 18:35 ` Maciej W. Rozycki
2017-10-17 9:08 ` Ulrich Weigand
2017-10-17 11:40 ` Maciej W. Rozycki
2017-10-17 13:19 ` Ulrich Weigand
2017-10-24 16:06 ` [pushed][RFC " Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).