Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
On 5/4/19 6:21 PM, Giuliano Belinassi wrote: > Hi > > On 04/30, Jeff Law wrote: >> On 4/30/19 8:00 AM, Jakub Jelinek wrote: >>> On Tue, Apr 30, 2019 at 07:57:20AM -0600, Jeff Law wrote: > Just curious, do we want to add math identities like above to match.pd ? I'd think so. > In practice, I am not sure how often we'd see "tanh * cosh" instead > of sinh directly in source, We're often surprised when what ultimately shows up in sources :-) And a transformation that allows us to turn two transcendentals and a multiplication into a single transcendental is going to be a big win. >>> > > I wonder why these kind of optimization are not in the EasyHacks page. I doubt anyone has really thought about them. It's certainly appropriate to add more stuff to that page ;-) > It is somewhat easy to add it and can give huge performance > improvements. Agreed. > > There is a blogpost that I wrote about the previous patches I submitted > here. I tried to be didadic, discussing from the mathematical > standpoint, to floating point numbers, and GCC implementation. > Although there may be several english errors, I think it is still > useful: > > https://flusp.ime.usp.br/gcc/2019/03/26/making-gcc-optimize-some-trigonometric-functions/ Thanks. It's funny you mention high school math, I had to do a fair amount of refreshing when we were working on your patches ;-) jeff
Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
Hi On 04/30, Jeff Law wrote: > On 4/30/19 8:00 AM, Jakub Jelinek wrote: > > On Tue, Apr 30, 2019 at 07:57:20AM -0600, Jeff Law wrote: > >>> Just curious, do we want to add math identities like above to match.pd ? > >> I'd think so. > >> > >> > >>> In practice, I am not sure how often we'd see "tanh * cosh" instead > >>> of sinh directly in source, > >> We're often surprised when what ultimately shows up in sources :-) And > >> a transformation that allows us to turn two transcendentals and a > >> multiplication into a single transcendental is going to be a big win. > > I wonder why these kind of optimization are not in the EasyHacks page. It is somewhat easy to add it and can give huge performance improvements. There is a blogpost that I wrote about the previous patches I submitted here. I tried to be didadic, discussing from the mathematical standpoint, to floating point numbers, and GCC implementation. Although there may be several english errors, I think it is still useful: https://flusp.ime.usp.br/gcc/2019/03/26/making-gcc-optimize-some-trigonometric-functions/ > > I guess an important question is if such transformations need to be guarded > > by some -ffast-math suboptions, whether those transformations work properly > > for signed zeros, NaNs, sNaNs, infinities, guarantee the same ulp, have the > > same -ferrno-math behavior etc. > Yes. If you look at the discussion with Gualiano for the first set in > this space that was the hardest part. And for Barbara's we know there's > at least one issue in this space as well -- in particular it looks like > the long double handling is incorrect in the sign bit for very small > inputs. The question is precisely where that's gone wrong :-) > > jeff Giuliano.
Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
On 4/30/19 8:00 AM, Jakub Jelinek wrote: > On Tue, Apr 30, 2019 at 07:57:20AM -0600, Jeff Law wrote: >>> Just curious, do we want to add math identities like above to match.pd ? >> I'd think so. >> >> >>> In practice, I am not sure how often we'd see "tanh * cosh" instead >>> of sinh directly in source, >> We're often surprised when what ultimately shows up in sources :-) And >> a transformation that allows us to turn two transcendentals and a >> multiplication into a single transcendental is going to be a big win. > > I guess an important question is if such transformations need to be guarded > by some -ffast-math suboptions, whether those transformations work properly > for signed zeros, NaNs, sNaNs, infinities, guarantee the same ulp, have the > same -ferrno-math behavior etc. Yes. If you look at the discussion with Gualiano for the first set in this space that was the hardest part. And for Barbara's we know there's at least one issue in this space as well -- in particular it looks like the long double handling is incorrect in the sign bit for very small inputs. The question is precisely where that's gone wrong :-) jeff
Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
On Tue, Apr 30, 2019 at 07:57:20AM -0600, Jeff Law wrote: > > Just curious, do we want to add math identities like above to match.pd ? > I'd think so. > > > > In practice, I am not sure how often we'd see "tanh * cosh" instead > > of sinh directly in source, > We're often surprised when what ultimately shows up in sources :-) And > a transformation that allows us to turn two transcendentals and a > multiplication into a single transcendental is going to be a big win. I guess an important question is if such transformations need to be guarded by some -ffast-math suboptions, whether those transformations work properly for signed zeros, NaNs, sNaNs, infinities, guarantee the same ulp, have the same -ferrno-math behavior etc. Jakub
Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
On 4/29/19 9:58 PM, Prathamesh Kulkarni wrote: > On Tue, 30 Apr 2019 at 02:56, Jeff Law wrote: >> >> On 1/30/19 7:10 AM, Bárbara de Castro Fernandes wrote: >>> This patch simplifies the function tanh (x) * cosh (x) -> sinh (x). >>> This rule is derived from the relationship between hyperbolic >>> functions. >>> >>> I ran the tests and gfortran.dg/pr79966.f90 failed, but this failure >>> is unrelated to the patch (see >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88711 for more >>> information). My architecture is x86_64. >>> >>> gcc/ChangeLog: >>> 2019-01-30 Bárbara Fernandes >>> >>> * match.pd (tanh (x) * cosh (x)): New simplification rule. >>> >>> gcc/testsuite/ChangeLog: >>> 2019-01-30 Bárbara Fernandes >>> >>> * tanhtimescosh.c: New test. >> So how does this behave for numbers extremely close to zero in practice >> and do we have to worry about overflow problems when cosh(x) gets large? > Hi Jeff, > Just curious, do we want to add math identities like above to match.pd ? I'd think so. > In practice, I am not sure how often we'd see "tanh * cosh" instead > of sinh directly in source, We're often surprised when what ultimately shows up in sources :-) And a transformation that allows us to turn two transcendentals and a multiplication into a single transcendental is going to be a big win. > altho it seems we do simplify tan * cos -> sin. > In general, I suppose there are lot of identities we haven't added > because the likelihood of "match" part isn't high. Perhaps. I have to assume the submitter submitted this particular simplification for a reason. jeff
Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
On Tue, 30 Apr 2019 at 02:56, Jeff Law wrote: > > On 1/30/19 7:10 AM, Bárbara de Castro Fernandes wrote: > > This patch simplifies the function tanh (x) * cosh (x) -> sinh (x). > > This rule is derived from the relationship between hyperbolic > > functions. > > > > I ran the tests and gfortran.dg/pr79966.f90 failed, but this failure > > is unrelated to the patch (see > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88711 for more > > information). My architecture is x86_64. > > > > gcc/ChangeLog: > > 2019-01-30 Bárbara Fernandes > > > > * match.pd (tanh (x) * cosh (x)): New simplification rule. > > > > gcc/testsuite/ChangeLog: > > 2019-01-30 Bárbara Fernandes > > > > * tanhtimescosh.c: New test. > So how does this behave for numbers extremely close to zero in practice > and do we have to worry about overflow problems when cosh(x) gets large? Hi Jeff, Just curious, do we want to add math identities like above to match.pd ? In practice, I am not sure how often we'd see "tanh * cosh" instead of sinh directly in source, altho it seems we do simplify tan * cos -> sin. In general, I suppose there are lot of identities we haven't added because the likelihood of "match" part isn't high. Thanks, Prathamesh > > jeff > > >
Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
On 1/30/19 7:10 AM, Bárbara de Castro Fernandes wrote: > This patch simplifies the function tanh (x) * cosh (x) -> sinh (x). > This rule is derived from the relationship between hyperbolic > functions. > > I ran the tests and gfortran.dg/pr79966.f90 failed, but this failure > is unrelated to the patch (see > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88711 for more > information). My architecture is x86_64. > > gcc/ChangeLog: > 2019-01-30 Bárbara Fernandes > > * match.pd (tanh (x) * cosh (x)): New simplification rule. > > gcc/testsuite/ChangeLog: > 2019-01-30 Bárbara Fernandes > > * tanhtimescosh.c: New test. So how does this behave for numbers extremely close to zero in practice and do we have to worry about overflow problems when cosh(x) gets large? jeff >
Re: [PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
On 1/30/19 7:10 AM, Bárbara de Castro Fernandes wrote: > This patch simplifies the function tanh (x) * cosh (x) -> sinh (x). > This rule is derived from the relationship between hyperbolic > functions. > > I ran the tests and gfortran.dg/pr79966.f90 failed, but this failure > is unrelated to the patch (see > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88711 for more > information). My architecture is x86_64. > > gcc/ChangeLog: > 2019-01-30 Bárbara Fernandes > > * match.pd (tanh (x) * cosh (x)): New simplification rule. > > gcc/testsuite/ChangeLog: > 2019-01-30 Bárbara Fernandes > > * tanhtimescosh.c: New test. > Just a note. The trunk is only open for regression bugfixes right now as we prepare for the spring release. This patch has been queued for analysis after the gcc-9 release. jeff
[PATCH] Add simplification rule tanh (x) * cosh (x) -> sinh (x)
This patch simplifies the function tanh (x) * cosh (x) -> sinh (x).
This rule is derived from the relationship between hyperbolic
functions.
I ran the tests and gfortran.dg/pr79966.f90 failed, but this failure
is unrelated to the patch (see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88711 for more
information). My architecture is x86_64.
gcc/ChangeLog:
2019-01-30 Bárbara Fernandes
* match.pd (tanh (x) * cosh (x)): New simplification rule.
gcc/testsuite/ChangeLog:
2019-01-30 Bárbara Fernandes
* tanhtimescosh.c: New test.
Index: gcc/match.pd
===
--- gcc/match.pd (revision 268384)
+++ gcc/match.pd (working copy)
@@ -4545,6 +4545,11 @@
&& ! HONOR_INFINITIES (@0))
(rdiv { build_one_cst (type); } (COS @0
+ /* Simplify tanh (x) * cosh (x) -> sinh (x). */
+ (simplify
+ (mult:c (TANH:s @0) (COSH:s @0))
+ (SINH @0))
+
/* Simplify pow(x,y) * pow(x,z) -> pow(x,y+z). */
(simplify
(mult (POW:s @0 @1) (POW:s @0 @2))
Index: gcc/testsuite/gcc.dg/tanhtimescosh.c
===
--- gcc/testsuite/gcc.dg/tanhtimescosh.c (nonexistent)
+++ gcc/testsuite/gcc.dg/tanhtimescosh.c (working copy)
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-optimized" } */
+
+extern float coshf (float);
+extern float tanhf (float);
+extern double cosh (double);
+extern double tanh (double);
+extern long double coshl (long double);
+extern long double tanhl (long double);
+
+double __attribute__ ((noinline))
+sinh_ (double x)
+{
+return tanh (x) * cosh (x);
+}
+
+float __attribute__ ((noinline))
+sinhf_(float x)
+{
+return tanhf (x) * coshf (x);
+}
+
+long double __attribute__ ((noinline))
+sinhl_ (long double x)
+{
+return tanhl (x) * coshl (x);
+}
+
+/* There must be no calls to cosh, or tanh */
+/* {dg-final { scan-tree-dump-not "cosh " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "tanh " "optimized" }} */
+/* {dg-final { scan-tree-dump-not "coshf " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "tanhf " "optimized" }} */
+/* {dg-final { scan-tree-dump-not "coshl " "optimized" } } */
+/* {dg-final { scan-tree-dump-not "tanhl " "optimized" }} */
+/* {dg-final { scan-tree-dump-times "sinh " "1" "optimized" }} */
+/* {dg-final { scan-tree-dump-times "sinhf " "1" "optimized" }} */
+/* {dg-final { scan-tree-dump-times "sinhl " "1" "optimized" }} */
