Re: PR57073 - Optimize __builtin_powif (-1.0, k) to k & 1 ? -1.0 : 1.0

[Tobias Burnus <burnus at net-b dot de>]

Am 31.05.2013 10:24, schrieb Richard Biener:
> On Thu, May 30, 2013 at 10:54 PM, Jeff Law <law@redhat.com> wrote:
> > Don't worry about it. The patch is good as-is. 
> Why sink the !host_integerp check?  Please keep it where it is now.

Answer: Because it doesn't work. And if I had a cup of coffee and didn't 
mess up my regtesting (by excluding the newly added test case), I had 
also seen that.

The very old code had (assume: powi(x,n)):
  if (n is not a constant)
    break;
  expand powi to "n" multiplications.

The original patch changed it to:

  if (n is a not constant and x == -1)
    result =  n & 1 ? -1.0 : 1.0
  else
    {
      if (n is not a constant)
        break;
      expand powi to "n" multiplications.
    }

Thus, if one moves up the condition
  if (n is not a constant)
    break;
the newly added code becomes unreachable.

However, I think the code is more readable if one simply removes the "&& 
!host_integerp (arg1,0)" from the x==1 case. Due to fold_builtin_powi 
having x==-1 and n == const should not happen - and if, the "n & 1 ? 
-1.0 : 1.0" is also not worse than an expanded multiplication (if n is 
large). [Alternatively, one can also keep (re-add) the "&& 
!host_integerp (arg1,0)".]

OK? (After successful bootstrap and regtesting.)

Tobias

diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index b4de411..e9c32b3 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1447,9 +1447,6 @@ execute_cse_sincos (void)
 		  arg1 = gimple_call_arg (stmt, 1);
 		  loc = gimple_location (stmt);
 
-		  if (!host_integerp (arg1, 0))
-		    break;
-
 		  if (real_minus_onep (arg0))
 		    {
                       tree t0, t1, cond, one, minus_one;
@@ -1477,6 +1474,9 @@ execute_cse_sincos (void)
 		    }
 		  else
 		    {
+		      if (!host_integerp (arg1, 0))
+			break;
+
 		      n = TREE_INT_CST_LOW (arg1);
 		      result = gimple_expand_builtin_powi (&gsi, loc, arg0, n);
 		    }