[PATCH] Fix truncf for sNaN input

[PATCH] Fix truncf for sNaN input

[Fabian Schriever]

Make line 47 in sf_trunc.c reachable. While converting the double
precision function trunc to the single precision version truncf an error
was introduced into the special case. This special case is meant to
catch both NaNs and infinities, however qNaNs and infinities work just
fine with the simple return of x (line 51). The only error occurs for
sNaNs where the same sNaN is returned and no invalid exception is
raised.
---
 newlib/libm/common/sf_trunc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/newlib/libm/common/sf_trunc.c b/newlib/libm/common/sf_trunc.c
index 74ea933ce..8eb0554d8 100644
--- a/newlib/libm/common/sf_trunc.c
+++ b/newlib/libm/common/sf_trunc.c
@@ -42,7 +42,7 @@
     }
   else
     {
-      if (exponent_less_127 == 255)
+      if (exponent_less_127 == 128)
         /* x is NaN or infinite. */
         return x + x;
 
-- 
2.24.1.windows.2

Re: [PATCH] Fix truncf for sNaN input

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

On Mar 11 10:58, Fabian Schriever wrote:
> Make line 47 in sf_trunc.c reachable. While converting the double
> precision function trunc to the single precision version truncf an error
> was introduced into the special case. This special case is meant to
> catch both NaNs and infinities, however qNaNs and infinities work just
> fine with the simple return of x (line 51). The only error occurs for
> sNaNs where the same sNaN is returned and no invalid exception is
> raised.
> ---
>  newlib/libm/common/sf_trunc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/newlib/libm/common/sf_trunc.c b/newlib/libm/common/sf_trunc.c
> index 74ea933ce..8eb0554d8 100644
> --- a/newlib/libm/common/sf_trunc.c
> +++ b/newlib/libm/common/sf_trunc.c
> @@ -42,7 +42,7 @@
>      }
>    else
>      {
> -      if (exponent_less_127 == 255)
> +      if (exponent_less_127 == 128)
>          /* x is NaN or infinite. */
>          return x + x;
>  
> -- 
> 2.24.1.windows.2
> 

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Fix truncf for sNaN input

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

Fabian Schriever <fabian.schriever@gtd-gmbh.de> writes:

> Make line 47 in sf_trunc.c reachable. While converting the double
> precision function trunc to the single precision version truncf an error
> was introduced into the special case. This special case is meant to
> catch both NaNs and infinities, however qNaNs and infinities work just
> fine with the simple return of x (line 51). The only error occurs for
> sNaNs where the same sNaN is returned and no invalid exception is
> raised.

I think this isn't right. According to the trunc/truncf man page:

       If x is integral, infinite, or NaN, x itself is returned.

I think this means that we should just return x in these cases, and not
raise an exception?

The same appears to be true for the other similar functions, including
floor, ceil, round, rint and nearbyint.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] Fix truncf for sNaN input

[Joseph Myers]

On Wed, 11 Mar 2020, Keith Packard via Newlib wrote:

> I think this isn't right. According to the trunc/truncf man page:
> 
>        If x is integral, infinite, or NaN, x itself is returned.
> 
> I think this means that we should just return x in these cases, and not
> raise an exception?

That manpage is wrong.  The underlying IEEE operation should raise 
"invalid" and return a qNaN for an sNaN operand (but never raises 
"inexact").

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix truncf for sNaN input

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

Joseph Myers <joseph@codesourcery.com> writes:

> That manpage is wrong.  The underlying IEEE operation should raise 
> "invalid" and return a qNaN for an sNaN operand (but never raises 
> "inexact").

Hrm. IEEE 754 says that you only get "invalid" if the NaN or infinite
operand cannot be represented in the destination format. As these
functions return the same type as their operand, NaN and inf values can
be represented in the return value.

Are you referring to some other standard?

Also, glibc works the way the manual says, and I'd really like newlib
and glibc to have the same general behavior, even if newlib isn't quite
as accurate as glibc for some operations.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] Fix truncf for sNaN input

[Joseph Myers]

On Wed, 11 Mar 2020, Keith Packard via Newlib wrote:

> Joseph Myers <joseph@codesourcery.com> writes:
> 
> > That manpage is wrong.  The underlying IEEE operation should raise 
> > "invalid" and return a qNaN for an sNaN operand (but never raises 
> > "inexact").
> 
> Hrm. IEEE 754 says that you only get "invalid" if the NaN or infinite
> operand cannot be represented in the destination format. As these
> functions return the same type as their operand, NaN and inf values can
> be represented in the return value.
> 
> Are you referring to some other standard?

