math-emu issue with fp divide

math-emu issue with fp divide

[Kumar Gala]

David,

You seem to be the last person to touch the math-emu code in the  
kernel so I figured you might have some insights.  I'm trying to move  
the powerpc math-emu code to use the include/math-emu bits.

In doing so I've been using TestFloat to see how good or bad we are  
doing.  For the most part the current math-emu code that PPC uses has  
a number of issues that the code in include/math-emu seems to solve  
(plus bugs we've had for ever that no one every realized).

Anyways, I've come across a case that we are flagging underflow and  
inexact because we think we have a denormalized result from a double  
precision divide:

000.FFFFFFFFFFFFF / 3FE.FFFFFFFFFFFFE
	soft: 001.0000000000000 .....  syst: 001.0000000000000 ...ux

What it looks like is the results out of FP_DIV_D are:

D:
sign:	  0
mantissa: 01000000 00000000
exp:	 -1023 (0)

The problem seems like we aren't normalizing the result and bumping  
the exp.  So I was wondering if you had any idea if FP_DIV_D should be  
doing that or if when we call FP_PACK_DP/_FP_PACK_CANONICAL if that  
could should be doing it.

thanks

- k

Re: math-emu issue with fp divide

[David Miller]

From: Kumar Gala <galak@kernel.crashing.org>
Date: Wed, 4 Jun 2008 15:38:22 -0500

> You seem to be the last person to touch the math-emu code in the  
> kernel so I figured you might have some insights.  I'm trying to move  
> the powerpc math-emu code to use the include/math-emu bits.

I'll try to look at this after I'm done with my current 3 week
stint of travelling, but right now I really don't have time for
stuff like this.

I tried running TestFloat a couple weeks ago but I couldn't
get the damn thing to even build properly after fighting with
it for what seemed like hours :-/  I'll try again at some point.

Re: math-emu issue with fp divide

[Joseph S. Myers]

On Wed, 4 Jun 2008, Kumar Gala wrote:

> I'm trying to move the powerpc math-emu
> code to use the include/math-emu bits.

I'd like to remind people that the include/math-emu code is based on an 
old, buggy version of the soft-fp code in glibc.  RTH previously noted 
<http://sourceware.org/ml/libc-alpha/2006-02/msg00075.html> that the fixes 
should go in the kernel, when approving a patch of mine fixing various 
bugs and speeding up soft-fp, and several more bugs have been fixed since 
then in the course of GCC work on soft-float for Power and __float128 
support for x86_64.

So it may be better than the old code you're replacing, but it might still 
be a good idea to merge things with the current glibc code, as a matter of 
both correctness and performance.  (And there may well still be bugs left 
in the code.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: math-emu issue with fp divide

[David Miller]

From: "Joseph S. Myers" <joseph@codesourcery.com>
Date: Wed, 4 Jun 2008 21:42:55 +0000 (UTC)

> On Wed, 4 Jun 2008, Kumar Gala wrote:
> 
> > I'm trying to move the powerpc math-emu
> > code to use the include/math-emu bits.
> 
> I'd like to remind people that the include/math-emu code is based on an 
> old, buggy version of the soft-fp code in glibc.  RTH previously noted 
> <http://sourceware.org/ml/libc-alpha/2006-02/msg00075.html> that the fixes 
> should go in the kernel, when approving a patch of mine fixing various 
> bugs and speeding up soft-fp, and several more bugs have been fixed since 
> then in the course of GCC work on soft-float for Power and __float128 
> support for x86_64.
> 
> So it may be better than the old code you're replacing, but it might still 
> be a good idea to merge things with the current glibc code, as a matter of 
> both correctness and performance.  (And there may well still be bugs left 
> in the code.)

Once the GCC folks started looking into using the soft-fp bits I knew
this situation would develop.  The kernel side would have some set of
fixes and the glibc/gcc side would have yet another set.

To be honest this doesn't surprise me...

Re: math-emu issue with fp divide

[Kumar Gala]

On Wed, 4 Jun 2008, David Miller wrote:

> From: "Joseph S. Myers" <joseph@codesourcery.com>
> Date: Wed, 4 Jun 2008 21:42:55 +0000 (UTC)
>
> > On Wed, 4 Jun 2008, Kumar Gala wrote:
> >
> > > I'm trying to move the powerpc math-emu
> > > code to use the include/math-emu bits.
> >
> > I'd like to remind people that the include/math-emu code is based on an
> > old, buggy version of the soft-fp code in glibc.  RTH previously noted
> > <http://sourceware.org/ml/libc-alpha/2006-02/msg00075.html> that the fixes
> > should go in the kernel, when approving a patch of mine fixing various
> > bugs and speeding up soft-fp, and several more bugs have been fixed since
> > then in the course of GCC work on soft-float for Power and __float128
> > support for x86_64.
> >
> > So it may be better than the old code you're replacing, but it might still
> > be a good idea to merge things with the current glibc code, as a matter of
> > both correctness and performance.  (And there may well still be bugs left
> > in the code.)
>
> Once the GCC folks started looking into using the soft-fp bits I knew
> this situation would develop.  The kernel side would have some set of
> fixes and the glibc/gcc side would have yet another set.
>
> To be honest this doesn't surprise me...
>

Now that I'm digging into this a bit I'm thinking my issue has to do with
the fix you put in place from back in Aug 2007 (commit
405849610fd96b4f34cd1875c4c033228fea6c0f):

[MATH-EMU]: Fix underflow exception reporting.

    2) we ended up rounding back up to normal (this is the case where
       we set the exponent to 1 and set the fraction to zero), this
       should set inexact too
...

    Another example, "0x0.0000000000001p-1022 / 16.0", should signal both
    inexact and underflow.  The cpu implementations and ieee1754
    literature is very clear about this.  This is case #2 above.

I'm not clear from your commit comment on what actual number
0x0.0....01p-1022 is?

It looks like the case I have we are exact before rounding, but think it
looks like the rounding case since it appears as if "overflow is set".

000.FFFFFFFFFFFFF / 3FE.FFFFFFFFFFFFE = 001.0000000000000

I think the following adds the check for my case and still works for the
issue your commit was trying to resolve.  Take a look any and all comments
are welcome since this code is pretty complicated:

- k

diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h
index cc1ec39..bc50aa0 100644
--- a/include/math-emu/op-common.h
+++ b/include/math-emu/op-common.h
@@ -139,18 +139,27 @@ do {								\
 	if (X##_e <= _FP_WFRACBITS_##fs)			\
 	  {							\
 	    _FP_FRAC_SRS_##wc(X, X##_e, _FP_WFRACBITS_##fs);	\
-	    _FP_ROUND(wc, X);					\
 	    if (_FP_FRAC_HIGH_##fs(X)				\
 		& (_FP_OVERFLOW_##fs >> 1))			\
 	      {							\
 	        X##_e = 1;					\
 	        _FP_FRAC_SET_##wc(X, _FP_ZEROFRAC_##wc);	\
-	        FP_SET_EXCEPTION(FP_EX_INEXACT);		\
 	      }							\
 	    else						\
 	      {							\
-		X##_e = 0;					\
-		_FP_FRAC_SRL_##wc(X, _FP_WORKBITS);		\
+		_FP_ROUND(wc, X);				\
+		if (_FP_FRAC_HIGH_##fs(X)			\
+		   & (_FP_OVERFLOW_##fs >> 1))			\
+		  {						\
+		    X##_e = 1;					\
+		    _FP_FRAC_SET_##wc(X, _FP_ZEROFRAC_##wc);	\
+		    FP_SET_EXCEPTION(FP_EX_INEXACT);		\
+		  }						\
+		else						\
+		  {						\
+		    X##_e = 0;					\
+		    _FP_FRAC_SRL_##wc(X, _FP_WORKBITS);		\
+		  }						\
 	      }							\
 	    if ((FP_CUR_EXCEPTIONS & FP_EX_INEXACT) ||		\
 		(FP_TRAPPING_EXCEPTIONS & FP_EX_UNDERFLOW))	\

Re: math-emu issue with fp divide

[David Miller]

From: Kumar Gala <galak@kernel.crashing.org>
Date: Thu, 5 Jun 2008 08:38:44 -0500 (CDT)

> Now that I'm digging into this a bit I'm thinking my issue has to do with
> the fix you put in place from back in Aug 2007 (commit
> 405849610fd96b4f34cd1875c4c033228fea6c0f):
> 
> [MATH-EMU]: Fix underflow exception reporting.
> 
>     2) we ended up rounding back up to normal (this is the case where
>        we set the exponent to 1 and set the fraction to zero), this
>        should set inexact too
> ...
> 
>     Another example, "0x0.0000000000001p-1022 / 16.0", should signal both
>     inexact and underflow.  The cpu implementations and ieee1754
>     literature is very clear about this.  This is case #2 above.
> 
> I'm not clear from your commit comment on what actual number
> 0x0.0....01p-1022 is?

I haven't been able to look closely at this yet but I think I
happened to stumble over the test case that lead me to that
changeset you are referencing here.

The "actual number" is exactly as listed "0x0.0000000000001p-1022",
I don't know what's so confusing about it :-)))

I think this was distilled by Jakub Jelinek from some glibc test case.

#include <float.h>
#include <fenv.h>
#include <stdio.h>

volatile double d = DBL_MIN;
volatile double e = 0x0.0000000000001p-1022;
volatile double f = 16.0;
int
main (void)
{
  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
  d /= f;
  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
  e /= f;
  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
  return 0;
}

Re: math-emu issue with fp divide

[Kumar Gala]

On Jun 12, 2008, at 11:24 PM, David Miller wrote:

> From: Kumar Gala <galak@kernel.crashing.org>
> Date: Thu, 5 Jun 2008 08:38:44 -0500 (CDT)
>
>> Now that I'm digging into this a bit I'm thinking my issue has to  
>> do with
>> the fix you put in place from back in Aug 2007 (commit
>> 405849610fd96b4f34cd1875c4c033228fea6c0f):
>>
>> [MATH-EMU]: Fix underflow exception reporting.
>>
>>    2) we ended up rounding back up to normal (this is the case where
>>       we set the exponent to 1 and set the fraction to zero), this
>>       should set inexact too
>> ...
>>
>>    Another example, "0x0.0000000000001p-1022 / 16.0", should signal  
>> both
>>    inexact and underflow.  The cpu implementations and ieee1754
>>    literature is very clear about this.  This is case #2 above.
>>
>> I'm not clear from your commit comment on what actual number
>> 0x0.0....01p-1022 is?
>
> I haven't been able to look closely at this yet but I think I
> happened to stumble over the test case that lead me to that
> changeset you are referencing here.
>
> The "actual number" is exactly as listed "0x0.0000000000001p-1022",
> I don't know what's so confusing about it :-)))

