[match.pd] PR tree-optimization/126467: 0.0-x -> -x vs. signed zeros.

Message ID 003101dd24b1$0c462d80$24d28880$@nextmovesoftware.com
State New
Headers
Series [match.pd] PR tree-optimization/126467: 0.0-x -> -x vs. signed zeros. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed

Commit Message

Roger Sayle Aug. 5, 2026, 8:04 a.m. UTC
  This is my proposed solution to PR tree-optimization/126467, where we're
inappropriately converting 0.0 - x to -x when we honor IEEE signed zeros.
This transformation is valid with -Ofast, but by default +0.0 - +0.0
should return +0.0, but -(+0.0) is -0.0.  Likewise when x is NaN, 0.0 - x
may change the payload, but -x is guaranteed not to.  My fix is to
separate the logic for this transformation from that for FP addition.

Technically, we could do slightly better by introducing a
tree_expr_negative_p (complementing and mutually recursive with the
existing tree_expr_nonnegative_p), but that's a bigger change and
less suitable for backporting to release branches, i.e. a follow-up.

I agree with Alexander Monakov that an alternate fix might be to
correctly reuse the existing fold_real_zero_addition_p functionality
by constructing and garbage collecting a NEGATE_EXPR tree on each call,
but this seems a little less efficient.

This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32}
with no new failures.  Ok for mainline?


2026-08-05  Roger Sayle  <roger@nextmovesofware.com>

gcc/ChangeLog
        PR tree-optimization/126467
        * match.pd (0.0 - x -> -x): Update the conditions under which
        the transformation is performed, disallowing x = +0.0 when we
        honor signed zeros.

gcc/testsuite/ChangeLog
        PR tree-optimization/126467
        * gcc.dg/pr126467-1.c: New test case.
        * gcc.dg/pr126467-2.c: Likewise.
        * gcc.dg/pr96392.c: Fix incorrect test case.
  

Comments

Andrea Pinski Aug. 7, 2026, 6:07 a.m. UTC | #1
On Wed, Aug 5, 2026 at 1:06 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This is my proposed solution to PR tree-optimization/126467, where we're
> inappropriately converting 0.0 - x to -x when we honor IEEE signed zeros.
> This transformation is valid with -Ofast, but by default +0.0 - +0.0
> should return +0.0, but -(+0.0) is -0.0.  Likewise when x is NaN, 0.0 - x
> may change the payload, but -x is guaranteed not to.  My fix is to
> separate the logic for this transformation from that for FP addition.
>
> Technically, we could do slightly better by introducing a
> tree_expr_negative_p (complementing and mutually recursive with the
> existing tree_expr_nonnegative_p), but that's a bigger change and
> less suitable for backporting to release branches, i.e. a follow-up.
>
> I agree with Alexander Monakov that an alternate fix might be to
> correctly reuse the existing fold_real_zero_addition_p functionality
> by constructing and garbage collecting a NEGATE_EXPR tree on each call,
> but this seems a little less efficient.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32}
> with no new failures.  Ok for mainline?

`!HONOR_NANS (type) || !tree_expr_maybe_nan_p (@1)`
You don't need the HONOR_NANS part here since tree_expr_maybe_nan_p
already handles that.
Otherwise ok.

Thanks,
Andrea

>
>
> 2026-08-05  Roger Sayle  <roger@nextmovesofware.com>
>
> gcc/ChangeLog
>         PR tree-optimization/126467
>         * match.pd (0.0 - x -> -x): Update the conditions under which
>         the transformation is performed, disallowing x = +0.0 when we
>         honor signed zeros.
>
> gcc/testsuite/ChangeLog
>         PR tree-optimization/126467
>         * gcc.dg/pr126467-1.c: New test case.
>         * gcc.dg/pr126467-2.c: Likewise.
>         * gcc.dg/pr96392.c: Fix incorrect test case.
>
  
Alexander Monakov Aug. 7, 2026, 6:44 a.m. UTC | #2
On Wed, 5 Aug 2026, Roger Sayle wrote:

> gcc/testsuite/ChangeLog
>         PR tree-optimization/126467
>         * gcc.dg/pr126467-1.c: New test case.
>         * gcc.dg/pr126467-2.c: Likewise.
>         * gcc.dg/pr96392.c: Fix incorrect test case.

Sorry, what is incorrect in this testcase? x is an int there:

-double negate(int x)
-{
-  return 0.0 - x;
-}

Alexander
  
