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] tree level if-conversion for vectorizer


On Wed, Aug 25, 2004 at 10:53:16PM -0700, Devang Patel wrote:
> +   num_uses = num_immediate_uses (df);
> + 
> +   for (j = 0; j < num_uses; j++)
> +     {
> +       tree use = immediate_use (df, j);
> +       if (TREE_CODE (use) == PHI_NODE)

Why are you iterating over the PHI nodes both here for
the modify_expr, and later via the PHI node?  Seems to
me the former is sufficient.

> +   switch (TREE_CODE (TREE_OPERAND (m_expr, 1)))
> +     {
> +     case CALL_EXPR:
> +     case TRUNC_DIV_EXPR:
> +     case INDIRECT_REF:
> +     case ARRAY_REF:

This list isn't anywhere near complete.  Anyway, I think all
of this should be reducible to tree_could_trap_p.  Well, you
probably don't want to ifcvt a call, even if it is to a pure
function.

> +       /* Attempt to do simple boolean expression simplification.  */

fold?

> +   /* Create temporary for C in < A = COND_EXPR < C, B>>.  */
> +   new_cond = ifc_temp_var (boolean_type_node, unshare_expr (cond), bsi, true);

A temporary is not required if COND is a simple condition,
a-la is_gimple_condexpr.  In practice this means one level
comparison, no conjunction or disjunction.

> + static void
> + handle_sibling_pattern (struct loop *loop)
> + {
> +   basic_block *hsp_bbs;
> +   basic_block bb;
> +   block_stmt_iterator bsi;
> +   unsigned int i;
> + 
> +   compute_immediate_uses (TDFA_USE_OPS, NULL);
> +   compute_immediate_uses (TDFA_USE_VOPS, NULL);

Why are you traversing forward def->use rather than backward from use->def?  

> + /* FIRST and SECOND statements are sibling statements.
> +    If possible fold FIRST statement into SECOND statement
> +    and remove FIRST statement.  
> + 
> +    1) 
> +      T1 = cond ? (void)0 : B;
> +      T2 = cond ? A : (void)B;

Err, did I miss something earlier?  Are those void's really there?
They certainly should not be.
                                                                                
I suppose that (void)0 is supposed to be from the default statement?
Suppose you do not find a sibling for this conditional, because the
variable really was only partially set in the original.  Do you leave
this invalid code hanging about, or do you simplify to "T1 = B"?

Why would you be checking for default anyway?  Isn't

	t1 = cond ? a : b
	t2 = cond ? t1 : c
->
	t2 = cond ? a : c

correct no matter what the values of a/b/c?

> + /* Return TRUE iff, EXP is boolean exp.  
> +    CHECKME: Move this into tree.c?  */

What are you trying to accomplish?

> + /* Return TRUE iff, EXP is constant.
> +    CHECKME: Move this into tree.c?  */

TREE_CONSTANT?

> + /* Return TRUE iff, EXP needs speculative load support.  
> +    See above for a speculative load example.  */

tree_could_trap_p?

> *************** build_ssa_operands (tree stmt, stmt_ann_
> *************** get_expr_operands (tree stmt, tree *expr

You needed to modify both of these for operand handling?
I find that surprising.  The later should have been sufficient.

> !  	if (TREE_CODE (TREE_OPERAND (expr, 1)) == COND_EXPR)

And given that this doesn't start with "case COND_EXPR:" 
I'll claim you've done it in the wrong place.



r~


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