X+Y < X iff Y<0 moved to match.pd

[Marc Glisse]

On Mon, 9 Oct 2017, Richard Biener wrote:

> On Sun, Oct 8, 2017 at 1:22 PM, Marc Glisse <marc.glisse@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 :-(


Here is a new version of the patch, same changelog/testing.

-- 
Marc Glisse
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cmp.patch
Type: text/x-diff
Size: 14270 bytes
Desc: 
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20171010/6d6fde28/attachment.bin>