On Thu, 6 Nov 2014, Andrew Pinski wrote: > On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener <rguent...@suse.de> wrote: > > On Thu, 6 Nov 2014, Richard Biener wrote: > > > >> On Wed, 5 Nov 2014, Andrew Pinski wrote: > >> > >> > Hi, > >> > I was trying to hook up tree-ssa-phiopt to match-and-simplify using > >> > either gimple_build (or rather using gimple_simplify depending on if > >> > we want to produce cond_expr for conditional move). I ran into a > >> > problem. > >> > With the pattern below: > >> > /* a ? 0 : 1 -> a if 0 and 1 are integral types. */ > >> > (simplify > >> > (cond_expr @0 integer_zerop integer_onep) > >> > (if (INTEGRAL_TYPE_P (type)) > >> > (convert @0))) > >> > >> Ok, so you are capturing a GENERIC expr here but nothing knows that. > >> It would work if you'd do (ugh) > >> > >> (for op (lt le eq ne ge gt) > >> (simplify > >> (cond_expr (op @0 @1) integer_zerop integer_onep) > >> (if (INTEGRAL_TYPE_P (type)) > >> (convert (op @0 @1))))) > >> (simplify > >> (cond_expr SSA_NAME@0 integer_zerop integer_onep) > >> (if (INTEGRAL_TYPE_P (type)) > >> (convert @0)))) > >> > >> as a workaround. To make your version work will require (quite) > >> some special-casing in the code generator or maybe the resimplify > >> helper. Let me see if I can cook up a "simple" fix. > > > > Sth like below (for the real fix this has to be replicated in > > all gimple_resimplifyN functions). I'm missing a testcase > > where the pattern would apply (and not be already folded by fold), > > so I didn't check if it actually works. > > You do need to check if seq is NULL though as gimple_build depends on > seq not being NULL. But otherwise yes this works for me.
Ok, here is a better and more complete fix. The optimization whether a captured expression may be a GENERIC condition isn't implemented so a lot of checks are emitted right now. Also if gimple_build would fail this terminates the whole matching process, not just matching of the current pattern (failing "late" isn't supported with the style of code we emit - well, without adding ugly gotos and labels). At least it avoids splitting up conditions if substituted into a COND_EXPR, so simplifications of COND_EXPRs in-place (w/o seq) should still work. Can you check whether this works for you? Thanks, Richard. Index: gcc/genmatch.c =================================================================== --- gcc/genmatch.c (revision 217213) +++ gcc/genmatch.c (working copy) @@ -419,7 +419,8 @@ struct operand { operand (enum op_type type_) : type (type_) {} enum op_type type; virtual void gen_transform (FILE *, const char *, bool, int, - const char *, dt_operand ** = 0) + const char *, dt_operand ** = 0, + bool = true) { gcc_unreachable (); } }; @@ -449,7 +450,7 @@ struct expr : public operand later lowered to two separate patterns. */ bool is_commutative; virtual void gen_transform (FILE *f, const char *, bool, int, - const char *, dt_operand ** = 0); + const char *, dt_operand ** = 0, bool = true); }; /* An operator that is represented by native C code. This is always @@ -479,7 +480,7 @@ struct c_expr : public operand /* The identifier replacement vector. */ vec<id_tab> ids; virtual void gen_transform (FILE *f, const char *, bool, int, - const char *, dt_operand **); + const char *, dt_operand ** = 0, bool = true); }; /* A wrapper around another operand that captures its value. */ @@ -493,7 +494,7 @@ struct capture : public operand /* The captured value. */ operand *what; virtual void gen_transform (FILE *f, const char *, bool, int, - const char *, dt_operand ** = 0); + const char *, dt_operand ** = 0, bool = true); }; template<> @@ -1358,7 +1359,7 @@ get_operand_type (id_base *op, const cha void expr::gen_transform (FILE *f, const char *dest, bool gimple, int depth, - const char *in_type, dt_operand **indexes) + const char *in_type, dt_operand **indexes, bool) { bool conversion_p = is_conversion (operation); const char *type = expr_type; @@ -1405,7 +1406,10 @@ expr::gen_transform (FILE *f, const char const char *optype = get_operand_type (operation, in_type, expr_type, i == 0 ? NULL : op0type); - ops[i]->gen_transform (f, dest, gimple, depth + 1, optype, indexes); + ops[i]->gen_transform (f, dest, gimple, depth + 1, optype, indexes, + ((!(*operation == COND_EXPR) + && !(*operation == VEC_COND_EXPR)) + || i != 0)); } const char *opr; @@ -1455,7 +1459,7 @@ expr::gen_transform (FILE *f, const char void c_expr::gen_transform (FILE *f, const char *dest, - bool, int, const char *, dt_operand **) + bool, int, const char *, dt_operand **, bool) { if (dest && nr_stmts == 1) fprintf (f, "%s = ", dest); @@ -1524,7 +1528,8 @@ c_expr::gen_transform (FILE *f, const ch void capture::gen_transform (FILE *f, const char *dest, bool gimple, int depth, - const char *in_type, dt_operand **indexes) + const char *in_type, dt_operand **indexes, + bool expand_compares) { if (what && is_a<expr *> (what)) { @@ -1537,6 +1542,24 @@ capture::gen_transform (FILE *f, const c } fprintf (f, "%s = captures[%u];\n", dest, where); + + /* ??? Stupid tcc_comparison GENERIC trees in COND_EXPRs. Deal + with substituting a capture of that. + ??? We can optimize this in the generator because we should + know whether this capture is from a COND_EXPR condition. Also + technically splitting the comparison up isn't required if we + are substituting into a COND_EXPR condition (so simplification + may unnecessarily fail if seq is NULL and a toplevel cond result). + ??? Returning false here will also not allow any other patterns + to match. */ + if (gimple && expand_compares) + fprintf (f, "if (COMPARISON_CLASS_P (%s))\n" + " {\n" + " if (!seq) return false;\n" + " %s = gimple_build (seq, TREE_CODE (%s)," + " TREE_TYPE (%s), TREE_OPERAND (%s, 0)," + " TREE_OPERAND (%s, 1));\n" + " }\n", dest, dest, dest, dest, dest, dest); } /* Return the name of the operand representing the decision tree node. @@ -1999,7 +2022,8 @@ capture_info::walk_match (operand *o, un { bool cond_p = conditional_p; if (i == 0 - && *e->operation == COND_EXPR) + && (*e->operation == COND_EXPR + || *e->operation == VEC_COND_EXPR)) cond_p = true; else if (*e->operation == TRUTH_ANDIF_EXPR || *e->operation == TRUTH_ORIF_EXPR) @@ -2046,7 +2070,8 @@ capture_info::walk_result (operand *o, b { bool cond_p = conditional_p; if (i == 0 - && *e->operation == COND_EXPR) + && (*e->operation == COND_EXPR + || *e->operation == VEC_COND_EXPR)) cond_p = true; else if (*e->operation == TRUTH_ANDIF_EXPR || *e->operation == TRUTH_ORIF_EXPR) @@ -2226,7 +2251,11 @@ dt_simplify::gen (FILE *f, bool gimple) "type", e->expr_type, j == 0 ? NULL : "TREE_TYPE (res_ops[0])"); - e->ops[j]->gen_transform (f, dest, true, 1, optype, indexes); + e->ops[j]->gen_transform (f, dest, true, 1, optype, indexes, + !is_predicate + && ((!(*e->operation == COND_EXPR) + && !(*e->operation == VEC_COND_EXPR)) + || j != 0)); } /* Re-fold the toplevel result. It's basically an embedded @@ -2238,8 +2267,21 @@ dt_simplify::gen (FILE *f, bool gimple) else if (result->type == operand::OP_CAPTURE || result->type == operand::OP_C_EXPR) { - result->gen_transform (f, "res_ops[0]", true, 1, "type", indexes); - fprintf (f, "*res_code = TREE_CODE (res_ops[0]);\n"); + result->gen_transform (f, "res_ops[0]", true, 1, "type", + indexes, false); + /* ??? Stupid tcc_comparison GENERIC trees in COND_EXPRs. Deal + with substituting a capture of that. + ??? We can optimize this in the generator because we should + know whether this capture is from a COND_EXPR condition. */ + fprintf (f, "if (COMPARISON_CLASS_P (res_ops[0]))\n" + " {\n" + " tree tem = res_ops[0];\n" + " res_ops[0] = TREE_OPERAND (tem, 0);\n" + " res_ops[1] = TREE_OPERAND (tem, 1);\n" + " *res_code = TREE_CODE (tem);\n" + " }\n" + "else\n" + " *res_code = TREE_CODE (res_ops[0]);\n"); } else gcc_unreachable ();