Submitted by Andrew Pinski on Aug. 28, 2012, 7:09 a.m.

Message ID | CA+=Sn1kn3WFShS4NQNSteMf4R9s=QgDddQDTq1jCbxf-Z2J3xg@mail.gmail.com |
---|---|

State | New |

Headers | show |

On Tue, Aug 28, 2012 at 9:09 AM, Andrew Pinski <pinskia@gmail.com> wrote: > On Mon, May 21, 2012 at 2:56 AM, Richard Guenther > <richard.guenther@gmail.com> wrote: >> On Sun, May 20, 2012 at 1:40 AM, Andrew Pinski <pinskia@gmail.com> 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. Ok. Thanks, Richard. > 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); }