I don't think I've ever seen the notation before :)

> I think this was distilled by Jakub Jelinek from some glibc test case.
>
> #include <float.h>
> #include <fenv.h>
> #include <stdio.h>
>
> volatile double d = DBL_MIN;
> volatile double e = 0x0.0000000000001p-1022;
> volatile double f = 16.0;
> int
> main (void)
> {
>  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
>  d /= f;
>  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
>  e /= f;
>  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
>  return 0;
> }

Cool, I'll try this out and see what it does on PPC HW and w/my  
current EMU.  I'll also see if I can work up a test case to show the  
issue I've set a patch for.

- k

Re: math-emu issue with fp divide

[Kumar Gala]

On Jun 12, 2008, at 11:24 PM, David Miller wrote:

> From: Kumar Gala <galak@kernel.crashing.org>
> Date: Thu, 5 Jun 2008 08:38:44 -0500 (CDT)
>
>> Now that I'm digging into this a bit I'm thinking my issue has to  
>> do with
>> the fix you put in place from back in Aug 2007 (commit
>> 405849610fd96b4f34cd1875c4c033228fea6c0f):
>>
>> [MATH-EMU]: Fix underflow exception reporting.
>>
>>    2) we ended up rounding back up to normal (this is the case where
>>       we set the exponent to 1 and set the fraction to zero), this
>>       should set inexact too
>> ...
>>
>>    Another example, "0x0.0000000000001p-1022 / 16.0", should signal  
>> both
>>    inexact and underflow.  The cpu implementations and ieee1754
>>    literature is very clear about this.  This is case #2 above.
>>
>> I'm not clear from your commit comment on what actual number
>> 0x0.0....01p-1022 is?
>
> I haven't been able to look closely at this yet but I think I
> happened to stumble over the test case that lead me to that
> changeset you are referencing here.

