[Bug c++/95351] New: Comparison with NAN optimizes incorrectly with -ffast-math disabled

[Bug c++/95351] New: Comparison with NAN optimizes incorrectly with -ffast-math disabled

[chris.dahlberg at cytovale dot com]

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

            Bug ID: 95351
           Summary: Comparison with NAN optimizes incorrectly with
                    -ffast-math disabled
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chris.dahlberg at cytovale dot com
  Target Milestone: ---

Created attachment 48610
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48610&action=edit
nan_bug.ii:  preprocessed output from --save-temps

The function

bool Foo(double possiblyNAN, double b, double c)
{
    return (possiblyNAN <= 2.0) || ((possiblyNAN  > 2.0) && (b > c));
}


is optimized incorrectly with -O1.  It gets reduced to just '(b > c)', ignoring
the possibility that 'possiblyNAN' is NAN and both comparisons vs 2.0 are
false.  -ffinite-math-only is off (both by default and explicitly, no
difference)

The command line I'm using is:

"g++ -Wall -Wextra -O -fno-strict-aliasing -fwrapv
-fno-aggressive-loop-optimizations -fno-finite-math-only nan_bug.cpp"

No warnings are output

Looking at the assembly, 'possiblyNAN' is never accessed at all

Tried this on these gcc versions and all have the same behavior:
- gcc version 8.3.1 20190311 (Red Hat 8.3.1-3) (GCC)
- gcc version 8.4.0 (Ubuntu 8.4.0-1ubuntu1~18.04)
- gcc version 10.1.0 (Compiler-Explorer-Build)
- gcc version 11.0.0 20200525 (experimental) (Compiler-Explorer-Build)

[Bug c++/95351] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-05-26
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed, this is transformed before the optimizations happen.

Oh this works with the C front-end ...

[Bug c++/95351] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[glisse at gcc dot gnu.org]

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

--- Comment #2 from Marc Glisse <glisse at gcc dot gnu.org> ---
It might not be the issue, but merge_truthop_with_opposite_arm has a suspicious
HONOR_NANS (type) where type is bool: the result of the comparison instead of
one of the arguments.

