diff mbox

[2/7] cgraph_node -> cgraph_get_node conversions accepting NULL results

Message ID 20110406232245.369880493@virgil.suse.cz
State New
Headers show

Commit Message

Martin Jambor April 6, 2011, 11:22 p.m. UTC 
Hi,

this patch converts a number of calls to cgraph_node to calls to
cgraph_get_node and provides means to deal with returned NULL value.
These are essentially the places where lazy node creation was
happening for no good reason.

Bootstrapped and tested separately on x86_64-linux without any
problems, tests on other platforms (together with the other patches)
in progress.

OK for trunk?

Thanks,

Martin


2011-04-06  Martin Jambor  <mjambor@suse.cz>

gcc/
	* cgraph.c (cgraph_local_info): Call cgraph_get_node instead
	of cgraph_node,	handle NULL return value.
	(cgraph_global_info): Likewise.
	(cgraph_rtl_info): Likewise.
	* tree-inline.c (estimate_num_insns): Likewise.
	* gimplify.c (unshare_body): Likewise.
	(unvisit_body): Likewise.
	(gimplify_body): Likewise.
	* predict.c (optimize_function_for_size_p): Likewise.
	* tree-ssa-alias.c (ref_maybe_used_by_call_p_1): Likewise.
	(call_may_clobber_ref_p_1): Likewise.
	* varasm.c (function_section_1): Likewise.
	(assemble_start_function): Likewise.

gcc/java/
	* decl.c (java_mark_decl_local): Call cgraph_get_node instead of
	cgraph_node and handle returned NULL.

Comments

Jan Hubicka April 11, 2011, 10:18 a.m. UTC | #1 
> 2011-04-06  Martin Jambor  <mjambor@suse.cz>
> 
> gcc/
> 	* cgraph.c (cgraph_local_info): Call cgraph_get_node instead
> 	of cgraph_node,	handle NULL return value.
> 	(cgraph_global_info): Likewise.
> 	(cgraph_rtl_info): Likewise.
> 	* tree-inline.c (estimate_num_insns): Likewise.
> 	* gimplify.c (unshare_body): Likewise.
> 	(unvisit_body): Likewise.
> 	(gimplify_body): Likewise.
> 	* predict.c (optimize_function_for_size_p): Likewise.
> 	* tree-ssa-alias.c (ref_maybe_used_by_call_p_1): Likewise.
> 	(call_may_clobber_ref_p_1): Likewise.
> 	* varasm.c (function_section_1): Likewise.
> 	(assemble_start_function): Likewise.
> 
> gcc/java/
> 	* decl.c (java_mark_decl_local): Call cgraph_get_node instead of
> 	cgraph_node and handle returned NULL.

OK.
> Index: src/gcc/predict.c
> ===================================================================
> --- src.orig/gcc/predict.c
> +++ src/gcc/predict.c
> @@ -214,10 +214,11 @@ probably_never_executed_bb_p (const_basi
>  bool
>  optimize_function_for_size_p (struct function *fun)
>  {
> +  struct cgraph_node *node;
>    return (optimize_size
>  	  || (fun && fun->decl
> -	      && (cgraph_node (fun->decl)->frequency
> -		  == NODE_FREQUENCY_UNLIKELY_EXECUTED)));
> +	      && (node = cgraph_get_node (fun->decl))
> +	      && (node->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED)));

I guess this is because optimize_function_for_size_p is used from folder
that in turn is called before cgraph is built.  Please, for consistency, add same
test into the other predicates that calls cgraph_node (fun->decl) here
and also uwind the statement into series of ifs.  It has grown in to
quite a beast.  This is preaproved either as this patch of followup.

Honza
Martin Jambor April 11, 2011, 1:33 p.m. UTC | #2 
Hi,

On Mon, Apr 11, 2011 at 12:18:24PM +0200, Jan Hubicka wrote:
> > 2011-04-06  Martin Jambor  <mjambor@suse.cz>
> > 
> > gcc/
> > 	* cgraph.c (cgraph_local_info): Call cgraph_get_node instead
> > 	of cgraph_node,	handle NULL return value.
> > 	(cgraph_global_info): Likewise.
> > 	(cgraph_rtl_info): Likewise.
> > 	* tree-inline.c (estimate_num_insns): Likewise.
> > 	* gimplify.c (unshare_body): Likewise.
> > 	(unvisit_body): Likewise.
> > 	(gimplify_body): Likewise.
> > 	* predict.c (optimize_function_for_size_p): Likewise.
> > 	* tree-ssa-alias.c (ref_maybe_used_by_call_p_1): Likewise.
> > 	(call_may_clobber_ref_p_1): Likewise.
> > 	* varasm.c (function_section_1): Likewise.
> > 	(assemble_start_function): Likewise.
> > 
> > gcc/java/
> > 	* decl.c (java_mark_decl_local): Call cgraph_get_node instead of
> > 	cgraph_node and handle returned NULL.
> 
> OK.

Thnanks! I'm in the process of re-bootstrapping and will start
committing the patches soon.

However...

