On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener <[email protected]> wrote:
> On Thu, 6 Nov 2014, Andrew Pinski wrote:
>
>> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener <[email protected]> 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?
This works for most cases but does not work for things like:
/* a != b ? a : b -> a */
(simplify
(cond_expr (ne @0 @1) @0 @1)
@0)
In gimple_simplify after it matches COND_EXPR. It does not look into
the operand 0 to see if it matches NE_EXPR, it just sees if it was a
SSA_NAME. In this case I was trying to do a simple replacement of
value_replacement in tree-ssa-phiopt.c.
But for my purpose right now this is sufficient and will be posting a
patch which does not remove things from tree-ssa-phiopt.c just yet.
Thanks,
Andrew Pinski
>
> 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 ();
