public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
@ 2022-03-14 19:25 Roger Sayle
  2022-03-15  7:29 ` Richard Biener
  0 siblings, 1 reply; 15+ messages in thread
From: Roger Sayle @ 2022-03-14 19:25 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]


I've been wondering about the possible performance/missed-optimization
impact of my patch for PR middle-end/98420 and similar IEEE correctness
fixes that disable constant folding optimizations when worrying about -0.0.
In the common situation where the floating point result is used by a
FP comparison, there's no distinction between +0.0 and -0.0, so some
HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.

Consider the following interesting example:

int foo(int x, double y) {
    return (x * 0.0) < y;
}

Although we know that x (when converted to double) can't be NaN or Inf,
we still worry that for negative values of x that (x * 0.0) may be -0.0
and so perform the multiplication at run-time.  But in this case, the
result of the comparison (-0.0 < y) will be exactly the same as (+0.0 < y)
for any y, hence the above may be safely constant folded to "0.0 < y"
avoiding the multiplication at run-time.

This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check with no new failures, and allows GCC to continue to
optimize cases that we optimized in GCC 11 (without regard to correctness).
Ok for mainline?


2022-03-14  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	* match.pd (X CMP (Y-Y) -> X CMP 0.0): New transformation.
	(X CMP (Y * 0.0) -> X CMP 0.0): Likewise.
	(X CMP X -> true): Test tree_expr_maybe_nan_p instead of HONOR_NANS.
	(X LTGT X -> false): Enable if X is not tree_expr_maybe_nan_p, as
	this can't trap/signal.

gcc/testsuite/ChangeLog
	* gcc.dg/fold-compare-9.c: New test case.


Thanks in advance,
Roger
--


[-- Attachment #2: patchrm3.txt --]
[-- Type: text/plain, Size: 2725 bytes --]

diff --git a/gcc/match.pd b/gcc/match.pd
index 8b44b5c..f3f7865 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4671,6 +4671,35 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    (if (single_use (@2))
     (cmp @0 @1)))))
 
+/* Floating point comparisons can ignore signed zeros.  */
+(for cmp (tcc_comparison)
+ /* Simplify (X - X) CMP Y even with -frounding-math.  */
+ (simplify
+  (cmp (minus @0 @0) @1)
+  (if (FLOAT_TYPE_P (TREE_TYPE (@0))
+       && !tree_expr_maybe_nan_p (@0)
+       && !tree_expr_maybe_infinite_p (@0))
+   (cmp { build_zero_cst (TREE_TYPE (@0)); } @1)))
+ /* Simplify X CMP (Y - Y) even with -frounding-math.  */
+ (simplify
+  (cmp @0 (minus @1 @1))
+  (if (FLOAT_TYPE_P (TREE_TYPE (@1))
+       && !tree_expr_maybe_nan_p (@1)
+       && !tree_expr_maybe_infinite_p (@1))
+   (cmp @0 { build_zero_cst (TREE_TYPE (@1)); })))
+ /* Simplify (X * 0.0) CMP Y.  */
+ (simplify
+  (cmp (mult @0 real_zerop@1) @2)
+  (if (!tree_expr_maybe_nan_p (@0)
+       && !tree_expr_maybe_infinite_p (@0))
+   (cmp @1 @2)))
+ /* Simplify X CMP (Y * 0.0).  */
+ (simplify
+  (cmp @0 (mult @1 real_zerop@2))
+  (if (!tree_expr_maybe_nan_p (@1)
+       && !tree_expr_maybe_infinite_p (@0))
+   (cmp @0 @2))))
+
 /* Simplify (x < 0) ^ (y < 0) to (x ^ y) < 0 and
    (x >= 0) ^ (y >= 0) to (x ^ y) < 0.  */
 (for cmp (lt ge)
@@ -4743,7 +4772,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (simplify
   (cmp @0 @0)
   (if (! FLOAT_TYPE_P (TREE_TYPE (@0))
-       || ! HONOR_NANS (@0))
+       || ! tree_expr_maybe_nan_p (@0))
    { constant_boolean_node (true, type); }
    (if (cmp != EQ_EXPR
 	/* With -ftrapping-math conversion to EQ loses an exception.  */
@@ -4755,7 +4784,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (cmp @0 @0)
   (if (cmp != NE_EXPR
        || ! FLOAT_TYPE_P (TREE_TYPE (@0))
-       || ! HONOR_NANS (@0))
+       || ! tree_expr_maybe_nan_p (@0))
    { constant_boolean_node (false, type); })))
 (for cmp (unle unge uneq)
  (simplify
@@ -4767,7 +4796,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (unordered @0 @0)))
 (simplify
  (ltgt @0 @0)
- (if (!flag_trapping_math)
+ (if (!flag_trapping_math || !tree_expr_maybe_nan_p (@0))
   { constant_boolean_node (false, type); }))
 
 /* x == ~x -> false */
diff --git a/gcc/testsuite/gcc.dg/fold-compare-9.c b/gcc/testsuite/gcc.dg/fold-compare-9.c
new file mode 100644
index 0000000..e56f63d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-compare-9.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+int foo(int x, double y) {
+    return (x * 0.0) < y;
+}
+
+/* { dg-final { scan-tree-dump " y_\[0-9\]\\(D\\) > 0.0" "optimized" } } */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-14 19:25 [PATCH] Ignore (possible) signed zeros in operands of FP comparisons Roger Sayle
@ 2022-03-15  7:29 ` Richard Biener
  2022-03-15  8:03   ` Roger Sayle
  2022-03-16  9:44   ` Richard Sandiford
  0 siblings, 2 replies; 15+ messages in thread
From: Richard Biener @ 2022-03-15  7:29 UTC (permalink / raw)
  To: Roger Sayle; +Cc: GCC Patches

On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> I've been wondering about the possible performance/missed-optimization
> impact of my patch for PR middle-end/98420 and similar IEEE correctness
> fixes that disable constant folding optimizations when worrying about -0.0.
> In the common situation where the floating point result is used by a
> FP comparison, there's no distinction between +0.0 and -0.0, so some
> HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
>
> Consider the following interesting example:
>
> int foo(int x, double y) {
>     return (x * 0.0) < y;
> }
>
> Although we know that x (when converted to double) can't be NaN or Inf,
> we still worry that for negative values of x that (x * 0.0) may be -0.0
> and so perform the multiplication at run-time.  But in this case, the
> result of the comparison (-0.0 < y) will be exactly the same as (+0.0 < y)
> for any y, hence the above may be safely constant folded to "0.0 < y"
> avoiding the multiplication at run-time.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check with no new failures, and allows GCC to continue to
> optimize cases that we optimized in GCC 11 (without regard to correctness).
> Ok for mainline?

Isn't that something that gimple-ssa-backprop.c is designed to handle?  I wonder
if you can see whether the signed zero speciality can be retrofitted there?
It currently tracks "sign does not matter", so possibly another state,
"sign of zero
does not matter" could be introduced there.

Thanks,
Richard.

