On Fri, Nov 21, 2014 at 7:18 AM, Richard Biener
<[email protected]> wrote:
> On Fri, Nov 21, 2014 at 12:29 PM, Patrick Palka <[email protected]> wrote:
>> When adjusting the value range of an induction variable using SCEV, VRP
>> calls scev_probably_wraps_p() with use_overflow_semantics=true. This
>> parameter set to true makes scev_probably_wraps_p() assume that signed
>> induction variables never wrap, so for these variables it always returns
>> false (when strict overflow rules are in effect). This is wrong because
>> if a signed induction variable really does overflow then we want to give
>> it an INF(OVF) value range and not the (finite) estimation returned by
>> SCEV.
>>
>> While this change shouldn't make a difference in code generation, it
>> should help improve the coverage of -Wstrict-overflow warnings on
>> induction variables like in the test case.
>>
>> OK after bootstrap + regtest on x86_64-unknown-linux-gnu?
>
> Hmm, I don't think the change won't affect code-generation. In fact
> we check for overflow ourselves in the most interesting case
> (the first block) - only the path adjusting min/max based on the
> init value and the max value of the type needs to know whether
> overflow may happen and fail or drop to +-INF(OVF).
>
> So I'd rather open-code the relevant cases and not call
> scev_probably_wraps_p at all.
What kind of tests for overflow do you have in mind?
max_loop_iterations() in this test case always return INT_MAX so there
will be no overflow when computing the upper bound using the number of
loop iterations. Do you mean to compare what max_loop_iterations()
returns with the range that VRP has inferred for the induction
variable?
>
> Richard.
>
>> gcc/
>> * tree-vrp.c (adjust_range_with_scev): Call
>> scev_probably_wraps_p with use_overflow_semantics=false.
>>
>> gcc/testsuite/
>> * gcc.dg/Wstrict-overflow-27.c: New test.
>> ---
>> gcc/testsuite/gcc.dg/Wstrict-overflow-27.c | 22 ++++++++++++++++++++++
>> gcc/tree-vrp.c | 2 +-
>> 2 files changed, 23 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.dg/Wstrict-overflow-27.c
>>
>> diff --git a/gcc/testsuite/gcc.dg/Wstrict-overflow-27.c
>> b/gcc/testsuite/gcc.dg/Wstrict-overflow-27.c
>> new file mode 100644
>> index 0000000..c1f27ab
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/Wstrict-overflow-27.c
>> @@ -0,0 +1,22 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-fstrict-overflow -O2 -Wstrict-overflow" } */
>> +
>> +/* Warn about an overflow when folding i < 0. */
>> +
>> +void bar (unsigned *p);
>> +
>> +int
>> +foo (unsigned *p)
>> +{
>> + int i;
>> + int sum = 0;
>> +
>> + for (i = 0; i < *p; i++)
>> + {
>> + if (i < 0) /* { dg-warning "signed overflow" } */
>> + sum += 2;
>> + bar (p);
>> + }
>> +
>> + return sum;
>> +}
>> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
>> index a75138f..bf9ff61 100644
>> --- a/gcc/tree-vrp.c
>> +++ b/gcc/tree-vrp.c
>> @@ -4270,7 +4270,7 @@ adjust_range_with_scev (value_range_t *vr, struct loop
>> *loop,
>> dir == EV_DIR_UNKNOWN
>> /* ... or if it may wrap. */
>> || scev_probably_wraps_p (init, step, stmt, get_chrec_loop (chrec),
>> - true))
>> + /*use_overflow_semantics=*/false))
>> return;
>>
>> /* We use TYPE_MIN_VALUE and TYPE_MAX_VALUE here instead of
>> --
>> 2.2.0.rc1.23.gf570943
>>
