[Bug c/103193] New: gcc for x86_64: wrong code generation: ucomiss instead of comiss

[Bug c/103193] New: gcc for x86_64: wrong code generation: ucomiss instead of comiss

[pavel.morozkin at gmail dot com]

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

            Bug ID: 103193
           Summary: gcc for x86_64: wrong code generation: ucomiss instead
                    of comiss
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pavel.morozkin at gmail dot com
  Target Milestone: ---

This code:
#pragma STDC FENV_ACCESS ON
float f;
_Bool b;
f = NAN;
b = f >= f; // ucomiss (wrong), comiss (correct)

results in:
ucomiss xmm0, DWORD PTR [rbp-4]


Per IEEE 754-2008 the ">=" is compareSignalingGreaterEqual, which can be
implemented using comiss.

Notes:
1. Yes, the #pragma STDC FENV_ACCESS ON is not yet supported. Just wanted to
point out that under #pragma STDC FENV_ACCESS ON gcc needs to generate comiss
(not ucomiss).
2. Adding volatile to float f; results in generation of comiss (expected).

[Bug target/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[joseph at codesourcery dot com]

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

--- Comment #1 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
See bug 52451 and bug 91323 for previous cases of unordered comparisons 
being wrongly used on x86.

[Bug target/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[ubizjak at gmail dot com]

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

--- Comment #2 from Uroš Bizjak <ubizjak at gmail dot com> ---
Here is compilable testcase:

_Bool a (void)
{
#pragma STDC FENV_ACCESS ON
  float f;
  _Bool b;
  f = __builtin_nan ("");
  b = f >= f; // ucomiss (wrong), comiss (correct)
  return b;
}

Tree optimizers (_.optimized dump) already give us:

  <bb 2> :
  f_1 =  Nan;
  b_2 = f_1 == f_1;
  _3 = b_2;


However, f_1 == f_1 expands with UNSPEC_NOTRAP RTX which gives us ucomiss.

[Bug target/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[joseph at codesourcery dot com]

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

--- Comment #3 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
Converting from >= to == is inappropriate (because >= should raise invalid 
for all NaN operands but == should do so only for signaling NaNs).  If 
that's happening in architecture-independent code, then this is an 
architecture-independent bug.

[Bug middle-end/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-* i?86-*-*
          Component|target                      |middle-end
     Ever confirmed|0                           |1
                 CC|                            |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-11-12

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to joseph@codesourcery.com from comment #3)
> Converting from >= to == is inappropriate (because >= should raise invalid 
> for all NaN operands but == should do so only for signaling NaNs).  If 
> that's happening in architecture-independent code, then this is an 
> architecture-independent bug.

/* Simplify comparison of something with itself.  For IEEE
   floating-point, we can only do some of these simplifications.  */
(for cmp (eq ge le)
 (simplify
  (cmp @0 @0)
  (if (! FLOAT_TYPE_P (TREE_TYPE (@0))
       || ! HONOR_NANS (@0))
   { constant_boolean_node (true, type); }
   (if (cmp != EQ_EXPR)
    (eq @0 @0)))))

does this.  The folding to == happens unconditionally.  As I understand you
the condition that applies to the constant folding should apply to the
folding to EQ as well, which means we effectively need to remove the
canonicalization to EQ (since when it would be valid we can fold to constant
true)?

There are similar transforms following:

(for cmp (unle unge uneq)
 (simplify
  (cmp @0 @0)
  { constant_boolean_node (true, type); }))
(for cmp (unlt ungt)
 (simplify
  (cmp @0 @0)
  (unordered @0 @0)))
(simplify
 (ltgt @0 @0)
 (if (!flag_trapping_math)
  { constant_boolean_node (false, type); }))

[Bug middle-end/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[joseph at codesourcery dot com]

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

--- Comment #5 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
On Fri, 12 Nov 2021, rguenth at gcc dot gnu.org via Gcc-bugs wrote:

> /* Simplify comparison of something with itself.  For IEEE
>    floating-point, we can only do some of these simplifications.  */
> (for cmp (eq ge le)
>  (simplify
>   (cmp @0 @0)
>   (if (! FLOAT_TYPE_P (TREE_TYPE (@0))
>        || ! HONOR_NANS (@0))
>    { constant_boolean_node (true, type); }
>    (if (cmp != EQ_EXPR)
>     (eq @0 @0)))))
> 
> does this.  The folding to == happens unconditionally.  As I understand you
> the condition that applies to the constant folding should apply to the
> folding to EQ as well, which means we effectively need to remove the
> canonicalization to EQ (since when it would be valid we can fold to constant
> true)?

It's invalid with -ftrapping-math because it loses an exception.  With 
-fno-trapping-math, but NaNs supported, you can convert to EQ but can't 
fold to constant true.

[Bug middle-end/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine then.

[Bug middle-end/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

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

commit r12-5495-gd9ca2ca381e44a332703155d07b50b84aa21f80d
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Nov 15 12:13:40 2021 +0100

    middle-end/103193 - avoid canonicalizing <= and >= to == for floats

    This avoids doing aforementioned canoncalization when -ftrapping-math
    is in effect and we honor NaNs.

    2021-11-15  Richard Biener  <rguenther@suse.de>

            PR middle-end/103193
            * match.pd: Avoid canonicalizing (le/ge @0 @0) to (eq @0 @0)
            with NaNs and -ftrapping-math.

[Bug middle-end/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |12.0

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar, I'm going to backport to at least GCC 11 which where the
issue was reported for but it's been broken forever (originally in
fold-const.c:fold_comparison, moved in 2015).

[Bug middle-end/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

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

commit r11-10168-ge2b97d6883a72b0c51dd0455acea43e21b5537d9
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Nov 15 12:13:40 2021 +0100

    middle-end/103193 - avoid canonicalizing <= and >= to == for floats

    This avoids doing aforementioned canoncalization when -ftrapping-math
    is in effect and we honor NaNs.

    2021-11-15  Richard Biener  <rguenther@suse.de>

            PR middle-end/103193
            * match.pd: Avoid canonicalizing (le/ge @0 @0) to (eq @0 @0)
            with NaNs and -ftrapping-math.

    (cherry picked from commit d9ca2ca381e44a332703155d07b50b84aa21f80d)

[Bug middle-end/103193] gcc for x86_64: wrong code generation: ucomiss instead of comiss

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
      Known to fail|                            |11.3.0
      Known to work|                            |11.3.1
             Status|ASSIGNED                    |RESOLVED

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for GCC 11.4.