Re: Make dominated_by_p and get_immediate_dominator inline

[Jan Hubicka <hubicka at ucw dot cz>]

> On Tue, Jun 22, 2010 at 8:36 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > Hi,
> > this is change I forgot in my tree for a while. ?get_immediate_dominator and
> > dominated_by_p are one of most frequently called functions and they are good
> > candidates for inlining. Dominated_by_p has fallback path calling et_forest
> > code. ?Most of passes don't use dynamic dominance info and thus we never get
> > there. ?I was thinking about making specific version for non-dynamic dominators,
> > but since the most common calls from dominated_by_p comes from cfg.c
> > (dominance frontiers computation) that is sort of generic, it would
> > require more bookkeping of when info is dynamic and when not.
> > Hopefully the cold path is not going to cause too much of trouble.
> >
> > The patch needs to include et-forest.h in basic-block.h, but I do not see
> > much way around.
> 
> Do not make this inline?
> 
> I think making all these small functions inline makes things only more
> entangled. Now you include et-forest.h in every place that includes
> basic-block.h. I see no point in continuing my little project of
> removing #include's if you plan to add more of these.
> 
> I would much rather *remove* inline functions and let WHOPR do its job!

I am defnitly trying to push things towards making WHOPR the official way
to build GCC binary.  Hopefully we will get there for next release.

Even with WHOPR we won't inline dominated_by_p, at least as long as we
build with -O2 because inlining it increases cc1 binary size (by 14kb).
So at least we would have to declare it inline in dominance.c

Averaging 5 runs of each I got 9m4s withtout the patch and 9m2s with the
patch. dominated_by_p accounts to about 0.4% of runtime according to oprofile.
It seems that we do too many queries using et_splay that acounts 0.56% of
compilation time.  I guess it is because we do not recompute and keep
dominance tree in NO_FAST_QUERY mode for a while.  Will check.

Well, if we agree that we do not care about this relatively little amount
of perfomrance for non-WHOPR build, I guess following patch would be
less intrusive ;)

Bootstrapping/regtesting x86_64-linux, OK?

Honza

	* dominance.c (get_immediate_dominator, set_immediate_dominator,
	dom_convert_dir_to_idx): Make inline; use checking assert.
Index: dominance.c
===================================================================
--- dominance.c	(revision 161199)
+++ dominance.c	(working copy)
@@ -188,10 +188,10 @@ init_dom_info (struct dom_info *di, enum
    to be modified, obviously, if additional values are added to
    cdi_direction.  */
 
-static unsigned int
+static inline unsigned int
 dom_convert_dir_to_idx (enum cdi_direction dir)
 {
-  gcc_assert (dir == CDI_DOMINATORS || dir == CDI_POST_DOMINATORS);
+  gcc_checking_assert (dir == CDI_DOMINATORS || dir == CDI_POST_DOMINATORS);
   return dir - 1;
 }
 
@@ -696,13 +696,13 @@ free_dominance_info (enum cdi_direction
 }
 
 /* Return the immediate dominator of basic block BB.  */
-basic_block
+inline basic_block
 get_immediate_dominator (enum cdi_direction dir, basic_block bb)
 {
   unsigned int dir_index = dom_convert_dir_to_idx (dir);
   struct et_node *node = bb->dom[dir_index];
 
-  gcc_assert (dom_computed[dir_index]);
+  gcc_checking_assert (dom_computed[dir_index]);
 
   if (!node->father)
     return NULL;
@@ -712,14 +712,14 @@ get_immediate_dominator (enum cdi_direct
 
 /* Set the immediate dominator of the block possibly removing
    existing edge.  NULL can be used to remove any edge.  */
-void
+inline void
 set_immediate_dominator (enum cdi_direction dir, basic_block bb,
 			 basic_block dominated_by)
 {
   unsigned int dir_index = dom_convert_dir_to_idx (dir);
   struct et_node *node = bb->dom[dir_index];
 
-  gcc_assert (dom_computed[dir_index]);
+  gcc_checking_assert (dom_computed[dir_index]);
 
   if (node->father)
     {