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.
Index: tree-vn.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-vn.c,v
retrieving revision 2.6
diff -u -p -r2.6 tree-vn.c
--- tree-vn.c 25 Sep 2004 14:36:40 -0000 2.6
+++ tree-vn.c 15 Feb 2005 14:43:01 -0000
@@ -267,11 +267,15 @@ vn_lookup_or_add (tree expr, vuse_optype
/* Get the value handle of EXPR. This is the only correct way to get
the value handle for a "thing". If EXPR does not have a value
- handle associated, it returns NULL_TREE. */
+ handle associated, it returns NULL_TREE.
+ NB: If EXPR is min_invariant, this function is *required* to return EXPR. */
tree
get_value_handle (tree expr)
{
+ if (is_gimple_min_invariant (expr))
+ return expr;
+
if (TREE_CODE (expr) == SSA_NAME)
return SSA_NAME_VALUE (expr);
else if (EXPR_P (expr) || DECL_P (expr))
@@ -280,10 +284,7 @@ get_value_handle (tree expr)
return ((ann) ? ann->common.value_handle : NULL_TREE);
}
else
- {
- gcc_assert (is_gimple_min_invariant (expr));
- return expr;
- }
+ gcc_unreachable ();
}
