[PATCH, RFA]: Revamp NaN detection & discussion

[PATCH, RFA]: Revamp NaN detection & discussion

[Mark Kettenis]

Hi,

In an attempt to get rid of the Linux-specific TARGET_ANALYZE_FLOATING
macro I revamped the code that detects and prints NaN's in
valprint.c:print_floating().  The new code uses the "floatformat.h"
information already present in GDB/libiberty, which means that in
principle all targets that correctly set TARGET_LONG_DOUBLE_FORMAT get
NaN detection for free.

I also changed the way NaN's are printed to something that conforms to
the ISO C99, which means that instead of say

-NaN(0x4000)

GDB now prints

-nan(0x4000)

I think the new format is prettier :-) and it matches what glibc's
printf() will do (although in that case you won't get the sign and the
mantissa in hex).  But if people on the list think this could cause
problems, it wouldn't be a problem to change it back to the old
format.

While working on this code I realized that the IEEE_FLOAT target macro
is almost redundant.  If only we wouldn't default TARGET_FLOAT_FORMAT
and TARGET_DOUBLE_FORMAT to IEEE formats, we could get rid of it.
Any bright idea's on how to accomplish this?

I also have an itch about floatformat.{ch} and its functions.  I'd
rather put all floatformat_* functions in there rather than in
defs.h/utils.c.  But since it's part of libiberty that's a bit of a
pain.  Is this stuff really used anywhere outside GDB?  If not,
perhaps we could claim it back.  Otherwise I'd like to add a
gdb_floatformat.{ch} and put all the floatformat stuff there.

Mark


2001-01-25  Mark Kettenis  <kettenis@gnu.org>

	* defs.h: Provide prototypes for floatformat_is_negative,
	floatformat_is_nan and floatformat_mantissa.
	* utils.c: Include "gdb_assert.h".
	(floatformat_is_negative): New function.
	(floatformat_is_nan): New function.
	(floatformat_mantissa): New function.
	* valprint.c: Include "floatformat.h".
	(print_floating): Get rid of the Linux-specific
	TARGET_ANALYZE_FLOATING macro and rewrite NaN detection with the
	help these new functions.  Print NaN's in a format conforming to
	ISO C99.


Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.34
diff -u -p -r1.34 defs.h
--- defs.h 2001/01/19 08:01:46 1.34
+++ defs.h 2001/01/25 20:38:59
@@ -1,6 +1,6 @@
 /* *INDENT-OFF* */ /* ATTR_FORMAT confuses indent, avoid running it for now */
 /* Basic, host-specific, and target-specific definitions for GDB.
-   Copyright (C) 1986, 1989, 1991-1996, 1998-2000
+   Copyright (C) 1986, 1989, 1991-1996, 1998-2000, 2001
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -1121,8 +1121,12 @@ extern void floatformat_to_doublest (con
 				     char *, DOUBLEST *);
 extern void floatformat_from_doublest (const struct floatformat *,
 				       DOUBLEST *, char *);
-extern DOUBLEST extract_floating (void *, int);
 
+extern int floatformat_is_negative (const struct floatformat *, char *);
+extern int floatformat_is_nan (const struct floatformat *, char *);
+extern char *floatformat_mantissa (const struct floatformat *, char *);
+
+extern DOUBLEST extract_floating (void *, int);
 extern void store_floating (void *, int, DOUBLEST);
 \f
 /* On some machines there are bits in addresses which are not really
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.25
diff -u -p -r1.25 utils.c
--- utils.c 2000/12/15 01:01:50 1.25
+++ utils.c 2001/01/25 20:39:02
@@ -1,5 +1,5 @@
 /* General utility routines for GDB, the GNU debugger.
-   Copyright 1986, 1989, 1990-1992, 1995, 1996, 1998, 2000
+   Copyright 1986, 1989, 1990-1992, 1995, 1996, 1998, 2000, 2001
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -20,6 +20,7 @@
    Boston, MA 02111-1307, USA.  */
 
 #include "defs.h"
+#include "gdb_assert.h"
 #include <ctype.h>
 #include "gdb_string.h"
 #include "event-top.h"
@@ -2729,6 +2730,106 @@ floatformat_from_doublest (CONST struct 
 	  *swaphigh++ = tmp;
 	}
     }
