[PATCH] Simplify floating point comparisons
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 Jackson Woodruff 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 ..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
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 Jackson Woodruff 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 ..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 ..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
On Fri, Nov 9, 2018 at 6:05 PM Wilco Dijkstra 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 > Jackson Woodruff > > 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 > ..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 > ..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
On 17 October 2017 at 21:58, Wilco Dijkstra 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 > Jackson Woodruff > > 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 > ..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
On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra 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 > Jackson Woodruff > > 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 > ..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
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 * 0xfep0f == 3.0f * 0xfdp0f (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 jos...@codesourcery.com
Re: [PATCH] Simplify floating point comparisons
Richard Biener wrote: >On Thu, Jan 4, 2018 at 10:27 PM, Marc Glisse 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> 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
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 Jackson Woodruff 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 ..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 ..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
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 > Jackson Woodruff > > 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
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 Jackson Woodruff 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
On Mon, Apr 30, 2018 at 8:49 PM, Marc Glisse 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 >> Jackson Woodruff >> >>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
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 > 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 Jackson Woodruff 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 ..ceaba334cce770eb1cbec9283ba8a0c64f725630 --- /dev/null +++ b/gcc/testsuite/gcc.dg/associate_comparison_1.c @@ -0,0 +1,35 @@ +/* { dg-do compile } */ +
Re: [PATCH] Simplify floating point comparisons
(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 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 0difference 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
On Thu, Jan 4, 2018 at 10:27 PM, Marc Glisse 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 >>> 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 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
Richard Biener wrote: > On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra > 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 Jackson Woodruff 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 ..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
On 11/15/2017 08:36 AM, Wilco Dijkstra wrote: > Richard Biener wrote: >> On Tue, Oct 17, 2017 at 6:28 PM, Wilco Dijkstra >> 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