Bug 106579 - ieee_signaling_nan problem in fortran on powerpc64
Summary: ieee_signaling_nan problem in fortran on powerpc64
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-08-10 15:14 UTC by seurer
Modified: 2022-08-30 09:10 UTC (History)
4 users (show)

See Also:
Host:
Target: powerpc
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-08-11 00:00:00


Attachments
gcc13-builtin-issignaling.patch (3.70 KB, patch)
2022-08-11 16:22 UTC, Jakub Jelinek
Details | Diff
gcc13-builtin-issignaling.patch (5.89 KB, patch)
2022-08-11 20:35 UTC, Jakub Jelinek
Details | Diff
gcc13-builtin-issignaling.patch (6.88 KB, patch)
2022-08-12 11:19 UTC, Jakub Jelinek
Details | Diff
gcc13-pr106579.patch (2.02 KB, patch)
2022-08-12 17:15 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description seurer 2022-08-10 15:14:19 UTC
There is an issue with ieee_signaling_nan as used in fortran on powerpc64 when --with-long-double-format=ieee is used.

Extracted from gfortran.dg/ieee/signaling_3.f90:

program test
  use, intrinsic :: iso_c_binding
  use, intrinsic :: ieee_arithmetic
  implicit none

  real(kind=c_long_double) :: z

  if (ieee_support_nan(z)) then
    z = ieee_value(z, ieee_signaling_nan)
    if (ieee_class(z) /= ieee_signaling_nan) stop 100
  end if

end program test

seurer@fowler:~/gcc/git/build/gcc-12-test$ /home/seurer/gcc/git/build/gcc-12-test/gcc/testsuite/gfortran/../../gfortran -B/home/seurer/gcc/git/build/gcc-12-test/gcc/testsuite/gfortran/../../ -B/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libgfortran/ signaling_3a.f90 -fdiagnostics-plain-output -fdiagnostics-plain-output -O0 -pedantic-errors -fintrinsic-modules-path /home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libgfortran/ -fno-unsafe-math-optimizations -frounding-math -fsignaling-nans -B/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libgfortran/.libs -L/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libgfortran/.libs -L/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libgfortran/.libs -L/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libatomic/.libs -B/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libquadmath/.libs -L/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libquadmath/.libs -L/home/seurer/gcc/git/build/gcc-12-test/powerpc64le-unknown-linux-gnu/./libquadmath/.libs -lm -o ./signaling_3a.exe

seurer@fowler:~/gcc/git/build/gcc-12-test$ ./signaling_3a.exe 
STOP 100


This happens for both gcc 12 and 13 (trunk).

It occurs on powerpc64 LE with the compiler being tested configured with --with-long-double-format=ieee on Fedora 36 which is the first powerpc64 distro built with --with-long-double-format=ieee.  The distro compiler is gcc version 12.1.1 20220507 (Red Hat 12.1.1-1) (GCC).


Note the specific test case, signaling_3.f90, was added here but another test case signaling_2.f90 fails the same way.

commit e89d0befe3ec3238fca6de2cb078eb403b8c7e99 (HEAD)
Author: Francois-Xavier Coudert <fxcoudert@gcc.gnu.org>
Date:   Mon Jan 17 12:46:48 2022 +0100

    Fortran: provide a fallback implementation of issignaling