+}
+
+/* Check if VAL (which is assumed to be a floating point number whose
+   format is described by FMT) is negative.  */
+
+int
+floatformat_is_negative (const struct floatformat *fmt, char *val)
+{
+  unsigned char *uval = (unsigned char *) val;
+
+  return get_field (uval, fmt->byteorder, fmt->totalsize, fmt->sign_start, 1);
+}
+
+/* Check if VAL is "not a number" (NaN) for FMT.  */
+
+int
+floatformat_is_nan (const struct floatformat *fmt, char *val)
+{
+  unsigned char *uval = (unsigned char *) val;
+  long exponent;
+  unsigned long mant;
+  unsigned int mant_bits, mant_off;
+  int mant_bits_left;
+
+  if (! fmt->exp_nan)
+    return 0;
+
+  exponent = get_field (uval, fmt->byteorder, fmt->totalsize,
+			fmt->exp_start, fmt->exp_len);
+
+  if (exponent != fmt->exp_nan)
+    return 0;
+
+  mant_bits_left = fmt->man_len;
+  mant_off = fmt->man_start;
+
+  while (mant_bits_left > 0)
+    {
+      mant_bits = min (mant_bits_left, 32);
+
+      mant = get_field (uval, fmt->byteorder, fmt->totalsize,
+			mant_off, mant_bits);
+
+      /* If there is an explicit integer bit, mask it off.  */
+      if (mant_off == fmt->man_start
+	  && fmt->intbit == floatformat_intbit_yes)
+	mant &= ~(1 << (mant_bits - 1));
+
+      if (mant)
+	return 1;
+
+      mant_off += mant_bits;
+      mant_bits_left -= mant_bits;
+    }
+
+  return 0;
+}
+
+/* Convert the mantissa of VAL (which is assumed to be a floating
+   point number whose format is described by FMT) into a hexadecimal
+   and store it in a static string.  Return a pointer to that string.  */
+
+char *
+floatformat_mantissa (const struct floatformat *fmt, char *val)
+{
+  unsigned char *uval = (unsigned char *) val;
+  unsigned long mant;
+  unsigned int mant_bits, mant_off;
+  int mant_bits_left;
+  static char res[50];
+  char buf[9];
+
+  /* Make sure we have enough room to store the mantissa.  */
+  gdb_assert (sizeof res > ((fmt->man_len + 7) / 8) * 2);
+
+  mant_off = fmt->man_start;
+  mant_bits_left = fmt->man_len;
+  mant_bits = (mant_bits_left % 32) > 0 ? mant_bits_left % 32 : 32;
+
+  mant = get_field (uval, fmt->byteorder, fmt->totalsize,
+		    mant_off, mant_bits);
+
+  sprintf (res, "%lx", mant);
+
+  mant_off += mant_bits;
+  mant_bits_left -= mant_bits;
+  
+  while (mant_bits_left > 0)
+    {
+      mant = get_field (uval, fmt->byteorder, fmt->totalsize,
+			mant_off, 32);
+
+      sprintf (buf, "%08lx", mant);
+      strcat (res, buf);
+
+      mant_off += 32;
+      mant_bits_left -= 32;
+    }
+
+  return res;
 }
 
 /* print routines to handle variable size regs, etc. */
Index: valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/valprint.c,v
retrieving revision 1.9
diff -u -p -r1.9 valprint.c
--- valprint.c 2000/12/15 01:01:50 1.9
+++ valprint.c 2001/01/25 20:39:03
@@ -1,5 +1,5 @@
 /* Print values for GDB, the GNU debugger.
-   Copyright 1986, 1988, 1989, 1991-1994, 1998, 2000
+   Copyright 1986, 1988, 1989, 1991-1994, 1998, 2000, 2001
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -32,6 +32,7 @@
 #include "demangle.h"
 #include "annotate.h"
 #include "valprint.h"
+#include "floatformat.h"
 
 #include <errno.h>
 
@@ -538,92 +539,39 @@ longest_to_int (LONGEST arg)
   return (rtnval);
 }
 
+/* Print a floating point value of type TYPE, pointed to in GDB by
+   VALADDR, on STREAM.  */
 
