On Mon, May 21, 2012 at 2:56 AM, Richard Guenther
<[email protected]> wrote:
> On Sun, May 20, 2012 at 1:40 AM, Andrew Pinski <[email protected]> wrote:
>> The problem here is that tree-if-conv.c produces COND_EXPR instead of
>> the MAX/MIN EXPRs. When I added the expansion from COND_EXPR to
>> conditional moves, I assumes that the expressions which should have
>> been converted into MAX_EXPR/MIN_EXPR have already happened.
>>
>> This fixes the problem by having tree-if-conv fold the expression so
>> the MIN_EXPR/MAX_EXPR appears in the IR rather than COND_EXPR and the
>> expansion happens correctly to the min/max rtl rather than just
>> through the conditional move ones.
>>
>> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> As we are unconditionally building a gimple_assign from the folding result
> you need to re-gimplify it. The code was using build3 instead of fold_build3
> to avoid that and to make sure we create a plain COND_EXPR we can
> later CSE / simplify properly. Generating a MAX_EXPR directly is certainly
> fine (can we always vectorize that?), but I suppose simply changing the
> code to use fold_build3 will have unwanted fallout (consider folds habit
> to insert conversions that are not requried on gimple).
>
> So I'd rather prefer to abstract the build3 (COND_EXPR,... into a
> helper function that uses fold_ternary and only if the result is an
> invariant/register or a MIN/MAX_EXPR use the result, canonicalizing
> it properly.
Here is an updated patch which does exactly that.
Note I noticed another regression dealing with my expansion for
COND_EXPR dealing with min/max and I will be submitting patches for
those issue tomorrow.
OK? Bootstrap and tested on x86_64-linux-gnu.
Thanks,
Andrew Pinski
ChangeLog:
* tree-if-conv.c (constant_or_ssa_name): New function.
(fold_build_cond_expr): New function.
(predicate_scalar_phi): Use fold_build_cond_expr instead of build3.
(predicate_mem_writes): Likewise.
Index: tree-if-conv.c
===================================================================
--- tree-if-conv.c (revision 190736)
+++ tree-if-conv.c (working copy)
@@ -307,6 +307,65 @@ fold_or_predicates (location_t loc, tree
return fold_build2_loc (loc, TRUTH_OR_EXPR, boolean_type_node, c1, c2);
}
+/* Returns true if N is either a constant or a SSA_NAME. */
+
+static bool
+constant_or_ssa_name (tree n)
+{
+ switch (TREE_CODE (n))
+ {
+ case SSA_NAME:
+ case INTEGER_CST:
+ case REAL_CST:
+ case COMPLEX_CST:
+ case VECTOR_CST:
+ return true;
+ default:
+ return false;
+ }
+}
+
+/* Returns either a COND_EXPR or the folded expression if the folded
+ expression is a MIN_EXPR, a MAX_EXPR, an ABS_EXPR,
+ a constant or a SSA_NAME. */
+
+static tree
+fold_build_cond_expr (tree type, tree cond, tree rhs, tree lhs)
+{
+ tree rhs1, lhs1, cond_expr;
+ cond_expr = fold_ternary (COND_EXPR, type, cond,
+ rhs, lhs);
+
+ if (cond_expr == NULL_TREE)
+ return build3 (COND_EXPR, type, cond, rhs, lhs);
+
+ STRIP_USELESS_TYPE_CONVERSION (cond_expr);
+
+ if (constant_or_ssa_name (cond_expr))
+ return cond_expr;
+
+ if (TREE_CODE (cond_expr) == ABS_EXPR)
+ {
+ rhs1 = TREE_OPERAND (cond_expr, 1);
+ STRIP_USELESS_TYPE_CONVERSION (rhs1);
+ if (constant_or_ssa_name (rhs1))
+ return build1 (ABS_EXPR, type, rhs1);
+ }
+
+ if (TREE_CODE (cond_expr) == MIN_EXPR
+ || TREE_CODE (cond_expr) == MAX_EXPR)
+ {
+ lhs1 = TREE_OPERAND (cond_expr, 0);
+ STRIP_USELESS_TYPE_CONVERSION (lhs1);
+ rhs1 = TREE_OPERAND (cond_expr, 1);
+ STRIP_USELESS_TYPE_CONVERSION (rhs1);
+ if (constant_or_ssa_name (rhs1)
+ && constant_or_ssa_name (lhs1))
+ return build2 (TREE_CODE (cond_expr), type, lhs1, rhs1);
+ }
+ return build3 (COND_EXPR, type, cond, rhs, lhs);
+}
+
/* Add condition NC to the predicate list of basic block BB. */
static inline void
@@ -1293,8 +1352,8 @@ predicate_scalar_phi (gimple phi, tree c
|| bb_postdominates_preds (bb));
/* Build new RHS using selected condition and arguments. */
- rhs = build3 (COND_EXPR, TREE_TYPE (res),
- unshare_expr (cond), arg_0, arg_1);
+ rhs = fold_build_cond_expr (TREE_TYPE (res), unshare_expr (cond),
+ arg_0, arg_1);
}
new_stmt = gimple_build_assign (res, rhs);
@@ -1554,7 +1613,7 @@ predicate_mem_writes (loop_p loop)
cond = force_gimple_operand_gsi_1 (&gsi, unshare_expr (cond),
is_gimple_condexpr, NULL_TREE,
true, GSI_SAME_STMT);
- rhs = build3 (COND_EXPR, type, unshare_expr (cond), rhs, lhs);
+ rhs = fold_build_cond_expr (type, unshare_expr (cond), rhs, lhs);
gimple_assign_set_rhs1 (stmt, ifc_temp_var (type, rhs, &gsi));
update_stmt (stmt);
}
