Re: [patch] for PRs 27639 and 26719

[Daniel Berlin <dberlin at dberlin dot org>]

Zdenek Dvorak wrote:
> Hello,
> 
> the code for folding casts of chrecs (chrec_convert and related
> functions) is currently in a quite bad shape.  This is mostly from
> historical reasons:  it was rewritten to have a different semantics
> than the original code (with respect to the wrapping of the produced
> chrecs), but on some pieces of the code still followed the old
> semantics, causing several bugs.  To make the situation worse, there
> are two places that check for overflows (scev_probably_wraps_p and
> convert_step_widening), and some of the fixes has affected only one of
> those places.  Also, we somehow managed to make quite a mess of
> where we can assume that e.g. signed arithmetics does not wrap and where
> we cannot, which is the reason for PR 26719.
> 
> This patch cleans up this code a bit, fixing the two PRs (27639 and
> 26719) as a side effect.  The most important change is that only
> scev_probably_wraps_p function is now responsible for determining
> whether the chrec wraps or not, and that we can specify to this function
> whether we want to assume that signed arithmetics may wrap or not
> (the former is important when we are using it to verify a correctness of
> folding the cast, i.e., to verify that the newly produced chrec does not
> overflow).  All the tests in chrec_convert are now expressed in terms
> of scev_probably_wraps_p.
> 
> The other smaller changes are:
> -- the mostly unrelated code to determine whether chrec is growing or
>    decreasing was split from scev_probably_wraps_p to a separate function
> -- the code testing for used_in_pointer_arithmetic_p in
>    scev_probably_wraps_p was removed; the reason is that it
>    unfortunately this code is not correct -- even if we assume that the
>    arithmetics used in the statements for the
>    used_in_pointer_arithmetic_p is true does not wrap (which is a bit
>    doubtful, because these statements do not necessarily have to come
>    from the expansion of pointer arithmetics),it still does not say
>    anything about the operands of these statements (that may be wrapping
>    induction variables). 

You say it is incorrect without real details.
The code (which i believe seb wrote) was built to answer questions about
variables, not statements.
Looking at it, it checks whether every use of a variable is either
casted to a pointer, or an operand to a statement whose lhs is a pointer.

IIRC, the basic pattern it was trying to match occurred *everywhere* we
had non-global array references, indexing into passed in
arrays/pointers, or pointer arithmetic.

I.E.
foo[i][j] would generate something like:


idxtemp = i * 50;
idxtemp2 = j + idxtemp;
idxtemp3 = (int) foo;  (or was it &foo)
idxtemp4 = idxtemp3 + idxtemp2;
pointer = (int *)idxtemp4;


 Leaving this code was causing testsuite
>    failures (gcc.c-torture/execute/20030916-1.c and several fortran
>    testcases), that used to be hidden before the cleanup due to
>    deficiences in using the fact that signed arithmetics does not
>    wrap.
>

IIRC, this code was important because it matched basically all the array
reference expansions we produce in the compiler, so removing it without
replacing it with something that *does* work seems to me a bad idea.

--Dan