>
> 2022-03-14  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * match.pd (X CMP (Y-Y) -> X CMP 0.0): New transformation.
>         (X CMP (Y * 0.0) -> X CMP 0.0): Likewise.
>         (X CMP X -> true): Test tree_expr_maybe_nan_p instead of HONOR_NANS.
>         (X LTGT X -> false): Enable if X is not tree_expr_maybe_nan_p, as
>         this can't trap/signal.
>
> gcc/testsuite/ChangeLog
>         * gcc.dg/fold-compare-9.c: New test case.
>
>
> Thanks in advance,
> Roger
> --
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-15  7:29 ` Richard Biener
@ 2022-03-15  8:03   ` Roger Sayle
  2022-03-15 10:50     ` Richard Biener
  2022-03-17 23:27     ` Jeff Law
  2022-03-16  9:44   ` Richard Sandiford
  1 sibling, 2 replies; 15+ messages in thread
From: Roger Sayle @ 2022-03-15  8:03 UTC (permalink / raw)
  To: 'Richard Biener'; +Cc: 'GCC Patches'


> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: 15 March 2022 07:29
> To: Roger Sayle <roger@nextmovesoftware.com>
> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
> comparisons.
> 
> On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
> <roger@nextmovesoftware.com> wrote:
> >
> >
> > I've been wondering about the possible performance/missed-optimization
> > impact of my patch for PR middle-end/98420 and similar IEEE
> > correctness fixes that disable constant folding optimizations when worrying
> about -0.0.
> > In the common situation where the floating point result is used by a
> > FP comparison, there's no distinction between +0.0 and -0.0, so some
> > HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
> >
> > Consider the following interesting example:
> >
> > int foo(int x, double y) {
> >     return (x * 0.0) < y;
> > }
> >
> > Although we know that x (when converted to double) can't be NaN or
> > Inf, we still worry that for negative values of x that (x * 0.0) may
> > be -0.0 and so perform the multiplication at run-time.  But in this
> > case, the result of the comparison (-0.0 < y) will be exactly the same
> > as (+0.0 < y) for any y, hence the above may be safely constant folded to "0.0 <
> y"
> > avoiding the multiplication at run-time.
> >
> > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> > and make -k check with no new failures, and allows GCC to continue to
> > optimize cases that we optimized in GCC 11 (without regard to correctness).
> > Ok for mainline?
> 
> Isn't that something that gimple-ssa-backprop.c is designed to handle?  I wonder
> if you can see whether the signed zero speciality can be retrofitted there?
> It currently tracks "sign does not matter", so possibly another state, "sign of
> zero does not matter" could be introduced there.

Two questions. Would adding tracking of "sign of zero does not matter" to
gimple-ssa-backprop.c be suitable for stage4?  Secondly, even if gimple-ssa-backprop.c
performed this kind of optimization, would that be a reason not to support
these transformations in match.pd?  Perhaps someone could open a missed
optimization PR for backprop in Bugzilla, but the above patch still needs to be
reviewed on its own merits.

Speaking of tree-ssa passes that could be improved, I was wondering whether
you could review my EVRP patch to fix regression PR/102950.  Pretty please?
https://gcc.gnu.org/pipermail/gcc-patches/2022-February/589569.html

Thanks (as always),
Roger

> Thanks,
> Richard.
> 
> >
> > 2022-03-14  Roger Sayle  <roger@nextmovesoftware.com>
> >
> > gcc/ChangeLog
> >         * match.pd (X CMP (Y-Y) -> X CMP 0.0): New transformation.
> >         (X CMP (Y * 0.0) -> X CMP 0.0): Likewise.
> >         (X CMP X -> true): Test tree_expr_maybe_nan_p instead of
> HONOR_NANS.
> >         (X LTGT X -> false): Enable if X is not tree_expr_maybe_nan_p, as
> >         this can't trap/signal.
> >
> > gcc/testsuite/ChangeLog
> >         * gcc.dg/fold-compare-9.c: New test case.
> >
> >
> > Thanks in advance,
> > Roger
> > --
> >


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-15  8:03   ` Roger Sayle
@ 2022-03-15 10:50     ` Richard Biener
  2022-03-17 23:27     ` Jeff Law
  1 sibling, 0 replies; 15+ messages in thread
From: Richard Biener @ 2022-03-15 10:50 UTC (permalink / raw)
  To: Roger Sayle; +Cc: GCC Patches

On Tue, Mar 15, 2022 at 9:03 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> > -----Original Message-----
> > From: Richard Biener <richard.guenther@gmail.com>
> > Sent: 15 March 2022 07:29
> > To: Roger Sayle <roger@nextmovesoftware.com>
> > Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> > Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
> > comparisons.
> >
> > On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
> > <roger@nextmovesoftware.com> wrote:
> > >
> > >
> > > I've been wondering about the possible performance/missed-optimization
> > > impact of my patch for PR middle-end/98420 and similar IEEE
> > > correctness fixes that disable constant folding optimizations when worrying
> > about -0.0.
> > > In the common situation where the floating point result is used by a
> > > FP comparison, there's no distinction between +0.0 and -0.0, so some
> > > HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
> > >
> > > Consider the following interesting example:
> > >
> > > int foo(int x, double y) {
> > >     return (x * 0.0) < y;
> > > }
> > >
> > > Although we know that x (when converted to double) can't be NaN or
> > > Inf, we still worry that for negative values of x that (x * 0.0) may
> > > be -0.0 and so perform the multiplication at run-time.  But in this
> > > case, the result of the comparison (-0.0 < y) will be exactly the same
> > > as (+0.0 < y) for any y, hence the above may be safely constant folded to "0.0 <
> > y"
> > > avoiding the multiplication at run-time.
> > >
> > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> > > and make -k check with no new failures, and allows GCC to continue to
> > > optimize cases that we optimized in GCC 11 (without regard to correctness).
> > > Ok for mainline?
> >
> > Isn't that something that gimple-ssa-backprop.c is designed to handle?  I wonder
> > if you can see whether the signed zero speciality can be retrofitted there?
> > It currently tracks "sign does not matter", so possibly another state, "sign of
> > zero does not matter" could be introduced there.
>
> Two questions. Would adding tracking of "sign of zero does not matter" to
> gimple-ssa-backprop.c be suitable for stage4?

Probably not.

>  Secondly, even if gimple-ssa-backprop.c
> performed this kind of optimization, would that be a reason not to support
> these transformations in match.pd?

The only reason would be to avoid growing match.pd with lots of special
patterns for cases that should rarely matter in practice.  For example the
pattern at hand wouldn't trigger for (x * 0.0) * z < y which is why I thought
of backprop.  Yes, we do have match.pd patterns with similar issues already.

Basically when the pattern doesn't simplify the outermost expression it
is prone to such issues.

> Perhaps someone could open a missed
> optimization PR for backprop in Bugzilla, but the above patch still needs to be
> reviewed on its own merits.

There's a few other pieces in the patch (didn't look at it before), changing
HONOR_NANS and ltgt, those are OK independently.

One comment, instead of matching both

 (cmp (mult ...) @2)

