[PATCH] Simplify floating point comparisons

[PATCH] Simplify floating point comparisons

[Wilco Dijkstra]

This patch implements some of the optimizations discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.

Simplify (C / x > 0.0) into x > 0.0.

If C is negative the comparison is reversed. 

Simplify (x * C1) > C2 into x > (C2 / C1).

Again, if C1 is negative the comparison is reversed.
Both transformations are only done with -funsafe-math-optimizations,
the constant is non-zero, and not a NaN.

OK for commit?

ChangeLog
2017-10-17  Wilco Dijkstra  <wdijkstr@arm.com>  
	    Jackson Woodruff  <jackson.woodruff@arm.com>

    gcc/
	PR 71026/tree-optimization
	* match.pd: Simplify floating point comparisons.

    gcc/testsuite/
	PR 71026/tree-optimization
	* gcc.dg/associate_comparison_1.c: New test.
--
diff --git a/gcc/match.pd b/gcc/match.pd
index e58a65af59b44a6b82ed8705f62966c5e6f251ac..cb48f079b4a310272e49cc319a1b3b0ff2023ba4 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -352,6 +352,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (rdiv @0 (rdiv:s @1 @2))
    (mult (rdiv @0 @1) @2)))
 
+(if (flag_unsafe_math_optimizations)
+  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
+  (for op (lt le gt ge)
+       neg_op (gt ge lt le)
+    (simplify
+      (op (rdiv REAL_CST@0 @1) real_zerop@2)
+      (switch
+       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
+	(op @1 @2))
+       /* For C < 0, use the inverted operator.  */
+       (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
+	(neg_op @1 @2))))))
+
 /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
 (for div (trunc_div ceil_div floor_div round_div exact_div)
  (simplify
@@ -3546,6 +3559,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        (rdiv @2 @1))
    (rdiv (op @0 @2) @1)))
 
+ (for cmp (lt le gt ge)
+      neg_cmp (gt ge lt le)
+  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
+  (simplify
+   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
+   (with
+    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
+    (if (tem)
+     (switch
+      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
+       (cmp @0 { tem; }))
+      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
+       (neg_cmp @0 { tem; })))))))
+
  /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
  (for root (SQRT CBRT)
   (simplify
diff --git a/gcc/testsuite/gcc.dg/associate_comparison_1.c b/gcc/testsuite/gcc.dg/associate_comparison_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..d051f052e13812c91cbd2d559bf2af8fae128ee1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/associate_comparison_1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized" } */
+
+int
+cmp_mul_1 (float x)
+{
+  return x * 3 <= 100;
+}
+
+int
+cmp_mul_2 (float x)
+{
+  return x * -5 > 100;
+}
+
+int
+div_cmp_1 (float x, float y)
+{
+  return x / 3 <= y;
+}
+
+int
+div_cmp_2 (float x, float y)
+{
+  return x / 3 <= 1;
+}
+
+int
+inv_cmp (float x)
+{
+  return 5 / x >= 0;
+}
+
+/* { dg-final { scan-tree-dump-not " / " "optimized" } } */

Re: [PATCH] Simplify floating point comparisons

[Prathamesh Kulkarni]

On 17 October 2017 at 21:58, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> This patch implements some of the optimizations discussed in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.
>
> Simplify (C / x > 0.0) into x > 0.0.
>
> If C is negative the comparison is reversed.
>
> Simplify (x * C1) > C2 into x > (C2 / C1).
>
> Again, if C1 is negative the comparison is reversed.
> Both transformations are only done with -funsafe-math-optimizations,
> the constant is non-zero, and not a NaN.
>
> OK for commit?
>
> ChangeLog
> 2017-10-17  Wilco Dijkstra  <wdijkstr@arm.com>
>             Jackson Woodruff  <jackson.woodruff@arm.com>
>
>     gcc/
>         PR 71026/tree-optimization
>         * match.pd: Simplify floating point comparisons.
>
>     gcc/testsuite/
>         PR 71026/tree-optimization
>         * gcc.dg/associate_comparison_1.c: New test.
> --
> diff --git a/gcc/match.pd b/gcc/match.pd
> index e58a65af59b44a6b82ed8705f62966c5e6f251ac..cb48f079b4a310272e49cc319a1b3b0ff2023ba4 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -352,6 +352,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (rdiv @0 (rdiv:s @1 @2))
>     (mult (rdiv @0 @1) @2)))
>
> +(if (flag_unsafe_math_optimizations)
> +  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
> +  (for op (lt le gt ge)
> +       neg_op (gt ge lt le)
> +    (simplify
> +      (op (rdiv REAL_CST@0 @1) real_zerop@2)
> +      (switch
> +       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
> +       (op @1 @2))
> +       /* For C < 0, use the inverted operator.  */
> +       (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
> +       (neg_op @1 @2))))))
> +
>  /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
>  (for div (trunc_div ceil_div floor_div round_div exact_div)
>   (simplify
> @@ -3546,6 +3559,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         (rdiv @2 @1))
>     (rdiv (op @0 @2) @1)))
>
> + (for cmp (lt le gt ge)
> +      neg_cmp (gt ge lt le)
> +  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
> +  (simplify
> +   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
> +   (with
> +    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
> +    (if (tem)
> +     (switch
> +      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
> +       (cmp @0 { tem; }))
> +      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
> +       (neg_cmp @0 { tem; })))))))
> +
>   /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
>   (for root (SQRT CBRT)
>    (simplify
> diff --git a/gcc/testsuite/gcc.dg/associate_comparison_1.c b/gcc/testsuite/gcc.dg/associate_comparison_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d051f052e13812c91cbd2d559bf2af8fae128ee1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/associate_comparison_1.c
> @@ -0,0 +1,34 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized" } */
> +
> +int
> +cmp_mul_1 (float x)
> +{
> +  return x * 3 <= 100;
> +}
> +
> +int
> +cmp_mul_2 (float x)
> +{
> +  return x * -5 > 100;
> +}
> +
> +int
> +div_cmp_1 (float x, float y)
> +{
> +  return x / 3 <= y;
> +}
> +
> +int
> +div_cmp_2 (float x, float y)
> +{
> +  return x / 3 <= 1;
> +}
> +
> +int
> +inv_cmp (float x)
> +{
> +  return 5 / x >= 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-not " / " "optimized" } } */
Hi Wilco,
Since the test checks for div operator, I wonder if it would be better
to scan for rdiv_expr in raw optimized dump instead ?

Thanks,
Prathamesh
>

Re: [PATCH] Simplify floating point comparisons

[Richard Biener]

On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> This patch implements some of the optimizations discussed in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.
>
> Simplify (C / x > 0.0) into x > 0.0.
>
> If C is negative the comparison is reversed.
>
> Simplify (x * C1) > C2 into x > (C2 / C1).
>
> Again, if C1 is negative the comparison is reversed.
> Both transformations are only done with -funsafe-math-optimizations,
> the constant is non-zero, and not a NaN.
>
> OK for commit?
>
> ChangeLog
> 2017-10-17  Wilco Dijkstra  <wdijkstr@arm.com>
>             Jackson Woodruff  <jackson.woodruff@arm.com>
>
>     gcc/
>         PR 71026/tree-optimization
>         * match.pd: Simplify floating point comparisons.
>
>     gcc/testsuite/
>         PR 71026/tree-optimization
>         * gcc.dg/associate_comparison_1.c: New test.
> --
> diff --git a/gcc/match.pd b/gcc/match.pd
> index e58a65af59b44a6b82ed8705f62966c5e6f251ac..cb48f079b4a310272e49cc319a1b3b0ff2023ba4 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -352,6 +352,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (rdiv @0 (rdiv:s @1 @2))
>     (mult (rdiv @0 @1) @2)))
>
> +(if (flag_unsafe_math_optimizations)
> +  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
> +  (for op (lt le gt ge)
> +       neg_op (gt ge lt le)
> +    (simplify
> +      (op (rdiv REAL_CST@0 @1) real_zerop@2)
> +      (switch
> +       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))

Note that real_less (0., +Inf) so I think you either need to check C is 'normal'
or ! HONOR_INFINITIES.

There's also the underflow issue I guess this is what -funsafe-math-optimization
is for.  I think ignoring underflows is dangerous though.

> +       (op @1 @2))
> +       /* For C < 0, use the inverted operator.  */
> +       (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
> +       (neg_op @1 @2))))))
> +
>  /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
>  (for div (trunc_div ceil_div floor_div round_div exact_div)
>   (simplify
> @@ -3546,6 +3559,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         (rdiv @2 @1))
>     (rdiv (op @0 @2) @1)))
>
> + (for cmp (lt le gt ge)
> +      neg_cmp (gt ge lt le)
> +  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
> +  (simplify
> +   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
> +   (with
> +    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
> +    (if (tem)
> +     (switch
> +      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
> +       (cmp @0 { tem; }))
> +      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
> +       (neg_cmp @0 { tem; })))))))
> +

