On Mon, 5 Nov 2018 at 15:10, Richard Biener <richard.guent...@gmail.com> wrote: > > On Fri, Nov 2, 2018 at 10:37 AM Prathamesh Kulkarni > <prathamesh.kulka...@linaro.org> wrote: > > > > Hi, > > This patch adds two transforms to match.pd to CSE erf/erfc pair. > > erfc(x) is canonicalized to 1 - erf(x) and is then reversed to 1 - > > erf(x) when canonicalization is disabled and result of erf(x) has > > single use within 1 - erf(x). > > > > The patch regressed builtin-nonneg-1.c. The following test-case > > reproduces the issue with patch: > > > > void test(double d1) { > > if (signbit(erfc(d1))) > > link_failure_erfc(); > > } > > > > ssa dump: > > > > <bb 2> : > > _5 = __builtin_erf (d1_4(D)); > > _1 = 1.0e+0 - _5; > > _6 = _1 < 0.0; > > _2 = (int) _6; > > if (_2 != 0) > > goto <bb 3>; [INV] > > else > > goto <bb 4>; [INV] > > > > <bb 3> : > > link_failure_erfc (); > > > > <bb 4> : > > return; > > > > As can be seen, erfc(d1) is folded to 1 - erf(d1). > > forwprop then transforms the if condition from _2 != 0 > > to _5 > 1.0e+0 and that defeats DCE thus resulting in link failure > > in undefined reference to link_failure_erfc(). > > > > So, the patch adds another transform erf(x) > 1 -> 0. > > Ick. > > Why not canonicalize erf (x) to 1-erfc(x) instead? Sorry I didn't quite follow, won't this cause similar issue with erf ? I changed the pattern to canonicalize erf(x) -> 1 - erfc(x) and 1 - erfc(x) -> erf(x) after canonicalization is disabled.
This caused undefined reference to link_failure_erf() in following test-case: extern int signbit(double); extern void link_failure_erf(void); extern double erf(double); void test(double d1) { if (signbit(erf(d1))) link_failure_erf(); } forwprop1 shows: <bb 2> : _5 = __builtin_erfc (d1_4(D)); _1 = 1.0e+0 - _5; _6 = _5 > 1.0e+0; _2 = (int) _6; if (_5 > 1.0e+0) goto <bb 3>; [INV] else goto <bb 4>; [INV] <bb 3> : link_failure_erf (); <bb 4> : return; which defeats DCE to remove call to link_failure_erf. Thanks, Prathamesh > > > which resolves the regression. > > > > Bootstrapped+tested on x86_64-unknown-linux-gnu. > > Cross-testing on arm and aarch64 variants in progress. > > OK for trunk if passes ? > > > > Thanks, > > Prathamesh