This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix the constant mess in PRE
On Thu, 17 Jul 2008, Daniel Berlin wrote:
> On Thu, Jul 17, 2008 at 7:36 AM, Richard Guenther <rguenther@suse.de> wrote:
> > On Wed, 16 Jul 2008, Daniel Berlin wrote:
> >
> >> In any case, if your patch works without the other workarounds i had,
> >> i'd rather use your patch, since it would be nice to go back to
> >> constants being their own reps.
> >
> > I verified that reverting
> >
> > 2008-07-15 Daniel Berlin <dberlin@dberlin.org>
> >
> > * tree-ssa-sccvn.c (expressions_equal_p): Check type equality.
> > * tree-ssa-pre.c (pre_expr_eq): Ditto
> > (get_constant_for_value_id): Take a type as an argument.
> > (fully_constant_expression): Pass in type.
> > (find_or_generate_expression): Short circuit constant case.
> > (create_expression_by_pieces): Remove special casing of
> > pointer_plus.
> > (do_regular_insertion): Short circuit constant case.
> > (do_partial_partial_insertion): Ditto.
> >
> > and applying the patch to trunk only fixes some more regressions.
> >
> Great!
>
> > Should we still keep the "Short circuit constant case" cases or will
> > that be fixed automatically by making constants represent themselves?
>
> It should be fixed, but it would be ideal if we bootstrapped it once
> with an assert in those places like
>
> gcc_assert(useless_type_conversion_p(TREE_TYPE (result from
> bitmap_find_leader), TREE_TYPE (expr we pass in)))
> .
> (In all those places you have the original expression a constant, and
> this should just check that the constant we get back has the same
> type).
>
> Just to make sure it's not slipping through the cracks somehow.
>
> I'd rather be finally done with this mess :)
>
> I'm happy to do this and commit the result if it all works, if you are
> busy with tuples.
Btw, on tuples (with some more patches) I seem to need the following
(otherwise we ICE in get_constant_for_value_id sometimes). I guess
it makes sense to shortcut here, but maybe there's still some latent
bug.
Richard.
Index: gimple-tuples-branch/gcc/tree-ssa-pre.c
===================================================================
*** gimple-tuples-branch.orig/gcc/tree-ssa-pre.c 2008-07-17 15:05:27.000000000 +0200
--- gimple-tuples-branch/gcc/tree-ssa-pre.c 2008-07-17 15:09:20.000000000 +0200
*************** fully_constant_expression (pre_expr e)
*** 1061,1073 ****
constants. */
tree naryop0 = nary->op[0];
tree naryop1 = nary->op[1];
! pre_expr rep0 = get_or_alloc_expr_for (naryop0);
! pre_expr rep1 = get_or_alloc_expr_for (naryop1);
! unsigned int vrep0 = get_expr_value_id (rep0);
! unsigned int vrep1 = get_expr_value_id (rep1);
! tree const0 = get_constant_for_value_id (vrep0);
! tree const1 = get_constant_for_value_id (vrep1);
! tree result = NULL;
if (const0 && const1)
{
tree type1 = TREE_TYPE (nary->op[0]);
--- 1061,1084 ----
constants. */
tree naryop0 = nary->op[0];
tree naryop1 = nary->op[1];
! tree const0, const1, result;
! if (is_gimple_min_invariant (naryop0))
! const0 = naryop0;
! else
! {
! pre_expr rep0 = get_or_alloc_expr_for (naryop0);
! unsigned int vrep0 = get_expr_value_id (rep0);
! const0 = get_constant_for_value_id (vrep0);
! }
! if (is_gimple_min_invariant (naryop1))
! const1 = naryop1;
! else
! {
! pre_expr rep1 = get_or_alloc_expr_for (naryop1);
! unsigned int vrep1 = get_expr_value_id (rep1);
! const1 = get_constant_for_value_id (vrep1);
! }
! result = NULL;
if (const0 && const1)
{
tree type1 = TREE_TYPE (nary->op[0]);
*************** fully_constant_expression (pre_expr e)
*** 1086,1095 ****
/* We have to go from trees to pre exprs to value ids to
constants. */
tree naryop0 = nary->op[0];
! pre_expr rep0 = get_or_alloc_expr_for (naryop0);
! unsigned int vrep0 = get_expr_value_id (rep0);
! tree const0 = get_constant_for_value_id (vrep0);
! tree result = NULL;
if (const0)
{
tree type1 = TREE_TYPE (nary->op[0]);
--- 1097,1112 ----
/* We have to go from trees to pre exprs to value ids to
constants. */
tree naryop0 = nary->op[0];
! tree const0, result;
! if (is_gimple_min_invariant (naryop0))
! const0 = naryop0;
! else
! {
! pre_expr rep0 = get_or_alloc_expr_for (naryop0);
! unsigned int vrep0 = get_expr_value_id (rep0);
! const0 = get_constant_for_value_id (vrep0);
! }
! result = NULL;
if (const0)
{
tree type1 = TREE_TYPE (nary->op[0]);