and

  (cmp @2 (mult ..))

you can use :c on the 'cmp' - it will do the "right" thing (swap the
comparison code)
when matching the other way around.  That will reduce repetition.

>
> Speaking of tree-ssa passes that could be improved, I was wondering whether
> you could review my EVRP patch to fix regression PR/102950.  Pretty please?
> https://gcc.gnu.org/pipermail/gcc-patches/2022-February/589569.html

I've left this to the ranger folks - you may want to ping Andrew here.

Richard.

> Thanks (as always),
> Roger
>
> > Thanks,
> > Richard.
> >
> > >
> > > 2022-03-14  Roger Sayle  <roger@nextmovesoftware.com>
> > >
> > > gcc/ChangeLog
> > >         * match.pd (X CMP (Y-Y) -> X CMP 0.0): New transformation.
> > >         (X CMP (Y * 0.0) -> X CMP 0.0): Likewise.
> > >         (X CMP X -> true): Test tree_expr_maybe_nan_p instead of
> > HONOR_NANS.
> > >         (X LTGT X -> false): Enable if X is not tree_expr_maybe_nan_p, as
> > >         this can't trap/signal.
> > >
> > > gcc/testsuite/ChangeLog
> > >         * gcc.dg/fold-compare-9.c: New test case.
> > >
> > >
> > > Thanks in advance,
> > > Roger
> > > --
> > >
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-15  7:29 ` Richard Biener
  2022-03-15  8:03   ` Roger Sayle
@ 2022-03-16  9:44   ` Richard Sandiford
  1 sibling, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2022-03-16  9:44 UTC (permalink / raw)
  To: Richard Biener via Gcc-patches; +Cc: Roger Sayle, Richard Biener

Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>> I've been wondering about the possible performance/missed-optimization
>> impact of my patch for PR middle-end/98420 and similar IEEE correctness
>> fixes that disable constant folding optimizations when worrying about -0.0.
>> In the common situation where the floating point result is used by a
>> FP comparison, there's no distinction between +0.0 and -0.0, so some
>> HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
>>
>> Consider the following interesting example:
>>
>> int foo(int x, double y) {
>>     return (x * 0.0) < y;
>> }
>>
>> Although we know that x (when converted to double) can't be NaN or Inf,
>> we still worry that for negative values of x that (x * 0.0) may be -0.0
>> and so perform the multiplication at run-time.  But in this case, the
>> result of the comparison (-0.0 < y) will be exactly the same as (+0.0 < y)
>> for any y, hence the above may be safely constant folded to "0.0 < y"
>> avoiding the multiplication at run-time.
>>
>> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
>> and make -k check with no new failures, and allows GCC to continue to
>> optimize cases that we optimized in GCC 11 (without regard to correctness).
>> Ok for mainline?
>
> Isn't that something that gimple-ssa-backprop.c is designed to handle?  I wonder
> if you can see whether the signed zero speciality can be retrofitted there?
> It currently tracks "sign does not matter", so possibly another state,
> "sign of zero
> does not matter" could be introduced there.

I agree that would be a nice thing to have FWIW.  gimple-ssa-backprop.c
was added to avoid regressions in one specific fold-const -> match.pd
move, but it was supposed to be suitable for other similar things
in future.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-15  8:03   ` Roger Sayle
  2022-03-15 10:50     ` Richard Biener
@ 2022-03-17 23:27     ` Jeff Law
  2022-03-18  2:12       ` Andrew MacLeod
                         ` (2 more replies)
  1 sibling, 3 replies; 15+ messages in thread
From: Jeff Law @ 2022-03-17 23:27 UTC (permalink / raw)
  To: Roger Sayle, 'Richard Biener'; +Cc: 'GCC Patches'


On 3/15/2022 2:03 AM, Roger Sayle wrote:
>> -----Original Message-----
>> From: Richard Biener <richard.guenther@gmail.com>
>> Sent: 15 March 2022 07:29
>> To: Roger Sayle <roger@nextmovesoftware.com>
>> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
>> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
>> comparisons.
>>
>> On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
>> <roger@nextmovesoftware.com> wrote:
>>>
>>> I've been wondering about the possible performance/missed-optimization
>>> impact of my patch for PR middle-end/98420 and similar IEEE
>>> correctness fixes that disable constant folding optimizations when worrying
>> about -0.0.
>>> In the common situation where the floating point result is used by a
>>> FP comparison, there's no distinction between +0.0 and -0.0, so some
>>> HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
>>>
>>> Consider the following interesting example:
>>>
>>> int foo(int x, double y) {
>>>      return (x * 0.0) < y;
>>> }
>>>
>>> Although we know that x (when converted to double) can't be NaN or
>>> Inf, we still worry that for negative values of x that (x * 0.0) may
>>> be -0.0 and so perform the multiplication at run-time.  But in this
>>> case, the result of the comparison (-0.0 < y) will be exactly the same
>>> as (+0.0 < y) for any y, hence the above may be safely constant folded to "0.0 <
>> y"
>>> avoiding the multiplication at run-time.
>>>
>>> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
>>> and make -k check with no new failures, and allows GCC to continue to
>>> optimize cases that we optimized in GCC 11 (without regard to correctness).
>>> Ok for mainline?
>> Isn't that something that gimple-ssa-backprop.c is designed to handle?  I wonder
>> if you can see whether the signed zero speciality can be retrofitted there?
>> It currently tracks "sign does not matter", so possibly another state, "sign of
>> zero does not matter" could be introduced there.
> Two questions. Would adding tracking of "sign of zero does not matter" to
> gimple-ssa-backprop.c be suitable for stage4?  Secondly, even if gimple-ssa-backprop.c
> performed this kind of optimization, would that be a reason not to support
> these transformations in match.pd?  Perhaps someone could open a missed
> optimization PR for backprop in Bugzilla, but the above patch still needs to be
> reviewed on its own merits.

Can't see how it's appropriate for stage4, but definitely interesting 
for gcc-13.

It'd fit well into some of the Ranger plans too -- Aldy and Andrew have 
been talking about tracking the special FP values in Ranger.   This is 
related, though not exactly the same since rather than tracking the 
special value, you're tracking if those special values actually matter.  
If you're going to do more work in this space, you might want to reach 
out to Aldy and Andrew to see if there's space for collaboration.


>
> Speaking of tree-ssa passes that could be improved, I was wondering whether
> you could review my EVRP patch to fix regression PR/102950.  Pretty please?
> https://gcc.gnu.org/pipermail/gcc-patches/2022-February/589569.html

I forwarded this to Aldy & Andrew.  I suspect they missed it.


>
> Thanks (as always),

No, thank you.  I'm so happy to see you contributing to GCC regularly again!


Jeff

>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-17 23:27     ` Jeff Law
@ 2022-03-18  2:12       ` Andrew MacLeod
  2022-03-18  7:43       ` Roger Sayle
  2022-03-18 13:16       ` Andrew MacLeod
  2 siblings, 0 replies; 15+ messages in thread
