On Tue, Oct 10, 2017 at 4:22 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
> On Mon, 9 Oct 2017, Richard Biener wrote:
>
>> On Sun, Oct 8, 2017 at 1:22 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
>>>
>>> Hello,
>>>
>>> this moves (and extends a bit) one more transformation from fold-const.c
>>> to
>>> match.pd. The single_use restriction is necessary for consistency with
>>> the
>>> existing X+CST1 CMP CST2 transformation (if we do only one of the 2
>>> transformations, gcc.dg/tree-ssa/vrp54.c regresses because DOM fails to
>>> simplify 2 conditions based on different variables). The relaxation of
>>> the
>>> condition to simplify (T) X == (T) Y is an easier way to avoid regressing
>>> gcc.dg/tree-ssa/foldaddr-1.c than adding plenty of convert? in the
>>> patterns,
>>> although that may still prove necessary at some point. I left a few odd
>>> float cases in fold-const.c for later.
>>
>>
>> +/* X + Y < Y is the same as X < 0 when there is no overflow. */
>> +(for op (lt le ge gt)
>> + rop (gt ge le lt)
>> + (simplify
>> + (op (plus:c@2 @0 @1) @1)
>> + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
>> + && (CONSTANT_CLASS_P (@0) || single_use (@2)))
>> + (op @0 { build_zero_cst (TREE_TYPE (@0)); })))
>> + (simplify
>> + (op @1 (plus:c@2 @0 @1))
>> + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
>> + && (CONSTANT_CLASS_P (@0) || single_use (@2)))
>> + (rop @0 { build_zero_cst (TREE_TYPE (@0)); }))))
>>
>> any reason (op:c (plus... on the first of the two patterns wouldn't have
>> catched
>> the second? :c on comparison swaps the comparison code.
>
>
> I had completely forgotten that you had added this cool feature.
>
>> +/* For equality, this is also true with wrapping overflow. */
>> +(for op (eq ne)
>> + (simplify
>> + (op:c (plus:c@2 @0 @1) @1)
>> + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> + && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
>> + || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
>> + && (CONSTANT_CLASS_P (@0) || single_use (@2)))
>> + (op @0 { build_zero_cst (TREE_TYPE (@0)); })))
>> + (simplify
>> + (op:c (convert?@3 (pointer_plus@2 @0 @1)) (convert? @0))
>> + (if (CONSTANT_CLASS_P (@1) || (single_use (@2) && single_use (@3)))
>> + (op @1 { build_zero_cst (TREE_TYPE (@1)); }))))
>>
>> for consistency can you add the convert?s to the integer variant as well?
>
>
> The only relevant case I could think of is, for unsigned u,
> (unsigned)((int)u+1)==u. All the other cases seem to be handled some other
> way. I still wrote it to allow conversions all over the place, but that's
> uglier than the MINUS_EXPR transformation below where I didn't.
>
>> I'm not sure you'll see the convers like you write them (with matching
>> @0).
>
>
> int f(char*p){
> char*q=p+5;
> return (long)q==(long)p;
> }
>
>> Eventually on GENERIC when we still have pointer conversions around?
>
>
> For generic we might want @@0 or pointers sometimes fail to match. At least
> in the POINTER_DIFF_EXPR experiment I had to do that in some transformations
> for constexpr.
>
>> You are allowing arbitrary conversions here - is that desired? Isn't
>> a tree_nop_conversion_p missing?
>
>
> I had the impression that casts from a pointer to whatever were always NOP,
> for instance when I try to cast char* to short, gcc produces (short)(long)p.
> But reading the comment in verify_gimple_assign_unary it is more
> complicated, so yes I should add a constraint.
>
> /* Allow conversions from pointer type to integral type only if
> there is no sign or zero extension involved.
> For targets were the precision of ptrofftype doesn't match that
> of pointers we need to allow arbitrary conversions to ptrofftype.
> */
> if ((POINTER_TYPE_P (lhs_type)
> && INTEGRAL_TYPE_P (rhs1_type))
> || (POINTER_TYPE_P (rhs1_type)
> && INTEGRAL_TYPE_P (lhs_type)
> && (TYPE_PRECISION (rhs1_type) >= TYPE_PRECISION (lhs_type)
> || ptrofftype_p (sizetype))))
> return false;
>
> The code "|| ptrofftype_p (sizetype)" doesn't seem to match the comment :-(
Ugh. Should have been || ptrofftype_p (lhs_type) of course...
The whole thing is fragile in the context of address-spaces as well (who can
theoretically have a different pointer size and thus different
[u]intptr_t). But
that's not news and similar to the issues we have with the POINTER_PLUS_EXPR
requirement ...
>
> Here is a new version of the patch, same changelog/testing.
Ok.
Thanks,
Richard.
> --
> Marc Glisse
