diff mbox

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

Message ID 52694269.9080109@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Oct. 24, 2013, 3:53 p.m. UTC
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

Thanks!
Paolo.

PS: there are interesting issues about the non-compile-time constant 
case, especially vs long double, which I didn't touch here.

////////////////////////

Comments

Richard Biener Oct. 25, 2013, 8:01 a.m. UTC | #1
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.
diff mbox

Patch

Index: builtins.c
===================================================================
--- builtins.c	(revision 204016)
+++ builtins.c	(working copy)
@@ -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.  */
@@ -13527,7 +13529,7 @@  do_mpfr_ckconv (mpfr_srcptr m, tree type, int inex
   /* Proceed iff we get a normal number, i.e. not NaN or Inf and no
      overflow/underflow occurred.  If -frounding-math, proceed iff the
      result of calling FUNC was exact.  */
-  if (mpfr_number_p (m) && !mpfr_overflow_p () && !mpfr_underflow_p ()
+  if (!mpfr_nan_p (m) && !mpfr_overflow_p () && !mpfr_underflow_p ()
       && (!flag_rounding_math || !inexact))
     {
       REAL_VALUE_TYPE rr;
@@ -13537,7 +13539,7 @@  do_mpfr_ckconv (mpfr_srcptr m, tree type, int inex
 	 check for overflow/underflow.  If the REAL_VALUE_TYPE is zero
 	 but the mpft_t is not, then we underflowed in the
 	 conversion.  */
-      if (real_isfinite (&rr)
+      if (!real_isnan (&rr)
 	  && (rr.cl == rvc_zero) == (mpfr_zero_p (m) != 0))
         {
 	  REAL_VALUE_TYPE rmode;
@@ -13623,7 +13625,7 @@  do_mpfr_arg1 (tree arg, tree type, int (*func)(mpf
     {
       const REAL_VALUE_TYPE *const ra = &TREE_REAL_CST (arg);
 
-      if (real_isfinite (ra)
+      if (!real_isnan (ra)
 	  && (!min || real_compare (inclusive ? GE_EXPR: GT_EXPR , ra, min))
 	  && (!max || real_compare (inclusive ? LE_EXPR: LT_EXPR , ra, max)))
         {
@@ -13669,7 +13671,7 @@  do_mpfr_arg2 (tree arg1, tree arg2, tree type,
       const REAL_VALUE_TYPE *const ra1 = &TREE_REAL_CST (arg1);
       const REAL_VALUE_TYPE *const ra2 = &TREE_REAL_CST (arg2);
 
-      if (real_isfinite (ra1) && real_isfinite (ra2))
+      if (!real_isnan (ra1) && !real_isnan (ra2))
         {
 	  const struct real_format *fmt = REAL_MODE_FORMAT (TYPE_MODE (type));
 	  const int prec = fmt->p;
@@ -13717,7 +13719,7 @@  do_mpfr_arg3 (tree arg1, tree arg2, tree arg3, tre
       const REAL_VALUE_TYPE *const ra2 = &TREE_REAL_CST (arg2);
       const REAL_VALUE_TYPE *const ra3 = &TREE_REAL_CST (arg3);
 
-      if (real_isfinite (ra1) && real_isfinite (ra2) && real_isfinite (ra3))
+      if (!real_isnan (ra1) && !real_isnan (ra2) && !real_isnan (ra3))
         {
 	  const struct real_format *fmt = REAL_MODE_FORMAT (TYPE_MODE (type));
 	  const int prec = fmt->p;
@@ -13762,7 +13764,7 @@  do_mpfr_sincos (tree arg, tree arg_sinp, tree arg_
     {
       const REAL_VALUE_TYPE *const ra = &TREE_REAL_CST (arg);
 
-      if (real_isfinite (ra))
+      if (!real_isnan (ra))
         {
 	  const struct real_format *fmt = REAL_MODE_FORMAT (TYPE_MODE (type));
 	  const int prec = fmt->p;