From: Andrew MacLeod @ 2022-03-18  2:12 UTC (permalink / raw)
  To: Jeff Law, Roger Sayle, 'Richard Biener', Jakub Jelinek
  Cc: 'GCC Patches'

On 3/17/22 19:27, Jeff Law via Gcc-patches wrote:
>
> On 3/15/2022 2:03 AM, Roger Sayle wrote:
>
>
>>
>> Speaking of tree-ssa passes that could be improved, I was wondering 
>> whether
>> you could review my EVRP patch to fix regression PR/102950. Pretty 
>> please?
>> https://gcc.gnu.org/pipermail/gcc-patches/2022-February/589569.html
>
> I forwarded this to Aldy & Andrew.  I suspect they missed it.
>
>
I saw it originally, and was happy to not have to figure out the bits 
myself :-)  The more people pitching in to range-ops the better!!

I'm certainly fine with it, but the gory details of the bit twiddling it 
beyond my scope of expertise. Figured Jakub or richi were better shots 
at that part.

Andrew




^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-17 23:27     ` Jeff Law
  2022-03-18  2:12       ` Andrew MacLeod
@ 2022-03-18  7:43       ` Roger Sayle
  2022-03-18 13:07         ` Andrew MacLeod
  2022-03-18 13:16       ` Andrew MacLeod
  2 siblings, 1 reply; 15+ messages in thread
From: Roger Sayle @ 2022-03-18  7:43 UTC (permalink / raw)
  To: 'Jeff Law', 'Andrew MacLeod'
  Cc: 'GCC Patches', 'Richard Biener'


Hi Jeff/Andrew,
> If you're going to do more work in this space, you might want to reach out to
> Aldy and Andrew to see if there's space for collaboration.

One (clever?) suggestion that I do have for ranger would be to add support for
an additional value_range_kind, VR_NONZEROBITS, which would be a variant of
VR_RANGE (for unsigned types?) and require very few changes to the existing
code.  Just like VR_RANGE all values would lie in [MIN, MAX], so by default
treating this value_range_kind identically to VR_RANGE there should be no
visible changes, but the change in semantics is that MIN has the minimum bits
set, and MAX, the maximum bits set [equivalent to the RVAL and RMASK pairs
from CCP's bit_value_{bin,un}op].  Hence, the VR_NONZEROBITS range [2,7]
would represent the possible values {2, 3, 6, 7} rather than {2, 3, 4, 5, 6, 7}. 
For a small number of bits, int_range can already handle this with multiple
irange spans, but adding this representation would allow the unification of the
bit-based propagation performed in tree-ssa-ccp with the range-value based
propagation performed in EVRP/ranger, allowing the clever forwards/backwards
functionality.

As Andrew's recent (partial) review points out, tracking the effect of operations
like BIT_XOR_EXPR on VR_RANGE is much more complicated than on the
proposed VR_NONZEROBITS.

Alas, I'm not the sort of contributor to make large infrastructure changes
myself, but if the above functionality were in place, I/the compiler would
be able to make use of it.

Cheers,
Roger
--

> -----Original Message-----
> From: Jeff Law <jeffreyalaw@gmail.com>
> Sent: 17 March 2022 23:28
> To: Roger Sayle <roger@nextmovesoftware.com>; 'Richard Biener'
> <richard.guenther@gmail.com>
> Cc: 'GCC Patches' <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
> comparisons.
> 
> 
> On 3/15/2022 2:03 AM, Roger Sayle wrote:
> >> -----Original Message-----
> >> From: Richard Biener <richard.guenther@gmail.com>
> >> Sent: 15 March 2022 07:29
> >> To: Roger Sayle <roger@nextmovesoftware.com>
> >> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> >> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
> >> comparisons.
> >>
> >> On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
> >> <roger@nextmovesoftware.com> wrote:
> >>>
> >>> I've been wondering about the possible
> >>> performance/missed-optimization impact of my patch for PR
> >>> middle-end/98420 and similar IEEE correctness fixes that disable
> >>> constant folding optimizations when worrying
> >> about -0.0.
> >>> In the common situation where the floating point result is used by a
> >>> FP comparison, there's no distinction between +0.0 and -0.0, so some
> >>> HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
> >>>
> >>> Consider the following interesting example:
> >>>
> >>> int foo(int x, double y) {
> >>>      return (x * 0.0) < y;
> >>> }
> >>>
> >>> Although we know that x (when converted to double) can't be NaN or
> >>> Inf, we still worry that for negative values of x that (x * 0.0) may
> >>> be -0.0 and so perform the multiplication at run-time.  But in this
> >>> case, the result of the comparison (-0.0 < y) will be exactly the
> >>> same as (+0.0 < y) for any y, hence the above may be safely constant
> >>> folded to "0.0 <
> >> y"
> >>> avoiding the multiplication at run-time.
> >>>
> >>> This patch has been tested on x86_64-pc-linux-gnu with make
> >>> bootstrap and make -k check with no new failures, and allows GCC to
> >>> continue to optimize cases that we optimized in GCC 11 (without regard to
> correctness).
> >>> Ok for mainline?
> >> Isn't that something that gimple-ssa-backprop.c is designed to
> >> handle?  I wonder if you can see whether the signed zero speciality can be
> retrofitted there?
> >> It currently tracks "sign does not matter", so possibly another
> >> state, "sign of zero does not matter" could be introduced there.
> > Two questions. Would adding tracking of "sign of zero does not matter"
> > to gimple-ssa-backprop.c be suitable for stage4?  Secondly, even if
> > gimple-ssa-backprop.c performed this kind of optimization, would that
> > be a reason not to support these transformations in match.pd?  Perhaps
> > someone could open a missed optimization PR for backprop in Bugzilla,
> > but the above patch still needs to be reviewed on its own merits.
> 
> Can't see how it's appropriate for stage4, but definitely interesting for gcc-13.
> 
> It'd fit well into some of the Ranger plans too -- Aldy and Andrew have been
> talking about tracking the special FP values in Ranger.   This is related, though
> not exactly the same since rather than tracking the special value, you're tracking
> if those special values actually matter. If you're going to do more work in this
> space, you might want to reach out to Aldy and Andrew to see if there's space
> for collaboration.
> 
> 
> >
> > Speaking of tree-ssa passes that could be improved, I was wondering
> > whether you could review my EVRP patch to fix regression PR/102950.  Pretty
> please?
> > https://gcc.gnu.org/pipermail/gcc-patches/2022-February/589569.html
> 
> I forwarded this to Aldy & Andrew.  I suspect they missed it.
> 
> 
> >
> > Thanks (as always),
> 
> No, thank you.  I'm so happy to see you contributing to GCC regularly again!
> 
> 
> Jeff
> 
> >


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-18  7:43       ` Roger Sayle
@ 2022-03-18 13:07         ` Andrew MacLeod
  2022-03-18 18:07           ` Aldy Hernandez
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew MacLeod @ 2022-03-18 13:07 UTC (permalink / raw)
  To: Roger Sayle, 'Jeff Law'
  Cc: 'GCC Patches', 'Richard Biener', Aldy Hernandez

