Jeff Law wrote:
On 05/30/2013 12:08 PM, Tobias Burnus wrote:
+            }
+          else
+            {
+              if (!host_integerp (arg1, 0))
+            break;
+
+              n = TREE_INT_CST_LOW (arg1);
+              result = gimple_expand_builtin_powi (&gsi, loc, arg0, n);
+            }

In this case, why even bother with gimple_expand_builtin_powi. Don't we know the result simply by looking at N and setting the dest to -1.0 or 1.0 appropriately?

I am a bit lost. The code quoted above is the old code - just moved down a bit. It is supposed to handle powi(x,n) for unknown x with known n - while the new code handles x == -1.0 for unknown n. Thus, gimple_expand_builtin_powi should be unreachable for x == -1.

My - possibly wrong - impression is that the case of n being constant *and* x being also a constant (e.g. x == -1) is handled before. One place where it is handled is at fold_builtin_powi. However, I do not understand GCC's passes well enough to know whether it is possible that one can reach tree-ssa-math-opts.c's execute_cse_sincos with x and n being becoming constant after the call to builtins.c's fold_builtin_powi.


Side remark: fold_builtin_powi would have been a more suitable case to do the conversion of this patch. However, re-gimplification does not like if one suddenly add an COND_EXPR at tree level, where the condition is neither a constant nor a variable. The problem is in the call to gimplify_modify_expr (from gimplify_cond_expr). If I recall correctly, for re-gimplification (but not for the initial gimplification), the code assumes that for "cond" is_gimple_val() is true. But as one has "x & 1", it is false - leading to an ICE. Thus, Richard + Jakub suggested to handle this case not on tree level during folding but on gimple level. That's how it ended up in tree-ssa-math-opts.c, where a special case for BUILT_IN_POWI already existed.

I don't see that powi_as_mults optimizes the case where both args are constants and thus the result is a trivially computable compile time constant. Am I missing something? Granted, I wouldn't expect it to happen often, but we might have started with a variable exponent and through various opts eventually collapsed it down to a known constant.

If I understood it correctly, you would like to have an additional case before the newly added "k == 1", which does something like:

result = fold_builtin_powi (loc, NULL_TREE, arg1, arg2, TREE_TYPE (arg1);

if (result != NULL_TREE &&   .... /* Newly added  x == -1.0 case.  */

Is that what you propose?

Tobias

Reply via email to