> > Index: src/gcc/predict.c
> > ===================================================================
> > --- src.orig/gcc/predict.c
> > +++ src/gcc/predict.c
> > @@ -214,10 +214,11 @@ probably_never_executed_bb_p (const_basi
> >  bool
> >  optimize_function_for_size_p (struct function *fun)
> >  {
> > +  struct cgraph_node *node;
> >    return (optimize_size
> >  	  || (fun && fun->decl
> > -	      && (cgraph_node (fun->decl)->frequency
> > -		  == NODE_FREQUENCY_UNLIKELY_EXECUTED)));
> > +	      && (node = cgraph_get_node (fun->decl))
> > +	      && (node->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED)));
> 
> I guess this is because optimize_function_for_size_p is used from folder
> that in turn is called before cgraph is built.
>  Please, for consistency, add same
> test into the other predicates that calls cgraph_node (fun->decl) here

...there are no such places in in predict.c, the other two calls
to cgraph_(get_)node pass current_function_decl to the function.
Or did you mean some other file?

> and also uwind the statement into series of ifs.  It has grown in to
> quite a beast.  This is preaproved either as this patch of followup.

OK, I've turned the body of the predicate into:

/* Return true when current function should always be optimized for size.  */

bool
optimize_function_for_size_p (struct function *fun)
{
  struct cgraph_node *node;

  if (optimize_size)
    return true;
  if (!fun || !fun->decl)
    return false;
  node = cgraph_get_node (fun->decl);
  if (node && (node->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED))
    return true;
  else
    return false;
}


Thanks,

Martin
diff mbox

Patch

Index: src/gcc/cgraph.c
===================================================================
--- src.orig/gcc/cgraph.c
+++ src/gcc/cgraph.c
@@ -1766,7 +1766,9 @@  cgraph_local_info (tree decl)
   struct cgraph_node *node;
 
   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
-  node = cgraph_node (decl);
+  node = cgraph_get_node (decl);
+  if (!node)
+    return NULL;
   return &node->local;
 }
 
@@ -1778,7 +1780,9 @@  cgraph_global_info (tree decl)
   struct cgraph_node *node;
 
   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL && cgraph_global_info_ready);
-  node = cgraph_node (decl);
+  node = cgraph_get_node (decl);
+  if (!node)
+    return NULL;
   return &node->global;
 }
 
@@ -1790,9 +1794,10 @@  cgraph_rtl_info (tree decl)
   struct cgraph_node *node;
 
   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
-  node = cgraph_node (decl);
-  if (decl != current_function_decl
-      && !TREE_ASM_WRITTEN (node->decl))
+  node = cgraph_get_node (decl);
+  if (!node
+      || (decl != current_function_decl
+	  && !TREE_ASM_WRITTEN (node->decl)))
     return NULL;
   return &node->rtl;
 }
