PATCH for Re: Problems with floatformat on Alpha

PATCH for Re: Problems with floatformat on Alpha

[Daniel Jacobowitz]

On Sun, Nov 24, 2002 at 04:44:47PM -0500, Daniel Jacobowitz wrote:
> I spent some time today tracking problems in Alpha/Linux testsuite results. 
> One of the biggies was SIGFPE in convert_floatformat_to_doublest.  The first
> problem was pretty easy:
> 
>   else if (exponent == 0)
>     exponent = 1 - fmt->exp_bias;
> 
> 1 is an int, exp_bias is an unsigned int, exponent is a long int.  The math
> is done unsigned and exponent is ridiculously large.
> 
> I'm not sending a patch yet because I can't test it; something goes wrong
> later, in the call to ldexp and elsewhere, that looks suspiciously like GDB
> is miscompiled.

Blech.  Doublest is assuming an IEEE-ish host.  More particularly, it
appears that the mechanism doublest.c uses to construct doubles is not
usable on Alpha in non-IEE mode.  The instruction "addt $f10,$f11,$f10"
can trap if $f10 is a denormal - even if $f11 is 0.0.  I have test code
which demonstrates this to my satisfaction.  So when we accumulate in
dto, we take a SIGFPE.

Here's the patch I'm using.  The doublest part is obvious and no one's
caring for alpha-linux right now; so I'll check this in in a few days
unless someone sees a problem.  It assumes that the compiler for an
alpha-linux host recognizes -mieee, which I'm comfortable with.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2002-11-24  Daniel Jacobowitz  <drow@mvista.com>

	* doublest.c (convert_floatformat_to_doublest): Cast exp_bias to int.
	* config/alpha/alpha-linux.mh (MH_CFLAGS): Add -mieee.

--- gdb-5.2.debian90.cvs20021120/gdb/doublest.c.orig	2002-11-24 17:48:16.000000000 -0500
+++ gdb-5.2.debian90.cvs20021120/gdb/doublest.c	2002-11-24 17:48:25.000000000 -0500
@@ -177,7 +177,7 @@
   if (!special_exponent)
     exponent -= fmt->exp_bias;
   else if (exponent == 0)
-    exponent = 1 - fmt->exp_bias;
+    exponent = 1 - (int)fmt->exp_bias;
 
   /* Build the result algebraically.  Might go infinite, underflow, etc;
      who cares. */
--- gdb-5.2.debian90.cvs20021120/gdb/config/alpha/alpha-linux.mh.orig	2002-11-24 17:50:30.000000000 -0500
+++ gdb-5.2.debian90.cvs20021120/gdb/config/alpha/alpha-linux.mh	2002-11-24 17:50:41.000000000 -0500
@@ -8,3 +8,5 @@
 
 MMALLOC = 
 MMALLOC_CFLAGS = -DNO_MMALLOC 
+
+MH_CFLAGS = -mieee

Re: PATCH for Re: Problems with floatformat on Alpha

[Andrew Cagney]

> 
> Blech.  Doublest is assuming an IEEE-ish host.  More particularly, it
> appears that the mechanism doublest.c uses to construct doubles is not
> usable on Alpha in non-IEE mode.  The instruction "addt $f10,$f11,$f10"
> can trap if $f10 is a denormal - even if $f11 is 0.0.  I have test code
> which demonstrates this to my satisfaction.  So when we accumulate in
> dto, we take a SIGFPE.

Suggest a bug report explaining this (more amunition for getting a 
proper floating-point library).

Andrew

Re: PATCH for Re: Problems with floatformat on Alpha

[Daniel Jacobowitz]

