Re: [Patch Preview/RFC] tree-optimization/57994: Constant folding of infinity

[Richard Biener <rguenther at suse dot de>]

On Thu, 24 Oct 2013, Paolo Carlini wrote:

> Hi,
> 
> this is just a preview, but I decided to send it out early to understand if
> I'm on the right track or not. As you can see in the Bug, this started as a
> spin-off of a library issue with complex pow, which led us to:
> 
>     __builtin_exp(-__builtin_huge_val())
> 
> not being folded to a constant at compile-time. The reason is simple: the
> various do_mpfr_arg*, etc, all check real_isfinite on the arguments. In the
> audit trail of the bug we came to the conclusion that allowing non-NANs could
> make sense (the mpfr facilities appear to be quite solid in this case - maybe
> for NANs too, at least in the non-complex case, but that's another story).
> However, when today I started fiddling with this kind of change, I noticed
> that isn't enough for the kind of code we really care about for the original
> issue, involving logs too, thus something like:
> 
>   long double num = __builtin_logl(0L);
>   long double res = __builtin_expl(num);
> 
> because the log isn't folded at all for zero: fold_builtin_logarithm calls
> do_mpfr_arg1 with true as last argument. We can make progress if, when it's
> safe - is !flag_trapping_math && !flag_errno_math enough? - we pass true
> instead. Then we also have to tweak do_mpfr_ckconv, because it checks
> mpfr_number_p (and real_isfinite) on the result of the folding, thus ruling
> out infinities. Then we are almost there: the latter can be actually fully
> folded if -O1 is used, -O0 is not Ok, because otherwise num isn't const
> propagated to the evaluation of res. Is this so far by and large Ok? I'm
> attaching a corresponding patchlet (it of course lacks testcases and testsuite
> adjustments)
> 
> Note: in the patchlet I'm not trying to handle NaNs; neither complex numbers;
> neither bessel, remquo and lgamma (which could probably be added?) Also, I
> suppose we could discover other cases like fold_builtin_logarithm, where, in
> the context of folding infinities too, we may have to tweak a bit the way
> do_mpfr_* functions are called

Generally I think it's quite odd that do_mpfr_arg1 restricts operands
and results in any way.  mpfr should be able to return the correct
(exceptional) value for any inputs.  We have to restrict ourselves
with using the result only in the case we have to set errno (where
we can still constant propagate but have to leave the call around
for errno setting purposes) and if the function is supposed to trap on
exceptional values (same handling as errno).  But this logic should
be really in the callers.  Restricting what arguments we feed into
mpfr should never be necessary unless there are bugs in mpfr (which
should better be reported and fixed).

Not sure if we have to care about SNaN inputs in any way, but we'd
have -fsignalling-nans to control this anyway.

I have no opinion on the patch itself for now, we have to decide
first whether to trust mpfr enough to feed it with exceptional
values (where obviously the range test of do_mpfr_arg? doesn't work,
but AFAICS it's only to avoid errno cases which should be done
separately).

For example

@@ -8191,7 +8191,9 @@ fold_builtin_logarithm (location_t loc, tree fndec
       const enum built_in_function fcode = builtin_mathfn_code (arg);
 
       /* Calculate the result when the argument is a constant.  */
-      if ((res = do_mpfr_arg1 (arg, type, func, &dconst0, NULL, false)))
+      if ((res = do_mpfr_arg1 (arg, type, func, &dconst0, NULL,
+                              !flag_trapping_math && !flag_errno_math
+                              ? true : false)))
        return res;
 
       /* Special case, optimize logN(expN(x)) = x.  */

we should be able to constant fold for all inputs, but if arg <= 0
and flag_trapping_math || flag_errno_math then we have to fold to

 ( log (arg), <constant-folding-result> )

especially for the -ferrno-math case this allows to continue constant
folding with the result, removing otherwise dead code.

Not sure if this fixes the "not constant folding" (as the result
is a constant but with side-effects that still need to be executed
and represented by calls with unused result).

Richard.