On 3/18/22 03:43, Roger Sayle wrote:
> Hi Jeff/Andrew,
>> If you're going to do more work in this space, you might want to reach out to
>> Aldy and Andrew to see if there's space for collaboration.
> One (clever?) suggestion that I do have for ranger would be to add support for
> an additional value_range_kind, VR_NONZEROBITS, which would be a variant of
> VR_RANGE (for unsigned types?) and require very few changes to the existing


I think were ahead of you here.. Tracking known zero and one bits within 
irange as an adjunct has been in plan for awhile, just priorities 
haven't allowed us to get to it until recently...

Theres a bunch of stuff already in the hopper for the next stage1 that 
Aldy has been working with... he can expound upon it, but we plan to use 
both masks and ranges together  as appropriate.


> code.  Just like VR_RANGE all values would lie in [MIN, MAX], so by default
> treating this value_range_kind identically to VR_RANGE there should be no
> visible changes, but the change in semantics is that MIN has the minimum bits
> set, and MAX, the maximum bits set [equivalent to the RVAL and RMASK pairs
> from CCP's bit_value_{bin,un}op].  Hence, the VR_NONZEROBITS range [2,7]
> would represent the possible values {2, 3, 6, 7} rather than {2, 3, 4, 5, 6, 7}.
> For a small number of bits, int_range can already handle this with multiple
> irange spans, but adding this representation would allow the unification of the
> bit-based propagation performed in tree-ssa-ccp with the range-value based
> propagation performed in EVRP/ranger, allowing the clever forwards/backwards
> functionality.
>
> As Andrew's recent (partial) review points out, tracking the effect of operations
> like BIT_XOR_EXPR on VR_RANGE is much more complicated than on the
> proposed VR_NONZEROBITS.
>
> Alas, I'm not the sort of contributor to make large infrastructure changes
> myself, but if the above functionality were in place, I/the compiler would
> be able to make use of it.
>

And this is exactly what we are hoping.. we provide the structure and 
someone who is better at the underlying detail interaction can flesh out 
whatever specifics they find interesting.


Andrew


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-17 23:27     ` Jeff Law
  2022-03-18  2:12       ` Andrew MacLeod
  2022-03-18  7:43       ` Roger Sayle
@ 2022-03-18 13:16       ` Andrew MacLeod
  2022-03-18 16:01         ` Jeff Law
  2 siblings, 1 reply; 15+ messages in thread
From: Andrew MacLeod @ 2022-03-18 13:16 UTC (permalink / raw)
  To: Jeff Law, Roger Sayle, 'Richard Biener', Aldy Hernandez
  Cc: 'GCC Patches'

On 3/17/22 19:27, Jeff Law via Gcc-patches wrote:
>
> On 3/15/2022 2:03 AM, Roger Sayle wrote:
>>> -----Original Message-----
>>> From: Richard Biener <richard.guenther@gmail.com>
>>> Sent: 15 March 2022 07:29
>>> To: Roger Sayle <roger@nextmovesoftware.com>
>>> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
>>> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
>>> comparisons.
>>>
>>> On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
>>> <roger@nextmovesoftware.com> wrote:
>>>>
>>>> I've been wondering about the possible performance/missed-optimization
>>>> impact of my patch for PR middle-end/98420 and similar IEEE
>>>> correctness fixes that disable constant folding optimizations when 
>>>> worrying
>>> about -0.0.
>>>> In the common situation where the floating point result is used by a
>>>> FP comparison, there's no distinction between +0.0 and -0.0, so some
>>>> HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
>>>>
>>>> Consider the following interesting example:
>>>>
>>>> int foo(int x, double y) {
>>>>      return (x * 0.0) < y;
>>>> }
>>>>
>>>> Although we know that x (when converted to double) can't be NaN or
>>>> Inf, we still worry that for negative values of x that (x * 0.0) may
>>>> be -0.0 and so perform the multiplication at run-time.  But in this
>>>> case, the result of the comparison (-0.0 < y) will be exactly the same
>>>> as (+0.0 < y) for any y, hence the above may be safely constant 
>>>> folded to "0.0 <
>>> y"
>>>> avoiding the multiplication at run-time.

I'm going to hazard a guess that this can be handled in the upcoming 
floating point range support?  there was a start of a conversation in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24021 about a month ago.

I know *zero* about floating point, but It seems like when we track 
floating point ranges, we will naturally be able to integrate

   _2 = _1 * 0.0;
   _3 = _2 < y_5(D);

that _2 evaluates to +/- 0.0  and when we do look at  (_2 < y_5)   that 
VRP  can simplify that to 0.0 < y?  (or any patch which uses 
simplification and ranges).    It seems like it should be 
straightforward anyway.

Andrew


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-18 13:16       ` Andrew MacLeod
@ 2022-03-18 16:01         ` Jeff Law
  2022-03-18 18:33           ` Aldy Hernandez
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Law @ 2022-03-18 16:01 UTC (permalink / raw)
  To: Andrew MacLeod, Roger Sayle, 'Richard Biener', Aldy Hernandez
  Cc: 'GCC Patches'



On 3/18/2022 7:16 AM, Andrew MacLeod wrote:
> On 3/17/22 19:27, Jeff Law via Gcc-patches wrote:
>>
>> On 3/15/2022 2:03 AM, Roger Sayle wrote:
>>>> -----Original Message-----
>>>> From: Richard Biener <richard.guenther@gmail.com>
>>>> Sent: 15 March 2022 07:29
>>>> To: Roger Sayle <roger@nextmovesoftware.com>
>>>> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
>>>> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
>>>> comparisons.
>>>>
>>>> On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
>>>> <roger@nextmovesoftware.com> wrote:
>>>>>
>>>>> I've been wondering about the possible 
>>>>> performance/missed-optimization
>>>>> impact of my patch for PR middle-end/98420 and similar IEEE
>>>>> correctness fixes that disable constant folding optimizations when 
>>>>> worrying
>>>> about -0.0.
>>>>> In the common situation where the floating point result is used by a
>>>>> FP comparison, there's no distinction between +0.0 and -0.0, so some
>>>>> HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
>>>>>
>>>>> Consider the following interesting example:
>>>>>
>>>>> int foo(int x, double y) {
>>>>>      return (x * 0.0) < y;
>>>>> }
>>>>>
>>>>> Although we know that x (when converted to double) can't be NaN or
>>>>> Inf, we still worry that for negative values of x that (x * 0.0) may
>>>>> be -0.0 and so perform the multiplication at run-time. But in this
>>>>> case, the result of the comparison (-0.0 < y) will be exactly the 
>>>>> same
>>>>> as (+0.0 < y) for any y, hence the above may be safely constant 
>>>>> folded to "0.0 <
>>>> y"
>>>>> avoiding the multiplication at run-time.
>
> I'm going to hazard a guess that this can be handled in the upcoming 
> floating point range support?  there was a start of a conversation in 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24021 about a month ago.
>
> I know *zero* about floating point, but It seems like when we track 
> floating point ranges, we will naturally be able to integrate
>
>   _2 = _1 * 0.0;
>   _3 = _2 < y_5(D);
>
> that _2 evaluates to +/- 0.0  and when we do look at  (_2 < y_5)   
> that VRP  can simplify that to 0.0 < y?  (or any patch which uses 
> simplification and ranges).    It seems like it should be 
> straightforward anyway.
Yea, I guess we'd pick it up that way and that's probably cleaner than 
what I was originally thinking in this space.

We realize that 2 is +-0.0.  Then we realize that for the comparison, we 
can constant propagate +0.0 for _2 since +0.0 and -0.0 behave the same 
way.  Ideally that removes the last reference to _2 and we DCE away the 
multiplication.


jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-18 13:07         ` Andrew MacLeod
@ 2022-03-18 18:07           ` Aldy Hernandez
  0 siblings, 0 replies; 15+ messages in thread
From: Aldy Hernandez @ 2022-03-18 18:07 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: Roger Sayle, Jeff Law, GCC Patches, Richard Biener

On Fri, Mar 18, 2022 at 2:07 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>
> On 3/18/22 03:43, Roger Sayle wrote:
> > Hi Jeff/Andrew,
> >> If you're going to do more work in this space, you might want to reach out to
> >> Aldy and Andrew to see if there's space for collaboration.
> > One (clever?) suggestion that I do have for ranger would be to add support for
> > an additional value_range_kind, VR_NONZEROBITS, which would be a variant of
> > VR_RANGE (for unsigned types?) and require very few changes to the existing
>
>
> I think were ahead of you here.. Tracking known zero and one bits within
> irange as an adjunct has been in plan for awhile, just priorities
> haven't allowed us to get to it until recently...
>
> Theres a bunch of stuff already in the hopper for the next stage1 that
> Aldy has been working with... he can expound upon it, but we plan to use
> both masks and ranges together  as appropriate.

Yes, for the next stage1 I have patches queued up to provide nonzero
bits within the irange object.

In working on providing a global range storage (an irange replacement
for SSA_NAME_RANGE_INFO) for the next release, it became clear that
nonzero and ranges are probably best suited to live in the same space.
The current global range mechanism already does this-- by refining
nonzero bits with each change to the range for an SSA name.  However,
we sometimes pessimize global ranges, presumably because we couldn´t
properly represent an intersection with our value_range anti-range
business.  Also, this nonzero+range symbiosis only currently works for
global ranges (not iranges or value_ranges).  In the upcoming work,
the nonzero bits will live in irange, and work in tandem-- for
example, sometimes refining the range when a new nonzero mask is
available (0xff is really [0,255]) etc.  I have the core
infrastructure done.  I could probably use range-op help from the
relevant experts when the time comes.

And yes, as Roger points out, the mask tracking bits in CCP could play
very well with the ranger world.  For that matter, while I was doing
the above work I noticed that many of the bitmasks that CCP could
determine, would eventually be figured out by evrp, just in the form
of ranges.  Having nonzero bits in irange opens up all sorts of
possibilities.

>
>
> > code.  Just like VR_RANGE all values would lie in [MIN, MAX], so by default
> > treating this value_range_kind identically to VR_RANGE there should be no
> > visible changes, but the change in semantics is that MIN has the minimum bits
> > set, and MAX, the maximum bits set [equivalent to the RVAL and RMASK pairs
> > from CCP's bit_value_{bin,un}op].  Hence, the VR_NONZEROBITS range [2,7]
> > would represent the possible values {2, 3, 6, 7} rather than {2, 3, 4, 5, 6, 7}.
> > For a small number of bits, int_range can already handle this with multiple
> > irange spans, but adding this representation would allow the unification of the
> > bit-based propagation performed in tree-ssa-ccp with the range-value based
> > propagation performed in EVRP/ranger, allowing the clever forwards/backwards
> > functionality.
> >
> > As Andrew's recent (partial) review points out, tracking the effect of operations
> > like BIT_XOR_EXPR on VR_RANGE is much more complicated than on the
> > proposed VR_NONZEROBITS.
> >
> > Alas, I'm not the sort of contributor to make large infrastructure changes
> > myself, but if the above functionality were in place, I/the compiler would
> > be able to make use of it.
> >
>
> And this is exactly what we are hoping.. we provide the structure and
> someone who is better at the underlying detail interaction can flesh out
> whatever specifics they find interesting.

Yup.  We´re on the other side of the spectrum, the ability to provide
infrastructure with little expertise in the fine details (range ops,
floats, etc). :-)

Aldy

>
>
> Andrew
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-18 16:01         ` Jeff Law
@ 2022-03-18 18:33           ` Aldy Hernandez
  2022-03-21 15:56             ` Aldy Hernandez
  0 siblings, 1 reply; 15+ messages in thread
From: Aldy Hernandez @ 2022-03-18 18:33 UTC (permalink / raw)
  To: Jeff Law; +Cc: Andrew MacLeod, Roger Sayle, Richard Biener, GCC Patches

On Fri, Mar 18, 2022 at 5:01 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 3/18/2022 7:16 AM, Andrew MacLeod wrote:
> > On 3/17/22 19:27, Jeff Law via Gcc-patches wrote:
> >>
> >> On 3/15/2022 2:03 AM, Roger Sayle wrote:
> >>>> -----Original Message-----
> >>>> From: Richard Biener <richard.guenther@gmail.com>
> >>>> Sent: 15 March 2022 07:29
> >>>> To: Roger Sayle <roger@nextmovesoftware.com>
> >>>> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> >>>> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
> >>>> comparisons.
> >>>>
> >>>> On Mon, Mar 14, 2022 at 8:26 PM Roger Sayle
> >>>> <roger@nextmovesoftware.com> wrote:
> >>>>>
> >>>>> I've been wondering about the possible
> >>>>> performance/missed-optimization
> >>>>> impact of my patch for PR middle-end/98420 and similar IEEE
> >>>>> correctness fixes that disable constant folding optimizations when
> >>>>> worrying
> >>>> about -0.0.
> >>>>> In the common situation where the floating point result is used by a
> >>>>> FP comparison, there's no distinction between +0.0 and -0.0, so some
> >>>>> HONOR_SIGNED_ZEROS optimizations that we'd usually disable, are safe.
> >>>>>
> >>>>> Consider the following interesting example:
> >>>>>
> >>>>> int foo(int x, double y) {
> >>>>>      return (x * 0.0) < y;
> >>>>> }
> >>>>>
> >>>>> Although we know that x (when converted to double) can't be NaN or
> >>>>> Inf, we still worry that for negative values of x that (x * 0.0) may
> >>>>> be -0.0 and so perform the multiplication at run-time. But in this
> >>>>> case, the result of the comparison (-0.0 < y) will be exactly the
> >>>>> same
> >>>>> as (+0.0 < y) for any y, hence the above may be safely constant
> >>>>> folded to "0.0 <
> >>>> y"
> >>>>> avoiding the multiplication at run-time.
> >
> > I'm going to hazard a guess that this can be handled in the upcoming
> > floating point range support?  there was a start of a conversation in
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24021 about a month ago.
> >
> > I know *zero* about floating point, but It seems like when we track
> > floating point ranges, we will naturally be able to integrate
> >
> >   _2 = _1 * 0.0;
> >   _3 = _2 < y_5(D);
> >
> > that _2 evaluates to +/- 0.0  and when we do look at  (_2 < y_5)
> > that VRP  can simplify that to 0.0 < y?  (or any patch which uses
> > simplification and ranges).    It seems like it should be
> > straightforward anyway.

Yes, definitely.  Ranger is really a propagation engine.  The plan all
along was to make it type agnostic with a generic vrange class that
provides core functionality that each implementation overrides (union,
intersect, varying, undefined, etc).  Right now we have plans for
float ranges, pointer tracking, and eventually string tracking.   The
generic vrange abstraction is done and queued up for the next release.

I have a core float implementation that handles relationals and end
points, but the plan is to add bits to track nonzero, infs, nans, etc.
Since ranger will just be a propagation engine, range-ops, union,
intersection, etc, could propagate not NaN and non Inf for the cast
operator above, plus +-0.0 for _2.  The float range-op entries would
be able to do all sorts of magic by querying the range for _2.  Also
the simplify_using_ranges class vrp uses could simplify the _2 < y_5
with appropriate smarts.

It really does seem pretty straightforward once the vrange and frange
classes are in place.  All this should be doable for the next stage1,
but we could use help with the nuances of frange when the time comes.

> Yea, I guess we'd pick it up that way and that's probably cleaner than
> what I was originally thinking in this space.
>
> We realize that 2 is +-0.0.  Then we realize that for the comparison, we
> can constant propagate +0.0 for _2 since +0.0 and -0.0 behave the same
> way.  Ideally that removes the last reference to _2 and we DCE away the
> multiplication.

Yup yup.

Aldy
>
>
> jeff
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-18 18:33           ` Aldy Hernandez
@ 2022-03-21 15:56             ` Aldy Hernandez
  2022-03-26 18:52               ` Roger Sayle
  0 siblings, 1 reply; 15+ messages in thread
From: Aldy Hernandez @ 2022-03-21 15:56 UTC (permalink / raw)
  To: Jeff Law; +Cc: Andrew MacLeod, Roger Sayle, Richard Biener, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]