Comment 1 Jakub Jelinek 2022-08-10 17:51:17 UTC
Ouch, indeed.
Looking at ieee/ieee_exceptions.F90, I think that one is ok, while
__ieee_exceptions_MOD_ieee_support_flag_16
is exported from the library, all of __ieee_exceptions_MOD_ieee_support_flag_{4,8,16} return the same result and I think it is the same for both IEEE quad and IBM double double formats too,
so using __ieee_exceptions_MOD_ieee_support_flag_16 for it is fine.
On the other side, ieee/ieee_arithmetic.F90 is problematic.
I must say I don't understand at all stuff like IEEE_IS_FINITE, because it refers
to stuff like _gfortran_ieee_is_finite_16 which I don't see anywhere in libgfortran.{so.5,a}.
Looking purely on what is exported from libgfortran.so.5 from the ieee_arithmetic module symbols, I see:
readelf -Wa ../powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5 | grep ieee_arith | grep _16@@
   483: 00000000001ca770    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_io_16@@GFORTRAN_8
   555: 00000000001ca670    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_sqrt_16@@GFORTRAN_8
   753: 00000000001cac10    72 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_rounding_16@@GFORTRAN_8 	[<localentry>: 8]
   771: 00000000001caef0    56 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_class_16@@GFORTRAN_8 	[<localentry>: 8]
   795: 00000000001ca8f0    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_subnormal_16@@GFORTRAN_9
   823: 00000000001ca970    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_denormal_16@@GFORTRAN_8
   961: 00000000001ca6f0    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_nan_16@@GFORTRAN_8
  1220: 00000000001cae30    60 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_value_16@@GFORTRAN_8 	[<localentry>: 8]
  1265: 00000000001ca7f0    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_inf_16@@GFORTRAN_8
  1502: 00000000001caad0    72 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_underflow_control_16@@GFORTRAN_8 	[<localentry>: 8]
  1532: 00000000001ca5f0    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_standard_16@@GFORTRAN_8
  1596: 00000000001ca870    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_divide_16@@GFORTRAN_8
  1703: 00000000001ca9f0    20 FUNC    GLOBAL DEFAULT   11 __ieee_arithmetic_MOD_ieee_support_datatype_16@@GFORTRAN_8

Now, most of the above are probably fine too, they just return true and again that answer seems to be fine for both when
real(kind=16) is IBM double double or IEEE quad.
The problematic cases from those are:
__ieee_arithmetic_MOD_ieee_class_16
__ieee_arithmetic_MOD_ieee_value_16
The above 2 clearly need different behavior when real(kind=16) is IEEE quad vs. IBM double double, the
implementation right now implements those for IBM double double.
So, either we need to arrange for those to be expanded inline by the frontend (and it can be done either
by hardcoding the enumerators or say transforming the calls from working with IEEE quad to working with
IBM double double or vice versa), or we need to add
__ieee_arithmetic_MOD_ieee_class_17
__ieee_arithmetic_MOD_ieee_value_17
entrypoints to the library and arrange for the _16 to be remapped to _17 for these 2.
Other possibly problematic functions are
__ieee_arithmetic_MOD_ieee_support_rounding_16
__ieee_arithmetic_MOD_ieee_support_underflow_control_16
The former seems to do the exact same call in each case, so is probably fine like the true cases,
the latter actually calls support_underflow_control_helper with kind value, but it is ignored everywhere but alpha,
so is fine too.

For the transformation above, I mean something like handling ieee_class by doing __builtin_fpclassify + issignalling
and __builtin_signbit and depending on the result call __ieee_arithmetic_MOD_ieee_class_16 on sample IBM double double
values with the same properties.
Similarly, ieee_value by calling __ieee_arithmetic_MOD_ieee_value_16 first, analyzing the result with
__builtin_fpclassify and in most cases just converting the IBM double double to IEEE quad.
Guess NaNs (especially signalling NaN) and denormals need to be an exception.

Another question is if one needs to implement these as actual out of line functions (even hidden), whether it is
permissible to say call some function/subroutine with ieee_value or ieee_class as procedure argument.
Given that they are overloaded, I'd hope it is not possible.
Comment 2 Segher Boessenkool 2022-08-10 22:46:49 UTC
Let me say again that IEEE QP and double-double should not be the same kind.
This very obviously cannot work at all.
Comment 3 Francois-Xavier Coudert 2022-08-11 07:52:53 UTC
The module in the runtime library is compiled only once, unless the library is multilib (one libgfortran compiled for IBM double double, one libgfortran compiled for IEEE quad).