Drops possible overflow/underflow in x * C1 and may create underflow
or overflow with C2/C1
which you should detect here at least.

Existing overflows may be guarded against with a HONOR_INFINITIES check.

When overflow/underflow can be disregarded is there any reason remaining to
make this guarded by flag_unsafe_math_optimizations?  Are there any cases
where rounding issues can flip the comparison result?

Richard.

>   /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
>   (for root (SQRT CBRT)
>    (simplify
> diff --git a/gcc/testsuite/gcc.dg/associate_comparison_1.c b/gcc/testsuite/gcc.dg/associate_comparison_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d051f052e13812c91cbd2d559bf2af8fae128ee1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/associate_comparison_1.c
> @@ -0,0 +1,34 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized" } */
> +
> +int
> +cmp_mul_1 (float x)
> +{
> +  return x * 3 <= 100;
> +}
> +
> +int
> +cmp_mul_2 (float x)
> +{
> +  return x * -5 > 100;
> +}
> +
> +int
> +div_cmp_1 (float x, float y)
> +{
> +  return x / 3 <= y;
> +}
> +
> +int
> +div_cmp_2 (float x, float y)
> +{
> +  return x / 3 <= 1;
> +}
> +
> +int
> +inv_cmp (float x)
> +{
> +  return 5 / x >= 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-not " / " "optimized" } } */
>

Re: [PATCH] Simplify floating point comparisons

[Joseph Myers]

On Wed, 18 Oct 2017, Richard Biener wrote:

> When overflow/underflow can be disregarded is there any reason remaining to
> make this guarded by flag_unsafe_math_optimizations?  Are there any cases
> where rounding issues can flip the comparison result?

Yes.  E.g. (in round-to-nearest) 3.0f * 0xfffffep0f == 3.0f * 0xfffffdp0f 
(generically, multiplication / division by any non-power-of-2 constant 
should be expected sometimes to map two different values to the same value 
even when overflow and underflow cannot occur).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Simplify floating point comparisons

[Wilco Dijkstra]

Richard Biener wrote:
> On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:

>> +(if (flag_unsafe_math_optimizations)
>> +  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
>> +  (for op (lt le gt ge)
>> +       neg_op (gt ge lt le)
>> +    (simplify
>> +      (op (rdiv REAL_CST@0 @1) real_zerop@2)
>> +      (switch
>> +       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
>
> Note that real_less (0., +Inf) so I think you either need to check C is 'normal'
> or ! HONOR_INFINITIES.

Yes, it was missing an explicit check for infinity, now added.

> There's also the underflow issue I guess this is what -funsafe-math-optimizations
> is for.  I think ignoring underflows is dangerous though.

We could change C / x > 0 to x >= 0 so the underflow case is included.
However that still means x == 0.0 would behave differently - so the question is
what exactly does -funsafe-math-optimization allow?


>> + (for cmp (lt le gt ge)
>> +      neg_cmp (gt ge lt le)
>> +  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
>> +  (simplify
>> +   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
>> +   (with
>> +    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
>> +    (if (tem)
>> +     (switch
>> +      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
>> +       (cmp @0 { tem; }))
>> +      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
>> +       (neg_cmp @0 { tem; })))))))
>
>
> Drops possible overflow/underflow in x * C1 and may create underflow
> or overflow with C2/C1 which you should detect here at least.

I've added checks for this, however I thought -funsafe-math-optimizations is
allowed to insert/remove underflow/overflow, like in these cases:

(x * 1e20f) * 1e20f and (x * 1e40f) * 1e-30f.

> Existing overflows may be guarded against with a HONOR_INFINITIES check.

Not sure what you mean with this?

> When overflow/underflow can be disregarded is there any reason remaining to
> make this guarded by flag_unsafe_math_optimizations?  Are there any cases
> where rounding issues can flip the comparison result?

I think it needs to remain under -funsafe-math-optimizations. Here is the updated
version:


ChangeLog
2017-11-15  Wilco Dijkstra  <wdijkstr@arm.com>  
	    Jackson Woodruff  <jackson.woodruff@arm.com>

    gcc/
	PR 71026/tree-optimization
	* match.pd: Simplify floating point comparisons.

    gcc/testsuite/
	PR 71026/tree-optimization
	* gcc.dg/associate_comparison_1.c: New.
--

diff --git a/gcc/match.pd b/gcc/match.pd
index 4d56847d6889923938625beb579b7bbb0cbbad91..967dbf8946fd12a161330f4c8b58dada5d9cb871 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -359,6 +359,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (rdiv @0 (negate @1))
  (rdiv (negate @0) @1))
 
+(if (flag_unsafe_math_optimizations)
+  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
+  (for op (lt le gt ge)
+       neg_op (gt ge lt le)
+    (simplify
+      (op (rdiv REAL_CST@0 @1) real_zerop@2)
+      (if (!REAL_VALUE_ISINF (TREE_REAL_CST (@0)))
+       (switch
+	(if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
+	 (op @1 @2))
+	/* For C < 0, use the inverted operator.  */
+	(if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
+	 (neg_op @1 @2)))))))
+
 /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
 (for div (trunc_div ceil_div floor_div round_div exact_div)
  (simplify
@@ -3703,6 +3717,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        (rdiv @2 @1))
    (rdiv (op @0 @2) @1)))
 
+ (for cmp (lt le gt ge)
+      neg_cmp (gt ge lt le)
+  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
+  (simplify
+   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
+   (with
+    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
+    (if (tem
+	 && !(REAL_VALUE_ISINF (TREE_REAL_CST (tem))
+	      || (real_zerop (tem) && !real_zerop (@1))))
+     (switch
+      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
+       (cmp @0 { tem; }))
+      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
+       (neg_cmp @0 { tem; })))))))
+
  /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
  (for root (SQRT CBRT)
   (simplify
diff --git a/gcc/testsuite/gcc.dg/associate_comparison_1.c b/gcc/testsuite/gcc.dg/associate_comparison_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..ceaba334cce770eb1cbec9283ba8a0c64f725630
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/associate_comparison_1.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized-raw" } */
+
+int
+cmp_mul_1 (float x)
+{
+  return x * 3 <= 100;
+}
+
+int
+cmp_mul_2 (float x)
+{
+  return x * -5 > 100;
+}
+
+int
+div_cmp_1 (float x, float y)
+{
+  return x / 3 <= y;
+}
+
+int
+div_cmp_2 (float x, float y)
+{
+  return x / 3 <= 1;
+}
+
+int
+inv_cmp (float x)
+{
+  return 5 / x >= 0;
+}
+
+/* { dg-final { scan-tree-dump-times "mult_expr" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "rdiv_expr" "optimized" } } */

Re: [PATCH] Simplify floating point comparisons

[Jeff Law]

On 11/15/2017 08:36 AM, Wilco Dijkstra wrote:
> Richard Biener wrote:
>> On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> 
>>> +(if (flag_unsafe_math_optimizations)
>>> +  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
>>> +  (for op (lt le gt ge)
>>> +       neg_op (gt ge lt le)
>>> +    (simplify
>>> +      (op (rdiv REAL_CST@0 @1) real_zerop@2)
>>> +      (switch
>>> +       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
>>
>> Note that real_less (0., +Inf) so I think you either need to check C is 'normal'
>> or ! HONOR_INFINITIES.
> 
> Yes, it was missing an explicit check for infinity, now added.
> 
>> There's also the underflow issue I guess this is what -funsafe-math-optimizations
>> is for.  I think ignoring underflows is dangerous though.
> 
> We could change C / x > 0 to x >= 0 so the underflow case is included.
> However that still means x == 0.0 would behave differently - so the question is
> what exactly does -funsafe-math-optimization allow?
Well, we largely define what it means.  I believe we have in the past
stated that it can't break SPEC.  It's as good a rule as anything,
though it is underspecified (what version, what other flags, what
targets, etc).

With that bit of background, I believe that -funsafe-math-optimizations
has been allowed to inhibit or introduce underflows (reassociation in
particular I think does this and is enabled by
-funsafe-math-optimizations).  So I don't think ignoring underflow
should inherently block this patch.



> 
> 
>>> + (for cmp (lt le gt ge)
>>> +      neg_cmp (gt ge lt le)
>>> +  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
>>> +  (simplify
>>> +   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
>>> +   (with
>>> +    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
>>> +    (if (tem)
>>> +     (switch
>>> +      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
>>> +       (cmp @0 { tem; }))
>>> +      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
>>> +       (neg_cmp @0 { tem; })))))))
>>
>>
>> Drops possible overflow/underflow in x * C1 and may create underflow
>> or overflow with C2/C1 which you should detect here at least.
> 
> I've added checks for this, however I thought -funsafe-math-optimizations is
> allowed to insert/remove underflow/overflow, like in these cases:
> 
> (x * 1e20f) * 1e20f and (x * 1e40f) * 1e-30f.
Right.  That's my understanding as well.


> 
>> Existing overflows may be guarded against with a HONOR_INFINITIES check.
> 
> Not sure what you mean with this?
> 
>> When overflow/underflow can be disregarded is there any reason remaining to
>> make this guarded by flag_unsafe_math_optimizations?  Are there any cases
>> where rounding issues can flip the comparison result?
> 
> I think it needs to remain under -funsafe-math-optimizations. Here is the updated
> version:
Richi has taken the lead on this review and should probably own it.
Just thought it was worth chiming in on the underflow issue.

jeff

Re: [PATCH] Simplify floating point comparisons

[Wilco Dijkstra]

ping (note also Jeff's reply https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01916.html)


From: Wilco Dijkstra
Sent: 15 November 2017 15:36
To: Richard Biener
Cc: GCC Patches; nd
Subject: Re: [PATCH] Simplify floating point comparisons
  

Richard Biener wrote:
> On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:

>> +(if (flag_unsafe_math_optimizations)
>> +  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
>> +  (for op (lt le gt ge)
>> +       neg_op (gt ge lt le)
>> +    (simplify
>> +      (op (rdiv REAL_CST@0 @1) real_zerop@2)
>> +      (switch
>> +       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
>
> Note that real_less (0., +Inf) so I think you either need to check C is 'normal'
> or ! HONOR_INFINITIES.

Yes, it was missing an explicit check for infinity, now added.

> There's also the underflow issue I guess this is what -funsafe-math-optimizations
> is for.  I think ignoring underflows is dangerous though.

We could change C / x > 0 to x >= 0 so the underflow case is included.
However that still means x == 0.0 would behave differently - so the question is
what exactly does -funsafe-math-optimization allow?


>> + (for cmp (lt le gt ge)
>> +      neg_cmp (gt ge lt le)
>> +  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
>> +  (simplify
>> +   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
>> +   (with
>> +    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
>> +    (if (tem)
>> +     (switch
>> +      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
>> +       (cmp @0 { tem; }))
>> +      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
>> +       (neg_cmp @0 { tem; })))))))
>
>
> Drops possible overflow/underflow in x * C1 and may create underflow
> or overflow with C2/C1 which you should detect here at least.

I've added checks for this, however I thought -funsafe-math-optimizations is
allowed to insert/remove underflow/overflow, like in these cases:

(x * 1e20f) * 1e20f and (x * 1e40f) * 1e-30f.

> Existing overflows may be guarded against with a HONOR_INFINITIES check.

Not sure what you mean with this?

> When overflow/underflow can be disregarded is there any reason remaining to
> make this guarded by flag_unsafe_math_optimizations?  Are there any cases
> where rounding issues can flip the comparison result?

I think it needs to remain under -funsafe-math-optimizations. Here is the updated
version:


ChangeLog
2017-11-15  Wilco Dijkstra  <wdijkstr@arm.com>  
            Jackson Woodruff  <jackson.woodruff@arm.com>

    gcc/
        PR 71026/tree-optimization
        * match.pd: Simplify floating point comparisons.

    gcc/testsuite/
        PR 71026/tree-optimization
        * gcc.dg/associate_comparison_1.c: New.
--

diff --git a/gcc/match.pd b/gcc/match.pd
index 4d56847d6889923938625beb579b7bbb0cbbad91..967dbf8946fd12a161330f4c8b58dada5d9cb871 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -359,6 +359,20 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (rdiv @0 (negate @1))
  (rdiv (negate @0) @1))
 
+(if (flag_unsafe_math_optimizations)
+  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
+  (for op (lt le gt ge)
+       neg_op (gt ge lt le)
+    (simplify
+      (op (rdiv REAL_CST@0 @1) real_zerop@2)
+      (if (!REAL_VALUE_ISINF (TREE_REAL_CST (@0)))
+       (switch
+       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
+        (op @1 @2))
+       /* For C < 0, use the inverted operator.  */
+       (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
+        (neg_op @1 @2)))))))
+
 /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
 (for div (trunc_div ceil_div floor_div round_div exact_div)
  (simplify
@@ -3703,6 +3717,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        (rdiv @2 @1))
    (rdiv (op @0 @2) @1)))
 
+ (for cmp (lt le gt ge)
+      neg_cmp (gt ge lt le)
+  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
+  (simplify
+   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
+   (with
+    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
+    (if (tem
+        && !(REAL_VALUE_ISINF (TREE_REAL_CST (tem))
+             || (real_zerop (tem) && !real_zerop (@1))))
+     (switch
+      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
+       (cmp @0 { tem; }))
+      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
+       (neg_cmp @0 { tem; })))))))
+
  /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
  (for root (SQRT CBRT)
   (simplify
diff --git a/gcc/testsuite/gcc.dg/associate_comparison_1.c b/gcc/testsuite/gcc.dg/associate_comparison_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..ceaba334cce770eb1cbec9283ba8a0c64f725630
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/associate_comparison_1.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized-raw" } */
+
+int
+cmp_mul_1 (float x)
+{
+  return x * 3 <= 100;
+}
+
+int
+cmp_mul_2 (float x)
+{
+  return x * -5 > 100;
+}
+
+int
+div_cmp_1 (float x, float y)
+{
+  return x / 3 <= y;
+}
+
+int
+div_cmp_2 (float x, float y)
+{
+  return x / 3 <= 1;
+}
+
+int
+inv_cmp (float x)
+{
+  return 5 / x >= 0;
+}
+
+/* { dg-final { scan-tree-dump-times "mult_expr" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "rdiv_expr" "optimized" } } */

    

Re: [PATCH] Simplify floating point comparisons

[Marc Glisse]

(just a log of my thoughts while reading the patch)

On Thu, 4 Jan 2018, Wilco Dijkstra wrote:

> Richard Biener wrote:
>> On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>
>>> +(if (flag_unsafe_math_optimizations)
>>> +  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
>>> +  (for op (lt le gt ge)
>>> +       neg_op (gt ge lt le)
>>> +    (simplify
>>> +      (op (rdiv REAL_CST@0 @1) real_zerop@2)
>>> +      (switch
>>> +       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
>>
>> Note that real_less (0., +Inf) so I think you either need to check C is 'normal'
>> or ! HONOR_INFINITIES.
>
> Yes, it was missing an explicit check for infinity, now added.

I don't understand how the check you added helps.

>> There's also the underflow issue I guess this is what -funsafe-math-optimizations
>> is for.  I think ignoring underflows is dangerous though.
>
> We could change C / x > 0 to x >= 0 so the underflow case is included.
> However that still means x == 0.0 would behave differently - so the question is
> what exactly does -funsafe-math-optimization allow?

That is indeed unclear. HONOR_SIGNED_ZEROS, HONOR_NANS, HONOR_INFINITIES 
are better defined and could also see some use.

1/X>=0: if X can be zero (and the inverse can be infinite), the result 
depends on the sign of that zero. If !HONOR_SIGNED_ZEROS || 
!HONOR_INFINITIES, it looks like we can simplify to either X>0 or X>=0, 
doesn't matter which.
1/X>=0 is true iff X is not NaN and the sign bit is not set, so with 
!HONOR_NANS it could also be turned into a bit test.

It works the same for C/X>=0 with 0<C<infinity. For C=infinity, the main 
difference is that X=infinity now gives false (if HONOR_NANS).

C/X>0 is more tricky because small/big may round to zero. For C large 
enough (assuming denormals, 1 is large enough for instance), the only new 
issue is X=infinity. For smaller C, we start getting wrong results for 
finite values of X, and presumably that's where 
flag_unsafe_math_optimizations comes into play.

If we consider flag_unsafe_math_optimizations as a kitchen sink that 
implies !HONOR_SIGNED_ZEROS && !HONOR_INFINITIES, then you don't even need 
your REAL_VALUE_ISINF test.

-- 
Marc Glisse

Re: [PATCH] Simplify floating point comparisons

[Richard Biener]

On Thu, Jan 4, 2018 at 10:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> (just a log of my thoughts while reading the patch)
>
> On Thu, 4 Jan 2018, Wilco Dijkstra wrote:
>
>> Richard Biener wrote:
>>>
>>> On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com>
>>> wrote:
>>
>>
>>>> +(if (flag_unsafe_math_optimizations)
>>>> +  /* Simplify (C / x op 0.0) to x op 0.0 for C > 0.  */
>>>> +  (for op (lt le gt ge)
>>>> +       neg_op (gt ge lt le)
>>>> +    (simplify
>>>> +      (op (rdiv REAL_CST@0 @1) real_zerop@2)
>>>> +      (switch
>>>> +       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
>>>
>>>
>>> Note that real_less (0., +Inf) so I think you either need to check C is
>>> 'normal'
>>> or ! HONOR_INFINITIES.
>>
>>
>> Yes, it was missing an explicit check for infinity, now added.
>
>
> I don't understand how the check you added helps.
>
>>> There's also the underflow issue I guess this is what
>>> -funsafe-math-optimizations
>>> is for.  I think ignoring underflows is dangerous though.
>>
>>
>> We could change C / x > 0 to x >= 0 so the underflow case is included.
>> However that still means x == 0.0 would behave differently - so the
>> question is
>> what exactly does -funsafe-math-optimization allow?
>
>
> That is indeed unclear. HONOR_SIGNED_ZEROS, HONOR_NANS, HONOR_INFINITIES are
> better defined and could also see some use.
>
> 1/X>=0: if X can be zero (and the inverse can be infinite), the result
> depends on the sign of that zero. If !HONOR_SIGNED_ZEROS ||
> !HONOR_INFINITIES, it looks like we can simplify to either X>0 or X>=0,
> doesn't matter which.
> 1/X>=0 is true iff X is not NaN and the sign bit is not set, so with
> !HONOR_NANS it could also be turned into a bit test.
>
> It works the same for C/X>=0 with 0<C<infinity. For C=infinity, the main
> difference is that X=infinity now gives false (if HONOR_NANS).
>
> C/X>0 is more tricky because small/big may round to zero. For C large enough
> (assuming denormals, 1 is large enough for instance), the only new issue is
> X=infinity. For smaller C, we start getting wrong results for finite values
> of X, and presumably that's where flag_unsafe_math_optimizations comes into
> play.
>
> If we consider flag_unsafe_math_optimizations as a kitchen sink that implies
> !HONOR_SIGNED_ZEROS && !HONOR_INFINITIES, then you don't even need your
> REAL_VALUE_ISINF test.

Without looking at the latest patch -funsafe-math-optimizations is a
kitchen-sink
we should avoid at all cost.  In the past we somewhat decided to break it up
properly (-fassociative-math and friends have been added for example),
and we really
don't want to add more stuff under this umbrella (there's too much
under it already).
I'm not sure if C / x op 0.0 to x op 0.0 is association though (I'm
quite sure it isn't),
so citing association examples doesn't work for me here ;)

It might be we need some new flag like -fassume-infinite-precision-math to
cover the underflow issue?  We do have -ffinite-math-only but that
shouldn't allow
GCC to possibly introduce +-Inf for overflow - GCC just can assume there are
no overflows in the original user expression.

Richard.

> --
> Marc Glisse

Re: [PATCH] Simplify floating point comparisons

[Wilco Dijkstra]

Richard Biener wrote:
>On Thu, Jan 4, 2018 at 10:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:

>> I don't understand how the check you added helps.

It simply blocks the transformation for infinity:

+      (if (!REAL_VALUE_ISINF (TREE_REAL_CST (@0)))
+       (switch
+       (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
+        (op @1 @2))
+       /* For C < 0, use the inverted operator.  */
+       (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
+        (neg_op @1 @2)))))))

I can't see what is potentially problematic here, it excludes infinity. I can replace
the flag_unsafe_math_optimizations with HONOR variants and then the infinity 
check is no longer required.


>>> We could change C / x > 0 to x >= 0 so the underflow case is included.
>>> However that still means x == 0.0 would behave differently - so the
>>> question is
>>> what exactly does -funsafe-math-optimization allow?
>>
>>
>> That is indeed unclear. HONOR_SIGNED_ZEROS, HONOR_NANS, HONOR_INFINITIES are
>> better defined and could also see some use.
>>
>> 1/X>=0: if X can be zero (and the inverse can be infinite), the result
>> depends on the sign of that zero. If !HONOR_SIGNED_ZEROS ||
>> !HONOR_INFINITIES, it looks like we can simplify to either X>0 or X>=0,
>> doesn't matter which.

Agreed.

>> 1/X>=0 is true iff X is not NaN and the sign bit is not set, so with
>> !HONOR_NANS it could also be turned into a bit test.

That's unlikely more efficient than a compare with zero.

>> It works the same for C/X>=0 with 0<C<infinity. For C=infinity, the main
>> difference is that X=infinity now gives false (if HONOR_NANS).

Yes that's why C=infinity is excluded in the latest version. However if we already
check HONOR_INFINITIES it wouldn't be needed.

>> C/X>0 is more tricky because small/big may round to zero. For C large enough
>> (assuming denormals, 1 is large enough for instance), the only new issue is
>> X=infinity. For smaller C, we start getting wrong results for finite values
>> of X, and presumably that's where flag_unsafe_math_optimizations comes into
>> play.

Well we can change C/X > 0 to X >= 0. That deals with underflow cases when
X is a normal, and results in the same cases that are wrong as C/X >= 0
(X = -0 and X = +Inf assuming C>0), so  !HONOR_SIGNED_ZEROS && 
!HONOR_INFINITIES should be sufficient for all cases.

>> If we consider flag_unsafe_math_optimizations as a kitchen sink that implies
>> !HONOR_SIGNED_ZEROS && !HONOR_INFINITIES, then you don't even need your
>> REAL_VALUE_ISINF test.

Indeed. Changing it to use more descriptive tests is a good idea indeed as it makes it
more explicit which cases are not supported.


> Without looking at the latest patch -funsafe-math-optimizations is a
> kitchen-sink
> we should avoid at all cost.  In the past we somewhat decided to break it up
> properly (-fassociative-math and friends have been added for example),
> and we really
> don't want to add more stuff under this umbrella (there's too much
> under it already).
> I'm not sure if C / x op 0.0 to x op 0.0 is association though (I'm
> quite sure it isn't),
> so citing association examples doesn't work for me here ;)

Marc's proposal to use HONOR_* instead of flag_unsafe_math_optimizations
should work, I'll give it a go. Maybe in the future we should aim to replace all
existing uses.

> It might be we need some new flag like -fassume-infinite-precision-math to
> cover the underflow issue?  We do have -ffinite-math-only but that
> shouldn't allow
> GCC to possibly introduce +-Inf for overflow - GCC just can assume there are
> no overflows in the original user expression.

We can avoid the underflow for normal numbers, so I don't think there is need
for a new flag.

Wilco
    

Re: [PATCH] Simplify floating point comparisons

[Wilco Dijkstra]

Hi,

Here is the updated version:

This patch implements some of the optimizations discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.

Simplify (C / x >= 0.0) into x >= 0.0 with -fno-signed-zeros
and -ffinite-math-only.  If C is negative the comparison is reversed.
Only handle >= and <= for now since C / x can underflow if C is small.


Simplify (x * C1) > C2 into x > (C2 / C1) with -funsafe-math-optimizations.
If C1 is negative the comparison is reversed.

OK for commit?

ChangeLog
2018-01-10  Wilco Dijkstra  <wdijkstr@arm.com>  
	    Jackson Woodruff  <jackson.woodruff@arm.com>

    gcc/
	PR 71026/tree-optimization
	* match.pd: Simplify floating point comparisons.

    gcc/testsuite/
	PR 71026/tree-optimization
	* gcc.dg/div-cmp-1.c: New test.
	* gcc.dg/div-cmp-2.c: New test.
--

diff --git a/gcc/match.pd b/gcc/match.pd
index 435125a317275527661fba011a9d26e507d293a6..8a6fee906de6a750201362119862f8326868f26b 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -376,6 +376,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (rdiv @0 (negate @1))
  (rdiv (negate @0) @1))
 
+/* Simplify (C / x op 0.0) to x op 0.0 for C != 0, C != Inf/Nan.
+   Only handle >= and <= since C / x may underflow to zero.  */
+(for op (le ge)
+     res_op (lt ge)
+     neg_op (ge lt)
+ (simplify
+  (op (rdiv REAL_CST@0 @1) real_zerop@2)
+  (if (!HONOR_SIGNED_ZEROS (@1) && !HONOR_INFINITIES (@1))
+   (switch
+    (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
+     (res_op @1 @2))
+    /* For C < 0, use the inverted operator.  */
+    (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
+     (neg_op @1 @2))))))
+
 /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
 (for div (trunc_div ceil_div floor_div round_div exact_div)
  (simplify
@@ -3842,6 +3857,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        (rdiv @2 @1))
    (rdiv (op @0 @2) @1)))
 
+ (for cmp (lt le gt ge)
+      neg_cmp (gt ge lt le)
+  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
+  (simplify
+   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
+   (with
+    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
+    (if (tem
+	 && !(REAL_VALUE_ISINF (TREE_REAL_CST (tem))
+	      || (real_zerop (tem) && !real_zerop (@1))))
+     (switch
+      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
+       (cmp @0 { tem; }))
+      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
+       (neg_cmp @0 { tem; })))))))
+
  /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
  (for root (SQRT CBRT)
   (simplify
diff --git a/gcc/testsuite/gcc.dg/div-cmp-1.c b/gcc/testsuite/gcc.dg/div-cmp-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..cd1a5cd3d6fee5a10e9859ca99b344fa3fdb7f5f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/div-cmp-1.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized-raw" } */
+
+int
+cmp_mul_1 (float x)
+{
+  return x * 3 <= 100;
+}
+
+int
+cmp_mul_2 (float x)
+{
+  return x * -5 > 100;
+}
+
+int
+div_cmp_1 (float x, float y)
+{
+  return x / 3 <= y;
+}
+
+int
+div_cmp_2 (float x, float y)
+{
+  return x / 3 <= 1;
+}
+
+/* { dg-final { scan-tree-dump-times "mult_expr" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "rdiv_expr" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/div-cmp-2.c b/gcc/testsuite/gcc.dg/div-cmp-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..1aa0797c2ff7a3fa1d6b93bc150a7c280541aed5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/div-cmp-2.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-signed-zeros -ffinite-math-only -fdump-tree-optimized-raw" } */
+
+int
+cmp_1 (float x)
+{
+  return 5 / x >= 0;
+}
+
+int
+cmp_2 (float x)
+{
+  return 1 / x <= 0;
+}
+
+int
+cmp_3 (float x)
+{
+  return -2 / x >= 0;
+}
+
+int
+cmp_4 (float x)
+{
+  return -5 / x <= 0;
+}

Re: [PATCH] Simplify floating point comparisons

[Jeff Law]

On 01/12/2018 06:21 AM, Wilco Dijkstra wrote:
> Hi,
> 
> Here is the updated version:
> 
> This patch implements some of the optimizations discussed in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.
> 
> Simplify (C / x >= 0.0) into x >= 0.0 with -fno-signed-zeros
> and -ffinite-math-only.  If C is negative the comparison is reversed.
> Only handle >= and <= for now since C / x can underflow if C is small.
> 
> 
> Simplify (x * C1) > C2 into x > (C2 / C1) with -funsafe-math-optimizations.
> If C1 is negative the comparison is reversed.
> 
> OK for commit?
> 
> ChangeLog
> 2018-01-10  Wilco Dijkstra  <wdijkstr@arm.com>  
> 	    Jackson Woodruff  <jackson.woodruff@arm.com>
> 
>     gcc/
> 	PR 71026/tree-optimization
> 	* match.pd: Simplify floating point comparisons.
> 
>     gcc/testsuite/
> 	PR 71026/tree-optimization
> 	* gcc.dg/div-cmp-1.c: New test.
> 	* gcc.dg/div-cmp-2.c: New test.
OK for the trunk.  I don't think there is a need to backport to gcc-8.

jeff

Re: [PATCH] Simplify floating point comparisons

[Marc Glisse]

On Fri, 12 Jan 2018, Wilco Dijkstra wrote:

> Hi,
>
> Here is the updated version:
>
> This patch implements some of the optimizations discussed in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.
>
> Simplify (C / x >= 0.0) into x >= 0.0 with -fno-signed-zeros
> and -ffinite-math-only.  If C is negative the comparison is reversed.
> Only handle >= and <= for now since C / x can underflow if C is small.
>
>
> Simplify (x * C1) > C2 into x > (C2 / C1) with -funsafe-math-optimizations.
> If C1 is negative the comparison is reversed.
>
> OK for commit?
>
> ChangeLog
> 2018-01-10  Wilco Dijkstra  <wdijkstr@arm.com>
> 	    Jackson Woodruff  <jackson.woodruff@arm.com>
>
>    gcc/
> 	PR 71026/tree-optimization
> 	* match.pd: Simplify floating point comparisons.
>
>    gcc/testsuite/
> 	PR 71026/tree-optimization
> 	* gcc.dg/div-cmp-1.c: New test.
> 	* gcc.dg/div-cmp-2.c: New test.
> --
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 435125a317275527661fba011a9d26e507d293a6..8a6fee906de6a750201362119862f8326868f26b 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -376,6 +376,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (rdiv @0 (negate @1))
>  (rdiv (negate @0) @1))
>
> +/* Simplify (C / x op 0.0) to x op 0.0 for C != 0, C != Inf/Nan.
> +   Only handle >= and <= since C / x may underflow to zero.  */
> +(for op (le ge)
> +     res_op (lt ge)
> +     neg_op (ge lt)
> + (simplify
> +  (op (rdiv REAL_CST@0 @1) real_zerop@2)
> +  (if (!HONOR_SIGNED_ZEROS (@1) && !HONOR_INFINITIES (@1))
> +   (switch
> +    (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
> +     (res_op @1 @2))
> +    /* For C < 0, use the inverted operator.  */
> +    (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
> +     (neg_op @1 @2))))))

Let's try with C = DBL_MIN and x = ±DBL_MAX. I don't believe it involves 
signed zeros or infinities, just an underflow. First, the result depends 
on the rounding mode. And in the default round-to-nearest, both divisions 
give 0, and thus compare the same with 0, but we replace that with a sign 
test on x, where they clearly give opposite answers.

What would be the proper flag to test to check if we care about underflow?

-- 
Marc Glisse

Re: [PATCH] Simplify floating point comparisons

[Richard Biener]

On Mon, Apr 30, 2018 at 8:49 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Fri, 12 Jan 2018, Wilco Dijkstra wrote:
>
>> Hi,
>>
>> Here is the updated version:
>>
>> This patch implements some of the optimizations discussed in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.
>>
>> Simplify (C / x >= 0.0) into x >= 0.0 with -fno-signed-zeros
>> and -ffinite-math-only.  If C is negative the comparison is reversed.
>> Only handle >= and <= for now since C / x can underflow if C is small.
>>
>>
>> Simplify (x * C1) > C2 into x > (C2 / C1) with
>> -funsafe-math-optimizations.
>> If C1 is negative the comparison is reversed.
>>
>> OK for commit?
>>
>> ChangeLog
>> 2018-01-10  Wilco Dijkstra  <wdijkstr@arm.com>
>>             Jackson Woodruff  <jackson.woodruff@arm.com>
>>
>>    gcc/
>>         PR 71026/tree-optimization
>>         * match.pd: Simplify floating point comparisons.
>>
>>    gcc/testsuite/
>>         PR 71026/tree-optimization
>>         * gcc.dg/div-cmp-1.c: New test.
>>         * gcc.dg/div-cmp-2.c: New test.
>> --
>>
>> diff --git a/gcc/match.pd b/gcc/match.pd
>> index
>> 435125a317275527661fba011a9d26e507d293a6..8a6fee906de6a750201362119862f8326868f26b
>> 100644
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -376,6 +376,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>>  (rdiv @0 (negate @1))
>>  (rdiv (negate @0) @1))
>>
>> +/* Simplify (C / x op 0.0) to x op 0.0 for C != 0, C != Inf/Nan.
>> +   Only handle >= and <= since C / x may underflow to zero.  */
>> +(for op (le ge)
>> +     res_op (lt ge)
>> +     neg_op (ge lt)
>> + (simplify
>> +  (op (rdiv REAL_CST@0 @1) real_zerop@2)
>> +  (if (!HONOR_SIGNED_ZEROS (@1) && !HONOR_INFINITIES (@1))
>> +   (switch
>> +    (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
>> +     (res_op @1 @2))
>> +    /* For C < 0, use the inverted operator.  */
>> +    (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
>> +     (neg_op @1 @2))))))
>
>
> Let's try with C = DBL_MIN and x = 婊BL_MAX. I don't believe it involves
> signed zeros or infinities, just an underflow. First, the result depends on
> the rounding mode. And in the default round-to-nearest, both divisions give
> 0, and thus compare the same with 0, but we replace that with a sign test on
> x, where they clearly give opposite answers.
>
> What would be the proper flag to test to check if we care about underflow?

We have none specific so this makes it flag_unsafe_math_optimizations.

Richard.

> --
> Marc Glisse

Re: [PATCH] Simplify floating point comparisons

[Wilco Dijkstra]

Richard Biener wrote:
>Marc Glisse wrote:
>> Let's try with C = DBL_MIN and x = 婊BL_MAX. I don't believe it involves
>> signed zeros or infinities, just an underflow. First, the result depends on
>> the rounding mode. And in the default round-to-nearest, both divisions give
>> 0, and thus compare the same with 0, but we replace that with a sign test on
>> x, where they clearly give opposite answers.
>>
>> What would be the proper flag to test to check if we care about underflow?
>
> We have none specific so this makes it flag_unsafe_math_optimizations.

Right I have added the unsafe math check again like in the previous version:


The patch implements some of the optimizations discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.

Simplify (C / x >= 0.0) into x >= 0.0 with -funsafe-math-optimizations
(since C / x can underflow to zero if x is huge, it's not safe otherwise).
If C is negative the comparison is reversed.


Simplify (x * C1) > C2 into x > (C2 / C1) with -funsafe-math-optimizations.
If C1 is negative the comparison is reversed.

OK for commit?

ChangeLog
2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>  
	    Jackson Woodruff  <jackson.woodruff@arm.com>

    gcc/
	PR 71026/tree-optimization
	* match.pd: Simplify floating point comparisons.

    gcc/testsuite/
	PR 71026/tree-optimization
	* gcc.dg/div-cmp-1.c: New test.
	* gcc.dg/div-cmp-2.c: New test.

--
diff --git a/gcc/match.pd b/gcc/match.pd
index 94fbab841f5e36bd33fda849a686fd80886ee1ff..f6c76510f95be2485e5bacd07edab336705cbd25 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -405,6 +405,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (rdiv @0 (negate @1))
  (rdiv (negate @0) @1))
 
+(if (flag_unsafe_math_optimizations)
+ /* Simplify (C / x op 0.0) to x op 0.0 for C != 0, C != Inf/Nan.
+    Since C / x may underflow to zero, do this only for unsafe math.  */
+ (for op (lt le gt ge)
+      neg_op (gt ge lt le)
+  (simplify
+   (op (rdiv REAL_CST@0 @1) real_zerop@2)
+   (if (!HONOR_SIGNED_ZEROS (@1) && !HONOR_INFINITIES (@1))
+    (switch
+     (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
+      (op @1 @2))
+     /* For C < 0, use the inverted operator.  */
+     (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
+      (neg_op @1 @2)))))))
+
 /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
 (for div (trunc_div ceil_div floor_div round_div exact_div)
  (simplify
@@ -4049,6 +4064,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        (rdiv @2 @1))
    (rdiv (op @0 @2) @1)))
 
+ (for cmp (lt le gt ge)
+      neg_cmp (gt ge lt le)
+  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
+  (simplify
+   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
+   (with
+    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
+    (if (tem
+	 && !(REAL_VALUE_ISINF (TREE_REAL_CST (tem))
+	      || (real_zerop (tem) && !real_zerop (@1))))
+     (switch
+      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
+       (cmp @0 { tem; }))
+      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
+       (neg_cmp @0 { tem; })))))))
+
  /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
  (for root (SQRT CBRT)
   (simplify
diff --git a/gcc/testsuite/gcc.dg/div-cmp-1.c b/gcc/testsuite/gcc.dg/div-cmp-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..cd1a5cd3d6fee5a10e9859ca99b344fa3fdb7f5f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/div-cmp-1.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized-raw" } */
+
+int
+cmp_mul_1 (float x)
+{
+  return x * 3 <= 100;
+}
+
+int
+cmp_mul_2 (float x)
+{
+  return x * -5 > 100;
+}
+
+int
+div_cmp_1 (float x, float y)
+{
+  return x / 3 <= y;
+}
+
+int
+div_cmp_2 (float x, float y)
+{
+  return x / 3 <= 1;
+}
+
+/* { dg-final { scan-tree-dump-times "mult_expr" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "rdiv_expr" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/div-cmp-2.c b/gcc/testsuite/gcc.dg/div-cmp-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..f4ac42a196a804747d0b578e0aa2131671c8d3cf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/div-cmp-2.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funsafe-math-optimizations -ffinite-math-only -fdump-tree-optimized-raw" } */
+
+int
+cmp_1 (float x)
+{
+  return 5 / x >= 0;
+}
+
+int
+cmp_2 (float x)
+{
+  return 1 / x <= 0;
+}
+
+int
+cmp_3 (float x)
+{
+  return -2 / x >= 0;
+}
+
+int
+cmp_4 (float x)
+{
+  return -5 / x <= 0;
+}
+
+/* { dg-final { scan-tree-dump-not "rdiv_expr" "optimized" } } */
+

Re: [PATCH] Simplify floating point comparisons

[Richard Biener]

On Fri, Nov 9, 2018 at 6:05 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>
> Richard Biener wrote:
> >Marc Glisse wrote:
> >> Let's try with C = DBL_MIN and x = 婊BL_MAX. I don't believe it involves
> >> signed zeros or infinities, just an underflow. First, the result depends on
> >> the rounding mode. And in the default round-to-nearest, both divisions give
> >> 0, and thus compare the same with 0, but we replace that with a sign test on
> >> x, where they clearly give opposite answers.
> >>
> >> What would be the proper flag to test to check if we care about underflow?
> >
> > We have none specific so this makes it flag_unsafe_math_optimizations.
>
> Right I have added the unsafe math check again like in the previous version:
>
>
> The patch implements some of the optimizations discussed in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71026.
>
> Simplify (C / x >= 0.0) into x >= 0.0 with -funsafe-math-optimizations
> (since C / x can underflow to zero if x is huge, it's not safe otherwise).
> If C is negative the comparison is reversed.
>
>
> Simplify (x * C1) > C2 into x > (C2 / C1) with -funsafe-math-optimizations.
> If C1 is negative the comparison is reversed.
>
> OK for commit?

OK.

Thanks,
Richard.

> ChangeLog
> 2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>
>             Jackson Woodruff  <jackson.woodruff@arm.com>
>
>     gcc/
>         PR 71026/tree-optimization
>         * match.pd: Simplify floating point comparisons.
>
>     gcc/testsuite/
>         PR 71026/tree-optimization
>         * gcc.dg/div-cmp-1.c: New test.
>         * gcc.dg/div-cmp-2.c: New test.
>
> --
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 94fbab841f5e36bd33fda849a686fd80886ee1ff..f6c76510f95be2485e5bacd07edab336705cbd25 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -405,6 +405,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   (rdiv @0 (negate @1))
>   (rdiv (negate @0) @1))
>
> +(if (flag_unsafe_math_optimizations)
> + /* Simplify (C / x op 0.0) to x op 0.0 for C != 0, C != Inf/Nan.
> +    Since C / x may underflow to zero, do this only for unsafe math.  */
> + (for op (lt le gt ge)
> +      neg_op (gt ge lt le)
> +  (simplify
> +   (op (rdiv REAL_CST@0 @1) real_zerop@2)
> +   (if (!HONOR_SIGNED_ZEROS (@1) && !HONOR_INFINITIES (@1))
> +    (switch
> +     (if (real_less (&dconst0, TREE_REAL_CST_PTR (@0)))
> +      (op @1 @2))
> +     /* For C < 0, use the inverted operator.  */
> +     (if (real_less (TREE_REAL_CST_PTR (@0), &dconst0))
> +      (neg_op @1 @2)))))))
> +
>  /* Optimize (X & (-A)) / A where A is a power of 2, to X >> log2(A) */
>  (for div (trunc_div ceil_div floor_div round_div exact_div)
>   (simplify
> @@ -4049,6 +4064,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         (rdiv @2 @1))
>     (rdiv (op @0 @2) @1)))
>
> + (for cmp (lt le gt ge)
> +      neg_cmp (gt ge lt le)
> +  /* Simplify (x * C1) cmp C2 -> x cmp (C2 / C1), where C1 != 0.  */
> +  (simplify
> +   (cmp (mult @0 REAL_CST@1) REAL_CST@2)
> +   (with
> +    { tree tem = const_binop (RDIV_EXPR, type, @2, @1); }
> +    (if (tem
> +        && !(REAL_VALUE_ISINF (TREE_REAL_CST (tem))
> +             || (real_zerop (tem) && !real_zerop (@1))))
> +     (switch
> +      (if (real_less (&dconst0, TREE_REAL_CST_PTR (@1)))
> +       (cmp @0 { tem; }))
> +      (if (real_less (TREE_REAL_CST_PTR (@1), &dconst0))
> +       (neg_cmp @0 { tem; })))))))
> +
>   /* Simplify sqrt(x) * sqrt(y) -> sqrt(x*y).  */
>   (for root (SQRT CBRT)
>    (simplify
> diff --git a/gcc/testsuite/gcc.dg/div-cmp-1.c b/gcc/testsuite/gcc.dg/div-cmp-1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..cd1a5cd3d6fee5a10e9859ca99b344fa3fdb7f5f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/div-cmp-1.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -funsafe-math-optimizations -fdump-tree-optimized-raw" } */
> +
> +int
> +cmp_mul_1 (float x)
> +{
> +  return x * 3 <= 100;
> +}
> +
> +int
> +cmp_mul_2 (float x)
> +{
> +  return x * -5 > 100;
> +}
> +
> +int
> +div_cmp_1 (float x, float y)
> +{
> +  return x / 3 <= y;
> +}
> +
> +int
> +div_cmp_2 (float x, float y)
> +{
> +  return x / 3 <= 1;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "mult_expr" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "rdiv_expr" "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/div-cmp-2.c b/gcc/testsuite/gcc.dg/div-cmp-2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f4ac42a196a804747d0b578e0aa2131671c8d3cf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/div-cmp-2.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -funsafe-math-optimizations -ffinite-math-only -fdump-tree-optimized-raw" } */
> +
> +int
> +cmp_1 (float x)
> +{
> +  return 5 / x >= 0;
> +}
> +
> +int
> +cmp_2 (float x)
> +{
> +  return 1 / x <= 0;
> +}
> +
> +int
> +cmp_3 (float x)
> +{
> +  return -2 / x >= 0;
> +}
> +
> +int
> +cmp_4 (float x)
> +{
> +  return -5 / x <= 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "rdiv_expr" "optimized" } } */
> +