*[PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations@ 2017-06-24 3:59 Andrew Pinski2017-06-24 3:59 ` Andrew Pinski 2017-06-24 6:50 ` Marc Glisse 0 siblings, 2 replies; 7+ messages in thread From: Andrew Pinski @ 2017-06-24 3:59 UTC (permalink / raw) To: GCC Patches Hi, I saw this on llvm's review site (https://reviews.llvm.org/D34579) and I thought why not add it to GCC. I expanded more than what was done on the LLVM patch. I added the following optimizations: Transform X * (X > 0 ? 1 : -1) into ABS(X). Transform X * (X >= 0 ? 1 : -1) into ABS(X). Transform X * (X > 0.0 ? 1.0 : -1.0) into ABS(X). Transform X * (X >= 0.0 ? 1.0 : -1.0) into ABS(X). Transform X * (X > 0 ? -1 : 1) into -ABS(X). Transform X * (X >= 0 ? -1 : 1) into -ABS(X). Transform X * (X > 0.0 ? -1.0 : 1.0) into -ABS(X). Transform X * (X >= 0.0 ? -1.0 : 1.0) into -ABS(X). Transform X * (X < 0 ? 1 : -1) into -ABS(X). Transform X * (X <= 0 ? 1 : -1) into -ABS(X). Transform X * (X < 0.0 ? 1.0 : -1.0) into -ABS(X). Transform X * (X <= 0.0 ? 1.0 : -1.0) into -ABS(X). Transform X * (X < 0 ? -1 : 1) into ABS(X). Transform X * (X <= 0 ? -1 : 1) into ABS(X). Transform X * (X < 0.0 ? -1.0 : 1.0) into ABS(X). Transform X * (X <= 0.0 ? -1.0 : 1.0) into ABS(X). The floating points ones only happen when not honoring SNANS and not honoring signed zeros. OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. Thanks, Andrew Pinski ChangeLog: * match.pd ( X * (X >/>=/</<= 0 ? 1 : -1)): New patterns. Testsuite/ChangeLog: * testsuite/gcc.dg/tree-ssa/mult-abs-1.c: New testcase. * testsuite/gcc.dg/tree-ssa/mult-abs-2.c: New testcase. ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations2017-06-24 3:59 [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations Andrew Pinski@ 2017-06-24 3:59 ` Andrew Pinski2017-06-24 6:50 ` Marc Glisse 1 sibling, 0 replies; 7+ messages in thread From: Andrew Pinski @ 2017-06-24 3:59 UTC (permalink / raw) To: GCC Patches [-- Attachment #1: Type: text/plain, Size: 1523 bytes --] Forgot the patch On Fri, Jun 23, 2017 at 8:59 PM, Andrew Pinski <pinskia@gmail.com> wrote: > Hi, > I saw this on llvm's review site (https://reviews.llvm.org/D34579) > and I thought why not add it to GCC. I expanded more than what was > done on the LLVM patch. > > I added the following optimizations: > Transform X * (X > 0 ? 1 : -1) into ABS(X). > Transform X * (X >= 0 ? 1 : -1) into ABS(X). > Transform X * (X > 0.0 ? 1.0 : -1.0) into ABS(X). > Transform X * (X >= 0.0 ? 1.0 : -1.0) into ABS(X). > Transform X * (X > 0 ? -1 : 1) into -ABS(X). > Transform X * (X >= 0 ? -1 : 1) into -ABS(X). > Transform X * (X > 0.0 ? -1.0 : 1.0) into -ABS(X). > Transform X * (X >= 0.0 ? -1.0 : 1.0) into -ABS(X). > Transform X * (X < 0 ? 1 : -1) into -ABS(X). > Transform X * (X <= 0 ? 1 : -1) into -ABS(X). > Transform X * (X < 0.0 ? 1.0 : -1.0) into -ABS(X). > Transform X * (X <= 0.0 ? 1.0 : -1.0) into -ABS(X). > Transform X * (X < 0 ? -1 : 1) into ABS(X). > Transform X * (X <= 0 ? -1 : 1) into ABS(X). > Transform X * (X < 0.0 ? -1.0 : 1.0) into ABS(X). > Transform X * (X <= 0.0 ? -1.0 : 1.0) into ABS(X). > > The floating points ones only happen when not honoring SNANS and not > honoring signed zeros. > > OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. > > Thanks, > Andrew Pinski > > ChangeLog: > * match.pd ( X * (X >/>=/</<= 0 ? 1 : -1)): New patterns. > > Testsuite/ChangeLog: > * testsuite/gcc.dg/tree-ssa/mult-abs-1.c: New testcase. > * testsuite/gcc.dg/tree-ssa/mult-abs-2.c: New testcase. [-- Attachment #2: mult-abs.diff.txt --] [-- Type: text/plain, Size: 4029 bytes --] Index: match.pd =================================================================== --- match.pd (revision 249613) +++ match.pd (working copy) @@ -155,6 +155,55 @@ || !COMPLEX_FLOAT_TYPE_P (type))) (negate @0))) +(for cmp (gt ge) + /* Transform X * (X > 0 ? 1 : -1) into ABS(X). */ + /* Transform X * (X >= 0 ? 1 : -1) into ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 integer_zerop) integer_onep integer_all_onesp)) + (abs @0)) + /* Transform X * (X > 0.0 ? 1.0 : -1.0) into ABS(X). */ + /* Transform X * (X >= 0.0 ? 1.0 : -1.0) into ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 real_zerop) real_onep real_minus_onep)) + (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type)) + (abs @0))) + /* Transform X * (X > 0 ? -1 : 1) into -ABS(X). */ + /* Transform X * (X >= 0 ? -1 : 1) into -ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 integer_zerop) integer_all_onesp integer_onep)) + (negate (abs @0))) + /* Transform X * (X > 0.0 ? -1.0 : 1.0) into -ABS(X). */ + /* Transform X * (X >= 0.0 ? -1.0 : 1.0) into -ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 real_zerop) real_minus_onep real_onep)) + (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type)) + (negate (abs @0))))) + +(for cmp (lt le) + /* Transform X * (X < 0 ? 1 : -1) into -ABS(X). */ + /* Transform X * (X <= 0 ? 1 : -1) into -ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 integer_zerop) integer_onep integer_all_onesp)) + (negate (abs @0))) + /* Transform X * (X < 0.0 ? 1.0 : -1.0) into -ABS(X). */ + /* Transform X * (X <= 0.0 ? 1.0 : -1.0) into -ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 real_zerop) real_onep real_minus_onep)) + (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type)) + (negate (abs @0)))) + /* Transform X * (X < 0 ? -1 : 1) into ABS(X). */ + /* Transform X * (X <= 0 ? -1 : 1) into ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 integer_zerop) integer_all_onesp integer_onep)) + (abs @0)) + /* Transform X * (X < 0.0 ? -1.0 : 1.0) into ABS(X). */ + /* Transform X * (X <= 0.0 ? -1.0 : 1.0) into ABS(X). */ + (simplify + (mult:c @0 (cond (cmp @0 real_zerop) real_minus_onep real_onep)) + (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type)) + (abs @0)))) + + /* X * 1, X / 1 -> X. */ (for op (mult trunc_div ceil_div floor_div round_div exact_div) (simplify Index: testsuite/gcc.dg/tree-ssa/mult-abs-1.c =================================================================== --- testsuite/gcc.dg/tree-ssa/mult-abs-1.c (nonexistent) +++ testsuite/gcc.dg/tree-ssa/mult-abs-1.c (working copy) @@ -0,0 +1,35 @@ +/* { dg-options "-O2 -fdump-tree-gimple" } */ +/* { dg-do compile } */ +int f(int x) +{ + return x * (x > 0 ? -1 : 1); +} +int f1(int x) +{ + return x * (x > 0 ? 1 : -1); +} +int g(int x) +{ + return x * (x >= 0 ? -1 : 1); +} +int g1(int x) +{ + return x * (x >= 0 ? 1 : -1); +} +int h(int x) +{ + return x * (x < 0 ? -1 : 1); +} +int h1(int x) +{ + return x * (x < 0 ? 1 : -1); +} +int i(int x) +{ + return x * (x <= 0 ? -1 : 1); +} +int i1(int x) +{ + return x * (x <= 0 ? 1 : -1); +} +/* { dg-final { scan-tree-dump-times "ABS" 8 "gimple"} } */ Index: testsuite/gcc.dg/tree-ssa/mult-abs-2.c =================================================================== --- testsuite/gcc.dg/tree-ssa/mult-abs-2.c (nonexistent) +++ testsuite/gcc.dg/tree-ssa/mult-abs-2.c (working copy) @@ -0,0 +1,35 @@ +/* { dg-options "-O2 -ffast-math -fdump-tree-gimple" } */ +/* { dg-do compile } */ +float f(float x) +{ + return x * (x > 0.f ? -1.f : 1.f); +} +float f1(float x) +{ + return x * (x > 0.f ? 1.f : -1.f); +} +float g(float x) +{ + return x * (x >= 0.f ? -1.f : 1.f); +} +float g1(float x) +{ + return x * (x >= 0.f ? 1.f : -1.f); +} +float h(float x) +{ + return x * (x < 0.f ? -1.f : 1.f); +} +float h1(float x) +{ + return x * (x < 0.f ? 1.f : -1.f); +} +float i(float x) +{ + return x * (x <= 0.f ? -1.f : 1.f); +} +float i1(float x) +{ + return x * (x <= 0.f ? 1.f : -1.f); +} +/* { dg-final { scan-tree-dump-times "ABS" 8 "gimple"} } */ ^ permalink raw reply [flat|nested] 7+ messages in thread

*2017-06-24 3:59 [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations Andrew Pinski 2017-06-24 3:59 ` Andrew PinskiRe: [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations@ 2017-06-24 6:50 ` Marc Glisse2017-06-24 18:51 ` Andrew Pinski 2017-06-26 15:02 ` Joseph Myers 1 sibling, 2 replies; 7+ messages in thread From: Marc Glisse @ 2017-06-24 6:50 UTC (permalink / raw) To: Andrew Pinski;+Cc:GCC Patches On Fri, 23 Jun 2017, Andrew Pinski wrote: > Hi, > I saw this on llvm's review site (https://reviews.llvm.org/D34579) > and I thought why not add it to GCC. I expanded more than what was > done on the LLVM patch. > > I added the following optimizations: > Transform X * (X > 0 ? 1 : -1) into ABS(X). > Transform X * (X >= 0 ? 1 : -1) into ABS(X). > Transform X * (X > 0.0 ? 1.0 : -1.0) into ABS(X). > Transform X * (X >= 0.0 ? 1.0 : -1.0) into ABS(X). > Transform X * (X > 0 ? -1 : 1) into -ABS(X). > Transform X * (X >= 0 ? -1 : 1) into -ABS(X). > Transform X * (X > 0.0 ? -1.0 : 1.0) into -ABS(X). > Transform X * (X >= 0.0 ? -1.0 : 1.0) into -ABS(X). > Transform X * (X < 0 ? 1 : -1) into -ABS(X). > Transform X * (X <= 0 ? 1 : -1) into -ABS(X). > Transform X * (X < 0.0 ? 1.0 : -1.0) into -ABS(X). > Transform X * (X <= 0.0 ? 1.0 : -1.0) into -ABS(X). > Transform X * (X < 0 ? -1 : 1) into ABS(X). > Transform X * (X <= 0 ? -1 : 1) into ABS(X). > Transform X * (X < 0.0 ? -1.0 : 1.0) into ABS(X). > Transform X * (X <= 0.0 ? -1.0 : 1.0) into ABS(X). > > The floating points ones only happen when not honoring SNANS and not > honoring signed zeros. Some random comments (not a review): * if X is NaN, we may get a qNaN with the wrong sign bit. We probably don't care much though... * I am surprised (X<0.?-1.:1.) and copysign(1., X) remain different for the whole optimization pipeline with -ffast-math. X*copysign(1., X) is another candidate to become fabs(X). * Whenever you get -ABS(X) for integers, what about the case where X is INT_MIN? * I guess we can't get there with an unsigned type because X>0 would have become X!=0 . * I wonder if we could use something like (for cmp (gt ge lt le) outp (convert convert negate negate) outn (negate negate convert convert) [...] (outp (abs @0)) to reduce duplication or if that would be less readable. * Some of the cases are handled by PRE turning # iftmp.0_1 = PHI <1.0e+0(5), -1.0e+0(3)> _3 = iftmp.0_1 * a_2(D); into _5 = -a_2(D); [...] # iftmp.0_1 = PHI <1.0e+0(2), -1.0e+0(3)> # prephitmp_6 = PHI <a_2(D)(2), _5(3)> which phiopt3 can handle (quite late). * With cond, this currently (?) only affects generic, so I am not sure it will hit very often... But it will be there if someone later writes a match.pd->phiopt generator ;-) -- Marc Glisse ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations2017-06-24 6:50 ` Marc Glisse@ 2017-06-24 18:51 ` Andrew Pinski2017-06-24 19:47 ` Marc Glisse 2017-06-26 15:02 ` Joseph Myers 1 sibling, 1 reply; 7+ messages in thread From: Andrew Pinski @ 2017-06-24 18:51 UTC (permalink / raw) To: GCC Patches On Fri, Jun 23, 2017 at 11:50 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Fri, 23 Jun 2017, Andrew Pinski wrote: > >> Hi, >> I saw this on llvm's review site (https://reviews.llvm.org/D34579) >> and I thought why not add it to GCC. I expanded more than what was >> done on the LLVM patch. >> >> I added the following optimizations: >> Transform X * (X > 0 ? 1 : -1) into ABS(X). >> Transform X * (X >= 0 ? 1 : -1) into ABS(X). >> Transform X * (X > 0.0 ? 1.0 : -1.0) into ABS(X). >> Transform X * (X >= 0.0 ? 1.0 : -1.0) into ABS(X). >> Transform X * (X > 0 ? -1 : 1) into -ABS(X). >> Transform X * (X >= 0 ? -1 : 1) into -ABS(X). >> Transform X * (X > 0.0 ? -1.0 : 1.0) into -ABS(X). >> Transform X * (X >= 0.0 ? -1.0 : 1.0) into -ABS(X). >> Transform X * (X < 0 ? 1 : -1) into -ABS(X). >> Transform X * (X <= 0 ? 1 : -1) into -ABS(X). >> Transform X * (X < 0.0 ? 1.0 : -1.0) into -ABS(X). >> Transform X * (X <= 0.0 ? 1.0 : -1.0) into -ABS(X). >> Transform X * (X < 0 ? -1 : 1) into ABS(X). >> Transform X * (X <= 0 ? -1 : 1) into ABS(X). >> Transform X * (X < 0.0 ? -1.0 : 1.0) into ABS(X). >> Transform X * (X <= 0.0 ? -1.0 : 1.0) into ABS(X). >> >> The floating points ones only happen when not honoring SNANS and not >> honoring signed zeros. > > > Some random comments (not a review): > > * if X is NaN, we may get a qNaN with the wrong sign bit. We probably don't > care much though... Ok, I changed it to when not honoring NANs. > > * I am surprised (X<0.?-1.:1.) and copysign(1., X) remain different for the > whole optimization pipeline with -ffast-math. X*copysign(1., X) is another > candidate to become fabs(X). This might be a better idea because of ... > > * Whenever you get -ABS(X) for integers, what about the case where X is > INT_MIN? This. Yes this is an issue; I guess I need to rethink the integer patterns. > > * I guess we can't get there with an unsigned type because X>0 would have > become X!=0 . No, unsigned is not an issue. > > * I wonder if we could use something like > > (for cmp (gt ge lt le) > outp (convert convert negate negate) > outn (negate negate convert convert) > [...] > (outp (abs @0)) > > to reduce duplication or if that would be less readable. I did thought of that but I added the lt/le parts latter on. > > * Some of the cases are handled by PRE turning > > # iftmp.0_1 = PHI <1.0e+0(5), -1.0e+0(3)> > _3 = iftmp.0_1 * a_2(D); > > into > > _5 = -a_2(D); > [...] > # iftmp.0_1 = PHI <1.0e+0(2), -1.0e+0(3)> > # prephitmp_6 = PHI <a_2(D)(2), _5(3)> > > which phiopt3 can handle (quite late). > > * With cond, this currently (?) only affects generic, so I am not sure it > will hit very often... But it will be there if someone later writes a > match.pd->phiopt generator ;-) I have a start of this patch but I have not finished it yet. Both phiopt and ifcombine should be moved over to gimple match and simplify. I will submit a new patch which implements some of the above by the end of the day; I might split up the patch into two (one for the integer case and one for the floating point case). Thanks, Andrew > > -- > Marc Glisse ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations2017-06-24 18:51 ` Andrew Pinski@ 2017-06-24 19:47 ` Marc Glisse2017-06-24 20:53 ` Andrew Pinski 0 siblings, 1 reply; 7+ messages in thread From: Marc Glisse @ 2017-06-24 19:47 UTC (permalink / raw) To: Andrew Pinski;+Cc:GCC Patches On Sat, 24 Jun 2017, Andrew Pinski wrote: >> * if X is NaN, we may get a qNaN with the wrong sign bit. We probably don't >> care much though... > > Ok, I changed it to when not honoring NANs. Note that I have no idea what guarantees we give in gcc. It is quite possible that your patch is fine without this change, I only wanted to raise the question in case someone knows. -- Marc Glisse ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations2017-06-24 19:47 ` Marc Glisse@ 2017-06-24 20:53 ` Andrew Pinski0 siblings, 0 replies; 7+ messages in thread From: Andrew Pinski @ 2017-06-24 20:53 UTC (permalink / raw) To: GCC Patches On Sat, Jun 24, 2017 at 12:47 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Sat, 24 Jun 2017, Andrew Pinski wrote: > >>> * if X is NaN, we may get a qNaN with the wrong sign bit. We probably >>> don't >>> care much though... >> >> >> Ok, I changed it to when not honoring NANs. > > > Note that I have no idea what guarantees we give in gcc. It is quite > possible that your patch is fine without this change, I only wanted to raise > the question in case someone knows. So looking through, we do guarantee the sign of the NaNs except when not honoring NaNs in the first place. So the conversion from a>0?1.0:-1.0 to copysign will be conditional on honoring NaNs. But the x*copysign(1.0,x) will only conditional on not honoring sNaNs. > > -- > Marc Glisse ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations2017-06-24 6:50 ` Marc Glisse 2017-06-24 18:51 ` Andrew Pinski@ 2017-06-26 15:02 ` Joseph Myers1 sibling, 0 replies; 7+ messages in thread From: Joseph Myers @ 2017-06-26 15:02 UTC (permalink / raw) To: GCC Patches;+Cc:Andrew Pinski On Sat, 24 Jun 2017, Marc Glisse wrote: > * if X is NaN, we may get a qNaN with the wrong sign bit. We probably don't > care much though... The sign bit from a multiplication involving a NaN is not specified. *But* making any of these transformations with a qNaN loses the "invalid" exception from an ordered comparison involving a qNaN, so isn't valid in the case of (qNaNs respected and trapping-math). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-06-26 15:02 UTC | newest]Thread overview:7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-24 3:59 [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations Andrew Pinski 2017-06-24 3:59 ` Andrew Pinski 2017-06-24 6:50 ` Marc Glisse 2017-06-24 18:51 ` Andrew Pinski 2017-06-24 19:47 ` Marc Glisse 2017-06-24 20:53 ` Andrew Pinski 2017-06-26 15:02 ` Joseph Myers

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