Message ID | alpine.DEB.2.02.1604211217410.22599@laptop-mg.saclay.inria.fr |
---|---|
State | New |
Headers | show |
On Thu, Apr 21, 2016 at 12:32 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > Hello, > > another simple transformation. > > Instead of the ":s", I had single_use (@2) || single_use (@3), but changed > it for simplicity. There may be some patterns in match.pd where we want > something like that though, as requiring single_use on many expressions may > be stricter than we need. > > We could generalize to cases where overflow is not undefined if we know > (VRP) that the variables are not TYPE_MIN_VALUE, but that didn't look like a > priority. > > Bootstrap+regtest on powerpc64le-unknown-linux-gnu. Ok. I thought about using negate_expr_p but min(-x,5) -> -max(x, -5) doesn't look like an obvious win. Thanks, Richard. > 2016-04-21 Marc Glisse <marc.glisse@inria.fr> > > gcc/ > * match.pd (min(-x, -y), max(-x, -y), min(~x, ~y), max(~x, ~y)): > New transformations. > > gcc/testsuite/ > * gcc.dg/tree-ssa/minmax-2.c: New testcase. > > > -- > Marc Glisse > Index: gcc/match.pd > =================================================================== > --- gcc/match.pd (revision 235292) > +++ gcc/match.pd (working copy) > @@ -1215,20 +1215,36 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > MIN and MAX don't honor that, so only transform if -ffinite-math-only > is set. C99 doesn't require -0.0 to be handled, so we don't have to > worry about it either. */ > (if (flag_finite_math_only) > (simplify > (FMIN @0 @1) > (min @0 @1)) > (simplify > (FMAX @0 @1) > (max @0 @1))) > +/* min (-A, -B) -> -max (A, B) */ > +(for minmax (min max FMIN FMAX) > + maxmin (max min FMAX FMIN) > + (simplify > + (minmax (negate:s@2 @0) (negate:s@3 @1)) > + (if (FLOAT_TYPE_P (TREE_TYPE (@0)) > + || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0)) > + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))) > + (negate (maxmin @0 @1))))) > +/* MIN (~X, ~Y) -> ~MAX (X, Y) > + MAX (~X, ~Y) -> ~MIN (X, Y) */ > +(for minmax (min max) > + maxmin (max min) > + (simplify > + (minmax (bit_not:s@2 @0) (bit_not:s@3 @1)) > + (bit_not (maxmin @0 @1)))) > > /* Simplifications of shift and rotates. */ > > (for rotate (lrotate rrotate) > (simplify > (rotate integer_all_onesp@0 @1) > @0)) > > /* Optimize -1 >> x for arithmetic right shifts. */ > (simplify > Index: gcc/testsuite/gcc.dg/tree-ssa/minmax-2.c > =================================================================== > --- gcc/testsuite/gcc.dg/tree-ssa/minmax-2.c (revision 0) > +++ gcc/testsuite/gcc.dg/tree-ssa/minmax-2.c (working copy) > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O -fstrict-overflow -fdump-tree-optimized" } */ > + > +static int max(int a,int b){return (a<b)?b:a;} > +int f(int x,int y){return max(-x,-y);} > +int g(int x,int y){return max(~x,~y);} > +double h(double x,double y){return __builtin_fmax(-x,-y);} > + > +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "optimized" } } */ > +/* { dg-final { scan-tree-dump "__builtin_fmin" "optimized" } } */ >
Hi Marc, On 21/04/16 11:32, Marc Glisse wrote: > Hello, > > another simple transformation. > > Instead of the ":s", I had single_use (@2) || single_use (@3), but changed it for simplicity. There may be some patterns in match.pd where we want something like that though, as requiring single_use on many expressions may be stricter > than we need. > > We could generalize to cases where overflow is not undefined if we know (VRP) that the variables are not TYPE_MIN_VALUE, but that didn't look like a priority. > > Bootstrap+regtest on powerpc64le-unknown-linux-gnu. > > 2016-04-21 Marc Glisse <marc.glisse@inria.fr> > > gcc/ > * match.pd (min(-x, -y), max(-x, -y), min(~x, ~y), max(~x, ~y)): > New transformations. > > gcc/testsuite/ > * gcc.dg/tree-ssa/minmax-2.c: New testcase. > > I see the new testcase failing on aarch64: FAIL: gcc.dg/tree-ssa/minmax-2.c scan-tree-dump optimized "__builtin_fmin" The tree dump for the function 'h' is: h (double x, double y) { double _2; double _4; double _5; <bb 2>: _2 = -x_1(D); _4 = -y_3(D); _5 = __builtin_fmax (_2, _4); return _5; } Kyrill
On Fri, 22 Apr 2016, Kyrill Tkachov wrote: >> 2016-04-21 Marc Glisse <marc.glisse@inria.fr> >> >> gcc/ >> * match.pd (min(-x, -y), max(-x, -y), min(~x, ~y), max(~x, ~y)): >> New transformations. >> >> gcc/testsuite/ >> * gcc.dg/tree-ssa/minmax-2.c: New testcase. >> >> > > I see the new testcase failing on aarch64: > FAIL: gcc.dg/tree-ssa/minmax-2.c scan-tree-dump optimized "__builtin_fmin" Strange, it seems to work in https://gcc.gnu.org/ml/gcc-testresults/2016-04/msg02120.html Is that on some freestanding kind of setup where the builtin might be disabled?
On 22/04/16 10:42, Marc Glisse wrote: > On Fri, 22 Apr 2016, Kyrill Tkachov wrote: > >>> 2016-04-21 Marc Glisse <marc.glisse@inria.fr> >>> >>> gcc/ >>> * match.pd (min(-x, -y), max(-x, -y), min(~x, ~y), max(~x, ~y)): >>> New transformations. >>> >>> gcc/testsuite/ >>> * gcc.dg/tree-ssa/minmax-2.c: New testcase. >>> >>> >> >> I see the new testcase failing on aarch64: >> FAIL: gcc.dg/tree-ssa/minmax-2.c scan-tree-dump optimized "__builtin_fmin" > > Strange, it seems to work in https://gcc.gnu.org/ml/gcc-testresults/2016-04/msg02120.html > > Is that on some freestanding kind of setup where the builtin might be disabled? > Ah, this is aarch64-none-elf which uses newlib as the C library. Let me check on aarch64-none-linux-gnu and get back to you. Thanks, Kyrill
On 22/04/16 10:43, Kyrill Tkachov wrote: > > On 22/04/16 10:42, Marc Glisse wrote: >> On Fri, 22 Apr 2016, Kyrill Tkachov wrote: >> >>>> 2016-04-21 Marc Glisse <marc.glisse@inria.fr> >>>> >>>> gcc/ >>>> * match.pd (min(-x, -y), max(-x, -y), min(~x, ~y), max(~x, ~y)): >>>> New transformations. >>>> >>>> gcc/testsuite/ >>>> * gcc.dg/tree-ssa/minmax-2.c: New testcase. >>>> >>>> >>> >>> I see the new testcase failing on aarch64: >>> FAIL: gcc.dg/tree-ssa/minmax-2.c scan-tree-dump optimized "__builtin_fmin" >> >> Strange, it seems to work in https://gcc.gnu.org/ml/gcc-testresults/2016-04/msg02120.html >> >> Is that on some freestanding kind of setup where the builtin might be disabled? >> > > Ah, this is aarch64-none-elf which uses newlib as the C library. > Let me check on aarch64-none-linux-gnu and get back to you. > Yeah, I see it passing on aarch64-none-linux-gnu. Do we have an appropriate effective target check to gate this test on? Kyrill > Thanks, > Kyrill >
On Fri, 22 Apr 2016, Kyrill Tkachov wrote: > > On 22/04/16 10:43, Kyrill Tkachov wrote: >> >> On 22/04/16 10:42, Marc Glisse wrote: >>> On Fri, 22 Apr 2016, Kyrill Tkachov wrote: >>> >>>>> 2016-04-21 Marc Glisse <marc.glisse@inria.fr> >>>>> >>>>> gcc/ >>>>> * match.pd (min(-x, -y), max(-x, -y), min(~x, ~y), max(~x, ~y)): >>>>> New transformations. >>>>> >>>>> gcc/testsuite/ >>>>> * gcc.dg/tree-ssa/minmax-2.c: New testcase. >>>>> >>>>> >>>> >>>> I see the new testcase failing on aarch64: >>>> FAIL: gcc.dg/tree-ssa/minmax-2.c scan-tree-dump optimized >>>> "__builtin_fmin" >>> >>> Strange, it seems to work in >>> https://gcc.gnu.org/ml/gcc-testresults/2016-04/msg02120.html >>> >>> Is that on some freestanding kind of setup where the builtin might be >>> disabled? >>> >> >> Ah, this is aarch64-none-elf which uses newlib as the C library. >> Let me check on aarch64-none-linux-gnu and get back to you. >> > > Yeah, I see it passing on aarch64-none-linux-gnu. > Do we have an appropriate effective target check to gate this test on? I don't know, I have a hard time finding something related. I am not even convinced the test should be skipped. It looks like __builtin_fmax was recognized, otherwise you would get a warning and a conversion int-double. Maybe gimple_call_combined_fn rejects it? Ah, builtins.def declares it with DEF_C99_BUILTIN, which checks targetm.libc_has_function (function_c99_misc). I assume newlib fails that check? That would make c99_runtime a relevant target check.
On 22/04/16 11:34, Marc Glisse wrote: > On Fri, 22 Apr 2016, Kyrill Tkachov wrote: > >> >> On 22/04/16 10:43, Kyrill Tkachov wrote: >>> >>> On 22/04/16 10:42, Marc Glisse wrote: >>>> On Fri, 22 Apr 2016, Kyrill Tkachov wrote: >>>> >>>>>> 2016-04-21 Marc Glisse <marc.glisse@inria.fr> >>>>>> >>>>>> gcc/ >>>>>> * match.pd (min(-x, -y), max(-x, -y), min(~x, ~y), max(~x, ~y)): >>>>>> New transformations. >>>>>> >>>>>> gcc/testsuite/ >>>>>> * gcc.dg/tree-ssa/minmax-2.c: New testcase. >>>>>> >>>>>> >>>>> >>>>> I see the new testcase failing on aarch64: >>>>> FAIL: gcc.dg/tree-ssa/minmax-2.c scan-tree-dump optimized "__builtin_fmin" >>>> >>>> Strange, it seems to work in https://gcc.gnu.org/ml/gcc-testresults/2016-04/msg02120.html >>>> >>>> Is that on some freestanding kind of setup where the builtin might be disabled? >>>> >>> >>> Ah, this is aarch64-none-elf which uses newlib as the C library. >>> Let me check on aarch64-none-linux-gnu and get back to you. >>> >> >> Yeah, I see it passing on aarch64-none-linux-gnu. >> Do we have an appropriate effective target check to gate this test on? > > I don't know, I have a hard time finding something related. I am not even convinced the test should be skipped. It looks like __builtin_fmax was recognized, otherwise you would get a warning and a conversion int-double. Maybe > gimple_call_combined_fn rejects it? Ah, builtins.def declares it with DEF_C99_BUILTIN, which checks targetm.libc_has_function (function_c99_misc). I assume newlib fails that check? That would make c99_runtime a relevant target check. > Yeah, adding the below makes this test UNSUPPORTED on aarch64-none-elf. /* { dg-add-options c99_runtime } */ /* { dg-require-effective-target c99_runtime } */ I'll prepare a patch. Thanks, Kyrill
Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 235292) +++ gcc/match.pd (working copy) @@ -1215,20 +1215,36 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) MIN and MAX don't honor that, so only transform if -ffinite-math-only is set. C99 doesn't require -0.0 to be handled, so we don't have to worry about it either. */ (if (flag_finite_math_only) (simplify (FMIN @0 @1) (min @0 @1)) (simplify (FMAX @0 @1) (max @0 @1))) +/* min (-A, -B) -> -max (A, B) */ +(for minmax (min max FMIN FMAX) + maxmin (max min FMAX FMIN) + (simplify + (minmax (negate:s@2 @0) (negate:s@3 @1)) + (if (FLOAT_TYPE_P (TREE_TYPE (@0)) + || (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0)) + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))) + (negate (maxmin @0 @1))))) +/* MIN (~X, ~Y) -> ~MAX (X, Y) + MAX (~X, ~Y) -> ~MIN (X, Y) */ +(for minmax (min max) + maxmin (max min) + (simplify + (minmax (bit_not:s@2 @0) (bit_not:s@3 @1)) + (bit_not (maxmin @0 @1)))) /* Simplifications of shift and rotates. */ (for rotate (lrotate rrotate) (simplify (rotate integer_all_onesp@0 @1) @0)) /* Optimize -1 >> x for arithmetic right shifts. */ (simplify Index: gcc/testsuite/gcc.dg/tree-ssa/minmax-2.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/minmax-2.c (revision 0) +++ gcc/testsuite/gcc.dg/tree-ssa/minmax-2.c (working copy) @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fstrict-overflow -fdump-tree-optimized" } */ + +static int max(int a,int b){return (a<b)?b:a;} +int f(int x,int y){return max(-x,-y);} +int g(int x,int y){return max(~x,~y);} +double h(double x,double y){return __builtin_fmax(-x,-y);} + +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "optimized" } } */ +/* { dg-final { scan-tree-dump "__builtin_fmin" "optimized" } } */