On Fri, Mar 18, 2022 at 7:33 PM Aldy Hernandez <aldyh@redhat.com> wrote:

> > >>>>> Consider the following interesting example:
> > >>>>>
> > >>>>> int foo(int x, double y) {
> > >>>>>      return (x * 0.0) < y;
> > >>>>> }
> > >>>>>
> > >>>>> Although we know that x (when converted to double) can't be NaN or
> > >>>>> Inf, we still worry that for negative values of x that (x * 0.0) may
> > >>>>> be -0.0 and so perform the multiplication at run-time. But in this
> > >>>>> case, the result of the comparison (-0.0 < y) will be exactly the
> > >>>>> same
> > >>>>> as (+0.0 < y) for any y, hence the above may be safely constant
> > >>>>> folded to "0.0 <
> > >>>> y"
> > >>>>> avoiding the multiplication at run-time.

Ok, once the "frange" infrastructure is in place, it's actually
trivial.  See attached patch and tests.  We can do everything with
small range-op entries and evrp / ranger will handle everything else.

Roger, I believe this is what you described:

+// { dg-do compile }
+// { dg-options "-O2 -fno-tree-forwprop -fno-thread-jumps
-fdump-tree-evrp -fdump-tree-optimized" }
+
+extern void link_error ();
+
+int foo(int x, double y)
+{
+      return (x * 0.0) < y;
+}
+
+// The multiply should be gone by *vrp time.
+// { dg-final { scan-tree-dump-not " \\* " "evrp" } }
+
+// Ultimately, there sound be no references to "x".
+// { dg-final { scan-tree-dump-not "x_" "optimized" } }

