Bug 66462 - GCC isinf/isnan/... builtins cause sNaN exceptions
Summary: GCC isinf/isnan/... builtins cause sNaN exceptions
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
: 59900 77832 (view as bug list)
Depends on:
Blocks: 122662 123015
  Show dependency treegraph
 
Reported: 2015-06-08 14:20 UTC by Wilco
Modified: 2025-12-18 04:14 UTC (History)
4 users (show)

See Also:
Host:
Target: aarch64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-06-09 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Wilco 2015-06-08 14:20:37 UTC
The isinf, isnan, isnormal, isfinite, fpclassify and signbit builtins use FP arithmetic to compute their result even with -fsignaling-nans (signbit only when -ffast-math is also used). This means sNaNs cause invalid operation exceptions which is incorrect according to the IEEE754-2008 standard.

The reason is the use of FP comparisons. Integer arithmetic avoids this, and is typically faster, especially when using softfloat.

int snan_fail(double x)
{
  return __builtin_isinf (x);
}

gcc6.0 -O2 -fsignaling-nans generates for AArch64:

snan_fail:
        ldr     d1, .LC7
        fabs    d0, d0
        fcmp    d0, d1  // will signal on sNaN...
        cset    w0, gt
        ret
Comment 1 Wilco 2015-06-08 14:56:57 UTC
Note when this is fixed, GLIBC math/math.h should be updated to enable the isinf builtins even with -fsignaling-nans.
Comment 2 Richard Biener 2016-10-04 07:51:26 UTC
*** Bug 77832 has been marked as a duplicate of this bug. ***
Comment 3 Drea Pinski 2016-10-10 04:24:55 UTC
*** Bug 59900 has been marked as a duplicate of this bug. ***
Comment 4 Tamar Christina 2017-06-08 07:39:16 UTC
Author: tnfchris
Date: Thu Jun  8 07:38:42 2017
New Revision: 249005

URL: https://gcc.gnu.org/viewcvs?rev=249005&root=gcc&view=rev
Log:
2017-06-08  Tamar Christina  <tamar.christina@arm.com>

	PR middle-end/77925
	PR middle-end/77926
	PR middle-end/66462

	* gcc/builtins.c (fold_builtin_fpclassify): Remove.
	(fold_builtin_interclass_mathfn): Remove.
	(expand_builtin): Add builtins to lowering list.
	(fold_builtin_n): Remove fold_builtin_varargs.
	(fold_builtin_varargs): Remove.
	* gcc/builtins.def (BUILT_IN_ISZERO, BUILT_IN_ISSUBNORMAL): New.
	* gcc/real.h (get_min_float): New.
	(real_format): Add is_ieee_compatible field.
	* gcc/real.c (get_min_float): New.
	(ieee_single_format): Set is_ieee_compatible flag.
	* gcc/gimple-low.c (lower_stm): Define BUILT_IN_FPCLASSIFY,
	CASE_FLT_FN (BUILT_IN_ISINF), BUILT_IN_ISINFD32, BUILT_IN_ISINFD64,
	BUILT_IN_ISINFD128, BUILT_IN_ISNAND32, BUILT_IN_ISNAND64,
	BUILT_IN_ISNAND128, BUILT_IN_ISNAN, BUILT_IN_ISNORMAL, BUILT_IN_ISZERO,
	BUILT_IN_ISSUBNORMAL, CASE_FLT_FN (BUILT_IN_FINITE), BUILT_IN_FINITED32
	BUILT_IN_FINITED64, BUILT_IN_FINITED128, BUILT_IN_ISFINITE.
	(lower_builtin_fpclassify, is_nan, is_normal, is_infinity): New.
	(is_zero, is_subnormal, is_finite, use_ieee_int_mode): Likewise.
	(lower_builtin_isnan, lower_builtin_isinfinite): Likewise.
	(lower_builtin_isnormal, lower_builtin_iszero): Likewise.
	(lower_builtin_issubnormal, lower_builtin_isfinite): Likewise.
	(emit_tree_cond, get_num_as_int, emit_tree_and_return_var): New.
	(mips_single_format): Likewise.
	(motorola_single_format): Likewise.
	(spu_single_format): Likewise.
	(ieee_double_format): Likewise.
	(mips_double_format): Likewise.
	(motorola_double_format): Likewise.
	(ieee_extended_motorola_format): Likewise.
	(ieee_extended_intel_128_format): Likewise.
	(ieee_extended_intel_96_round_53_format): Likewise.
	(ibm_extended_format): Likewise.
	(mips_extended_format): Likewise.
	(ieee_quad_format): Likewise.
	(mips_quad_format): Likewise.
	(vax_f_format): Likewise.
	(vax_d_format): Likewise.
	(vax_g_format): Likewise.
	(decimal_single_format): Likewise.
	(decimal_quad_format): Likewise.
	(iee_half_format): Likewise.
	(mips_single_format): Likewise.
	(arm_half_format): Likewise.
	(real_internal_format): Likewise.
	* gcc/doc/extend.texi: Add documentation for built-ins.
	* gcc/c/c-typeck.c (convert_arguments): Add BUILT_IN_ISZERO
	and BUILT_IN_ISSUBNORMAL.