Have you had a chance to look at this?

- k

Re: math-emu issue with fp divide

[David Miller]

From: Kumar Gala <galak@kernel.crashing.org>
Date: Fri, 27 Jun 2008 08:48:33 -0500

> Have you had a chance to look at this?

Nope.  I have some networking bits I want to get into 2.6.27 and
that's likely to eat me up for at least another week.

You really shouldn't depend upon me if you need to make forward
progress on this any time soon, sorry :-/

Re: math-emu issue with fp divide

[Kumar Gala]

On Jun 12, 2008, at 11:24 PM, David Miller wrote:

> From: Kumar Gala <galak@kernel.crashing.org>
> Date: Thu, 5 Jun 2008 08:38:44 -0500 (CDT)
>
>> Now that I'm digging into this a bit I'm thinking my issue has to  
>> do with
>> the fix you put in place from back in Aug 2007 (commit
>> 405849610fd96b4f34cd1875c4c033228fea6c0f):
>>
>> [MATH-EMU]: Fix underflow exception reporting.
>>
>>    2) we ended up rounding back up to normal (this is the case where
>>       we set the exponent to 1 and set the fraction to zero), this
>>       should set inexact too
>> ...
>>
>>    Another example, "0x0.0000000000001p-1022 / 16.0", should signal  
>> both
>>    inexact and underflow.  The cpu implementations and ieee1754
>>    literature is very clear about this.  This is case #2 above.
>>
>> I'm not clear from your commit comment on what actual number
>> 0x0.0....01p-1022 is?
>
> I haven't been able to look closely at this yet but I think I
> happened to stumble over the test case that lead me to that
> changeset you are referencing here.
>
> The "actual number" is exactly as listed "0x0.0000000000001p-1022",
> I don't know what's so confusing about it :-)))
>
> I think this was distilled by Jakub Jelinek from some glibc test case.
>
> #include <float.h>
> #include <fenv.h>
> #include <stdio.h>
>
> volatile double d = DBL_MIN;
> volatile double e = 0x0.0000000000001p-1022;
> volatile double f = 16.0;
> int
> main (void)
> {
>  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
>  d /= f;
>  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
>  e /= f;
>  printf ("%x\n", fetestexcept (FE_UNDERFLOW));
>  return 0;
> }

I tested this on PPC HW fp and kern emu w/my patch and get the same  
results:

0
0
8000000

(8000000 is underflow in the PPC FP status register)

- k

Re: math-emu issue with fp divide

[David Miller]

From: Kumar Gala <galak@kernel.crashing.org>
Date: Fri, 27 Jun 2008 09:15:40 -0500

> I tested this on PPC HW fp and kern emu w/my patch and get the same results:
> 
> 0
> 0
> 8000000
> 
> (8000000 is underflow in the PPC FP status register)

I did verifications of this on sparc64 as well with similar correct
results.

I'll merge your fix on the kernel side via the sparc tree, as I've
wasted enough of your time as it is :-)

Sorry for taking so long to get to this.