On Sun, Nov 19, 2017 at 11:41 AM, Marc Glisse <marc.gli...@inria.fr> wrote:
> On Mon, 13 Nov 2017, Richard Biener wrote:
>
>> On Sat, Nov 11, 2017 at 11:03 PM, Marc Glisse <marc.gli...@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.
Interesting - can you open a bugreport so we don't forget? I suspect it's
the effect of zero_nonzero_bits_from_vr handling VARYING and [INT_MIN, INT_MAX]
differently rippling down. At some point in time I wanted to get rid of VARYING
in favor of [INT_MIN, INT_MAX] ...
>
> 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.
Ok.
Thanks,
Richard.
> 2017-11-20 Marc Glisse <marc.gli...@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.
>
> --
> Marc Glisse
