This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][4.5] Move gimplifier predicates


On Fri, 5 Dec 2008, Diego Novillo wrote:

> On Thu, Dec 4, 2008 at 12:10, Richard Guenther <rguenther@suse.de> wrote:
> >
> > This moves gimplifier predicates to where they belong and makes them
> > private.  Apart from the uses in walk_gimple_op for which I have no
> > clue what this monster-function tries to do - Diego, do you remember?
> 
> Sure.  It walks the trees in the tuple operand slots.  This is still
> different than the operand scanner, as it is a pure tree walk.  It's
> seldom used, but still needed.
> 
> > Index: trunk/gcc/gimple.c
> > ===================================================================
> > *** trunk.orig/gcc/gimple.c     2008-12-04 11:18:08.000000000 +0100
> > --- trunk/gcc/gimple.c  2008-12-04 15:00:49.000000000 +0100
> > *************** walk_gimple_op (gimple stmt, walk_tree_f
> > *** 1373,1378 ****
> > --- 1373,1381 ----
> >    struct pointer_set_t *pset = (wi) ? wi->pset : NULL;
> >    unsigned i;
> >    tree ret = NULL_TREE;
> > +   /* ???  We shouldn't be using these here.  */
> > +   extern bool is_gimple_formal_tmp_var (tree);
> > +   extern bool is_gimple_mem_rhs (tree);
> 
> Why not?  gimple.c and gimplify.c are joined at the hip after all.
> It's perfectly fine for walk_gimple_op to want to use these
> predicates.  The only way I see to get rid of them would be if the
> functionality they provide is not needed (i.e., wi->val_only).
>
> In fact, is_gimple_formal_tmp_var and is_gimple_mem_rhs make more
> sense to be provided by gimple.c instead of gimplify.c.

The problem is that the _formal_tmp stuff does only work in the context
of gimplification.  Which means that these are not predicates that
you can reliably use from elsewhere which means that they better should
not be exported.  As is_gimple_mem_rhs calls a _formal_tmp variant the
same is true for it.

See the strange places we set DECL_GIMPLE_FORMAL_TEMP_P in gimplify.c
and how we clean that flag again in pop_gimplify_context (not
reliably though dependent on what set that flag).

This patch developed in the course of trying to understand the
correctness and optimization parts of DECL_GIMPLE_FORMAL_TEMP_P and
in trying to make DECL_GIMPLE_REG_P handling more general and not
dependent on is_gimple_reg_type.

I will just put this patch on hold until I have more convincing
evidence in this area, but I do not agree with your simple
rationale ;)

Thanks,
Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]