Re: Match-and-simplify and COND_EXPR

[Richard Biener <rguenther at suse dot de>]

On Thu, 6 Nov 2014, pinskia@gmail.com wrote:

> 
> 
> 
> 
> > On Nov 6, 2014, at 11:24 PM, Richard Biener <rguenther@suse.de> wrote:
> > 
> >> On November 7, 2014 5:03:19 AM CET, Andrew Pinski <pinskia@gmail.com> wrote:
> >> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener <rguenther@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.
> >> 
> >>> 
> >>> 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.