Fix sysdeps/ieee754 pow handling of sNaN arguments (bug 20916) [committed]

Various pow function implementations mishandle sNaN arguments in
various ways.  This includes returning sNaN instead of qNaN for sNaN
arguments.  For arguments (1, sNaN) and (sNaN, 0), TS 18661-1
semantics are also that the result should be qNaN, whereas with a qNaN
argument there the result should be 1, but for the dbl-64
implementation of pow there are issues with sNaN arguments beyond not
implementing the TS 18661-1 semantics in those special cases.

This patch makes the implementations in sysdeps/ieee754 follow the TS
18661-1 semantics consistently.  Because x86 / x86_64 implementations
still need fixing, testcases are not included with this patch; they
will be included with the fix for the x86 / x86_64 versions.

Tested for x86_64, x86, mips64 and powerpc (with such testcases, which
pass in the mips64 and powerpc cases).  Committed.

2016-12-02  Joseph Myers  <joseph@codesourcery.com>

	[BZ #20916]
	* sysdeps/ieee754/dbl-64/e_pow.c (__ieee754_pow): Do not return 1
	for arguments (sNaN, 0) or (1, sNaN).  Do arithmetic on NaN
	arguments to compute result.
	* sysdeps/ieee754/flt-32/e_powf.c (__ieee754_powf): Do not return
	1 for arguments (sNaN, 0) or (1, sNaN).
	* sysdeps/ieee754/ldbl-128/e_powl.c (__ieee754_powl): Likewise.
	* sysdeps/ieee754/ldbl-128ibm/e_powl.c (__ieee754_powl): Likewise.

On 12/03/2016 12:21 AM, Joseph Myers wrote:
> Various pow function implementations mishandle sNaN arguments in
> various ways.  This includes returning sNaN instead of qNaN for sNaN
> arguments.  For arguments (1, sNaN) and (sNaN, 0), TS 18661-1
> semantics are also that the result should be qNaN, whereas with a qNaN
> argument there the result should be 1, but for the dbl-64
> implementation of pow there are issues with sNaN arguments beyond not
> implementing the TS 18661-1 semantics in those special cases.
>
> This patch makes the implementations in sysdeps/ieee754 follow the TS
> 18661-1 semantics consistently.  Because x86 / x86_64 implementations
> still need fixing, testcases are not included with this patch; they
> will be included with the fix for the x86 / x86_64 versions.
>
> Tested for x86_64, x86, mips64 and powerpc (with such testcases, which
> pass in the mips64 and powerpc cases).  Committed.

Hi Joseph,
on s390, I get test fails in math/test-double / math/test-idouble:
testing double (without inline functions)
Failure: Test: pow (1, sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN
Failure: Test: pow (1, -sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN
Failure: Test: pow_downward (1, sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN
Failure: Test: pow_downward (1, -sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN
Failure: Test: pow_towardzero (1, sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN
Failure: Test: pow_towardzero (1, -sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN
Failure: Test: pow_upward (1, sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN
Failure: Test: pow_upward (1, -sNaN)
Result:
  is:          1.0000000000000000e+00   0x1.0000000000000p+0
  should be:  qNaN

Test suite completed:
   97258 test cases plus 86314 tests for exception flags and
     86314 tests for errno executed.
   8 errors occurred.


I've debugged in sysdeps/ieee754/dbl-64/e_pow.c and recognized that sNaN 
is converted to qNaN in line 85:
       if (y == 0)
	return 1.0;
This comparison is done with a load-and-test instruction from and to the 
same register, which results in a qNaN.
This value is passed to issignaling (y) in line 148:
   if (qy >= 0x7ff00000 && (qy > 0x7ff00000 || v.i[LOW_HALF] != 0))	/* 
NaN */
     return x == 1.0 && !issignaling (y) ? 1.0 : y + y;


 From ieee 754-2008 "6.2 Operations with NaNs":
"Under default exception handling, any operation signaling an invalid 
operation exception and for which a floating-point result is to be 
delivered shall deliver a quiet NaN."

As the used load-and-test instruction delivers a result, I think qNaN is 
correct. But is the compiler allowed to use this instruction for a 
comparision against zero?

How can I get rid of this issue?
Shall I use "if ((v.i[HIGH_HALF] & 0x7fffffff) == 0x0)" on s390?

Bye
Stefan

On Mon, 12 Dec 2016, Stefan Liebler wrote:

> I've debugged in sysdeps/ieee754/dbl-64/e_pow.c and recognized that sNaN is
> converted to qNaN in line 85:
>       if (y == 0)
> 	return 1.0;
> This comparison is done with a load-and-test instruction from and to the same
> register, which results in a qNaN.
> This value is passed to issignaling (y) in line 148:
>   if (qy >= 0x7ff00000 && (qy > 0x7ff00000 || v.i[LOW_HALF] != 0))	/* NaN
> */
>     return x == 1.0 && !issignaling (y) ? 1.0 : y + y;
> 
> 
> From ieee 754-2008 "6.2 Operations with NaNs":
> "Under default exception handling, any operation signaling an invalid
> operation exception and for which a floating-point result is to be delivered
> shall deliver a quiet NaN."
> 
> As the used load-and-test instruction delivers a result, I think qNaN is
> correct. But is the compiler allowed to use this instruction for a comparision
> against zero?

TS 18661-1 allows for floating-point assignments and argument passing 
(etc.) to act as convertFormat operations, so converting signaling NaNs to 
quiet.

Obviously when implemented that way, issignaling cannot work reliably.  As 
a quality-of-implementation issue it's probably best to avoid such 
instructions for simple loads of stored values, at least when 
-fsignaling-nans is in use.

So if -fsignaling-nans avoids the issue, compiling pow with 
-fsignaling-nans on s390 would make sense (most of libm isn't built with 
-fsignaling-nans and should still work fine).  Otherwise the tests of 
sNaNs can be disabled in a math-tests.h file for your architecture (like 
sysdeps/i386/fpu/math-tests.h), which should have a comment pointing to a 
bug report in GCC Bugzilla about the issue.

On Tue, Dec 13, 2016 at 09:42:14PM +0000, Joseph Myers wrote:
> On Mon, 12 Dec 2016, Stefan Liebler wrote:
> 
> > I've debugged in sysdeps/ieee754/dbl-64/e_pow.c and recognized that sNaN is
> > converted to qNaN in line 85:
> >       if (y == 0)
> > 	return 1.0;
> > This comparison is done with a load-and-test instruction from and to the same
> > register, which results in a qNaN.
> > This value is passed to issignaling (y) in line 148:
> >   if (qy >= 0x7ff00000 && (qy > 0x7ff00000 || v.i[LOW_HALF] != 0))	/* NaN
> > */
> >     return x == 1.0 && !issignaling (y) ? 1.0 : y + y;
> > 
> > 
> > From ieee 754-2008 "6.2 Operations with NaNs":
> > "Under default exception handling, any operation signaling an invalid
> > operation exception and for which a floating-point result is to be delivered
> > shall deliver a quiet NaN."
> > 
> > As the used load-and-test instruction delivers a result, I think qNaN is
> > correct. But is the compiler allowed to use this instruction for a comparision
> > against zero?
> 
> TS 18661-1 allows for floating-point assignments and argument passing 
> (etc.) to act as convertFormat operations, so converting signaling NaNs to 
> quiet.
> 
> Obviously when implemented that way, issignaling cannot work reliably.  As 
> a quality-of-implementation issue it's probably best to avoid such 
> instructions for simple loads of stored values, at least when 
> -fsignaling-nans is in use.
> 
> So if -fsignaling-nans avoids the issue, compiling pow with 
> -fsignaling-nans on s390 would make sense (most of libm isn't built with 
> -fsignaling-nans and should still work fine).  Otherwise the tests of 
> sNaNs can be disabled in a math-tests.h file for your architecture (like 
> sysdeps/i386/fpu/math-tests.h), which should have a comment pointing to a 
> bug report in GCC Bugzilla about the issue.

The problem with the load and test instruction is that it is not
consistent to doing a load and a compare separately.  Our floating
point loads do not quiet a NaN.

While the IEEE standard leaves it open to quiet NaNs with loads it is
probably supposed to be the same for all loads.

The plan is to disable the use of the FPR result of a load-and-test
instruction in the compiler.  We would leave the Glibc tests failing
since this appears to be a real finding to me.

Does this sound reasonable?

-Andreas-

On Wed, 14 Dec 2016, Andreas Krebbel wrote:

> The problem with the load and test instruction is that it is not
> consistent to doing a load and a compare separately.  Our floating
> point loads do not quiet a NaN.
> 
> While the IEEE standard leaves it open to quiet NaNs with loads it is
> probably supposed to be the same for all loads.
> 
> The plan is to disable the use of the FPR result of a load-and-test
> instruction in the compiler.  We would leave the Glibc tests failing
> since this appears to be a real finding to me.
> 
> Does this sound reasonable?

It sounds reasonable, given the quality-of-implementation goal to allow 
signaling NaNs to be copied without quieting.  If using that FPR result is 
useful for optimization, and harmless in the absence of signaling NaNs, of 
course you could restrict the disabling to the -fsignaling-nans case (and 
arrange for pow to be built with that option in glibc for s390).