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: [tree-ssa] Loop header copying on trees


On Fri, 2004-02-13 at 08:58, Zdenek Dvorak wrote:

> this patch moves loop header copying from rtl to trees; this is
> important since it should increase effectivity of code motion
> transformations (and also because the rtl loop header copying code
> is just ugly).
> 
> The loop header copying is done just after the first dominator
> optimization pass, so that we do not miss opportunities for redundancy
> elimination.
> 
This is not a full review.  Though it looks mostly OK, I'm not familiar
enough with the loop code to give you more solid feedback.

Besides the SPEC numbers, which look decent, have you measured any
difference in compile time versus copy-header on RTL?  The
transformation doesn't look too expensive, though.

> + 
> + /* In cfg.c */
> + extern void alloc_rbi_pool (void);
> + extern void initialize_bb_rbi (basic_block bb);
> + extern void free_rbi_pool (void);
>
Please mention somewhere what RBI means.  We don't seem to have a single
comment about it anywhere in the code.

> *** common.opt	13 Feb 2004 00:10:27 -0000	1.14.2.19
> --- common.opt	13 Feb 2004 11:27:39 -0000
> *************** ftree-ccp
> *** 711,716 ****
> --- 711,720 ----
>   Common
>   Enable SSA-CCP optimization on trees
>   
> + ftree-ch
> + Common
> + Enable loop header copying on trees
> + 
>
Include a small blurb describing why header copying is beneficial. 
Also, do we really want to control it with a flag?  Some of the -f flags
we have in tree-ssa should probably disappear.

>   /* The main entry into loop optimization pass.  PHASE indicates which dump file
> *************** struct tree_opt_pass pass_loop = 
> *** 76,79 ****
> --- 77,367 ----
>     0,					/* properties_destroyed */
>     0,					/* todo_flags_start */
>     TODO_dump_func | TODO_verify_ssa	/* todo_flags_finish */
> + };
> + 
> + /* Checks whether the STMT is a call, and if so, returns the call_expr.  */
> + 
> + static tree
> + call_expr_p (tree stmt)
> + {
> +   if (TREE_CODE (stmt) == MODIFY_EXPR)
> +     stmt = TREE_OPERAND (stmt, 1);
> + 
> +   return TREE_CODE (stmt) == CALL_EXPR ? stmt : NULL_TREE;
> + }
> + 
No.  Use tree-simple.c:get_call_expr_in().  This will not detect
CALL_EXPRs inside return statements.


> + 
> +   /* Aproximately copy the conditions that used to be used in jump.c --
>        ^^^^^^^^^^^^
Approximately

> + /* Marks variables defined in basic block BB for rewriting.  */
> + 
> + static void
> + mark_defs_for_rewrite (basic_block bb)
> + {
>
Hmm, only defs?  Is it because uses are guaranteed to not be moved past
their dominator def?  Could you add a small explanation?

> + /* For all loops, copy the condition at the end of the loop body in front
> +    of the loop.  */
> + 
Again, a small description as to what are the benefits of doing this
transformation.

> + 	 We do not really copy the blocks immediatelly, so that we do not have
>                                           ^^^^^^^^^^^^
immediately


Thanks.  Diego.


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