| Message ID | DB6PR0801MB205395FA43FC1A6547A981BC834C0@DB6PR0801MB2053.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Simplify floating point comparisons | expand |
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 >
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" } } */ >
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).
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" } } */
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
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" } } */
(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.
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
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
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;
+}
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
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?
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
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" } } */ +
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" } } */ > +
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" } } */
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. --