[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0
            Summary|Undue optimisation of       |[13 Regression] Undue
                   |floating-point comparisons  |optimisation of
                   |                            |floating-point comparisons

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54004
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54004&action=edit
gcc13-pr106805.patch

Untested patch.  I believe fold_relational_const does it right, in the match.pd
case we are doing it just for simple_comparison, so a subset of those, so
only ==/!= are quiet from those.

Unfortunately, the patch isn't enough.  On 12 branch it is enough until
expansion but during expansion we just drop the comparisons for some reason,
and on 13 branch it is worse, frange kicks in and we have the PR107608
problems.

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That said, perhaps for now committing the patch without the pr106805.c testcase
and work incrementally on the other issues might be a good idea.

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:109148dd16e4bcd50faee19c49082de69d0ba26e

commit r13-4493-g109148dd16e4bcd50faee19c49082de69d0ba26e
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Dec 5 11:54:45 2022 +0100

    match.pd: Don't fold nan < x etc. for -ftrapping-math [PR106805]

    As reported in the PR, the following pr106805.c testcase is miscompiled
    with the default -ftrapping-math, because we fold all the comparisons into
    constants and don't raise any exceptions.

    The match.pd pattern handles just simple comparisons, from those
    EQ/NE are quiet and don't raise exceptions on anything but sNaN, while
    GT/GE/LT/LE are signaling and do raise exceptions even on qNaN.

    fold_relational_const handles this IMHO correctly:
          /* Handle the cases where either operand is a NaN.  */
          if (real_isnan (c0) || real_isnan (c1))
            {
              switch (code)
                {
                case EQ_EXPR:
                case ORDERED_EXPR:
                  result = 0;
                  break;

                case NE_EXPR:
                case UNORDERED_EXPR:
                case UNLT_EXPR:
                case UNLE_EXPR:
                case UNGT_EXPR:
                case UNGE_EXPR:
                case UNEQ_EXPR:
                  result = 1;
                  break;

                case LT_EXPR:
                case LE_EXPR:
                case GT_EXPR:
                case GE_EXPR:
                case LTGT_EXPR:
                  if (flag_trapping_math)
                    return NULL_TREE;
                  result = 0;
                  break;

                default:
                  gcc_unreachable ();
                }

              return constant_boolean_node (result, type);
            }
    by folding the signaling comparisons only if -fno-trapping-math.
    The following patch does the same in match.pd.

    Unfortunately the pr106805.c testcase still fails, but no longer because of
    match.pd, but on the trunk because of the still unresolved ranger problems
    (same issue as for fold-overflow-1.c etc.) and on 12 branch (and presumably
    trunk too) somewhere during expansion the comparisons are also expanded
    into constants (which is ok for -fno-trapping-math, but not ok with that).

    Though, I think the patch is a small step in the direction, so I'd like
    to commit this patch without the gcc.dg/pr106805.c testcase for now.

    2022-12-05  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/106805
            * match.pd (cmp @0 REAL_CST@1): Don't optimize x cmp NaN
            or NaN cmp x to false/true for cmp >/>=/</<= if -ftrapping-math.

            * c-c++-common/pr57371-4.c: Revert 2021-09-19 changes.
            * c-c++-common/pr57371-5.c: New test.
            * gcc.c-torture/execute/ieee/fp-cmp-6.x: Add -fno-trapping-math.
            * gcc.c-torture/execute/ieee/fp-cmp-9.c: New test.
            * gcc.c-torture/execute/ieee/fp-cmp-9.x: New file.

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note phiopt turns some control-flow into straight-line code which is then also
susceptible to code motion across the foo() calls, even if that's not what
currently happens.  So even preserving those stmts will still have them
un-ordered with respect to the inspection of the FP exception state,
that is, -frounding-math is known to be buggy.

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=107608

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
PR107608 is practically the same underlying issue.

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[vincent-gcc at vinc17 dot net]

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

--- Comment #8 from Vincent Lefèvre <vincent-gcc at vinc17 dot net> ---
Isn't it the same as PR56020, which is due to the fact that the STDC
FENV_ACCESS pragma is not implemented and assumed to be OFF (PR34678)?

[Bug middle-end/106805] [13 Regression] Undue optimisation of floating-point comparisons

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, the current state is that this is now optimized away by dom2 using frange,
like in PR107608 (which is closed with a workaround), but in this case the NaN
vs. 1.0
or NaN vs. Inf >= and <= comparisons get a singleton integral range which dom2
then optimizes out.
GCC 12 optimizes it out currently even earlier because #c5 change isn't
backported there yet, but otherwise wouldn't suffer from the frange issue and
would be miscompiled only during expansion.

I'd say for now if you want such a function to raise exceptions even with
constant arguments, don't make it inline and add noipa attribute to it instead.
 That case I think has been fixed on the trunk already.

[Bug middle-end/106805] Undue optimisation of floating-point comparisons

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|13.0                        |---
      Known to fail|                            |10.1.0, 12.1.0, 3.4.6
           Priority|P1                          |P3
                 CC|                            |rguenth at gcc dot gnu.org
            Summary|[13 Regression] Undue       |Undue optimisation of
                   |optimisation of             |floating-point comparisons
                   |floating-point comparisons  |

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Since the original testcase also fails with GCC 12.2 andthis can't be P1.  I
can't find a single working version (but 2.95 doesn't like my libm headers, so
...) - even 3.4.6 fails.  GCC 3.3 didn't know -frounding-math.

So it's not even a regression.