With the attached patch (and pending patches), we keep track that a
cast from int to float is guaranteed to not be NaN, which allows us to
fold x*0.0 into 0.0, and DCE to remove x altogether.  Also, as the
other tests show, we are able to resolve __builtin_isnan's for the
operations implemented.  It should be straightforward for someone with
floating point foo to extend this to all operations.

Aldy

[-- Attachment #2: 0011-frange-Implement-NAN-aware-stubs-for-FLOAT_EXPR-UNOR.patch --]
[-- Type: text/x-patch, Size: 5082 bytes --]

From 2a6218e97782f63dfe9836e6024fbb28c8cbb803 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Mon, 21 Mar 2022 16:26:40 +0100
Subject: [PATCH] [frange] Implement NAN aware stubs for FLOAT_EXPR,
 UNORDERED_EXPR, and MULT_EXPR.

---
 gcc/range-op-float.cc                        | 90 ++++++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-01.c | 14 +++
 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-02.c | 14 +++
 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-03.c | 15 ++++
 4 files changed, 133 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-01.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-02.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-03.c

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 988a3938959..52aaa01ab0f 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -922,6 +922,93 @@ foperator_cst::fold_range (frange &r, tree type ATTRIBUTE_UNUSED,
   return true;
 }
 
+class foperator_cast : public range_operator
+{
+  using range_operator::fold_range;
+public:
+  virtual bool fold_range (frange &r, tree type,
+			   const irange &inner,
+			   const frange &outer,
+			   relation_kind rel = VREL_NONE) const override;
+} fop_convert;
+
+bool
+foperator_cast::fold_range (frange &r, tree type,
+			    const irange &inner,
+			    const frange &outer,
+			    relation_kind) const
+{
+  if (empty_range_varying (r, type, inner, outer))
+    return true;
+
+  r.set_varying (type);
+
+  // Some flags can be cleared when converting from ints.
+  r.clear_prop (FRANGE_PROP_NAN);
+
+  return true;
+}
+
+class foperator_unordered : public range_operator
+{
+  using range_operator::fold_range;
+public:
+  virtual bool fold_range (irange &r, tree type,
+			   const frange &lh,
+			   const frange &rh,
+			   relation_kind rel = VREL_NONE) const override;
+} fop_unordered;
+
+bool
+foperator_unordered::fold_range (irange &r, tree type,
+				 const frange &lh,
+				 const frange &rh,
+				 relation_kind) const
+{
+  if (empty_range_varying (r, type, lh, rh))
+    return true;
+
+  // Return FALSE if both operands are !NaN.
+  if (!lh.get_prop (FRANGE_PROP_NAN) && !rh.get_prop (FRANGE_PROP_NAN))
+    {
+      r = range_false (type);
+      return true;
+    }
+
+  return false;
+}
+
+class foperator_mult : public range_operator
+{
+  using range_operator::fold_range;
+public:
+  virtual bool fold_range (frange &r, tree type,
+			   const frange &lh,
+			   const frange &rh,
+			   relation_kind rel = VREL_NONE) const override;
+} fop_mult;
+
+bool
+foperator_mult::fold_range (frange &r, tree type,
+			    const frange &lh,
+			    const frange &rh,
+			    relation_kind) const
+{
+  if (empty_range_varying (r, type, lh, rh))
+    return true;
+
+  // When x is !Nan, x * 0.0 = 0.0
+  if (rh.zero_p ()
+      && !rh.get_prop (FRANGE_PROP_NAN)
+      && !lh.get_prop (FRANGE_PROP_NAN))
+    {
+      r.set_zero (type);
+      return true;
+    }
+
+  return false;
+}
+
 class floating_table : public range_op_table
 {
 public:
@@ -944,6 +1031,9 @@ floating_table::floating_table ()
   set (PAREN_EXPR, fop_identity);
   set (OBJ_TYPE_REF, fop_identity);
   set (REAL_CST, fop_real_cst);
+  set (FLOAT_EXPR, fop_convert);
+  set (UNORDERED_EXPR, fop_unordered);
+  set (MULT_EXPR, fop_mult);
 }
 
 #if CHECKING_P
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-01.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-01.c
new file mode 100644
index 00000000000..92af87091a8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-01.c
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-O2 -fno-tree-forwprop -fno-tree-ccp -fno-tree-fre -fdump-tree-evrp" }
+
+extern void link_error ();
+
+void
+foo ()
+{
+  float z = 0.0;
+  if (__builtin_isnan (z))
+    link_error ();
+}
+
+// { dg-final { scan-tree-dump-not "link_error" "evrp" } }
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-02.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-02.c
new file mode 100644
index 00000000000..d38ea523bea
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-02.c
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-O2 -fno-tree-forwprop -fno-thread-jumps -fdump-tree-evrp" }
+
+extern void link_error ();
+
+void
+foo (int i)
+{
+  float z = i;
+  if (__builtin_isnan (z))
+    link_error ();
+}
+
+// { dg-final { scan-tree-dump-not "link_error" "evrp" } }
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-03.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-03.c
new file mode 100644
index 00000000000..6c26a295f94
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-03.c
@@ -0,0 +1,15 @@
+// { dg-do compile }
+// { dg-options "-O2 -fno-tree-forwprop -fno-thread-jumps -fdump-tree-evrp -fdump-tree-optimized" }
+
+extern void link_error ();
+
+int foo(int x, double y) 
+{
+      return (x * 0.0) < y;
+}
+
+// The multiply should be gone by *vrp time.
+// { dg-final { scan-tree-dump-not " \\* " "evrp" } }
+
+// Ultimately, there sound be no references to "x".
+// { dg-final { scan-tree-dump-not "x_" "optimized" } }
-- 
2.35.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] Ignore (possible) signed zeros in operands of FP comparisons.
  2022-03-21 15:56             ` Aldy Hernandez
@ 2022-03-26 18:52               ` Roger Sayle
  0 siblings, 0 replies; 15+ messages in thread