gcc/testsuite/
2017-06-08  Tamar Christina  <tamar.christina@arm.com>

	* gcc.target/aarch64/builtin-fpclassify.c: New codegen test.
	* gcc.dg/fold-notunord.c: Removed.
	* gcc.dg/torture/floatn-tg-4.h: Add tests for iszero and issubnormal.
	* gcc.dg/torture/float128-tg-4.c: Likewise.
	* gcc.dg/torture/float128x-tg-4: Likewise.
	* gcc.dg/torture/float16-tg-4.c: Likewise.
	* gcc.dg/torture/float32-tg-4.c: Likewise.
	* gcc.dg/torture/float32x-tg-4.c: Likewise.
	* gcc.dg/torture/float64-tg-4.c: Likewise.
	* gcc.dg/torture/float64x-tg-4.c: Likewise.
	* gcc.dg/pr28796-1.c: Add -O2.
	* gcc.dg/builtins-43.c: Check lower instead of gimple.
	* gcc.dg/tg-tests.h: Add iszero and issubnormal.
	* gcc.dg/pr77925.c: Add to test safe cases.


Added:
    trunk/gcc/testsuite/gcc.dg/pr77925.c
    trunk/gcc/testsuite/gcc.dg/torture/float128-tg-4.c
    trunk/gcc/testsuite/gcc.dg/torture/float128x-tg-4.c
    trunk/gcc/testsuite/gcc.dg/torture/float16-tg-4.c
    trunk/gcc/testsuite/gcc.dg/torture/float32-tg-4.c
    trunk/gcc/testsuite/gcc.dg/torture/float32x-tg-4.c
    trunk/gcc/testsuite/gcc.dg/torture/float64-tg-4.c
    trunk/gcc/testsuite/gcc.dg/torture/float64x-tg-4.c
    trunk/gcc/testsuite/gcc.dg/torture/floatn-tg-4.h
    trunk/gcc/testsuite/gcc.target/aarch64/builtin-fpclassify.c
Removed:
    trunk/gcc/testsuite/gcc.dg/fold-notunord.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/builtins.def
    trunk/gcc/c/c-typeck.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/gimple-low.c
    trunk/gcc/real.c
    trunk/gcc/real.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/builtins-43.c
    trunk/gcc/testsuite/gcc.dg/pr28796-1.c
    trunk/gcc/testsuite/gcc.dg/tg-tests.h
Comment 5 Tamar Christina 2017-06-08 07:59:38 UTC
Fixed in r249005
Comment 6 jsm-csl@polyomino.org.uk 2017-06-08 16:25:20 UTC
I thought this was fixed only for certain floating-point formats - and 
even for those, not globally for all targets (not for binary128 on 32-bit 
targets)?
Comment 7 Tamar Christina 2017-06-09 07:42:46 UTC
That's true. The ticket was for aarch64 but even there it doesn't work for binary128 so I'll re-open.
Comment 8 Tamar Christina 2017-06-09 07:43:33 UTC
wrong state
Comment 9 Bill Sacks 2017-09-13 22:00:54 UTC
I have found that, with gfortran, the following all lead to a SIGFPE if I compile with -ffpe-trap=invalid:

(1) ieee_value(my_nan, ieee_quiet_nan)

(2) ieee_value(my_nan, ieee_signaling_nan)

(3) ieee_is_nan called on a signaling NaN

(4) gfortran's built-in isnan called on a signaling NaN

(5) ieee_class called on a signaling NaN

