Re: [RFA] Factor conversion out of COND_EXPR using match.pd pattern

[Jeff Law <law at redhat dot com>]

On 06/01/2015 04:55 AM, Richard Biener wrote:
> On Sat, May 30, 2015 at 11:11 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> > (only commenting on the technique, not on the transformation itself)
> >
> > > +(simplify
> > > +  (cond @0 (convert @1) INTEGER_CST@2)
> > > +  (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> > > +       && COMPARISON_CLASS_P (@0)
> >
> >
> > If you add COMPARISON_CLASS_P to define_predicates, you could write:
> > (cond COMPARISON_CLASS_P@0 (convert @1) INTEGER_CST@2)
>
> But that would fail to match on GIMPLE, so I don't like either variant
> as Jeffs relies on the awkward fact that on GIMPLE cond expr conditions
> are GENERIC and yours wouldn't work.
>
> That said - for this kind of patterns testcases that exercise the patterns
> on GIMPLE would be very appreciated.
OK.  I'll see if I can build a testcase to exercise this in gimple.  If 
that's not possible would you prefer the pattern be restricted to 
generic just to be safe?

> > or maybe use a for loop on comparisons, which would give names to
> > TREE_OPERAND (@0, *). This should even handle the operand_equal_p
> > alternative:
> >
> > (cond (cmp:c@0 @1 @2) (convert @1) INTEGER_CST@2)
>
> Yes, that would be my reference.
OK.  easily done.

> > > +       && int_fits_type_p (@2, TREE_TYPE (@1))
> > > +       && ((operand_equal_p (TREE_OPERAND (@0, 0), @2, 0)
> > > +           && operand_equal_p (TREE_OPERAND (@0, 1), @1, 0))
> > > +          || (operand_equal_p (TREE_OPERAND (@0, 0), @1, 0)
> > > +              && operand_equal_p (TREE_OPERAND (@0, 1), @2, 0))))
> > > +    (with { tree itype = TREE_TYPE (@1); tree otype = TREE_TYPE (@2); }
> > > +      (convert:otype (cond:itype @0 @1 (convert:itype @2))))))
> >
> >
> > This should be enough, no need to specify the outer type
> > (convert (cond:itype @0 @1 (convert:itype @2))))))
>
> Yes.
>
> > I believe we should not have to write cond:itype here, cond should be made
> > to use the type of its second argument instead of the first one, by default
> > (expr::gen_transform already has a few special cases).
>
> Indeed.  Patch welcome (I'd have expected it already works...)
One of them is needed, but I can't recall which :-)  I'll pull them to 
generate the failure I'd run into and simplify appropriately and explain 
whichever is remaining.

jeff