I'm referring to 6.2 Operations with NaNs, "Signaling NaNs shall be 
reserved operands that signal the invalid operation exception (see 7.2) 
for every general-computational and signaling-computational operation 
except for the conversions described in 5.12.".

> Also, glibc works the way the manual says, and I'd really like newlib
> and glibc to have the same general behavior, even if newlib isn't quite
> as accurate as glibc for some operations.

glibc's libm-test-trunc.inc explicitly verifies that sNaN inputs to trunc 
functions result in a qNaN output with "invalid" but not "inexact" raised.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix truncf for sNaN input

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

Joseph Myers <joseph@codesourcery.com> writes:

> I'm referring to 6.2 Operations with NaNs, "Signaling NaNs shall be 
> reserved operands that signal the invalid operation exception (see 7.2) 
> for every general-computational and signaling-computational operation 
> except for the conversions described in 5.12.".

I believe you are concerned with the exception behavior, where the
original code was wrong. Section 5.9 says that conversion functions
signal for sNaN input, while 6.2 says that they deliver a qNaN result.

I was concerned with what NaN values are delivered from a NaN
operand, given the POSIX specification which says that the operand
itself shall be delivered if it is a NaN.

I hadn't considered the exception behavior, nor had I realized that sNaN
operands cause a qNaN to be delivered.

I need to interpret the POSIX spec as saying that a NaN operand will
deliver *some* NaN, but not that it will deliver the NaN operand. The
functions cannot do this in the case of sNaN operands which must deliver
a qNaN according to the IEEE spec.

I'll update the newlib tests to verify that sNaN parameters deliver qNaN
results, and to allow any qNaN value for a qNaN operand, instead of
requiring that the functions return precisely the same qNaN value. Those
tests don't currently encode the signal behavior; I'll have to add
support for that at some point.

Thanks much for your clarifications and pointers to the relevant specs.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] Fix truncf for sNaN input

[Joseph Myers]

On Mon, 16 Mar 2020, Keith Packard via Newlib wrote:

> I was concerned with what NaN values are delivered from a NaN
> operand, given the POSIX specification which says that the operand
> itself shall be delivered if it is a NaN.

I don't think POSIX specifications should be trusted for how functions 
should behave with sNaN inputs.  Rather, prefer the specifications from TS 
18661-1 (or the latest C2x draft which has TS 18661-1 integrated).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix truncf for sNaN input

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

Joseph Myers <joseph@codesourcery.com> writes:

> I don't think POSIX specifications should be trusted for how functions 
> should behave with sNaN inputs.  Rather, prefer the specifications from TS 
> 18661-1 (or the latest C2x draft which has TS 18661-1 integrated).

From my reading of that, I should only check for signaling vs quiet NaN,
ensuring that values are the right kind of NaN, but not any specific
value of NaN.

I've adjusted the newlib tests like this:

  /* All signaling nans are the "same", as are all quiet nans */
  if (isnan(correct.value) && isnan(is)
      && (issignaling(correct.value) == issignaling(is)))
  {
	/* PASS */
  }

Does this seem correct?

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] Fix truncf for sNaN input

[Fabian Schriever]