On Sun, Nov 24, 2002 at 05:56:05PM -0500, Daniel Jacobowitz wrote:
> On Sun, Nov 24, 2002 at 04:44:47PM -0500, Daniel Jacobowitz wrote:
> > I spent some time today tracking problems in Alpha/Linux testsuite results. 
> > One of the biggies was SIGFPE in convert_floatformat_to_doublest.  The first
> > problem was pretty easy:
> > 
> >   else if (exponent == 0)
> >     exponent = 1 - fmt->exp_bias;
> > 
> > 1 is an int, exp_bias is an unsigned int, exponent is a long int.  The math
> > is done unsigned and exponent is ridiculously large.
> > 
> > I'm not sending a patch yet because I can't test it; something goes wrong
> > later, in the call to ldexp and elsewhere, that looks suspiciously like GDB
> > is miscompiled.
> 
> Blech.  Doublest is assuming an IEEE-ish host.  More particularly, it
> appears that the mechanism doublest.c uses to construct doubles is not
> usable on Alpha in non-IEE mode.  The instruction "addt $f10,$f11,$f10"
> can trap if $f10 is a denormal - even if $f11 is 0.0.  I have test code
> which demonstrates this to my satisfaction.  So when we accumulate in
> dto, we take a SIGFPE.
> 
> Here's the patch I'm using.  The doublest part is obvious and no one's
> caring for alpha-linux right now; so I'll check this in in a few days
> unless someone sees a problem.  It assumes that the compiler for an
> alpha-linux host recognizes -mieee, which I'm comfortable with.

Here's the updated version, with comments.  Also filed a PR about the
doublest.c problems.  Checked in on the trunk; Andrew, OK for the
branch?


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2002-12-04  Daniel Jacobowitz  <drow@mvista.com>

	* doublest.c (convert_floatformat_to_doublest): Cast exp_bias to int.
	* config/alpha/alpha-linux.mh (MH_CFLAGS): Add -mieee.

Index: doublest.c
===================================================================
RCS file: /cvs/src/src/gdb/doublest.c,v
retrieving revision 1.10
diff -u -p -r1.10 doublest.c
--- doublest.c	9 Aug 2002 00:45:10 -0000	1.10
+++ doublest.c	4 Dec 2002 05:38:25 -0000
@@ -172,12 +172,14 @@ convert_floatformat_to_doublest (const s
 
   special_exponent = exponent == 0 || exponent == fmt->exp_nan;
 
-/* Don't bias NaNs. Use minimum exponent for denorms. For simplicity,
-   we don't check for zero as the exponent doesn't matter. */
+  /* Don't bias NaNs. Use minimum exponent for denorms. For simplicity,
+     we don't check for zero as the exponent doesn't matter.  Note the cast
+     to int; exp_bias is unsigned, so it's important to make sure the
+     operation is done in signed arithmetic.  */
   if (!special_exponent)
     exponent -= fmt->exp_bias;
   else if (exponent == 0)
-    exponent = 1 - fmt->exp_bias;
+    exponent = 1 - (int) fmt->exp_bias;
 
   /* Build the result algebraically.  Might go infinite, underflow, etc;
      who cares. */
Index: config/alpha/alpha-linux.mh
===================================================================
RCS file: /cvs/src/src/gdb/config/alpha/alpha-linux.mh,v
retrieving revision 1.9
diff -u -p -r1.9 alpha-linux.mh
--- config/alpha/alpha-linux.mh	18 Jan 2002 04:50:57 -0000	1.9
+++ config/alpha/alpha-linux.mh	4 Dec 2002 05:38:25 -0000
@@ -8,3 +8,7 @@ LOADLIBES = -ldl -rdynamic
 
 MMALLOC = 
 MMALLOC_CFLAGS = -DNO_MMALLOC 
+
+# doublest.c currently assumes some properties of FP arithmetic
+# on the host which require this.
+MH_CFLAGS = -mieee

Re: PATCH for Re: Problems with floatformat on Alpha

[Andrew Cagney]

> 
> Here's the updated version, with comments.  Also filed a PR about the
> doublest.c problems.  Checked in on the trunk; Andrew, OK for the
> branch?

Yes.  (Having thought about it, I'm going to change exp_bias to an int. 
  This bug is going to keep happening :-)

Andrew