From: Roger Sayle @ 2022-03-26 18:52 UTC (permalink / raw)
  To: 'Aldy Hernandez', 'Jeff Law'
  Cc: 'Andrew MacLeod', 'Richard Biener',
	'GCC Patches'

Hi Aldy,
The proposed frange implementation looks cool.  The one technical tweak is
that if x is not NaN and not +Inf/-Inf, then x*0.0 is [-0.0,0.0].  It's because this
result is a range and not a constant that it can’t normally be constant folded,
unless it appears in a context where the sign of zero isn't important (such as
a comparison).

I suspect that frange needs pos_zero_p, neg_zero_p and any_zero_p instead
of just zero_p (and variants r.set_zero) to capture the subtleties.

Cheers,
Roger
--

> -----Original Message-----
> From: Aldy Hernandez <aldyh@redhat.com>
> Sent: 21 March 2022 05:56
> To: Jeff Law <jeffreyalaw@gmail.com>
> Cc: Andrew MacLeod <amacleod@redhat.com>; Roger Sayle
> <roger@nextmovesoftware.com>; Richard Biener
> <richard.guenther@gmail.com>; GCC Patches <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH] Ignore (possible) signed zeros in operands of FP
> comparisons.
> 
> On Fri, Mar 18, 2022 at 7:33 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> 
> > > >>>>> Consider the following interesting example:
> > > >>>>>
> > > >>>>> int foo(int x, double y) {
> > > >>>>>      return (x * 0.0) < y;
> > > >>>>> }
> > > >>>>>
> > > >>>>> Although we know that x (when converted to double) can't be
> > > >>>>> NaN or Inf, we still worry that for negative values of x that
> > > >>>>> (x * 0.0) may be -0.0 and so perform the multiplication at
> > > >>>>> run-time. But in this case, the result of the comparison (-0.0
> > > >>>>> < y) will be exactly the same as (+0.0 < y) for any y, hence
> > > >>>>> the above may be safely constant folded to "0.0 <
> > > >>>> y"
> > > >>>>> avoiding the multiplication at run-time.
> 
> Ok, once the "frange" infrastructure is in place, it's actually trivial.  See attached
> patch and tests.  We can do everything with small range-op entries and evrp /
> ranger will handle everything else.
> 
> Roger, I believe this is what you described:
> 
> +// { dg-do compile }
> +// { dg-options "-O2 -fno-tree-forwprop -fno-thread-jumps
> -fdump-tree-evrp -fdump-tree-optimized" }
> +
> +extern void link_error ();
> +
> +int foo(int x, double y)
> +{
> +      return (x * 0.0) < y;
> +}
> +
> +// The multiply should be gone by *vrp time.
> +// { dg-final { scan-tree-dump-not " \\* " "evrp" } }
> +
> +// Ultimately, there sound be no references to "x".
> +// { dg-final { scan-tree-dump-not "x_" "optimized" } }
> 
> With the attached patch (and pending patches), we keep track that a cast from
> int to float is guaranteed to not be NaN, which allows us to fold x*0.0 into 0.0,
> and DCE to remove x altogether.  Also, as the other tests show, we are able to
> resolve __builtin_isnan's for the operations implemented.  It should be
> straightforward for someone with floating point foo to extend this to all
> operations.
> 
> Aldy


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-03-26 18:52 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14 19:25 [PATCH] Ignore (possible) signed zeros in operands of FP comparisons Roger Sayle
2022-03-15  7:29 ` Richard Biener
2022-03-15  8:03   ` Roger Sayle
2022-03-15 10:50     ` Richard Biener
2022-03-17 23:27     ` Jeff Law
2022-03-18  2:12       ` Andrew MacLeod
2022-03-18  7:43       ` Roger Sayle
2022-03-18 13:07         ` Andrew MacLeod
2022-03-18 18:07           ` Aldy Hernandez
2022-03-18 13:16       ` Andrew MacLeod
2022-03-18 16:01         ` Jeff Law
2022-03-18 18:33           ` Aldy Hernandez
2022-03-21 15:56             ` Aldy Hernandez
2022-03-26 18:52               ` Roger Sayle
2022-03-16  9:44   ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).