-/* Print a floating point value of type TYPE, pointed to in GDB by VALADDR,
-   on STREAM.  */
-
 void
 print_floating (char *valaddr, struct type *type, struct ui_file *stream)
 {
   DOUBLEST doub;
   int inv;
+  const struct floatformat *fmt = &floatformat_unknown;
   unsigned len = TYPE_LENGTH (type);
-
-  /* Check for NaN's.  Note that this code does not depend on us being
-     on an IEEE conforming system.  It only depends on the target
-     machine using IEEE representation.  This means (a)
-     cross-debugging works right, and (2) IEEE_FLOAT can (and should)
-     be non-zero for systems like the 68881, which uses IEEE
-     representation, but is not IEEE conforming.  */
-  if (IEEE_FLOAT)
-    {
-      unsigned long low, high;
-      /* Is the sign bit 0?  */
-      int nonnegative;
-      /* Is it is a NaN (i.e. the exponent is all ones and
-	 the fraction is nonzero)?  */
-      int is_nan;
-
-      /* For lint, initialize these two variables to suppress warning: */
-      low = high = nonnegative = 0;
-      if (len == 4)
-	{
-	  /* It's single precision.  */
-	  /* Assume that floating point byte order is the same as
-	     integer byte order.  */
-	  low = extract_unsigned_integer (valaddr, 4);
-	  nonnegative = ((low & 0x80000000) == 0);
-	  is_nan = ((((low >> 23) & 0xFF) == 0xFF)
-		    && 0 != (low & 0x7FFFFF));
-	  low &= 0x7fffff;
-	  high = 0;
-	}
-      else if (len == 8)
-	{
-	  /* It's double precision.  Get the high and low words.  */
 
-	  /* Assume that floating point byte order is the same as
-	     integer byte order.  */
-	  if (TARGET_BYTE_ORDER == BIG_ENDIAN)
-	    {
-	      low = extract_unsigned_integer (valaddr + 4, 4);
-	      high = extract_unsigned_integer (valaddr, 4);
-	    }
-	  else
-	    {
-	      low = extract_unsigned_integer (valaddr, 4);
-	      high = extract_unsigned_integer (valaddr + 4, 4);
-	    }
-	  nonnegative = ((high & 0x80000000) == 0);
-	  is_nan = (((high >> 20) & 0x7ff) == 0x7ff
-		    && !((((high & 0xfffff) == 0)) && (low == 0)));
-	  high &= 0xfffff;
-	}
-      else
-	{
-#ifdef TARGET_ANALYZE_FLOATING
-	  TARGET_ANALYZE_FLOATING;
-#else
-	  /* Extended.  We can't detect extended NaNs for this target.
-	     Also note that currently extendeds get nuked to double in
-	     REGISTER_CONVERTIBLE.  */
-	  is_nan = 0;
-#endif 
-	}
-
-      if (is_nan)
-	{
-	  /* The meaning of the sign and fraction is not defined by IEEE.
-	     But the user might know what they mean.  For example, they
-	     (in an implementation-defined manner) distinguish between
-	     signaling and quiet NaN's.  */
-	  if (high)
-	    fprintf_filtered (stream, "-NaN(0x%lx%.8lx)" + !!nonnegative,
-			      high, low);
-	  else
-	    fprintf_filtered (stream, "-NaN(0x%lx)" + nonnegative, low);
-	  return;
-	}
+  /* FIXME: kettenis/2001-01-20: The check for IEEE_FLOAT is probably
+     still necessary since GDB by default assumes that the target uses
+     the IEEE 754 representation for its floats and doubles.  Of
+     course this is all crock and should be cleaned up.  */
+
+  if (len == TARGET_FLOAT_BIT / TARGET_CHAR_BIT && IEEE_FLOAT)
+    fmt = TARGET_FLOAT_FORMAT;
+  else if (len == TARGET_DOUBLE_BIT / TARGET_CHAR_BIT && IEEE_FLOAT)
+    fmt = TARGET_DOUBLE_FORMAT;
+  else if (len == TARGET_LONG_DOUBLE_BIT / TARGET_CHAR_BIT)
+    fmt = TARGET_LONG_DOUBLE_FORMAT;
+
+  if (floatformat_is_nan (fmt, valaddr))
+    {
+      if (floatformat_is_negative (fmt, valaddr))
+	fprintf_filtered (stream, "-");
+      fprintf_filtered (stream, "nan(");
+      fprintf_filtered (stream, local_hex_format_prefix ());
+      fprintf_filtered (stream, floatformat_mantissa (fmt, valaddr));
+      fprintf_filtered (stream, local_hex_format_suffix ());
+      fprintf_filtered (stream, ")");
+      return;
     }
 
   doub = unpack_double (type, valaddr, &inv);
@@ -633,6 +581,9 @@ print_floating (char *valaddr, struct ty
       return;
     }
 
+  /* FIXME: kettenis/2001-01-20: The following code makes too much
+     assumptions about the host and target floating point format.  */
+
   if (len < sizeof (double))
       fprintf_filtered (stream, "%.9g", (double) doub);
   else if (len == sizeof (double))
@@ -641,7 +592,8 @@ print_floating (char *valaddr, struct ty
 #ifdef PRINTF_HAS_LONG_DOUBLE
     fprintf_filtered (stream, "%.35Lg", doub);
 #else
-    /* This at least wins with values that are representable as doubles */
+    /* This at least wins with values that are representable as
+       doubles.  */
     fprintf_filtered (stream, "%.17g", (double) doub);
 #endif
 }

Re: [PATCH, RFA]: Revamp NaN detection & discussion

[Kevin Buettner]

On Jan 25, 10:03pm, Mark Kettenis wrote:

> I also have an itch about floatformat.{ch} and its functions.  I'd
> rather put all floatformat_* functions in there rather than in
> defs.h/utils.c.  But since it's part of libiberty that's a bit of a
> pain.  Is this stuff really used anywhere outside GDB?  If not,
> perhaps we could claim it back. > Otherwise I'd like to add a
> gdb_floatformat.{ch} and put all the floatformat stuff there.

To the best of my knowledge, the differences between the libiberty
floatformat support and that in GDB is that GDB converts to/from a
DOUBLEST whereas libiberty uses a double.  Also, GDB has fixed some
bugs that are still in the libiberty version.  The opposite may also
be true.

Clearly, it'd be best if only one version of the floatformat
functionality needed to be supported.  Ideally, the floatformat
functionality would reside in libiberty (or perhaps some other
library) and GDB would just make use of it.  It would then be
available for other programs to use as well.  (And indeed, I imagine
these were the original motives which caused this support to
be moved from GDB to libiberty.)  However, as shown by your patch,
GDB would like to extend the functionality from time to time in ways
that may break the other (perhaps mythical) applications which use it. 
So it would seem that GDB will need its own copy of this code.

I don't know if an immediate "claiming back" of the floatformat
support from libiberty is a realistic alternative.  While I don't know
of any other applications which use it, I haven't really looked around
either.  In any event, it seems to me that it would be really
difficult to know whether someone is using this functionality or not.

I think the best thing to do (which comes pretty close to "claiming it
back") is to stop using any of the libiberty floatformat support in
GDB.  In other words, GDB's floatformat support should be entirely
self contained.  (I like your idea of putting it in
gdb_floatformat.[hc].)  Once this is done, the libiberty maintainer
will have the freedom to slowly phase out the floatformat support
if that is deemed a desirable goal.

Kevin

Re: [PATCH, RFA]: Revamp NaN detection & discussion

[Andrew Cagney]

> I also have an itch about floatformat.{ch} and its functions.  I'd
> rather put all floatformat_* functions in there rather than in
> defs.h/utils.c.  But since it's part of libiberty that's a bit of a
> pain.  Is this stuff really used anywhere outside GDB?  If not,
> perhaps we could claim it back.  Otherwise I'd like to add a
> gdb_floatformat.{ch} and put all the floatformat stuff there.

Hmm, looked in the TODO file? (I'll append all the comments).
Given that no one appears to be using floatformat (unless GCC is?) I
think we can ``claim it back''.

At a more idealistic level we could instead use sim/common/sim-fpu.[ch]
and implement GDB's target floating point using that.  I can but dream
:-)

	Andrew

PS: The harris mess is sorted out - harris is obsolete.

--

Rationalize use of floatformat_unknown in GDB sources.

Instead of defaulting to floatformat_unknown, should hosts/targets
specify the value explicitly?

http://sourceware.cygnus.com/ml/gdb-patches/2000-05/msg00447.html

--

Add a ``name'' member to include/floatformat.h:struct floatformat.
Print that name in gdbarch.c.

--

Sort out the harris mess in include/floatformat.h (it hardwires two
different floating point formats).

--

See of the GDB local floatformat_do_doublest() and libiberty's
floatformat_to_double (which was once GDB's ...) can be merged some
how.

--

Deprecate current use of ``floatformat_unknown''.

Require all targets to explicitly provide their float format instead
of defaulting to floatformat unknown.  Doing the latter leads to nasty
bugs.

http://sourceware.cygnus.com/ml/gdb-patches/2000-05/msg00447.html

--

Rationalize floatformat_to_double() vs floatformat_to_doublest().

Looks like GDB migrated floatformat_to_double() to libiberty but then
turned around and created a ..._to_doublest() the latter containing
several bug fixes.

http://sourceware.cygnus.com/ml/gdb-patches/2000-05/msg00472.html

--

Move floatformat_ia64_ext to libiberty/include floatformat.[ch].

http://sourceware.cygnus.com/ml/gdb-patches/2000-05/msg00466.html

--