Re: Match-and-simplify and COND_EXPR
On Tue, 11 Nov 2014, Richard Biener wrote:
> On Tue, 11 Nov 2014, Richard Biener wrote:
>
> > On Mon, 10 Nov 2014, Andrew Pinski wrote:
> >
> > > On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener wrote:
> > > > On Thu, 6 Nov 2014, Andrew Pinski wrote:
> > > >
> > > >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener
> > > >> 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.
> >
> > Yeah - I hoped you wouldn't notice. It's not too difficult to fix.
> > Still this ugly ambiguity in the GIMPLE IL is bad and I wonder if
> > it is better to finally fix it with
> >
> > Index: gcc/gimple-expr.c
> > ===
> > --- gcc/gimple-expr.c (revision 216973)
> > +++ gcc/gimple-expr.c (working copy)
> > @@ -641,10 +641,7 @@ is_gimple_lvalue (tree t)
> > bool
> > is_gimple_condexpr (tree t)
> > {
> > - return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)
> > - && !tree_could_throw_p (t)
> > - && is_gimple_val (TREE_OPERAND (t, 0))
> > - && is_gimple_val (TREE_OPERAND (t, 1;
> > + return is_gimple_val (t);
> > }
> >
> > /* Return true if T is a gimple address. */
> >
> > I expect mostly fallout from passes building [VEC_]COND_EXPRs and
> > not gimplifying it and of course missed optimizations from the
> > vectorizer which will likely now try to vectorize the separate
> > comparisons in some maybe odd way (not sure).
> >
> > I did this experiment once but didn't finish it. Too bad :/
> >
> > > 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.
> >
> > Fair enough. I suppose for GCC 5 I will need to deal with the odd
> > GIMPLE IL. At least I can consider it a "bug" and thus fix this
> > during stage3 ;)
>
> Well. Like the following which creates the expected code for
>
> (simplify
> (cond_expr (eq @0 @1)
Re: Match-and-simplify and COND_EXPR
On Tue, 11 Nov 2014, Richard Biener wrote:
> On Mon, 10 Nov 2014, Andrew Pinski wrote:
>
> > On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener wrote:
> > > On Thu, 6 Nov 2014, Andrew Pinski wrote:
> > >
> > >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener 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.
>
> Yeah - I hoped you wouldn't notice. It's not too difficult to fix.
> Still this ugly ambiguity in the GIMPLE IL is bad and I wonder if
> it is better to finally fix it with
>
> Index: gcc/gimple-expr.c
> ===
> --- gcc/gimple-expr.c (revision 216973)
> +++ gcc/gimple-expr.c (working copy)
> @@ -641,10 +641,7 @@ is_gimple_lvalue (tree t)
> bool
> is_gimple_condexpr (tree t)
> {
> - return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)
> - && !tree_could_throw_p (t)
> - && is_gimple_val (TREE_OPERAND (t, 0))
> - && is_gimple_val (TREE_OPERAND (t, 1;
> + return is_gimple_val (t);
> }
>
> /* Return true if T is a gimple address. */
>
> I expect mostly fallout from passes building [VEC_]COND_EXPRs and
> not gimplifying it and of course missed optimizations from the
> vectorizer which will likely now try to vectorize the separate
> comparisons in some maybe odd way (not sure).
>
> I did this experiment once but didn't finish it. Too bad :/
>
> > 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.
>
> Fair enough. I suppose for GCC 5 I will need to deal with the odd
> GIMPLE IL. At least I can consider it a "bug" and thus fix this
> during stage3 ;)
Well. Like the following which creates the expected code for
(simplify
(cond_expr (eq @0 @1) @0 @1)
@1)
I'll install this on the branch and work on the other patch to
clean it up a bit.
Richard.
2014-11-11 Richard Biener
* genmatch.c (struct expr): Add is_generic member.
(cmp_operand): Also compare is_generic flag.
(dt_operand::gen_gimple_expr): If is_gen
Re: Match-and-simplify and COND_EXPR
On Mon, 10 Nov 2014, Andrew Pinski wrote:
> On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener wrote:
> > On Thu, 6 Nov 2014, Andrew Pinski wrote:
> >
> >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener 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.
Yeah - I hoped you wouldn't notice. It's not too difficult to fix.
Still this ugly ambiguity in the GIMPLE IL is bad and I wonder if
it is better to finally fix it with
Index: gcc/gimple-expr.c
===
--- gcc/gimple-expr.c (revision 216973)
+++ gcc/gimple-expr.c (working copy)
@@ -641,10 +641,7 @@ is_gimple_lvalue (tree t)
bool
is_gimple_condexpr (tree t)
{
- return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)
- && !tree_could_throw_p (t)
- && is_gimple_val (TREE_OPERAND (t, 0))
- && is_gimple_val (TREE_OPERAND (t, 1;
+ return is_gimple_val (t);
}
/* Return true if T is a gimple address. */
I expect mostly fallout from passes building [VEC_]COND_EXPRs and
not gimplifying it and of course missed optimizations from the
vectorizer which will likely now try to vectorize the separate
comparisons in some maybe odd way (not sure).
I did this experiment once but didn't finish it. Too bad :/
> 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.
Fair enough. I suppose for GCC 5 I will need to deal with the odd
GIMPLE IL. At least I can consider it a "bug" and thus fix this
during stage3 ;)
Thanks,
Richard.
Re: Match-and-simplify and COND_EXPR
On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener wrote:
> On Thu, 6 Nov 2014, Andrew Pinski wrote:
>
>> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener 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 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,
Re: Match-and-simplify and COND_EXPR
On Thu, 6 Nov 2014, Andrew Pinski wrote:
> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener 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 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 *d
Re: Match-and-simplify and COND_EXPR
On Thu, 6 Nov 2014, pins...@gmail.com wrote: > > > > > > On Nov 6, 2014, at 11:24 PM, Richard Biener wrote: > > > >> On November 7, 2014 5:03:19 AM CET, Andrew Pinski > >> wrote: > >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener > >> 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. > >> > >>> > >>> Bah, of course we should fix COND_EXPRs to not embed a GENERIC > >>> expr... > >> > >> Yes totally agree. For my changes to tree-ssa-phiopt, I no longer > >> embed it. Though we need to change loop ifconvert still. > > > > Istr expansion or code quality does not like us to cse the condition of two > > cobd_exprs either. After all I had a patch set at some point doing that > > conversion (though as well for gimple_conds). > > I thought I changed that when I did the expansion of cond_expr into > conditional move. We need to something similar for cond_expr of jumps > too. Well, SSA coalescing may make simple forwarding of the conditions impossible (and TER doesn't work for multiple uses anyway). That said, the most "interesting" issues for cond_exprs of jumps was PRE eliminating partial redundant conditions (thus it noted jump threading opportunities without actually performing the block duplication). Generated code for such PREd conditions was ... "interesting" :/ Richard.
Re: Match-and-simplify and COND_EXPR
> On Nov 6, 2014, at 11:24 PM, Richard Biener wrote:
>
>> On November 7, 2014 5:03:19 AM CET, Andrew Pinski wrote:
>> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener
>> 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.
>>
>>>
>>> Bah, of course we should fix COND_EXPRs to not embed a GENERIC
>>> expr...
>>
>> Yes totally agree. For my changes to tree-ssa-phiopt, I no longer
>> embed it. Though we need to change loop ifconvert still.
>
> Istr expansion or code quality does not like us to cse the condition of two
> cobd_exprs either. After all I had a patch set at some point doing that
> conversion (though as well for gimple_conds).
I thought I changed that when I did the expansion of cond_expr into conditional
move. We need to something similar for cond_expr of jumps too.
Thanks,
Andrew
>
> Richard.
>
>> Thanks,
>> Andrew
>>
>>>
>>> Richard.
>>>
>>> Index: gcc/gimple-match-head.c
>>> ===
>>> --- gcc/gimple-match-head.c (revision 217035)
>>> +++ gcc/gimple-match-head.c (working copy)
>>> @@ -90,6 +90,13 @@ gimple_resimplify1 (gimple_seq *seq,
>>>code_helper *res_code, tree type, tree *res_ops,
>>>tree (*valueize)(tree))
>>> {
>>> + /* ??? Stupid tcc_comparison GENERIC trees in COND_EXPRs. */
>>> + if (COMPARISON_CLASS_P (res_ops[0]))
>>> +res_ops[0] = gimple_build (seq,
>>> + TREE_CODE (res_ops[0]), TREE_TYPE
>> (res_ops[0]),
>>> + TREE_OPERAND (res_ops[0], 0),
>>> + TREE_OPERAND (res_ops[0], 1));
>>> +
>>> if (constant_for_folding (res_ops[0]))
>>> {
>>> tree tem = NULL_TREE;
>>>
>
>
Re: Match-and-simplify and COND_EXPR
On November 7, 2014 5:03:19 AM CET, Andrew Pinski wrote:
>On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener
>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.
>
>>
>> Bah, of course we should fix COND_EXPRs to not embed a GENERIC
>> expr...
>
>Yes totally agree. For my changes to tree-ssa-phiopt, I no longer
>embed it. Though we need to change loop ifconvert still.
Istr expansion or code quality does not like us to cse the condition of two
cobd_exprs either. After all I had a patch set at some point doing that
conversion (though as well for gimple_conds).
Richard.
>Thanks,
>Andrew
>
>>
>> Richard.
>>
>> Index: gcc/gimple-match-head.c
>> ===
>> --- gcc/gimple-match-head.c (revision 217035)
>> +++ gcc/gimple-match-head.c (working copy)
>> @@ -90,6 +90,13 @@ gimple_resimplify1 (gimple_seq *seq,
>> code_helper *res_code, tree type, tree *res_ops,
>> tree (*valueize)(tree))
>> {
>> + /* ??? Stupid tcc_comparison GENERIC trees in COND_EXPRs. */
>> + if (COMPARISON_CLASS_P (res_ops[0]))
>> +res_ops[0] = gimple_build (seq,
>> + TREE_CODE (res_ops[0]), TREE_TYPE
>(res_ops[0]),
>> + TREE_OPERAND (res_ops[0], 0),
>> + TREE_OPERAND (res_ops[0], 1));
>> +
>>if (constant_for_folding (res_ops[0]))
>> {
>>tree tem = NULL_TREE;
>>
Re: Match-and-simplify and COND_EXPR
On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener 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.
>
> Bah, of course we should fix COND_EXPRs to not embed a GENERIC
> expr...
Yes totally agree. For my changes to tree-ssa-phiopt, I no longer
embed it. Though we need to change loop ifconvert still.
Thanks,
Andrew
>
> Richard.
>
> Index: gcc/gimple-match-head.c
> ===
> --- gcc/gimple-match-head.c (revision 217035)
> +++ gcc/gimple-match-head.c (working copy)
> @@ -90,6 +90,13 @@ gimple_resimplify1 (gimple_seq *seq,
> code_helper *res_code, tree type, tree *res_ops,
> tree (*valueize)(tree))
> {
> + /* ??? Stupid tcc_comparison GENERIC trees in COND_EXPRs. */
> + if (COMPARISON_CLASS_P (res_ops[0]))
> +res_ops[0] = gimple_build (seq,
> + TREE_CODE (res_ops[0]), TREE_TYPE (res_ops[0]),
> + TREE_OPERAND (res_ops[0], 0),
> + TREE_OPERAND (res_ops[0], 1));
> +
>if (constant_for_folding (res_ops[0]))
> {
>tree tem = NULL_TREE;
>
Re: Match-and-simplify and COND_EXPR
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.
Bah, of course we should fix COND_EXPRs to not embed a GENERIC
expr...
Richard.
Index: gcc/gimple-match-head.c
===
--- gcc/gimple-match-head.c (revision 217035)
+++ gcc/gimple-match-head.c (working copy)
@@ -90,6 +90,13 @@ gimple_resimplify1 (gimple_seq *seq,
code_helper *res_code, tree type, tree *res_ops,
tree (*valueize)(tree))
{
+ /* ??? Stupid tcc_comparison GENERIC trees in COND_EXPRs. */
+ if (COMPARISON_CLASS_P (res_ops[0]))
+res_ops[0] = gimple_build (seq,
+ TREE_CODE (res_ops[0]), TREE_TYPE (res_ops[0]),
+ TREE_OPERAND (res_ops[0], 0),
+ TREE_OPERAND (res_ops[0], 1));
+
if (constant_for_folding (res_ops[0]))
{
tree tem = NULL_TREE;
Re: Match-and-simplify and COND_EXPR
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. Richard. > This produces a gimple statement with an incorrect gimple statement > where we have have a convert with an compare expression. > > t.c: In function ‘f’: > t.c:1:5: error: invalid operand in unary operation > int f(int c, int a, int b) > ^ > _4 = (int) (c_2(D) != 0); > > > Did I implement the pattern incorrectly or is there a bug due to the > way cond_expr handles its 0th operand in that it is valid for compares > there in gimple form. > > I don't have a good patch to test tree-ssa-phiopt.c connection due the > patch having extra code in it already which handles creating cond_expr > for conditional moves already. > > Thanks, > Andrew Pinski > > -- Richard Biener SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284 (AG Nuernberg) Maxfeldstrasse 5, 90409 Nuernberg, Germany
Re: Match-and-simplify and COND_EXPR
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))) > > This produces a gimple statement with an incorrect gimple statement > where we have have a convert with an compare expression. > > t.c: In function ‘f’: > t.c:1:5: error: invalid operand in unary operation > int f(int c, int a, int b) > ^ > _4 = (int) (c_2(D) != 0); > > > Did I implement the pattern incorrectly or is there a bug due to the > way cond_expr handles its 0th operand in that it is valid for compares > there in gimple form. There are probably still corner-cases where the GENERIC exprs we embed into GIMPLE operands are not handled correctly or optimally. We jump through multiple hoops to get that right already but I'm sure I missed some ;) COND_EXPRs are difficult because the embedded comparison is simplified separately but how the result needs to be represented depends on the context. So more example patterns certainly help here. Feel free to open a bugreport. Btw, I usually "test" this by inspecting the generated code for a single pattern in a test.pd file and ./build/genmatch --gimple test.pd Thanks, Richard. -- Richard Biener SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284 (AG Nuernberg) Maxfeldstrasse 5, 90409 Nuernberg, Germany
Match-and-simplify and COND_EXPR
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))) This produces a gimple statement with an incorrect gimple statement where we have have a convert with an compare expression. t.c: In function ‘f’: t.c:1:5: error: invalid operand in unary operation int f(int c, int a, int b) ^ _4 = (int) (c_2(D) != 0); Did I implement the pattern incorrectly or is there a bug due to the way cond_expr handles its 0th operand in that it is valid for compares there in gimple form. I don't have a good patch to test tree-ssa-phiopt.c connection due the patch having extra code in it already which handles creating cond_expr for conditional moves already. Thanks, Andrew Pinski
