Re: VRP: x+1 and -x cannot be INT_MIN

[Jeff Law <law@redhat.com>]

On 11/19/2017 03:41 AM, Marc Glisse wrote:
> On Mon, 13 Nov 2017, Richard Biener wrote:
> 
>> On Sat, Nov 11, 2017 at 11:03 PM, Marc Glisse <marc.glisse@inria.fr>
>> wrote:
>>> Hello,
>>>
>>> with undefined overflow, just because we know nothing about one of the
>>> arguments of an addition doesn't mean we can't say something about the
>>> result. We could constrain more the cases where we replace VR_VARYING
>>> with a
>>> full VR_RANGE, but I didn't want to duplicate too much logic.
>>>
>>> The 20040409 testcases were introduced to test an RTL transformation,
>>> so I
>>> don't feel too bad adding -fwrapv to work around the undefined overflows
>>> they exhibit.
>>>
>>> Bootstrap+regtest on powerpc64le-unknown-linux-gnu.
>>
>> Index: gcc/testsuite/gcc.c-torture/execute/20040409-1.c
>> ===================================================================
>> --- gcc/testsuite/gcc.c-torture/execute/20040409-1.c    (revision 254629)
>> +++ gcc/testsuite/gcc.c-torture/execute/20040409-1.c    (working copy)
>> @@ -1,10 +1,12 @@
>> +/* { dg-options "-fwrapv" } */
>> +
>>
>> I think you should use dg-additional-options (if that works).  As said
>> in the PR
>> it would be safest to copy the tests, add -fwrapv and just remove the
>> -fno-wrapv
>> cases that do not work.
>>
>> I think a better fix would be in the caller of
>> extract_range_from_binary_expr_1,
>> like simply always replacing VARYING with [min,max] if either of the two
>> ranges is not varying.  In vr_values::extract_range_from_binary_expr
>> that is,
>> and doing an early out for varying & varying in _1.  Might simplify some
>> special case code for other opts as well.
> 
> Like this? I didn't add the early out yet, among other things because I
> am tempted to add that pointer_diff_expr can never be the min value. I
> didn't see any obvious simplification of other special cases (I only
> looked briefly), the other place where we replace VR_VARYING with a full
> range is for conversions (unary). I guess I could drop the restriction
> to integers with undefined overflow...
> 
> I had to adapt one testcase where for VR_VARYING | [1, 1] we used to
> produce ~[0, 0] and now produce [-INT_MAX, INT_MAX]. I am surprised at
> how late the transformation now happens (only after removing
> __builtin_unreachable, in forwprop3, while trunk currently has it in
> evrp IIRC), but I didn't investigate, doesn't seem like the right time
> with all the VRP changes going on.
> 
> The tests that moved to -fwrapv are not undefined for all tested values,
> just a few, but subdividing further really doesn't seem worth the trouble.
> 
> Bootstrap+regtest on powerpc64le-unknown-linux-gnu.
> 
> 2017-11-20  Marc Glisse  <marc.glisse@inria.fr>
> 
> gcc/
>     * vr-values.c (extract_range_from_binary_expr): Use a full range
>     for VR_VARYING.
> 
> gcc/testsuite/
>     PR testsuite/82951
>     * gcc.c-torture/execute/20040409-1.c: Move invalid tests...
>     * gcc.c-torture/execute/20040409-1w.c: ... here with -fwrapv.
>     * gcc.c-torture/execute/20040409-2.c: Move invalid tests...
>     * gcc.c-torture/execute/20040409-2w.c: ... here with -fwrapv.
>     * gcc.c-torture/execute/20040409-3.c: Move invalid tests...
>     * gcc.c-torture/execute/20040409-3w.c: ... here with -fwrapv.
>     * gcc.dg/tree-ssa/cmpmul-1.c: Tweak condition.
>     * gcc.dg/tree-ssa/vrp118.c: New file.
> 
Slick.  I wish I had thought of this last year -- I think it would have
made a BZ I was looking at easier to tackle.  Richi's already engaged on
the issue, so I'll let him handle the review side, I just wanted to note
that I see value in discovering these ranges.

Jeff