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, Richard Guenther wrote:
> 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.
Which is, we do not fill in the expression-set for all constants.
The following fixes it. But I wonder why we need expression-sets
for constants at all ...
Richard.
2008-07-18 Richard Guenther <rguenther@suse.de>
* tree-ssa-sccvn.h (get_constant_value_id): Declare.
* tree-ssa-sccvn.c (get_constant_value_id): New function.
* tree-ssa-pre.c (get_expr_value_id): For newly created
constant value-ids make sure to add the expression to its
expression-set.
Index: gcc/tree-ssa-sccvn.c
===================================================================
*** gcc/tree-ssa-sccvn.c (revision 137930)
--- gcc/tree-ssa-sccvn.c (working copy)
*************** vn_constant_hash (const void *p1)
*** 328,333 ****
--- 328,351 ----
return vc1->hashcode;
}
+ /* Lookup a value id for CONSTANT and return it. If it does not
+ exist returns 0. */
+
+ unsigned int
+ get_constant_value_id (tree constant)
+ {
+ void **slot;
+ struct vn_constant_s vc;
+
+ vc.hashcode = vn_hash_constant_with_type (constant);
+ vc.constant = constant;
+ slot = htab_find_slot_with_hash (constant_to_value_id, &vc,
+ vc.hashcode, NO_INSERT);
+ if (slot)
+ return ((vn_constant_t)*slot)->value_id;
+ return 0;
+ }
+
/* Lookup a value id for CONSTANT, and if it does not exist, create a
new one and return it. If it does exist, return it. */
Index: gcc/tree-ssa-sccvn.h
===================================================================
*** gcc/tree-ssa-sccvn.h (revision 137930)
--- gcc/tree-ssa-sccvn.h (working copy)
*************** hashval_t vn_reference_compute_hash (con
*** 193,198 ****
--- 193,199 ----
int vn_reference_eq (const void *, const void *);
unsigned int get_max_value_id (void);
unsigned int get_next_value_id (void);
+ unsigned int get_constant_value_id (tree);
unsigned int get_or_alloc_constant_value_id (tree);
bool value_id_constant_p (unsigned int);
VEC (tree, gc) *shared_vuses_from_stmt (gimple);
Index: gcc/tree-ssa-pre.c
===================================================================
*** gcc/tree-ssa-pre.c (revision 137930)
--- gcc/tree-ssa-pre.c (working copy)
*************** get_expr_value_id (pre_expr expr)
*** 594,600 ****
switch (expr->kind)
{
case CONSTANT:
! return get_or_alloc_constant_value_id (PRE_EXPR_CONSTANT (expr));
case NAME:
return VN_INFO (PRE_EXPR_NAME (expr))->value_id;
case NARY:
--- 594,609 ----
switch (expr->kind)
{
case CONSTANT:
! {
! unsigned int id;
! id = get_constant_value_id (PRE_EXPR_CONSTANT (expr));
! if (id == 0)
! {
! id = get_or_alloc_constant_value_id (PRE_EXPR_CONSTANT (expr));
! add_to_value (id, expr);
! }
! return id;
! }
case NAME:
return VN_INFO (PRE_EXPR_NAME (expr))->value_id;
case NARY: