[PATCH]: Latent bug in value numbering (Was Re: latent bug in PRE?)

[Daniel Berlin]

On Tue, 2005-02-15 at 11:31 +0100, Richard Guenther wrote:
> On Tue, 15 Feb 2005, Richard Guenther wrote:
> 
> > Hi!
> >
> > I have isolated the patch (attached) that caused the previously reported
> > build ICE and a testcase.  The patch enables folding of
> > &a[i] + cst to &a[i+cst] in addition to &a[i] + cst*j -> &a[i+j].
> > If enabled, this transformation triggeres two times in the
> > testcase derived from libiberty/sort.c:
> >
> > #define UCHAR_MAX ((unsigned char)(-1))
> > #define DIGIT_MAX (UCHAR_MAX + 1)
> >
> > void sort_pointers (void)
> > {
> >   unsigned int count[DIGIT_MAX];
> >   unsigned int *countp;
> >
> >   for (countp = count + 1; countp < count + DIGIT_MAX; ++countp)
> >     ;
> > }
> 
> Ok, stepping through PRE is seems that folding of &count[1]+1 at
> tree-ssa-pre:1622 by calling fully_constant_expression on it
> will get us &count[2] (obviously) and this one does not have a
> value handle, and such we ICE.

No matter what, it should have a value handle at that point
1.  if it's is_gimple_min_invariant, get_value_handle should return the
expression.  
2. If it isn't, fully_constant_expression would have returned the
original, which should have been value numbered by compute_avail.
Thus, the assert is correct.



>   Wether fully_constant_expression
> is in error, or the assert, I do not know.  But I guess other
> kind of folding could trigger this, too.

Neither is really in error, it should catch exactly this case :).
This is a real bug, but in get_value_handle.

Fully_constant_expression only returns something other than what you
passed it when it folded to something to an is_gimple_min_invariant.
get_value_handle MUST return the expression when handed an
is_gimple_min_invariant thing.

And we have a winner!
get_value_handle is returning NULL instead of expr when handed your
expression.

Move the is_gimple_min_invariant check in get_value_handle above the
other checks, and your bug should be fixed.

I'll add a comment stating that get_value_handle is *required* to return
the expression when it is is_gimple_min_invariant when I do that.

Please try the attached


> 
> One could work around this either by removing the call to
> fully_constant_expression or by wrapping this in sth like
> 
>   tmp = fully_constant_expression (eprime);
>   vprime = get_value_handle (tmp);
>   if (!vprime)
>     vprime = get_value_handle (eprime);
>   else
>     eprime = tmp;
>   gcc_assert(vprime);
> 
> at least, this fixes the ICE.

This isn't right, but it would work. :)

PS feel free to copy me on any PRE bugs.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vndiff.diff
Type: text/x-patch
Size: 1103 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20050215/4cc7aa67/attachment.bin>