diff mbox

Make dominated_by_p and get_immediate_dominator inline

Message ID 20100622183636.GC23996@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka June 22, 2010, 6:36 p.m. UTC 
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.

Bootstrapped/regtested x86_64-linux
OK?

Honza

	* dominance.c (dom_convert_dir_to_idx, get_immediate_dominator,
	dominated_by_p): Move from here to...
	* basic-block.h: Include et-forest.h
	* dominance.c (dom_convert_dir_to_idx, get_immediate_dominator,
	dominated_by_p): ... here; use checking_assert.

Comments

Steven Bosscher June 22, 2010, 8:33 p.m. UTC | #1 
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!

Ciao!
Steven
Steven Bosscher June 22, 2010, 8:34 p.m. UTC | #2 
On Tue, Jun 22, 2010 at 10:33 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> 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!

Oh, and you don't even mention if there is any benefit of this patch.
It seems to me that moving random functions to header files only makes
sense if you get a measurable speedup.

Ciao!
Steven
diff mbox

Patch

Index: dominance.c
===================================================================
--- dominance.c	(revision 161199)
+++ dominance.c	(working copy)
@@ -183,18 +183,6 @@  init_dom_info (struct dom_info *di, enum
 
 #undef init_ar
 
-/* Map dominance calculation type to array index used for various
-   dominance information arrays.  This version is simple -- it will need
-   to be modified, obviously, if additional values are added to
-   cdi_direction.  */
-
-static unsigned int
-dom_convert_dir_to_idx (enum cdi_direction dir)
-{
-  gcc_assert (dir == CDI_DOMINATORS || dir == CDI_POST_DOMINATORS);
-  return dir - 1;
-}
-
 /* Free all allocated memory in DI, but not DI itself.  */
 
 static void
@@ -695,21 +683,6 @@  free_dominance_info (enum cdi_direction
   dom_computed[dir_index] = DOM_NONE;
 }
 
-/* Return the immediate dominator of basic block BB.  */
-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]);
-
-  if (!node->father)
-    return NULL;
-
-  return (basic_block) node->father->data;
-}
-
 /* Set the immediate dominator of the block possibly removing
    existing edge.  NULL can be used to remove any edge.  */
 void
@@ -948,22 +921,6 @@  nearest_common_dominator_for_set (enum c
             && DFS_Number_Out (A) <= DFS_Number_Out(B);
    }  */
 
-/* Return TRUE in case BB1 is dominated by BB2.  */
-bool
-dominated_by_p (enum cdi_direction dir, const_basic_block bb1, const_basic_block bb2)
-{
-  unsigned int dir_index = dom_convert_dir_to_idx (dir);
-  struct et_node *n1 = bb1->dom[dir_index], *n2 = bb2->dom[dir_index];
-
-  gcc_assert (dom_computed[dir_index]);
-
-  if (dom_computed[dir_index] == DOM_OK)
-    return (n1->dfs_num_in >= n2->dfs_num_in
-  	    && n1->dfs_num_out <= n2->dfs_num_out);
-
-  return et_below (n1, n2);
-}
-
 /* Returns the entry dfs number for basic block BB, in the direction DIR.  */
 
 unsigned
Index: basic-block.h
===================================================================
--- basic-block.h	(revision 161199)
+++ basic-block.h	(working copy)
@@ -24,6 +24,7 @@  along with GCC; see the file COPYING3.
 #include "predict.h"
 #include "vec.h"
 #include "function.h"
+#include "et-forest.h"
 
 /* Type we use to hold basic block counters.  Should be at least
    64bit.  Although a counter cannot be negative, we use a signed
@@ -837,6 +838,50 @@  enum cdi_direction
   CDI_POST_DOMINATORS = 2
 };
 
+/* Map dominance calculation type to array index used for various
+   dominance information arrays.  This version is simple -- it will need
+   to be modified, obviously, if additional values are added to
+   cdi_direction.  */
+
+static inline unsigned int
+dom_convert_dir_to_idx (enum cdi_direction dir)
+{
+  gcc_checking_assert (dir == CDI_DOMINATORS || dir == CDI_POST_DOMINATORS);
+  return dir - 1;
+}
+
+/* Return TRUE in case BB1 is dominated by BB2.  */
+static inline bool
+dominated_by_p (enum cdi_direction dir, const_basic_block bb1, const_basic_block bb2)
+{
+  unsigned int dir_index = dom_convert_dir_to_idx (dir);
+  struct et_node *n1 = bb1->dom[dir_index], *n2 = bb2->dom[dir_index];
+
+  gcc_checking_assert (dom_computed[dir_index]);
+
+  if (dom_computed[dir_index] == DOM_OK)
+    return (n1->dfs_num_in >= n2->dfs_num_in
+  	    && n1->dfs_num_out <= n2->dfs_num_out);
+
+  return et_below (n1, n2);
+}
+
+
+/* Return the immediate dominator of basic block BB.  */
+static 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_checking_assert (dom_computed[dir_index]);
+
+  if (!node->father)
+    return NULL;
+
+  return (basic_block) node->father->data;
+}
+
 extern enum dom_state dom_info_state (enum cdi_direction);
 extern void set_dom_info_availability (enum cdi_direction, enum dom_state);
 extern bool dom_info_available_p (enum cdi_direction);