[PATCH] tile: create new math-tests.h header

[PATCH] tile: create new math-tests.h header

[Chris Metcalf]

The header makes tile use the new mechanisms for suppressing
exception and rounding support (the ROUNDING_TESTS_xxx() and
EXCEPTION_TESTS_xxx macros).  More importantly, it also now sets
SNAN_TESTS_PRESERVE_PAYLOAD to 0, since the tile soft float does
not preserve NaN payloads.
---
This change fixes the math/test-{,i}{double,float} failures.

2016-11-16  Chris Metcalf  <cmetcalf@mellanox.com>

	* sysdeps/tile/math-tests.h: New file.

 sysdeps/tile/math-tests.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 sysdeps/tile/math-tests.h

diff --git a/sysdeps/tile/math-tests.h b/sysdeps/tile/math-tests.h
new file mode 100644
index 000000000000..eeade64d54da
--- /dev/null
+++ b/sysdeps/tile/math-tests.h
@@ -0,0 +1,31 @@
+/* Configuration for math tests.  Tile version.
+   Copyright (C) 2013-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Tile soft float does not support exceptions and rounding modes.  */
+#define ROUNDING_TESTS_float(MODE)	((MODE) == FE_TONEAREST)
+#define ROUNDING_TESTS_double(MODE)	((MODE) == FE_TONEAREST)
+#define ROUNDING_TESTS_long_double(MODE)	((MODE) == FE_TONEAREST)
+#define EXCEPTION_TESTS_float	0
+#define EXCEPTION_TESTS_double	0
+#define EXCEPTION_TESTS_long_double	0
+
+/* Tile soft float does not preserve NaN payloads when converting a
+   signaling NaN to quiet.  */
+#define SNAN_TESTS_PRESERVE_PAYLOAD	0
+
+#include_next <math-tests.h>
-- 
2.7.2

Re: [PATCH] tile: create new math-tests.h header

[Joseph Myers]

On Wed, 16 Nov 2016, Chris Metcalf wrote:

> The header makes tile use the new mechanisms for suppressing
> exception and rounding support (the ROUNDING_TESTS_xxx() and
> EXCEPTION_TESTS_xxx macros).  More importantly, it also now sets
> SNAN_TESTS_PRESERVE_PAYLOAD to 0, since the tile soft float does
> not preserve NaN payloads.

All the tile sfp-machine.h files in libgcc seem to define _FP_KEEPNANFRACP 
to 1, so not preserving NaN payloads seems odd here.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] tile: create new math-tests.h header

[Chris Metcalf]

On 11/16/2016 5:51 PM, Joseph Myers wrote:
> On Wed, 16 Nov 2016, Chris Metcalf wrote:
>
>> The header makes tile use the new mechanisms for suppressing
>> exception and rounding support (the ROUNDING_TESTS_xxx() and
>> EXCEPTION_TESTS_xxx macros).  More importantly, it also now sets
>> SNAN_TESTS_PRESERVE_PAYLOAD to 0, since the tile soft float does
>> not preserve NaN payloads.
> All the tile sfp-machine.h files in libgcc seem to define _FP_KEEPNANFRACP
> to 1, so not preserving NaN payloads seems odd here.

So perhaps it's some kind of testing bug, or I'm otherwise
confused.  The following small program seems to suggest
that the payload is copied in canonicalize.  I built it naively just
by compiling it with my system headers but linking statically
against "libc.a math/libm.a" in the glibc build tree.  It prints
out "0 nan nan 1 1" when run.

#include <stdio.h>
#include <math.h>

double getpayload (const double *x);
int canonicalize (double *__cx, const double *__x);

int main()
{
   const double nan = __builtin_nans("0x1");
   double nan_payload = getpayload(&nan);
   double cnan;
   int rc = canonicalize(&cnan, &nan);
   double cnan_payload = getpayload(&nan);

   printf("%d %a %a %g %g\n", rc, nan, cnan, nan_payload, cnan_payload);
   return 0;
}

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Re: [PATCH] tile: create new math-tests.h header

[Joseph Myers]

On Thu, 17 Nov 2016, Chris Metcalf wrote:

> > All the tile sfp-machine.h files in libgcc seem to define _FP_KEEPNANFRACP
> > to 1, so not preserving NaN payloads seems odd here.
> 
> So perhaps it's some kind of testing bug, or I'm otherwise
> confused.  The following small program seems to suggest

Indeed, you'll need to investigate why the test fails in more detail.  
(The rest of your math-tests.h generically makes sense for any system 
without exceptions / rounding modes support.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] tile: create new math-tests.h header

[Chris Metcalf]

On 11/17/2016 6:28 PM, Joseph Myers wrote:
> On Thu, 17 Nov 2016, Chris Metcalf wrote:
>
>>> All the tile sfp-machine.h files in libgcc seem to define _FP_KEEPNANFRACP
>>> to 1, so not preserving NaN payloads seems odd here.
>> So perhaps it's some kind of testing bug, or I'm otherwise
>> confused.  The following small program seems to suggest
> Indeed, you'll need to investigate why the test fails in more detail.
> (The rest of your math-tests.h generically makes sense for any system
> without exceptions / rounding modes support.)