Am 17/03/2020 um 02:30 schrieb Joseph Myers:
> On Mon, 16 Mar 2020, Keith Packard via Newlib wrote:
>
>> I was concerned with what NaN values are delivered from a NaN
>> operand, given the POSIX specification which says that the operand
>> itself shall be delivered if it is a NaN.
> I don't think POSIX specifications should be trusted for how functions
> should behave with sNaN inputs.  Rather, prefer the specifications from TS
> 18661-1 (or the latest C2x draft which has TS 18661-1 integrated).
>
I agree that both the IEEE-754 and C standards should take precedence 
over POSIX, nonetheless POSIX does contain a chapter on the treatment of 
NaNs including signaling NaNs: §4.20 (I only have access to the POSIX 
draft 3 from 2007: http://www.open-std.org/jtc1/sc22/open/n4217.pdf). It 
contains the following:

On implementations that support the IEC60559: 1989 standard floating 
point, functions with signaling NaN argument(s) shall be treated as if 
the function were called with an argument that is a required domain 
error and shall return a quiet NaN result, except where stated otherwise.
Note:    The function might never see the signaling NaN, since it might 
trigger when the arguments are evaluated during the function call.
On implementations that support the IEC60559: 1989standard floating 
point, for those functions that do not have a documented domain error, 
the following shall apply:
     These functions shall fail if:
     Domain Error   Any argument is a signaling NaN.
     Either, the integer expression (math_errhandling & MATH_ERRNO) is 
non-zero and errno shall be set to [EDOM], or the integer expression 
(math_errhandling &MATH_ERREXCEPT) is non-zeroand the invalid 
floating-point exception shall be raised.

For this change I only sought to have the single-precision function 
perform the same as it's double counterpart for now. I have not checked 
whether the POSIX requirements regarding errno are met.

Re: [PATCH] Fix truncf for sNaN input

[Brian Inglis]

On 2020-03-17 09:26, Fabian Schriever wrote:
> Am 17/03/2020 um 02:30 schrieb Joseph Myers:
>> On Mon, 16 Mar 2020, Keith Packard via Newlib wrote:
>>> I was concerned with what NaN values are delivered from a NaN
>>> operand, given the POSIX specification which says that the operand
>>> itself shall be delivered if it is a NaN.
>> I don't think POSIX specifications should be trusted for how functions
>> should behave with sNaN inputs.  Rather, prefer the specifications from TS
>> 18661-1 (or the latest C2x draft which has TS 18661-1 integrated).

> I agree that both the IEEE-754 and C standards should take precedence over 
> POSIX, nonetheless POSIX does contain a chapter on the treatment of NaNs 
> including signaling NaNs: §4.20 (I only have access to the POSIX draft 3
> from 2007: http://www.open-std.org/jtc1/sc22/open/n4217.pdf). It contains
> the following:

Latest Volume 1. "Base Definitions" Chapter 4. "General Concepts" 4.21
"Treatment of NaN Arguments for the Mathematical Functions" available at:

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_21

[free registration allows continuing access and downloads with minimal spam]

Unchanged:

> "On implementations that support the IEC 60559:1989 standard floating point, 
> functions with signaling NaN argument(s) shall be treated as if the function 
> were called with an argument that is a required domain error and shall return
> a quiet NaN result, except where stated otherwise.
> Note:
>      The function might never see the signaling NaN, since it might trigger
> when the arguments are evaluated during the function call.
> On implementations that support the IEC 60559:1989 standard floating point,
> for those functions that do not have a documented domain error, the following
> shall apply:
>     These functions shall fail if:
>     Domain Error   Any argument is a signaling NaN.
>     Either, the integer expression (math_errhandling & MATH_ERRNO) is non-zero
> and errno shall be set to [EDOM], or the integer expression (math_errhandling
> & MATH_ERREXCEPT) is non-zero and the invalid floating-point exception shall
> be raised."

Section 4.20 "Treatment of Error Conditions for Mathematical Functions" may also
apply.

> For this change I only sought to have the single-precision function perform the
> same as it's double counterpart for now. I have not checked whether the POSIX
> requirements regarding errno are met.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.

Re: [PATCH] Fix truncf for sNaN input

[Joseph Myers]

On Mon, 16 Mar 2020, Keith Packard via Newlib wrote:

> Joseph Myers <joseph@codesourcery.com> writes:
> 
> > I don't think POSIX specifications should be trusted for how functions 
> > should behave with sNaN inputs.  Rather, prefer the specifications from TS 
> > 18661-1 (or the latest C2x draft which has TS 18661-1 integrated).
> 
> From my reading of that, I should only check for signaling vs quiet NaN,
> ensuring that values are the right kind of NaN, but not any specific
> value of NaN.
> 
> I've adjusted the newlib tests like this:
> 
>   /* All signaling nans are the "same", as are all quiet nans */
>   if (isnan(correct.value) && isnan(is)
>       && (issignaling(correct.value) == issignaling(is)))
>   {
> 	/* PASS */
>   }
> 
> Does this seem correct?

Yes.  Only a few functions such as fabs and copysign are expected to 
preserve NaN payloads (and those functions also preserve whether the NaN 
is signaling and never raise exceptions).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix truncf for sNaN input

[Joseph Myers]

On Tue, 17 Mar 2020, Fabian Schriever wrote:

> On implementations that support the IEC60559: 1989 standard floating point,
> functions with signaling NaN argument(s) shall be treated as if the function
> were called with an argument that is a required domain error and shall return
> a quiet NaN result, except where stated otherwise.

This fails to allow for functions such as fabs where the corresponding 
IEEE operations pass through a signaling NaN rather than returning a quiet 
NaN with "invalid" raised.

Furthermore, TS 18661-1 makes it implementation-defined whether there is a 
domain error.  And in practice it's more convenient for implementations 
not to treat it as a domain error (not to set errno) as that means they 
can just e.g. add a NaN argument to itself rather than rather than 
explicitly testing for a signaling NaN.

-- 
Joseph S. Myers
joseph@codesourcery.com