On Mon, 18 Oct 2021 at 17:23, Richard Biener <rguent...@suse.de> wrote:
>
> On Mon, 18 Oct 2021, Prathamesh Kulkarni wrote:
>
> > On Mon, 18 Oct 2021 at 17:10, Richard Biener <rguent...@suse.de> wrote:
> > >
> > > On Mon, 18 Oct 2021, Prathamesh Kulkarni wrote:
> > >
> > > > On Mon, 18 Oct 2021 at 16:18, Richard Biener <rguent...@suse.de> wrote:
> > > > >
> > > > > On Mon, 18 Oct 2021, Prathamesh Kulkarni wrote:
> > > > >
> > > > > > Hi Richard,
> > > > > > As suggested in PR, I have attached WIP patch that adds two patterns
> > > > > > to match.pd:
> > > > > > erfc(x) --> 1 - erf(x) if canonicalize_math_p() and,
> > > > > > 1 - erf(x) --> erfc(x) if !canonicalize_math_p().
> > > > > >
> > > > > > This works to remove call to erfc for the following test:
> > > > > > double f(double x)
> > > > > > {
> > > > > > double g(double, double);
> > > > > >
> > > > > > double t1 = __builtin_erf (x);
> > > > > > double t2 = __builtin_erfc (x);
> > > > > > return g(t1, t2);
> > > > > > }
> > > > > >
> > > > > > with .optimized dump shows:
> > > > > > t1_2 = __builtin_erf (x_1(D));
> > > > > > t2_3 = 1.0e+0 - t1_2;
> > > > > >
> > > > > > However, for the following test:
> > > > > > double f(double x)
> > > > > > {
> > > > > > double g(double, double);
> > > > > >
> > > > > > double t1 = __builtin_erfc (x);
> > > > > > return t1;
> > > > > > }
> > > > > >
> > > > > > It canonicalizes erfc(x) to 1 - erf(x), but does not transform 1 -
> > > > > > erf(x) to erfc(x) again
> > > > > > post canonicalization.
> > > > > > -fdump-tree-folding shows that 1 - erf(x) --> erfc(x) gets applied,
> > > > > > but then it tries to
> > > > > > resimplify erfc(x), which fails post canonicalization. So we end up
> > > > > > with erfc(x) transformed to
> > > > > > 1 - erf(x) in .optimized dump, which I suppose isn't ideal.
> > > > > > Could you suggest how to proceed ?
> > > > >
> > > > > I applied your patch manually and it does the intended
> > > > > simplifications so I wonder what I am missing?
> > > > Would it be OK to always fold erfc(x) -> 1 - erf(x) even when there's
> > > > no erf(x) in the source ?
> > >
> > > I do think it's reasonable to expect erfc to be available when erf
> > > is and vice versa but note both are C99 specified functions (either
> > > requires -lm).
> > OK, thanks. Would it be OK to commit the patch after bootstrap+test ?
>
> Yes, but I'm confused because you say the patch doesn't work for you?
The patch works for me to CSE erf/erfc pair.
However when there's only erfc in the source, it canonicalizes erfc(x)
to 1 - erf(x) but later fails to uncanonicalize 1 - erf(x) back to
erfc(x)
with -O3 -funsafe-math-optimizations.
For,
t1 = __builtin_erfc(x),
.optimized dump shows:
_2 = __builtin_erf (x_1(D));
t1_3 = 1.0e+0 - _2;
and for,
double t1 = x + __builtin_erfc(x);
.optimized dump shows:
_3 = __builtin_erf (x_2(D));
_7 = x_2(D) + 1.0e+0;
t1_4 = _7 - _3;
I assume in both cases, we want erfc in the code-gen instead ?
I think the reason uncaonicalization fails is because the pattern 1 -
erf(x) to erfc(x)
gets applied, but then it fails in resimplifying erfc(x), and we end
up with 1 - erf(x) in code-gen.
>From gimple-match.c, it hits the simplification:
gimple_seq *lseq = seq;
if (__builtin_expect (!dbg_cnt
(match), 0)) goto next_after_fail1172;
if (__builtin_expect (dump_file &&
(dump_flags & TDF_FOLDING), 0)) fprintf (dump_file, "Applying pattern
%s:%d, %s:%d\n", "match.pd", 6162, __FILE__, __LINE__);
{
res_op->set_op (CFN_BUILT_IN_ERFC, type, 1);
res_op->ops[0] = captures[0];
res_op->resimplify (lseq, valueize);
return true;
}
But res_op->resimplify returns false, and doesn't end up adding to lseq.
As you suggest, should we instead handle this in fre to transform
erfc(x) to 1 - erf(x), only when
there's a matching erf(x) in the source ?
Thanks,
Prathamesh
>
> Btw, please add the testcase from the PR and also a testcase that shows
> the canonicalization is undone. Maybe you can also double-check that
> we handle x + erfc (x) because I see we associate that as
> (x + 1) - erf (x) which is then not recognized back to erfc.
>
> The less surprising (as to preserve the function called in the source)
> variant for the PR would be to teach CSE to lookup erf(x) when
> visiting erfc(x) and when found synthesize 1 - erf(x).
>
> That said, a mathematician should chime in on how important it is
> to preserve erfc vs. erf (precision or even speed).
>
> Thanks,
> Richard.
>
> > Thanks,
> > Prathamesh
> >
> > >
> > > Richard.
> > >
> > > > So for the following test:
> > > > double f(double x)
> > > > {
> > > > t1 = __builtin_erfc(x)
> > > > return t1;
> > > > }
> > > >
> > > > .optimized dump shows:
> > > > double f (double x)
> > > > {
> > > > double t1;
> > > > double _2;
> > > >
> > > > <bb 2> [local count: 1073741824]:
> > > > _2 = __builtin_erf (x_1(D));
> > > > t1_3 = 1.0e+0 - _2;
> > > > return t1_3;
> > > > }
> > > >
> > > > while before patch, it has:
> > > > t1_4 = __builtin_erfc (x_2(D)); [tail call]
> > > > return t1_4;
> > > >
> > > > Thanks,
> > > > Prathamesh
> > > >
> > > > >
> > > > > Richard.
> > > > >
> > > > > > Thanks,
> > > > > > Prathamesh
> > > > > >
> > > > >
> > > > > --
> > > > > Richard Biener <rguent...@suse.de>
> > > > > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > > > > Nuernberg,
> > > > > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
> > > >
> > >
> > > --
> > > Richard Biener <rguent...@suse.de>
> > > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> > > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
> >
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