(See https://github.com/NCAR/billsacks-gfortran-snan/tree/master_n01 for test code and more information.)

Are all of these duplicate problems to the one reported here, or are some of these separate issues that should be opened?
Comment 10 Wilco 2017-09-14 11:17:03 UTC
(In reply to Bill Sacks from comment #9)
> I have found that, with gfortran, the following all lead to a SIGFPE if I
> compile with -ffpe-trap=invalid:
> 
> (1) ieee_value(my_nan, ieee_quiet_nan)
> 
> (2) ieee_value(my_nan, ieee_signaling_nan)
> 
> (3) ieee_is_nan called on a signaling NaN
> 
> (4) gfortran's built-in isnan called on a signaling NaN
> 
> (5) ieee_class called on a signaling NaN
> 
> (See https://github.com/NCAR/billsacks-gfortran-snan/tree/master_n01 for
> test code and more information.)
> 
> Are all of these duplicate problems to the one reported here, or are some of
> these separate issues that should be opened?

If the code does what it appears to do (just creating NaN bitpatterns and testing them, not arithmetic) then it is likely something else as this shouldn't cause exceptions. It's worth narrowing down to a tiny example and find out where the exception happens in the generated assembly. Running on other targets (which support traps) may be worthwhile to find out whether this is a general issue or x64 specific.
Comment 11 Segher Boessenkool 2019-09-03 18:22:09 UTC
I currently have

===
diff --git a/gcc/builtins.c b/gcc/builtins.c
index ad5135c..bc3d318 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -9050,6 +9050,12 @@ fold_builtin_interclass_mathfn (location_t loc, tree fnde
   if (interclass_mathfn_icode (arg, fndecl) != CODE_FOR_nothing)
     return NULL_TREE;
 
+  /* None of these builtins are ever exceptional, not even for signaling NaNs,
+     so we cannot do any of these optimizations that involve a floating point
+     comparison.  */
+  if (flag_signaling_nans)
+    return NULL_TREE;
+
   mode = TYPE_MODE (TREE_TYPE (arg));
 
   bool is_ibm_extended = MODE_COMPOSITE_P (mode);
===

but we should really handle this with some non-signaling insns, not punt
it to libm to do.
Comment 12 Wilco 2019-09-03 19:12:54 UTC
(In reply to Segher Boessenkool from comment #11)
> I currently have
> 
> ===
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index ad5135c..bc3d318 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -9050,6 +9050,12 @@ fold_builtin_interclass_mathfn (location_t loc, tree
> fnde
>    if (interclass_mathfn_icode (arg, fndecl) != CODE_FOR_nothing)
>      return NULL_TREE;
>  
> +  /* None of these builtins are ever exceptional, not even for signaling
> NaNs,
> +     so we cannot do any of these optimizations that involve a floating
> point
> +     comparison.  */
> +  if (flag_signaling_nans)
> +    return NULL_TREE;
> +
>    mode = TYPE_MODE (TREE_TYPE (arg));
>  
>    bool is_ibm_extended = MODE_COMPOSITE_P (mode);
> ===
> 
> but we should really handle this with some non-signaling insns, not punt
> it to libm to do.

Well we should simply commit Tamar's patch again since it works fine on any IEEE targets and showed performance gains across many targets. Any issues with weird 128-bit FP formats can be addressed separately.
Comment 13 jsm-csl@polyomino.org.uk 2019-09-03 19:25:05 UTC
These functions have to be expanded inline, unconditionally; there are no 
library functions they can reliably fall back on in general.
Comment 14 Segher Boessenkool 2019-09-03 21:29:34 UTC
(In reply to Wilco from comment #12)
> > but we should really handle this with some non-signaling insns, not punt
> > it to libm to do.
> 
> Well we should simply commit Tamar's patch again since it works fine on any
> IEEE targets and showed performance gains across many targets. Any issues
> with weird 128-bit FP formats can be addressed separately.

Do you have a link to those problems?  And no, please don't regress us for no
reason at all, it's really easy to *not* regress this on double-double.
Comment 15 Segher Boessenkool 2019-09-03 21:30:44 UTC
(In reply to joseph@codesourcery.com from comment #13)
> These functions have to be expanded inline, unconditionally; there are no 
> library functions they can reliably fall back on in general.

Ugh, yes.  There probably *should* be, but oh well.
Comment 16 Tamar Christina 2019-09-04 08:46:07 UTC
> Do you have a link to those problems?  And no, please don't regress us for no
reason at all, it's really easy to *not* regress this on double-double.

As far as I am aware, the final version of the patch had no regressions for any target, including PowerPC which I used the GCC compile farm to verify (https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02567.html)

The patch ended up not getting committed because of questions around whether integer operations were fast enough on all targets and on the latest reviewer requesting a major change to the patch.

At this time the patch had gone through 3 completely different implementations (due to to every time having a different reviewer reviewing it) and so a 4th rewrite was deemed not productive use of time.
Comment 17 Segher Boessenkool 2019-09-04 13:51:51 UTC
(In reply to Tamar Christina from comment #16)
> > Do you have a link to those problems?  And no, please don't regress us for no
> > reason at all, it's really easy to *not* regress this on double-double.
> 
> As far as I am aware, the final version of the patch had no regressions for
> any target, including PowerPC which I used the GCC compile farm to verify
> (https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02567.html)

That sounds great!

> The patch ended up not getting committed because of questions around whether
> integer operations were fast enough on all targets

But floating point operations are *incorrect* (at least when SNaNs are enabled).

> and on the latest
> reviewer requesting a major change to the patch.

Hrm.

> At this time the patch had gone through 3 completely different
> implementations (due to to every time having a different reviewer reviewing
> it) and so a 4th rewrite was deemed not productive use of time.

Could you please retry anyway?  Maybe split the patch into smaller chunks,
so it is easier to digest?  (This also helps if anything regresses, to help
pinpoint what caused that).

Thanks!
Comment 18 jsm-csl@polyomino.org.uk 2019-09-04 16:42:42 UTC
On Wed, 4 Sep 2019, tnfchris at gcc dot gnu.org wrote:

> As far as I am aware, the final version of the patch had no regressions for any
> target, including PowerPC which I used the GCC compile farm to verify
> (https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02567.html)
> 
> The patch ended up not getting committed because of questions around whether
> integer operations were fast enough on all targets and on the latest reviewer
> requesting a major change to the patch.

It *was* committed (r249005).  Then reverted (r249050).  
<https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00565.html> reported "a 
large number of new failures on AIX, including compiler ICEs".  I noted it 
caused ICEs building glibc for powerpc.  Rainer noted Solaris/SPARC was 
affected <https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00602.html>.  
Other issues were also reported in that thread.

Clearly these problems need to be fixed before it can go back in.

That doesn't mean it needs to cover all cases.  But it needs to avoid 
introducing regressions (whether ICEs or wrong code), and existing cases 
that are expanded inline need to stay expanded inline (whether with the 
old or the new expansion), and the limited subset of cases where it's OK 
to take the address of some such built-in functions with the possibility 
of out-of-line expansion need to stay working in the cases where they 
currently work.
Comment 19 Tamar Christina 2019-09-04 17:05:12 UTC
> It *was* committed (r249005).  Then reverted (r249050).  
> <https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00565.html> reported "a 
> large number of new failures on AIX, including compiler ICEs".  I noted it 
> caused ICEs building glibc for powerpc.  Rainer noted Solaris/SPARC was 
> affected <https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00602.html>.  
> Other issues were also reported in that thread.

No it was not. The patch you're linking to is a couple of months older than the one I had linked to that was problem free.

Yes it was committed and reverted and fixed, but never committed again.

The patch you linked to was form June, the one I linked to is from November. Two different patches.
Comment 20 GCC Commits 2025-08-28 14:13:12 UTC
The master branch has been updated by Wilco Dijkstra <wilco@gcc.gnu.org>:

https://gcc.gnu.org/g:8187d2630fac77176a4a806a60b0399a1e0376d8

commit r16-3439-g8187d2630fac77176a4a806a60b0399a1e0376d8
Author: Wilco Dijkstra <wilco.dijkstra@arm.com>
Date:   Wed Aug 13 14:56:57 2025 +0000

    AArch64: Add isinf expander [PR 66462]
    
    Add an expander for isinf using integer arithmetic.  This is
    typically faster and avoids generating spurious exceptions on
    signaling NaNs.  This fixes part of PR66462.
    
    int isinf1 (float x) { return __builtin_isinf (x); }
    
    Before:
            fabs    s0, s0
            mov     w0, 2139095039
            fmov    s31, w0
            fcmp    s0, s31
            cset    w0, le
            eor     w0, w0, 1
            ret
    
    After:
            fmov    w1, s0
            mov     w0, -16777216
            cmp     w0, w1, lsl 1
            cset    w0, eq
            ret
    
    gcc:
            PR middle-end/66462
            * config/aarch64/aarch64.md (isinf<mode>2): Add new expander.
            * config/aarch64/iterators.md (mantissa_bits): Add new mode_attr.
    
    gcc/testsuite:
            PR middle-end/66462
            * gcc.target/aarch64/pr66462.c: Add new test.
Comment 21 GCC Commits 2025-09-11 11:45:11 UTC
The master branch has been updated by Wilco Dijkstra <wilco@gcc.gnu.org>:

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

commit r16-3803-gb996d4509f1724d92a08dae70d8354fbb5561fb7
Author: Wilco Dijkstra <wilco.dijkstra@arm.com>
Date:   Wed Aug 27 17:20:21 2025 +0000

    AArch64: Add isfinite expander [PR 66462]
    
    Add an expander for isfinite using integer arithmetic.  This is
    typically faster and avoids generating spurious exceptions on
    signaling NaNs.  This fixes part of PR66462.
    
    int isfinite1 (float x) { return __builtin_isfinite (x); }
    
    Before:
            fabs    s0, s0
            mov     w0, 2139095039
            fmov    s31, w0
            fcmp    s0, s31
            cset    w0, hi
            eor     w0, w0, 1
            ret
    
    After:
            fmov    w1, s0
            mov     w0, -16777216
            cmp     w0, w1, lsl 1
            cset    w0, hi
            ret
    
    gcc:
            PR middle-end/66462
            * config/aarch64/aarch64.md (isfinite<mode>2): Add new expander.
    
    gcc/testsuite:
            PR middle-end/66462
            * gcc.target/aarch64/pr66462.c: Add tests for isfinite.
Comment 22 GCC Commits 2025-09-16 12:40:01 UTC
The master branch has been updated by Wilco Dijkstra <wilco@gcc.gnu.org>:

https://gcc.gnu.org/g:5b531aa5cc216b5ec4d740940b43ca7a7728cc90

commit r16-3909-g5b531aa5cc216b5ec4d740940b43ca7a7728cc90
Author: Wilco Dijkstra <wilco.dijkstra@arm.com>
Date:   Thu Sep 4 14:31:16 2025 +0000

    AArch64: Add isnan expander [PR 66462]
    
    Add an expander for isnan using integer arithmetic.  Since isnan is
    just a compare, enable it only with -fsignaling-nans to avoid
    generating spurious exceptions.  This fixes part of PR66462.
    
    int isnan1 (float x) { return __builtin_isnan (x); }
    
    Before:
            fcmp    s0, s0
            cset    w0, vs
            ret
    
    After:
            fmov    w1, s0
            mov     w0, -16777216
            cmp     w0, w1, lsl 1
            cset    w0, cc
            ret
    
    gcc:
            PR middle-end/66462
            * config/aarch64/aarch64.md (isnan<mode>2): Add new expander.
    
    gcc/testsuite:
            PR middle-end/66462
            * gcc.target/aarch64/pr66462.c: Update test.
Comment 23 GCC Commits 2025-09-24 08:28:08 UTC
The master branch has been updated by Xi Ruoyao <xry111@gcc.gnu.org>:

https://gcc.gnu.org/g:2c98d7cf3a151e3d464a8e4a161d084494281d76

commit r16-4071-g2c98d7cf3a151e3d464a8e4a161d084494281d76
Author: Xi Ruoyao <xry111@xry111.site>
Date:   Tue Sep 16 23:10:26 2025 +0800

    LoongArch: Add isnan expander [PR 66462]
    
    Add an expander for isnan using fclass.  Since isnan is
    just a compare, enable it only with -fsignaling-nans to avoid
    generating spurious exceptions.  This fixes part of PR66462.
    
    int isnan1 (float x) { return __builtin_isnan (x); }
    
    With -fno-signaling-nans:
    
            fcmp.cun.s      $fcc0,$f0,$f0
            movcf2fr        $f0,$fcc0
            movfr2gr.s      $r4,$f0
            jr      $r1
    
    With -fsignaling-nans:
    
            fclass.s        $f0,$f0
            movfr2gr.s      $r4,$f0
            andi    $r4,$r4,3
            sltu    $r4,$r0,$r4
            jr      $r1
    
            PR middle-end/66462
    
    gcc/
    
            * config/loongarch/loongarch.md (FCLASS_MASK): Add 3.
            (fclass_optab): Assign isnan for 3.
            (<FCLASS_MASK:fclass_optab><ANYF:mode>2): If FCLASS_MASK is 3,
            only enable when -fsignaling-nans.
    
    gcc/testsuite:
    
            * gcc.target/loongarch/fclass-compile.c: Update test.
            * gcc.target/loongarch/fclass-run.c: Likewise.