Am 31.05.2013 10:24, schrieb Richard Biener:
On Thu, May 30, 2013 at 10:54 PM, Jeff Law <l...@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);
}
