This is the mail archive of the
`gcc-patches@gcc.gnu.org`
mailing list for the GCC project.

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |

Other format: | [Raw text] |

*From*: Richard Biener <richard dot guenther at gmail dot com>*To*: Christophe Lyon <christophe dot lyon at linaro dot org>*Cc*: Andrew Pinski <pinskia at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>*Date*: Wed, 28 Jun 2017 11:29:34 +0200*Subject*: Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)*Authentication-results*: sourceware.org; auth=none*References*: <CA+=Sn1m-pMkB1Vvoi3s_N0DSwLioB3T90778oSDQNOYME83txA@mail.gmail.com> <alpine.DEB.2.20.1706250909190.2070@stedding.saclay.inria.fr> <CA+=Sn1nj_TNWUCGn9V_hqF2gcPj=WbzxN565FOA02O6qFg8_cA@mail.gmail.com> <CA+=Sn1mdyqOy+6UuuTXJH6UvtdNT2AS_VLWCHU_kK3kXg+apBQ@mail.gmail.com> <CAKdteOZ_fJt0aeE3k3oo3SPAiRZDzFCFUq9_GgEE0F=Nn2RJMQ@mail.gmail.com>

On Wed, Jun 28, 2017 at 10:50 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote: > On 25 June 2017 at 23:28, Andrew Pinski <pinskia@gmail.com> wrote: >> On Sun, Jun 25, 2017 at 11:18 AM, Andrew Pinski <pinskia@gmail.com> wrote: >>> On Sun, Jun 25, 2017 at 1:28 AM, Marc Glisse <marc.glisse@inria.fr> wrote: >>>> +(for cmp (gt ge lt le) >>>> + outp (convert convert negate negate) >>>> + outn (negate negate convert convert) >>>> + /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */ >>>> + /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */ >>>> + /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */ >>>> + /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */ >>>> + (simplify >>>> + (cond (cmp @0 real_zerop) real_onep real_minus_onep) >>>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type) >>>> + && types_match (type, TREE_TYPE (@0))) >>>> + (switch >>>> + (if (types_match (type, float_type_node)) >>>> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outp @0))) >>>> + (if (types_match (type, double_type_node)) >>>> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outp @0))) >>>> + (if (types_match (type, long_double_type_node)) >>>> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outp @0)))))) >>>> >>>> There is already a 1.0 of the right type in the input, it would be easier to >>>> reuse it in the output than build a new one. >>> >>> Right. Fixed. >>> >>>> >>>> Non-generic builtins like copysign are such a pain... We also end up missing >>>> the 128-bit case that way (pre-existing problem, not your patch). We seem to >>>> have a corresponding internal function, but apparently it is not used until >>>> expansion (well, maybe during vectorization). >>> >>> Yes I noticed that while working on a different patch related to >>> copysign; The generic version of a*copysign(1.0, b) [see the other >>> thread where the ARM folks started a patch for it; yes it was by pure >>> accident that I was working on this and really did not notice that >>> thread until yesterday]. >>> I was looking into a nice way of creating copysign without having to >>> do the switch but I could not find one. In the end I copied was done >>> already in a different location in match.pd; this is also the reason >>> why I had the build_one_cst there. >>> >>>> >>>> + /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */ >>>> + /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */ >>>> + /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */ >>>> + /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */ >>>> + (simplify >>>> + (cond (cmp @0 real_zerop) real_minus_onep real_onep) >>>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type) >>>> + && types_match (type, TREE_TYPE (@0))) >>>> + (switch >>>> + (if (types_match (type, float_type_node)) >>>> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outn @0))) >>>> + (if (types_match (type, double_type_node)) >>>> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outn @0))) >>>> + (if (types_match (type, long_double_type_node)) >>>> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outn @0))))))) >>>> + >>>> +/* Transform X * copysign (1.0, X) into abs(X). */ >>>> +(simplify >>>> + (mult:c @0 (COPYSIGN real_onep @0)) >>>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)) >>>> + (abs @0))) >>>> >>>> I would have expected it do to the right thing for signed zero and qNaN. Can >>>> you describe a case where it would give the wrong result, or are the >>>> conditions just conservative? >>> >>> I was just being conservative; maybe too conservative but I was a bit >>> worried I could get it incorrect. >>> >>>> >>>> +/* Transform X * copysign (1.0, -X) into -abs(X). */ >>>> +(simplify >>>> + (mult:c @0 (COPYSIGN real_onep (negate @0))) >>>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)) >>>> + (negate (abs @0)))) >>>> + >>>> +/* Transform copysign (-1.0, X) into copysign (1.0, X). */ >>>> +(simplify >>>> + (COPYSIGN real_minus_onep @0) >>>> + (COPYSIGN { build_one_cst (type); } @0)) >>>> >>>> (simplify >>>> (COPYSIGN REAL_CST@0 @1) >>>> (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0))) >>>> (COPYSIGN (negate @0) @1))) >>>> ? Or does that create trouble with sNaN and only the 1.0 case is worth >>>> the trouble? >>> >>> No that is the correct way; I Noticed the other thread about copysign >>> had something similar as what should be done too. >>> >>> I will send out a new patch after testing soon. >> >> New patch. >> OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. >> > Hi Andrew, > > 2 of the new testcases fail on aarch64*-elf: > FAIL: gcc.dg/tree-ssa/copy-sign-1.c scan-tree-dump-times gimple "copysign" 8 > FAIL: gcc.dg/tree-ssa/mult-abs-2.c scan-tree-dump-times gimple "ABS" 8 > > The attached patch makes them unsupported by requiring c99_runtime > effective-target. > > OK? Ok. > >> Thanks, >> Andrew Pinski >> >> ChangeLog: >> * match.pd (X >/>=/</<= 0 ? 1.0 : -1.0): New patterns. >> (X * copysign (1.0, X)): New pattern. >> (X * copysign (1.0, -X)): New pattern. >> (copysign (-1.0, CST)): New pattern. >> >> testsuite/ChangeLog: >> * gcc.dg/tree-ssa/copy-sign-1.c: New testcase. >> * gcc.dg/tree-ssa/copy-sign-2.c: New testcase. >> * gcc.dg/tree-ssa/mult-abs-2.c: New testcase. >> >>> >>> Thanks, >>> Andrew >>> >>>> >>>> -- >>>> Marc Glisse

**References**:**[PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)***From:*Andrew Pinski

**Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)***From:*Marc Glisse

**Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)***From:*Andrew Pinski

**Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)***From:*Andrew Pinski

**Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)***From:*Christophe Lyon

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |