Re: [PATCH][4.5] Move gimplifier predicates

[Diego Novillo <dnovillo at google dot com>]

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.

> + /* ???  This function should be private to gimplify.c, but it is used
> +       by gimple.c:walk_gimple_op for unknown reasons.  */
> + bool is_gimple_formal_tmp_var (tree);

Move this to gimple.c

> +
> + /* Returns true if T is a GIMPLE formal temporary variable.  */
> +
> + bool
> + is_gimple_formal_tmp_var (tree t)
> + {
> +   if (TREE_CODE (t) == SSA_NAME)
> +     return true;
> +
> +   return TREE_CODE (t) == VAR_DECL && DECL_GIMPLE_FORMAL_TEMP_P (t);
> + }
> +
> + /* Returns true if T is a GIMPLE formal temporary register variable.  */
> +
> + static bool
> + is_gimple_formal_tmp_reg (tree t)
> + {
> +   /* The intent of this is to get hold of a value that won't change.
> +      An SSA_NAME qualifies no matter if its of a user variable or not.  */
> +   if (TREE_CODE (t) == SSA_NAME)
> +     return true;
> +
> +   /* We don't know the lifetime characteristics of user variables.  */
> +   if (!is_gimple_formal_tmp_var (t))
> +     return false;
> +
> +   /* Finally, it must be capable of being placed in a register.  */
> +   return is_gimple_reg (t);
> + }
> +
> + /* Return true if T is a GIMPLE RHS for an assignment to a temporary.  */
> +
> + static bool
> + is_gimple_formal_tmp_rhs (tree t)
> + {
> +   if (is_gimple_lvalue (t) || is_gimple_val (t))
> +     return true;
> +
> +   return get_gimple_rhs_class (TREE_CODE (t)) != GIMPLE_INVALID_RHS;
> + }
> +
> + /* Returns true iff T is a valid RHS for an assignment to a renamed
> +    user -- or front-end generated artificial -- variable.  */
> +
> + static bool
> + is_gimple_reg_rhs (tree t)
> + {
> +   /* If the RHS of the MODIFY_EXPR may throw or make a nonlocal goto
> +      and the LHS is a user variable, then we need to introduce a formal
> +      temporary.  This way the optimizers can determine that the user
> +      variable is only modified if evaluation of the RHS does not throw.
> +
> +      Don't force a temp of a non-renamable type; the copy could be
> +      arbitrarily expensive.  Instead we will generate a VDEF for
> +      the assignment.  */
> +
> +   if (is_gimple_reg_type (TREE_TYPE (t)) && tree_could_throw_p (t))
> +     return false;
> +
> +   return is_gimple_formal_tmp_rhs (t);
> + }
> +
> + /* ???  This function should be private to gimplify.c, but it is used
> +       by gimple.c:walk_gimple_op for unknown reasons.  */
> + bool is_gimple_mem_rhs (tree);
> +
> + /* Returns true iff T is a valid RHS for an assignment to an un-renamed
> +    LHS, or for a call argument.  */
> +
> + bool
> + is_gimple_mem_rhs (tree t)
> + {
> +   /* If we're dealing with a renamable type, either source or dest must be
> +      a renamed variable.  */
> +   if (is_gimple_reg_type (TREE_TYPE (t)))
> +     return is_gimple_val (t);
> +   else
> +     return is_gimple_formal_tmp_rhs (t);
> + }

Likewise.  All the public is_gimple_* predicate make more sense in gimple.c.

Not quite sure about the Java predicate, but Andrew seems to know, so
if he's OK with it the rest looks fine to me.


Diego.