[Bug libgcc/67624] New: arm/fp16.c __gnu_f2h_internal has wrong pattern for INF/NAN

[Bug libgcc/67624] New: arm/fp16.c __gnu_f2h_internal has wrong pattern for INF/NAN

[pekka.jaaskelainen at parmance dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67624

            Bug ID: 67624
           Summary: arm/fp16.c __gnu_f2h_internal has wrong pattern for
                    INF/NAN
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: libgcc
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pekka.jaaskelainen at parmance dot com
  Target Milestone: ---

I believe the f2h conversion routine uses wrong mask for INF/NAN in the early
shortcut. It seems to be the case also in the gcc HEAD.

This should fix it:

--- arm/fp16.c  2015-09-18 15:53:51.069011932 +0300
+++ fp16.c      2015-09-18 15:50:50.653013393 +0300
@@ -35,7 +35,7 @@
     {
       if (!ieee)
        return sign;
-      return sign | 0x7e00 | (mantissa >> 13);
+      return sign | 0x7c00 | (mantissa >> 13);
     }

   if (aexp == 0 && mantissa == 0)

[Bug libgcc/67624] arm/fp16.c __gnu_f2h_internal has wrong pattern for INF/NAN

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67624

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-09-18
     Ever confirmed|0                           |1

--- Comment #1 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
Yes, the existing code looks wrong in that infinity gets converted to NaN.

Your patch, however, is not right either, since it can convert some NaNs (where
only the bottom 13 bits are non-zero) to infinity.

I'd need to think a bit more on what the exact conversion needs to be in these
cases.

[Bug libgcc/67624] arm/fp16.c __gnu_f2h_internal has wrong pattern for INF/NAN

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67624

--- Comment #2 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
All NaNs should have the top mantissa bit set in the result of the 
conversion (i.e. the result of the conversion should always be a quiet 
NaN, not a signaling NaN) - setting that bit also ensures the result is 
not an infinity.  Then I think the remaining bits in the mantissa of the 
result should match the corresponding high bits in the mantissa of the 
source (which appears to be what the hardware conversion instructions are 
documented to do).

[Bug libgcc/67624] arm/fp16.c __gnu_f2h_internal has wrong pattern for INF/NAN

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67624

--- Comment #3 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
Author: rearnsha
Date: Thu Sep 24 09:40:06 2015
New Revision: 228082

URL: https://gcc.gnu.org/viewcvs?rev=228082&root=gcc&view=rev
Log:
ARM: fp16 Fix PR 67624 - Incorrect conversion of float Infinity to __fp16

        PR libgcc/67624
        libgcc:
        * config/arm/fp16.c (__gnu_f2h_internal): Handle infinity correctly.
        gcc/testsuite:
        * gcc.target/arm/fp16-inf.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/arm/fp16-inf.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/libgcc/ChangeLog
    trunk/libgcc/config/arm/fp16.c

[Bug libgcc/67624] arm/fp16.c __gnu_f2h_internal has wrong pattern for INF/NAN

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67624

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
Fixed.