I drilled into libm-test enough to create a single reproducer, which
then inspired me to go back to look at the little test program I
emailed yesterday, which turned out to have had a typo.  Oh well.

In any case, the hardware on tilegx appears not to preserve the
payload when you add nans of any kind, so we see this manifest in the
code for canonicalize(__builtin_nans) only because that's the only
case where we actually ask the hardware to do some math for us.

We clearly should also tell libgcc not to bother preserving nan
payloads given that the hardware doesn't do it in any case; I'll pass
that along to the compiler team, but I'll also commit my original
patch since it seems to match the hardware behavior.

Thanks!

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Re: [PATCH] tile: create new math-tests.h header

[Joseph Myers]

On Fri, 18 Nov 2016, Chris Metcalf wrote:

> We clearly should also tell libgcc not to bother preserving nan
> payloads given that the hardware doesn't do it in any case; I'll pass
> that along to the compiler team, but I'll also commit my original
> patch since it seems to match the hardware behavior.

FWIW, I wouldn't expect any significant performance difference between 
copying a payload and setting a payload to the canonical one (which is 
what you get if !_FP_KEEPNANFRACP).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] tile: create new math-tests.h header

[Chris Metcalf]

On 11/18/2016 1:11 PM, Joseph Myers wrote:
> On Fri, 18 Nov 2016, Chris Metcalf wrote:
>
>> We clearly should also tell libgcc not to bother preserving nan
>> payloads given that the hardware doesn't do it in any case; I'll pass
>> that along to the compiler team, but I'll also commit my original
>> patch since it seems to match the hardware behavior.
> FWIW, I wouldn't expect any significant performance difference between
> copying a payload and setting a payload to the canonical one (which is
> what you get if !_FP_KEEPNANFRACP).

My assumption was that we should tweak it for consistency, not for
performance.  Is your sense that we might as well keep the NaN
payloads in libgcc even if we don't with inline float operations?

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Re: [PATCH] tile: create new math-tests.h header

[Joseph Myers]

On Fri, 18 Nov 2016, Chris Metcalf wrote:

> On 11/18/2016 1:11 PM, Joseph Myers wrote:
> > On Fri, 18 Nov 2016, Chris Metcalf wrote:
> > 
> > > We clearly should also tell libgcc not to bother preserving nan
> > > payloads given that the hardware doesn't do it in any case; I'll pass
> > > that along to the compiler team, but I'll also commit my original
> > > patch since it seems to match the hardware behavior.
> > FWIW, I wouldn't expect any significant performance difference between
> > copying a payload and setting a payload to the canonical one (which is
> > what you get if !_FP_KEEPNANFRACP).
> 
> My assumption was that we should tweak it for consistency, not for
> performance.  Is your sense that we might as well keep the NaN
> payloads in libgcc even if we don't with inline float operations?

Consistency with hardware is logical (I think the reason for such hooks in 
soft-fp is to allows its use in Linux kernel floating-point emulation to 
behave the same as the hardware being emulated).  Note however that when 
soft-fp creates a default NaN it also gives it a default sign - if that's 
what you want for consistency with hardware, you'll need a new 
math-tests.h hook for these canonicalize tests to disregard the sign of 
the result as well as its payload.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] tile: create new math-tests.h header

[Chris Metcalf]

On 11/18/2016 4:18 PM, Joseph Myers wrote:
> On Fri, 18 Nov 2016, Chris Metcalf wrote:
>
>> On 11/18/2016 1:11 PM, Joseph Myers wrote:
>>> On Fri, 18 Nov 2016, Chris Metcalf wrote:
>>>
>>>> We clearly should also tell libgcc not to bother preserving nan
>>>> payloads given that the hardware doesn't do it in any case; I'll pass
>>>> that along to the compiler team, but I'll also commit my original
>>>> patch since it seems to match the hardware behavior.
>>> FWIW, I wouldn't expect any significant performance difference between
>>> copying a payload and setting a payload to the canonical one (which is
>>> what you get if !_FP_KEEPNANFRACP).
>> My assumption was that we should tweak it for consistency, not for
>> performance.  Is your sense that we might as well keep the NaN
>> payloads in libgcc even if we don't with inline float operations?
> Consistency with hardware is logical (I think the reason for such hooks in
> soft-fp is to allows its use in Linux kernel floating-point emulation to
> behave the same as the hardware being emulated).  Note however that when
> soft-fp creates a default NaN it also gives it a default sign - if that's
> what you want for consistency with hardware, you'll need a new
> math-tests.h hook for these canonicalize tests to disregard the sign of
> the result as well as its payload.

Good point, but, the hardware does preserve the sign of NaN's, so
just disabling the payload testing is sufficient for the tests to pass.

I think we may just leave the payload copying enabled in soft-fp
unless you think it's a bad choice; it seems harmless and arguably
might give us better results in some circumstances.

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Re: [PATCH] tile: create new math-tests.h header

[Joseph Myers]

On Mon, 21 Nov 2016, Chris Metcalf wrote:

> I think we may just leave the payload copying enabled in soft-fp
> unless you think it's a bad choice; it seems harmless and arguably
> might give us better results in some circumstances.

I think it's harmless to leave it as-is.

-- 
Joseph S. Myers
joseph@codesourcery.com