On 06/01/2015 04:55 AM, Richard Biener wrote:
On Sat, May 30, 2015 at 11:11 AM, Marc Glisse <marc.gli...@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.
It may be the case that these patterns don't make a lot of sense on
gimple and should be restricted to generic, at least with our current
infrastructure.
The problem is when we lower from generic to gimple, we end up with
branchy code, not straight line code and there's no good way I see to
write a match.pd pattern which encompasses flow control.
So to discover the MIN/MAX with typecast, we're probably stuck hacking
minmax_replacement to know when it can ignore the typecast.
That may in fact be a good thing -- I haven't looked closely yet, but
45397 may be of a similar nature (no good chance to see straight line
code for match.pd, thus we're forced to handle it in phiopt).
So do we want to restrict the new pattern on GENERIC, then rely on
phiopt to get smarter and catch this stuff for GIMPLE? Or drop the
pattern totally and do everything in phiopt on GIMPLE?
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.
But won't this require pointer equivalence? Are INTEGER_CST nodes fully
shared? What if @1 is something more complex than a _DECL node
(remember, we're working with GENERIC). So something like
(cond (cmp:c@0 @1 @2) (convert @3) INTEGER_CST@4))
And using operand_equal_p seems more appropriate to me (and is still
better than the original (cond @0 ...) and grubbing around inside @0 to
look at operands.
+ && 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...)
With Marc's fix, those explicit types are no longer needed.
Jeff
