On Mon, 10 Nov 2014, Andrew Pinski wrote:
> On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener <rguent...@suse.de> wrote:
> > 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?
>
> 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.
