This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR tree-optimization/19903
- From: Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>
- To: Sebastian Pop <sebastian dot pop at cri dot ensmp dot fr>
- Cc: Eric Botcazou <ebotcazou at libertysurf dot fr>,gcc-patches at gcc dot gnu dot org, dnovillo at redhat dot com
- Date: Fri, 18 Mar 2005 22:48:31 +0100
- Subject: Re: [PATCH] Fix PR tree-optimization/19903
- References: <200502172205.50074.ebotcazou@libertysurf.fr> <20050318204437.GC5017@napoca.cri.ensmp.fr>
Hello,
> > We have 2 recent ACATS failures on x86
> >
> > FAIL: cxa4006
> > FAIL: cxa4017
> >
> > that come from the same problem in scev. It boils down to this:
> >
> > t49.lim:
> > line_51 = (natural___XDLU_0__2147483647) R130b_50;
> >
> > t52.ivcanon:
> > (set_scalar_evolution
> > (scalar = R130b_50)
> > (scalar_evolution = {3, +, -1}_1))
> > )
> > (set_scalar_evolution
> > (scalar = line_51)
> > (scalar_evolution = {3, +, 0ffffffff}_1))
> > )
> >
> > What happened is that the whole chrec of R130b_50 has been converted to
> > natural___XDLU_0__2147483647, which is an unsigned type, by chrec_convert.
which is correct. CHREC_LEFT and CHREC_RIGHT should have the same type,
namely the type of the chrec. ~0u is correct representation for -1 in
unsigned type, and if there is some code that fails to notice that, it
is that code, not this place that ensures type sanity of the
expressions.
Both the patches below are wrong.
Zdenek
> > Now the head comment of chrec_convert reads
> > /* Convert the initial condition of chrec to type. */
> > so it appears that there is a discrepancy between the comment and the code.
> >
>
> Comments surrounding the function are correct, code is not. Zdenek
> has introduced the conversion of the steps without much comments in:
> http://gcc.gnu.org/ml/gcc-patches/2004-03/msg02321.html
>
> > I'm not really sure how to fix this. My understanding is that the initial
> > condition and the step of a chrec need to be differentiated when it comes to
> > casting, much like pointer types and ptrdiff_t. So I've come up with the
> > following solution. Bootstrapped/regtested on i586-redhat-linux-gnu.
> >
> >
> > 2005-02-17 Eric Botcazou <ebotcazou@libertysurf.fr>
> >
> > PR tree-optimization/19903
> > * tree-chrec.c (chrec_convert) <POLYNOMIAL_CHREC>: Convert
> > the step to the associated signed type.
> >
> >
> > --
> > Eric Botcazou
>
> > Index: tree-chrec.c
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/tree-chrec.c,v
> > retrieving revision 2.12
> > diff -u -p -r2.12 tree-chrec.c
> > --- tree-chrec.c 9 Dec 2004 16:17:00 -0000 2.12
> > +++ tree-chrec.c 17 Feb 2005 15:16:54 -0000
> > @@ -31,6 +31,7 @@ Software Foundation, 59 Temple Place - S
> > #include "errors.h"
> > #include "ggc.h"
> > #include "tree.h"
> > +#include "langhooks.h"
> > #include "diagnostic.h"
> > #include "varray.h"
> > #include "tree-chrec.h"
> > @@ -978,7 +979,7 @@ chrec_convert (tree type,
> > return build_polynomial_chrec (CHREC_VARIABLE (chrec),
> > chrec_convert (type,
> > CHREC_LEFT (chrec)),
> > - chrec_convert (type,
> > + chrec_convert (lang_hooks.types.signed_type (type),
> > CHREC_RIGHT (chrec)));
> >
> > default:
>
> I propose the following patch:
>
> Index: tree-chrec.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-chrec.c,v
> retrieving revision 2.13
> diff -d -u -p -r2.13 tree-chrec.c
> --- tree-chrec.c 1 Mar 2005 02:43:49 -0000 2.13
> +++ tree-chrec.c 18 Mar 2005 13:57:31 -0000
> @@ -1026,8 +1026,7 @@ chrec_convert (tree type,
> return build_polynomial_chrec (CHREC_VARIABLE (chrec),
> chrec_convert (type,
> CHREC_LEFT (chrec)),
> - chrec_convert (type,
> - CHREC_RIGHT (chrec)));
> + CHREC_RIGHT (chrec));
>
> default:
> {
>
> The patch was bootstrapped and regtested on x86_64-unknown-linux-gnu
> all languages except treelang. It fixes cxa4006 and cxa4017.
>
> Okay for mainline, and 4.0?
>
> Sebastian
>
> Changelog:
>
> * tree-chrec.c (chrec_convert): Don't convert CHREC_RIGHT of
> POLYNOMIAL_CHREC.
>
> As a side note for Diego, this patch also fixes your problem of the
> steps of -1 wrongly represented as a positive number 0ffffffff.