This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR47594: Sign extend constants while translating to Graphite
On Tue, 26 Jul 2011, Sebastian Pop wrote:
> On Tue, Jul 26, 2011 at 03:22, Richard Guenther <rguenther@suse.de> wrote:
> > On Mon, 25 Jul 2011, Sebastian Pop wrote:
> >
> >> "Bug 47594 - gfortran.dg/vect/vect-5.f90 execution test fails when
> >> compiled with -O2 -fgraphite-identity"
> >>
> >> The problem is due to the fact that Graphite generates this loop:
> >>
> >> ? ? for (scat_3=0;scat_3<=4294967295*scat_1+T_51-1;scat_3++) {
> >> ? ? ? S6(scat_1,scat_3);
> >> ? ? }
> >>
> >> that has a "-1" encoded as an unsigned "4294967295". ?This constant
> >> comes from the computation of the number of iterations "M - I" of
> >> the inner loop:
> >>
> >> ? ? ? ? do I = 1, N
> >> ? ? ? ? ? do J = I, M
> >> ? ? ? ? ? ? A(J,2) = B(J)
> >> ? ? ? ? ? end do
> >> ? ? ? ? end do
> >>
> >> The patch fixes the problem by sign-extending the constants for the
> >> step of a chain of recurrence in scan_tree_for_params_right_scev.
> >>
> >> The same patter could occur for multiplication by a scalar, like in
> >> "-1 * N" and so the patch also fixes these cases in
> >> scan_tree_for_params.
> >
> > That certainly feels odd (again). ?How does it end up being unsigned
> > in the first place?
>
> We got this expression from niter. niter analysis turns all expressions
> into unsigned types before starting computations. I tried to see if we
> could improve niter, but that would be a major work. I also thought
> about using PPL or ISL to implement niter for graphite.
Hmm, I see (I suppose to avoid introducing undefined overflow).
> > Randomly sign-extending stuff looks bogus to me.
> > Does graphite operate on infinite precision signed integers? ?Or
> > does it operate on twos-complement fixed precision integers?
>
> Graphite represents constants using mpz_t.
Not exactly an answer but I guess all mpz_t do have a sign and are
of arbitrary precision. Thus it's wrong to change unsigned + -1U
to mpz_t + -1 unless you truncate to unsigneds precision after
doing that operation. Do we properly handle this?
Richard.