Alexander Monakov Aug. 7, 2026, 7:30 a.m. UTC | #3
On Wed, 5 Aug 2026, Roger Sayle wrote:

> Likewise when x is NaN, 0.0 - x
> may change the payload, but -x is guaranteed not to.

Not saying you should drop the condition in your patch, but do we care
about NaN payload propagation anywhere else? The HONOR_NANS et al. only
seem to cover presence/absence of NaNs, not observability of payloads.

> gcc/ChangeLog
>         PR tree-optimization/126467
>         * match.pd (0.0 - x -> -x): Update the conditions under which
>         the transformation is performed, disallowing x = +0.0 when we
>         honor signed zeros.

(this doesn't seem to mention NaNs)

Alexander
  
Richard Biener Aug. 7, 2026, 8:15 a.m. UTC | #4
On Fri, Aug 7, 2026 at 9:31 AM Alexander Monakov <amonakov@ispras.ru> wrote:
>
>
> On Wed, 5 Aug 2026, Roger Sayle wrote:
>
> > Likewise when x is NaN, 0.0 - x
> > may change the payload, but -x is guaranteed not to.
>
> Not saying you should drop the condition in your patch, but do we care
> about NaN payload propagation anywhere else? The HONOR_NANS et al. only
> seem to cover presence/absence of NaNs, not observability of payloads.

I think we don't.  -x will still turn sNaN into qNaN?

> > gcc/ChangeLog
> >         PR tree-optimization/126467
> >         * match.pd (0.0 - x -> -x): Update the conditions under which
> >         the transformation is performed, disallowing x = +0.0 when we
> >         honor signed zeros.
>
> (this doesn't seem to mention NaNs)
>
> Alexander
  
Alexander Monakov Aug. 7, 2026, 8:54 a.m. UTC | #5
On Fri, 7 Aug 2026, Richard Biener wrote:

> On Fri, Aug 7, 2026 at 9:31 AM Alexander Monakov <amonakov@ispras.ru> wrote:
> >
> >
> > On Wed, 5 Aug 2026, Roger Sayle wrote:
> >
> > > Likewise when x is NaN, 0.0 - x
> > > may change the payload, but -x is guaranteed not to.
> >
> > Not saying you should drop the condition in your patch, but do we care
> > about NaN payload propagation anywhere else? The HONOR_NANS et al. only
> > seem to cover presence/absence of NaNs, not observability of payloads.
> 
> I think we don't.  -x will still turn sNaN into qNaN?

It may not, negation can only change the sign bit. It's a good point,
0-x would quieten the input sNaN and raise FE_INVALID for a sNaN but not qNaN,
negation wouldn't.

As for payload propagation, IEEE754 recommends that computational operations
propagate one of the input NaNs. Therefore 0-x should just propagate payload
of x when it's a qNaN, just like -x. But, conversely, propagating the input
NaN can result in 0-x having the same sign as x, not the opposite (this is
what actually happens on amd64, subsd preserves the sign of a NaN).

Alexander
  
Roger Sayle Aug. 7, 2026, 9:56 a.m. UTC | #6
> From: Alexander Monakov <amonakov@ispras.ru>
> Sent: 07 August 2026 08:45
> On Wed, 5 Aug 2026, Roger Sayle wrote:
> 
> > gcc/testsuite/ChangeLog
> >         PR tree-optimization/126467
> >         * gcc.dg/pr126467-1.c: New test case.
> >         * gcc.dg/pr126467-2.c: Likewise.
> >         * gcc.dg/pr96392.c: Fix incorrect test case.
> 
> Sorry, what is incorrect in this testcase? x is an int there:
> 
> -double negate(int x)
> -{
> -  return 0.0 - x;
> -}

Hi Alexander,
Yes, x is an int, so when promoted to double (because of the 0.0)
it becomes +0.0, and as described +0.0 - +0.0 is not the same
as -(double)0.  Interestingly, I ran "git blame" to figure out which
misguided fool wrote this test case... It turns out it was me.
Live and learn.

Short answer, this test case now fails with this (any) fix
for PR tree-optimization/126467.

Thanks for your help/feedback.
Roger
--
  
Andrea Pinski Aug. 7, 2026, 11:52 p.m. UTC | #7
On Fri, Aug 7, 2026 at 2:57 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> > From: Alexander Monakov <amonakov@ispras.ru>
> > Sent: 07 August 2026 08:45
> > On Wed, 5 Aug 2026, Roger Sayle wrote:
> >
> > > gcc/testsuite/ChangeLog
> > >         PR tree-optimization/126467
> > >         * gcc.dg/pr126467-1.c: New test case.
> > >         * gcc.dg/pr126467-2.c: Likewise.
> > >         * gcc.dg/pr96392.c: Fix incorrect test case.
> >
> > Sorry, what is incorrect in this testcase? x is an int there:
> >
> > -double negate(int x)
> > -{
> > -  return 0.0 - x;
> > -}
>
> Hi Alexander,
> Yes, x is an int, so when promoted to double (because of the 0.0)
> it becomes +0.0, and as described +0.0 - +0.0 is not the same
> as -(double)0.  Interestingly, I ran "git blame" to figure out which
> misguided fool wrote this test case... It turns out it was me.
> Live and learn.
>
> Short answer, this test case now fails with this (any) fix
> for PR tree-optimization/126467.

That is until finalized ranger support is hooked up to say int->fp
conversion does not produce -0.0.
This is being worked on.
Can you add a separate testcase and xfail currently?

Thanks,
Andrea

>
> Thanks for your help/feedback.
> Roger
> --
>
>
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 536d5125a0b..b69543f35f2 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -5910,12 +5910,20 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (pointer_diff integer_zerop @1)
  (negate (convert @1)))
 
-/* (ARG0 - ARG1) is the same as (-ARG1 + ARG0).  So check whether
-   ARG0 is zero and X + ARG0 reduces to X, since that would mean
-   (-ARG1 + ARG0) reduces to -ARG1.  */
+/* (0.0 - ARG1) can be transformed to -ARG1, if we don't honor NaNs
+   and signed zeros or ARG1 is known to be non-zero.  Subtraction of
+   NaN may signal or modify payload, but negation doesn't, so it is
+   unsafe to apply this transformation for any kind of NaN.  When
+   ARG1 is +0.0 or -0.0, the behaviour depends upon the rounding
+   mode.  With the default rounding mode, (-0.0 - ARG1) is -ARG1,
+   but (+0.0 - ARG1) is only -ARG1 if ARG1 cannot be +0.0.  */
 (simplify
  (minus real_zerop@0 @1)
- (if (fold_real_zero_addition_p (type, @1, @0, 0))
+ (if ((!HONOR_NANS (type) || !tree_expr_maybe_nan_p (@1))
+      && (!HONOR_SIGNED_ZEROS (type)
+	  || tree_expr_nonzero_p (@1)
+	  || (!flag_rounding_math
+	      && REAL_VALUE_MINUS_ZERO (TREE_REAL_CST (@0)))))
   (negate @1)))
 
 /* Transform x * -1 into -x.  */
diff --git a/gcc/testsuite/gcc.dg/pr126467-1.c b/gcc/testsuite/gcc.dg/pr126467-1.c
new file mode 100644
index 00000000000..b078b436454
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr126467-1.c
@@ -0,0 +1,21 @@ 
+/* PR tree-optimization/126467 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-add-options ieee } */
+
+/* 0.0 - x is not -x.
+   For x == +0.0, +0.0 - +0.0 is +0.0, but -x is -0.0.
+   Likewise fabs/negate preserve a NaN's payload but
+   subtraction doesn't.  */
+
+double foo (double x)
+{
+  return 0.0 - x;
+}
+
+double bar (double y)
+{
+  return 0.0 - __builtin_fabs (y);
+}
+
+/* { dg-final { scan-tree-dump-times " \\- " 2 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/pr126467-2.c b/gcc/testsuite/gcc.dg/pr126467-2.c
new file mode 100644
index 00000000000..28742553173
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr126467-2.c
@@ -0,0 +1,15 @@ 
+/* PR tree-optimization/126467 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-signed-zeros -ffinite-math-only -fdump-tree-optimized" } */
+
+double foo (double x)
+{
+  return 0.0 - x;
+}
+
+double bar (double y)
+{
+  return 0.0 - __builtin_fabs (y);
+}
+
+/* { dg-final { scan-tree-dump-not " \\- " "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/pr96392.c b/gcc/testsuite/gcc.dg/pr96392.c
index fb7de217f96..82756907f3f 100644
--- a/gcc/testsuite/gcc.dg/pr96392.c
+++ b/gcc/testsuite/gcc.dg/pr96392.c
@@ -12,11 +12,6 @@  double sub0(int x)
   return x - 0.0;
 }
 
-double negate(int x)
-{
-  return 0.0 - x;
-}
-
 double subtract(int x)
 {
   return (double)x - (double)x;