Index: src/gcc/tree-inline.c
===================================================================
--- src.orig/gcc/tree-inline.c
+++ src/gcc/tree-inline.c
@@ -3470,10 +3470,11 @@  estimate_num_insns (gimple stmt, eni_wei
     case GIMPLE_CALL:
       {
 	tree decl = gimple_call_fndecl (stmt);
+	struct cgraph_node *node;
 
 	/* Do not special case builtins where we see the body.
 	   This just confuse inliner.  */
-	if (!decl || cgraph_node (decl)->analyzed)
+	if (!decl || !(node = cgraph_get_node (decl)) || node->analyzed)
 	  ;
 	/* For buitins that are likely expanded to nothing or
 	   inlined do not account operand costs.  */
Index: src/gcc/gimplify.c
===================================================================
--- src.orig/gcc/gimplify.c
+++ src/gcc/gimplify.c
@@ -959,11 +959,11 @@  copy_if_shared (tree *tp)
 static void
 unshare_body (tree *body_p, tree fndecl)
 {
-  struct cgraph_node *cgn = cgraph_node (fndecl);
+  struct cgraph_node *cgn = cgraph_get_node (fndecl);
 
   copy_if_shared (body_p);
 
-  if (body_p == &DECL_SAVED_TREE (fndecl))
+  if (cgn && body_p == &DECL_SAVED_TREE (fndecl))
     for (cgn = cgn->nested; cgn; cgn = cgn->next_nested)
       unshare_body (&DECL_SAVED_TREE (cgn->decl), cgn->decl);
 }
@@ -1000,11 +1000,11 @@  unmark_visited (tree *tp)
 static void
 unvisit_body (tree *body_p, tree fndecl)
 {
-  struct cgraph_node *cgn = cgraph_node (fndecl);
+  struct cgraph_node *cgn = cgraph_get_node (fndecl);
 
   unmark_visited (body_p);
 
-  if (body_p == &DECL_SAVED_TREE (fndecl))
+  if (cgn && body_p == &DECL_SAVED_TREE (fndecl))
     for (cgn = cgn->nested; cgn; cgn = cgn->next_nested)
       unvisit_body (&DECL_SAVED_TREE (cgn->decl), cgn->decl);
 }
@@ -7695,6 +7695,7 @@  gimplify_body (tree *body_p, tree fndecl
   gimple_seq parm_stmts, seq;
   gimple outer_bind;
   struct gimplify_ctx gctx;
+  struct cgraph_node *cgn;
 
   timevar_push (TV_TREE_GIMPLIFY);
 
@@ -7712,7 +7713,8 @@  gimplify_body (tree *body_p, tree fndecl
   unshare_body (body_p, fndecl);
   unvisit_body (body_p, fndecl);
 
-  if (cgraph_node (fndecl)->origin)
+  cgn = cgraph_get_node (fndecl);
+  if (cgn && cgn->origin)
     nonlocal_vlas = pointer_set_create ();
 
   /* Make sure input_location isn't set to something weird.  */
Index: src/gcc/predict.c
===================================================================
--- src.orig/gcc/predict.c
+++ src/gcc/predict.c
@@ -214,10 +214,11 @@  probably_never_executed_bb_p (const_basi
 bool
 optimize_function_for_size_p (struct function *fun)
 {
+  struct cgraph_node *node;
   return (optimize_size
 	  || (fun && fun->decl
-	      && (cgraph_node (fun->decl)->frequency
-		  == NODE_FREQUENCY_UNLIKELY_EXECUTED)));
+	      && (node = cgraph_get_node (fun->decl))
+	      && (node->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED)));
 }
 
 /* Return true when current function should always be optimized for speed.  */
Index: src/gcc/tree-ssa-alias.c
===================================================================
--- src.orig/gcc/tree-ssa-alias.c
+++ src/gcc/tree-ssa-alias.c
@@ -1245,14 +1245,18 @@  ref_maybe_used_by_call_p_1 (gimple call,
 
   /* Check if base is a global static variable that is not read
      by the function.  */
-  if (TREE_CODE (base) == VAR_DECL
+  if (callee != NULL_TREE
+      && TREE_CODE (base) == VAR_DECL
       && TREE_STATIC (base))
     {
+      struct cgraph_node *node = cgraph_get_node (callee);
       bitmap not_read;
 
-      if (callee != NULL_TREE
-	  && (not_read
-	        = ipa_reference_get_not_read_global (cgraph_node (callee)))
+      /* FIXME: Callee can be an OMP builtin that does not have a call graph
+	 node yet.  We should enforce that there are nodes for all decls in the
+	 IL and remove this check instead.  */
+      if (node
+	  && (not_read = ipa_reference_get_not_read_global (node))
 	  && bitmap_bit_p (not_read, DECL_UID (base)))
 	goto process_args;
     }
@@ -1512,10 +1516,11 @@  call_may_clobber_ref_p_1 (gimple call, a
       && TREE_CODE (base) == VAR_DECL
       && TREE_STATIC (base))
     {
+      struct cgraph_node *node = cgraph_get_node (callee);
       bitmap not_written;
 
-      if ((not_written
-	     = ipa_reference_get_not_written_global (cgraph_node (callee)))
+      if (node
+	  && (not_written = ipa_reference_get_not_written_global (node))
 	  && bitmap_bit_p (not_written, DECL_UID (base)))
 	return false;
     }
Index: src/gcc/varasm.c
===================================================================
--- src.orig/gcc/varasm.c
+++ src/gcc/varasm.c
@@ -605,11 +605,14 @@  function_section_1 (tree decl, bool forc
 
   if (decl)
     {
-      struct cgraph_node *node = cgraph_node (decl);
+      struct cgraph_node *node = cgraph_get_node (decl);
 
-      freq = node->frequency;
-      startup = node->only_called_at_startup;
-      exit = node->only_called_at_exit;
+      if (node)
+	{
+	  freq = node->frequency;
+	  startup = node->only_called_at_startup;
+	  exit = node->only_called_at_exit;
+	}
     }
   if (force_cold)
     freq = NODE_FREQUENCY_UNLIKELY_EXECUTED;
@@ -1607,11 +1610,12 @@  assemble_start_function (tree decl, cons
     }
   else if (DECL_SECTION_NAME (decl))
     {
+      struct cgraph_node *node = cgraph_get_node (current_function_decl);
       /* Calls to function_section rely on first_function_block_is_cold
 	 being accurate.  */
-      first_function_block_is_cold
-	 = (cgraph_node (current_function_decl)->frequency
-	    == NODE_FREQUENCY_UNLIKELY_EXECUTED);
+      first_function_block_is_cold = (node
+				      && node->frequency
+				      == NODE_FREQUENCY_UNLIKELY_EXECUTED);
     }
 
   in_cold_section_p = first_function_block_is_cold;
Index: src/gcc/java/decl.c
===================================================================
--- src.orig/gcc/java/decl.c
+++ src/gcc/java/decl.c
@@ -1928,7 +1928,10 @@  java_mark_decl_local (tree decl)
 #ifdef ENABLE_CHECKING
   /* Double check that we didn't pass the function to the callgraph early.  */
   if (TREE_CODE (decl) == FUNCTION_DECL)
-    gcc_assert (!cgraph_node (decl)->local.finalized);
+    {
+      struct cgraph_node *node = cgraph_get_node (decl);
+      gcc_assert (!node || !node->local.finalized);
+    }
 #endif
   gcc_assert (!DECL_RTL_SET_P (decl));
 }