My goal would be to generate all IEEE intrinsics in the front-end, it should be possible. However, there is not enough support in the middle-end support at the current time: one would need to have GCC support type-generic built-ins for the various IEEE fp functions. This was discussed, there was even a patch at some point for some of them (there must be some discussion of this between myself and, I think, Richard Biener on the gcc list a couple of months ago, but I can't find it right now).
Comment 4 Jakub Jelinek 2022-08-11 09:55:37 UTC
Ok, I'll first implement __builtin_issignaling and then conv_intrinsic_ieee_{value,class}.
Comment 5 Francois-Xavier Coudert 2022-08-11 11:26:44 UTC
Previous discussion on the list:

- when I first implemented it in 2013: https://gcc.gnu.org/legacy-ml/gcc/2013-11/msg00109.html
- more recent: https://www.mail-archive.com/gcc@gcc.gnu.org/msg97340.html

I think Joseph pointed (either in another bugzilla issue, or on the list) to a first tentative implementation of __builtin_issignaling in the middle-end by Sandra Loosemore, which had to be reversed because it caused trouble on some targets. The difficulty that was cited (and why I didn't get to it myself) is the large number of floating-point formats to be handled.

That said, having __builtin_issignaling() would be great and we could emit a lot more of the IEEE functions in the front-end, if it were available.
Comment 6 Jakub Jelinek 2022-08-11 16:22:29 UTC
Created attachment 53438 [details]
gcc13-builtin-issignaling.patch

Current WIP on the __builtin_issignaling.
Still need to look at decimal arguments (if we need/want to deal with those) and add i386 issignalingxf2 optab so that it also handles pseudo numbers and add test coverage.
Comment 7 Jakub Jelinek 2022-08-11 20:35:03 UTC
Created attachment 53440 [details]
gcc13-builtin-issignaling.patch

Another WIP version, the i386.md expander needs some more work but otherwise it looks good on x86_64/i686.
Comment 8 Jakub Jelinek 2022-08-12 11:19:34 UTC
Created attachment 53444 [details]
gcc13-builtin-issignaling.patch

This one works on x86_64/i686, now to test ppc64le and ppc64.
Comment 9 Jakub Jelinek 2022-08-12 17:15:46 UTC
Created attachment 53448 [details]
gcc13-pr106579.patch

On top of that, this implements ieee_class.  ieee_value still to be implemented.
Comment 10 GCC Commits 2022-08-26 07:53:12 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:db630423a97ec6690a8eb0e5c3cb186c91e3740d

commit r13-2216-gdb630423a97ec6690a8eb0e5c3cb186c91e3740d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Aug 26 09:52:02 2022 +0200

    fortran: Expand ieee_arithmetic module's ieee_class inline [PR106579]
    
    The following patch expands IEEE_CLASS inline in the FE, using the
    __builtin_fpclassify, __builtin_signbit and the new __builtin_issignaling
    builtins.
    
    2022-08-26  Jakub Jelinek  <jakub@redhat.com>
    
            PR fortran/106579
    gcc/fortran/
            * f95-lang.cc (gfc_init_builtin_functions): Initialize
            BUILT_IN_FPCLASSIFY.
            * libgfortran.h (IEEE_OTHER_VALUE, IEEE_SIGNALING_NAN,
            IEEE_QUIET_NAN, IEEE_NEGATIVE_INF, IEEE_NEGATIVE_NORMAL,
            IEEE_NEGATIVE_DENORMAL, IEEE_NEGATIVE_SUBNORMAL,
            IEEE_NEGATIVE_ZERO, IEEE_POSITIVE_ZERO, IEEE_POSITIVE_DENORMAL,
            IEEE_POSITIVE_SUBNORMAL, IEEE_POSITIVE_NORMAL, IEEE_POSITIVE_INF):
            New enum.
            * trans-intrinsic.cc (conv_intrinsic_ieee_class): New function.
            (gfc_conv_ieee_arithmetic_function): Handle ieee_class.
    libgfortran/
            * ieee/ieee_helper.c (IEEE_OTHER_VALUE, IEEE_SIGNALING_NAN,
            IEEE_QUIET_NAN, IEEE_NEGATIVE_INF, IEEE_NEGATIVE_NORMAL,
            IEEE_NEGATIVE_DENORMAL, IEEE_NEGATIVE_SUBNORMAL,
            IEEE_NEGATIVE_ZERO, IEEE_POSITIVE_ZERO, IEEE_POSITIVE_DENORMAL,
            IEEE_POSITIVE_SUBNORMAL, IEEE_POSITIVE_NORMAL, IEEE_POSITIVE_INF):
            Move to gcc/fortran/libgfortran.h.
Comment 11 GCC Commits 2022-08-26 07:56:52 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:0c2d6aa1be2ea85e751852834986ae52d58134d3

commit r13-2217-g0c2d6aa1be2ea85e751852834986ae52d58134d3
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Aug 26 09:56:19 2022 +0200

    fortran: Expand ieee_arithmetic module's ieee_value inline [PR106579]
    
    The following patch expands IEEE_VALUE function inline in the FE.
    
    2022-08-26  Jakub Jelinek  <jakub@redhat.com>
    
            PR fortran/106579
            * trans-intrinsic.cc: Include realmpfr.h.
            (conv_intrinsic_ieee_value): New function.
            (gfc_conv_ieee_arithmetic_function): Handle ieee_value.
Comment 12 Jakub Jelinek 2022-08-26 08:05:16 UTC
Fixed on the trunk now.  Not really sure what to do for the backport.  Perhaps make the builtin with a space in the name and do the FE expansion only for the abi_kind == 17 cases?
Comment 13 GCC Commits 2022-08-29 10:06:13 UTC
The releases/gcc-12 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:c5d4e67e764fb157404142cfc8d2bc4436fcf0f5

commit r12-8723-gc5d4e67e764fb157404142cfc8d2bc4436fcf0f5
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Aug 26 09:52:02 2022 +0200

    fortran: Expand ieee_arithmetic module's ieee_class inline [PR106579]
    
    The following patch expands IEEE_CLASS inline in the FE but only for the
    powerpc64le-linux IEEE quad real(kind=16), using the __builtin_fpclassify
    builtin and explicit check of the MSB mantissa bit in place of missing
    __builtin_signbit builtin.
    
    2022-08-26  Jakub Jelinek  <jakub@redhat.com>
    
            PR fortran/106579
    gcc/fortran/
            * f95-lang.cc (gfc_init_builtin_functions): Initialize
            BUILT_IN_FPCLASSIFY.
            * libgfortran.h (IEEE_OTHER_VALUE, IEEE_SIGNALING_NAN,
            IEEE_QUIET_NAN, IEEE_NEGATIVE_INF, IEEE_NEGATIVE_NORMAL,
            IEEE_NEGATIVE_DENORMAL, IEEE_NEGATIVE_SUBNORMAL,
            IEEE_NEGATIVE_ZERO, IEEE_POSITIVE_ZERO, IEEE_POSITIVE_DENORMAL,
            IEEE_POSITIVE_SUBNORMAL, IEEE_POSITIVE_NORMAL, IEEE_POSITIVE_INF):
            New enum.
            * trans-intrinsic.cc (conv_intrinsic_ieee_class): New function.
            (gfc_conv_ieee_arithmetic_function): Handle ieee_class.
    libgfortran/
            * ieee/ieee_helper.c (IEEE_OTHER_VALUE, IEEE_SIGNALING_NAN,
            IEEE_QUIET_NAN, IEEE_NEGATIVE_INF, IEEE_NEGATIVE_NORMAL,
            IEEE_NEGATIVE_DENORMAL, IEEE_NEGATIVE_SUBNORMAL,
            IEEE_NEGATIVE_ZERO, IEEE_POSITIVE_ZERO, IEEE_POSITIVE_DENORMAL,
            IEEE_POSITIVE_SUBNORMAL, IEEE_POSITIVE_NORMAL, IEEE_POSITIVE_INF):
            Move to gcc/fortran/libgfortran.h.
    
    (cherry picked from commit db630423a97ec6690a8eb0e5c3cb186c91e3740d)
Comment 14 GCC Commits 2022-08-29 10:06:18 UTC
The releases/gcc-12 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:87a07e96dc4e6849ab6ac0b1ceeb5a19aebee9d6

commit r12-8724-g87a07e96dc4e6849ab6ac0b1ceeb5a19aebee9d6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Aug 26 09:56:19 2022 +0200

    fortran: Expand ieee_arithmetic module's ieee_value inline [PR106579]
    
    The following patch expands IEEE_VALUE function inline in the FE,
    but only for the powerpc64le-linux IEEE quad real(kind=16) case.
    
    2022-08-26  Jakub Jelinek  <jakub@redhat.com>
    
            PR fortran/106579
            * trans-intrinsic.cc: Include realmpfr.h.
            (conv_intrinsic_ieee_value): New function.
            (gfc_conv_ieee_arithmetic_function): Handle ieee_value.
    
    (cherry picked from commit 0c2d6aa1be2ea85e751852834986ae52d58134d3)
Comment 15 Jakub Jelinek 2022-08-30 09:10:57 UTC
Fixed also for GCC 12 by inlining in that case only the IEEE quad real(kind) ieee_{class,value} and just checking a single bit instead of using __builtin_issignaling.