[Bug c/110305] New: Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[Bug c/110305] New: Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[tkisuki at tachyum dot com]

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

            Bug ID: 110305
           Summary: Incorrect optimization with -O3 -fsignaling-nans
                    -fno-signed-zeros
           Product: gcc
           Version: og12 (devel/omp/gcc-12)
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tkisuki at tachyum dot com
  Target Milestone: ---

float f(float a)
{
  return a + 0.0f;
}

Compiling the above code with -O3 -fsignaling-nans -fno-signed-zeros simplifies
'a + 0' to 'a'.

It seems '-fsignaling-nans' is not considered for this simplification.

Simplification is done by simplify_context::simplify_binary_operation_1()
function in gcc/simplify-rtx.cc.

'-fsignaling-nans' is experimental, but can the condition also check
!HONOR_SNANS(mode)? 

--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -2698,7 +2698,8 @@ simplify_context::simplify_binary_operation_1 (rtx_code
code,
         when x is NaN, infinite, or finite and nonzero.  They aren't
         when x is -0 and the rounding mode is not towards -infinity,
         since (-0) + 0 is then 0.  */
-      if (!HONOR_SIGNED_ZEROS (mode) && trueop1 == CONST0_RTX (mode))
+      if (!HONOR_SIGNED_ZEROS (mode) && !HONOR_SNANS (mode)
+          && trueop1 == CONST0_RTX (mode))
        return op0;

[Bug c/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
First off using signal nans but not caring about signed zeros seems a combo
that normally won't happen in real life. Did you find this by accident or where
you testing this combo on purpose with real code you are writing. .

[Bug c/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[tkisuki at tachyum dot com]

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

--- Comment #2 from tkisuki at tachyum dot com ---
(In reply to Andrew Pinski from comment #1)
> First off using signal nans but not caring about signed zeros seems a combo
> that normally won't happen in real life. Did you find this by accident or
> where you testing this combo on purpose with real code you are writing. .

Thank you for reviewing. We are developing own architecture and gcc backend for
it. My colleague found this case and reported it to me. So, it's more like
accidental and we don't need this fix for existing architectures, like x86 or
aarch64.

I just wanted to share what my colleague reported in the upstream.

[Bug rtl-optimization/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-06-19
            Version|og12 (devel/omp/gcc-12)     |14.0
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |wrong-code
          Component|c                           |rtl-optimization
     Ever confirmed|0                           |1

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  GENERIC properly handles signalling NaNs via
fold_real_zero_addition_p.

If you send a patch to gcc-patches@ I'll approve it.

[Bug rtl-optimization/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[tkisuki at tachyum dot com]

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

--- Comment #4 from tkisuki at tachyum dot com ---
(In reply to Richard Biener from comment #3)
> Confirmed.  GENERIC properly handles signalling NaNs via
> fold_real_zero_addition_p.
> 
> If you send a patch to gcc-patches@ I'll approve it.

Thank you, I just sent the patch to gcc-patches.

[Bug rtl-optimization/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:827b2a279fc6ad5bb76e4d2c2eb3432955b5e11c

commit r14-1952-g827b2a279fc6ad5bb76e4d2c2eb3432955b5e11c
Author: Toru Kisuki <tkisuki@tachyum.com>
Date:   Mon Jun 19 11:51:09 2023 -0600

    Do not allow "x + 0.0" to "x" optimization with -fsignaling-nans

    gcc/
            PR rtl-optimization/110305
            * simplify-rtx.cc (simplify_context::simplify_binary_operation_1):
            Handle HONOR_SNANS for x + 0.0.

[Bug rtl-optimization/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[mmorrell at tachyum dot com]

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

Michael Morrell <mmorrell at tachyum dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mmorrell at tachyum dot com

--- Comment #6 from Michael Morrell <mmorrell at tachyum dot com> ---
I'm curious why this transformation is being done by both
fold_real_zero_addition_p AND simplify_binary_operation_1.  The checks in
fold_real_zero_addition_p are more complex and will leave "a + 0.0" unchanged
in more cases, yet later simplify_binary_operation_1 transforms the expression
for less complex reasons.

I also wonder if there aren't similar expressions (perhaps "a * 1.0" -> a) that
need to be looked at.

[Bug rtl-optimization/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Michael Morrell from comment #6)
> I'm curious why this transformation is being done by both
> fold_real_zero_addition_p AND simplify_binary_operation_1.  

The answer there involves the history of GCC and the history of how
optimizations were done in GCC.
Basically fold_real_zero_addition_p (fold) would only act a statement while
simplify_binary_operation_1 could happen between statements (while doing CSE
and combine, etc.). That changed with the merge of tree-ssa in
r0-58166-g6de9cd9a886ea6 (2004). 

simplify_binary_operation_1 had the optimization since the begining of git
(though it moved from cse.c to simplify-rtx in r0-24738-g0cedb36cbd7e0c and the
HONOR_SIGNED_ZEROS was done by r0-41258-g71925bc04f24a4, in 2002 before it was
just checking ieee float format and unsafe-math).

fold had it since the begining of git also (and changed in a similar fashion as
simplify-rtx for the HONOR_SIGNED_ZEROS).

[Bug rtl-optimization/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[mmorrell at tachyum dot com]

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

--- Comment #8 from Michael Morrell <mmorrell at tachyum dot com> ---
Interesting information.  I still feel that perhaps both functions should use
the same logic to determine whether to make this transformation, but, for
example, the extra checking for the vector case done by
fold_real_zero_addition_p may not be needed in simplify_binary_operation_1
because of when the latter is used.

[Bug rtl-optimization/110305] Incorrect optimization with -O3 -fsignaling-nans -fno-signed-zeros

[mmorrell at tachyum dot com]

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

--- Comment #9 from Michael Morrell <mmorrell at tachyum dot com> ---
And what about when -frounding-math is used?  The transformation will still
occur in simplify_binary_operation_1 if -frounding-math -fno-signed-zeros
-fno-signaling-nans is used.  Note that fold_real_zero_addition_p checks
HONOR_SIGN_DEPENDENT_ROUNDING.