[Bug middle-end/95351] [11/12/13/14 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.6.4
   Target Milestone|---                         |11.5
            Summary|Comparison with NAN         |[11/12/13/14 Regression]
                   |optimizes incorrectly with  |Comparison with NAN
                   |-ffast-math disabled        |optimizes incorrectly with
                   |                            |-ffast-math disabled
      Known to work|                            |4.5.3

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Marc Glisse from comment #2)
> It might not be the issue, but merge_truthop_with_opposite_arm has a
> suspicious HONOR_NANS (type) where type is bool: the result of the
> comparison instead of one of the arguments.

I suspect that is the issue since GCC 4.5 was fine and GCC 4.6 is broken. and
r0-99565-g27d0d96a8f4064 added merge_truthop_with_opposite_arm for GCC 4.6.

[Bug middle-end/95351] [11/12/13/14 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a patch to fix this:
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 43105d20be3..299c22bf391 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -6420,7 +6420,6 @@ static tree
 merge_truthop_with_opposite_arm (location_t loc, tree op, tree cmpop,
                                 bool rhs_only)
 {
-  tree type = TREE_TYPE (cmpop);
   enum tree_code code = TREE_CODE (cmpop);
   enum tree_code truthop_code = TREE_CODE (op);
   tree lhs = TREE_OPERAND (op, 0);
@@ -6436,6 +6435,8 @@ merge_truthop_with_opposite_arm (location_t loc, tree op,
tree cmpop,
   if (TREE_CODE_CLASS (code) != tcc_comparison)
     return NULL_TREE;

+  tree type = TREE_TYPE (TREE_OPERAND (cmpop, 0));
+
   if (rhs_code == truthop_code)
     {
       tree newrhs = merge_truthop_with_opposite_arm (loc, rhs, cmpop,
rhs_only);

[Bug middle-end/95351] [11/12/13/14 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:31ce2e993d09dcad1ce139a2848a28de5931056d

commit r14-9422-g31ce2e993d09dcad1ce139a2848a28de5931056d
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Sun Mar 10 22:17:09 2024 +0000

    Fold: Fix up merge_truthop_with_opposite_arm for NaNs [PR95351]

    The problem here is that merge_truthop_with_opposite_arm would
    use the type of the result of the comparison rather than the operands
    of the comparison to figure out if we are honoring NaNs.
    This fixes that oversight and now we get the correct results in this
    case.

    Committed as obvious after a bootstrap/test on x86_64-linux-gnu.

            PR middle-end/95351

    gcc/ChangeLog:

            * fold-const.cc (merge_truthop_with_opposite_arm): Use
            the type of the operands of the comparison and not the type
            of the comparison.

    gcc/testsuite/ChangeLog:

            * gcc.dg/float_opposite_arm-1.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

[Bug middle-end/95351] [11/12/13 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |14.0
            Summary|[11/12/13/14 Regression]    |[11/12/13 Regression]
                   |Comparison with NAN         |Comparison with NAN
                   |optimizes incorrectly with  |optimizes incorrectly with
                   |-ffast-math disabled        |-ffast-math disabled

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed on the trunk will backport in a few days.

[Bug middle-end/95351] [11/12/13 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Andrew Pinski
<pinskia@gcc.gnu.org>:

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

commit r13-8420-gac96973150b3279fe157f160efd83995077c7590
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Sun Mar 10 22:17:09 2024 +0000

    Fold: Fix up merge_truthop_with_opposite_arm for NaNs [PR95351]

    The problem here is that merge_truthop_with_opposite_arm would
    use the type of the result of the comparison rather than the operands
    of the comparison to figure out if we are honoring NaNs.
    This fixes that oversight and now we get the correct results in this
    case.

    Committed as obvious after a bootstrap/test on x86_64-linux-gnu.

            PR middle-end/95351

    gcc/ChangeLog:

            * fold-const.cc (merge_truthop_with_opposite_arm): Use
            the type of the operands of the comparison and not the type
            of the comparison.

    gcc/testsuite/ChangeLog:

            * gcc.dg/float_opposite_arm-1.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
    (cherry picked from commit 31ce2e993d09dcad1ce139a2848a28de5931056d)

[Bug middle-end/95351] [11/12 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13 Regression]       |[11/12 Regression]
                   |Comparison with NAN         |Comparison with NAN
                   |optimizes incorrectly with  |optimizes incorrectly with
                   |-ffast-math disabled        |-ffast-math disabled
      Known to work|                            |13.2.1

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Also fixed for GCC 13.3.0.

[Bug middle-end/95351] [11/12 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Andrew Pinski
<pinskia@gcc.gnu.org>:

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

commit r12-10430-gd88fe8210e4edc2f4ddf722ba788924452c6f6a0
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Sun Mar 10 22:17:09 2024 +0000

    Fold: Fix up merge_truthop_with_opposite_arm for NaNs [PR95351]

    The problem here is that merge_truthop_with_opposite_arm would
    use the type of the result of the comparison rather than the operands
    of the comparison to figure out if we are honoring NaNs.
    This fixes that oversight and now we get the correct results in this
    case.

    Committed as obvious after a bootstrap/test on x86_64-linux-gnu.

            PR middle-end/95351

    gcc/ChangeLog:

            * fold-const.cc (merge_truthop_with_opposite_arm): Use
            the type of the operands of the comparison and not the type
            of the comparison.

    gcc/testsuite/ChangeLog:

            * gcc.dg/float_opposite_arm-1.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
    (cherry picked from commit 31ce2e993d09dcad1ce139a2848a28de5931056d)

[Bug middle-end/95351] [11/12 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Andrew Pinski
<pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:6c00c3245e688d00dae3e928f0d03f530640caae

commit r11-11420-g6c00c3245e688d00dae3e928f0d03f530640caae
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Sun Mar 10 22:17:09 2024 +0000

    Fold: Fix up merge_truthop_with_opposite_arm for NaNs [PR95351]

    The problem here is that merge_truthop_with_opposite_arm would
    use the type of the result of the comparison rather than the operands
    of the comparison to figure out if we are honoring NaNs.
    This fixes that oversight and now we get the correct results in this
    case.

    Committed as obvious after a bootstrap/test on x86_64-linux-gnu.

            PR middle-end/95351

    gcc/ChangeLog:

            * fold-const.c (merge_truthop_with_opposite_arm): Use
            the type of the operands of the comparison and not the type
            of the comparison.

    gcc/testsuite/ChangeLog:

            * gcc.dg/float_opposite_arm-1.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
    (cherry picked from commit 31ce2e993d09dcad1ce139a2848a28de5931056d)

[Bug middle-end/95351] [11/12 Regression] Comparison with NAN optimizes incorrectly with -ffast-math disabled

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |12.3.1, 14.1.0
         Resolution|---                         |FIXED

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed everywhere now.