diff mbox

[2/4] Tweaks to C++ -> call graph interface

Message ID 20110319193005.258092777@alvy.suse.cz
State New
Headers show

Commit Message

Martin Jambor March 19, 2011, 7:29 p.m. UTC
I concede that my understanding of the C++ front-end inner workings
are quite narrow and so the folling is basically a suggestion.  But it
seems to me that at a few places where C++ queries the call graph for
a node, the lazy node creation is not necessary.  If a maintainer can
verify and approve (parts of) this after the big patch is committed,
it would be a nice cleanup too.

Thanks,

Martin


2011-03-18  Martin Jambor  <mjambor@suse.cz>

cp/
	* class.c (cp_fold_obj_type_ref): Call cgraph_get_node instead of
	cgraph_get_create_node.
	* decl2.c (cxx_callgraph_analyze_expr): Call cgraph_do_get_node
	instead of cgraph_get_create_node.
	(cp_write_global_declarations): Call cgraph_get_node instead of
	cgraph_get_create_node.
	* method.c (make_alias_for_thunk): Call cgraph_do_get_node instead
	of cgraph_get_create_node.
	(use_thunk): Likewise.
	* optimize.c (maybe_clone_body): Call cgraph_same_body_alias only
	when flag_syntax_only is not set.  Call cgraph_do_get_node instead of
	cgraph_get_create_node.
	(maybe_clone_body): Call cgraph_get_node instead of
	cgraph_get_create_node.

Comments

Jan Hubicka March 23, 2011, 4:55 p.m. UTC | #1
> I concede that my understanding of the C++ front-end inner workings
> are quite narrow and so the folling is basically a suggestion.  But it
> seems to me that at a few places where C++ queries the call graph for
> a node, the lazy node creation is not necessary.  If a maintainer can
> verify and approve (parts of) this after the big patch is committed,
> it would be a nice cleanup too.
> 
> Thanks,
> 
> Martin
> 
> 
> Index: src/gcc/cp/class.c
> ===================================================================
> --- src.orig/gcc/cp/class.c	2011-03-18 19:34:07.000000000 +0100
> +++ src/gcc/cp/class.c	2011-03-18 19:34:38.000000000 +0100
> @@ -8405,7 +8405,7 @@ cp_fold_obj_type_ref (tree ref, tree kno
>  				  DECL_VINDEX (fndecl)));
>  #endif
>  
> -  cgraph_get_create_node (fndecl)->local.vtable_method = true;
> +  cgraph_get_node (fndecl)->local.vtable_method = true;

I believe that vtable_method is ugly hack to work around the fact that we was
not able to optimize away functions that has address taken.  Perhaps this can
just be removed now?
>  
>    return build_address (fndecl);
>  }
> Index: src/gcc/cp/decl2.c
> ===================================================================
> --- src.orig/gcc/cp/decl2.c	2011-03-18 19:34:07.000000000 +0100
> +++ src/gcc/cp/decl2.c	2011-03-18 19:34:39.000000000 +0100
> @@ -3375,12 +3375,12 @@ cxx_callgraph_analyze_expr (tree *tp, in
>      case PTRMEM_CST:
>        if (TYPE_PTRMEMFUNC_P (TREE_TYPE (t)))
>  	cgraph_mark_address_taken_node (
> -			      cgraph_get_create_node (PTRMEM_CST_MEMBER (t)));
> +				  cgraph_do_get_node (PTRMEM_CST_MEMBER (t)));

I believe this si not safe in general, since it happens at cgraph construction time.
However perhaps all of those are gimplified away now?
>        break;
>      case BASELINK:
>        if (TREE_CODE (BASELINK_FUNCTIONS (t)) == FUNCTION_DECL)
>  	cgraph_mark_address_taken_node (
> -			      cgraph_get_create_node (BASELINK_FUNCTIONS (t)));
> +				  cgraph_do_get_node (BASELINK_FUNCTIONS (t)));
>        break;
>      case VAR_DECL:
>        if (DECL_CONTEXT (t)
> @@ -3893,7 +3893,7 @@ cp_write_global_declarations (void)
>  	  if (!DECL_EXTERNAL (decl)
>  	      && decl_needed_p (decl)
>  	      && !TREE_ASM_WRITTEN (decl)
> -	      && !cgraph_get_create_node (decl)->local.finalized)
> +	      && !cgraph_get_node (decl)->local.finalized)
>  	    {
>  	      /* We will output the function; no longer consider it in this
>  		 loop.  */
> Index: src/gcc/cp/method.c
> ===================================================================
> --- src.orig/gcc/cp/method.c	2011-03-18 19:34:07.000000000 +0100
> +++ src/gcc/cp/method.c	2011-03-18 19:34:40.000000000 +0100
> @@ -260,7 +260,7 @@ make_alias_for_thunk (tree function)
>    if (!flag_syntax_only)
>      {
>        struct cgraph_node *aliasn;
> -      aliasn = cgraph_same_body_alias (cgraph_get_create_node (function),
> +      aliasn = cgraph_same_body_alias (cgraph_do_get_node (function),
>  				       alias, function);
>        DECL_ASSEMBLER_NAME (function);
>        gcc_assert (aliasn != NULL);
> @@ -379,7 +379,7 @@ use_thunk (tree thunk_fndecl, bool emit_
>    a = nreverse (t);
>    DECL_ARGUMENTS (thunk_fndecl) = a;
>    TREE_ASM_WRITTEN (thunk_fndecl) = 1;
> -  cgraph_add_thunk (cgraph_get_create_node (function), thunk_fndecl, function,
> +  cgraph_add_thunk (cgraph_do_get_node (function), thunk_fndecl, function,

These two ought t be safe.
>  		    this_adjusting, fixed_offset, virtual_value,
>  		    virtual_offset, alias);
>  
> Index: src/gcc/cp/optimize.c
> ===================================================================
> --- src.orig/gcc/cp/optimize.c	2011-03-18 19:34:07.000000000 +0100
> +++ src/gcc/cp/optimize.c	2011-03-18 19:34:40.000000000 +0100
> @@ -309,8 +309,9 @@ maybe_clone_body (tree fn)
>  	  && (!DECL_ONE_ONLY (fns[0])
>  	      || (HAVE_COMDAT_GROUP
>  		  && DECL_WEAK (fns[0])))
> -	  && cgraph_same_body_alias (cgraph_get_create_node (fns[0]), clone,
> -				     fns[0]))
> +	  && (flag_syntax_only
> +	      || cgraph_same_body_alias (cgraph_do_get_node (fns[0]), clone,
> +					 fns[0])))

Same here, the node we are producing alias of should exist.
>  	{
>  	  alias = true;
>  	  if (DECL_ONE_ONLY (fns[0]))
> @@ -424,8 +425,8 @@ maybe_clone_body (tree fn)
>  	  /* If *[CD][12]* dtors go into the *[CD]5* comdat group and dtor is
>  	     virtual, it goes into the same comdat group as well.  */
>  	  DECL_COMDAT_GROUP (fns[2]) = comdat_group;
> -	  base_dtor_node = cgraph_get_create_node (fns[0]);
> -	  deleting_dtor_node = cgraph_get_create_node (fns[2]);
> +	  base_dtor_node = cgraph_get_node (fns[0]);
> +	  deleting_dtor_node = cgraph_get_node (fns[2]);
>  	  gcc_assert (base_dtor_node->same_comdat_group == NULL);
>  	  gcc_assert (deleting_dtor_node->same_comdat_group == NULL);
do_get_node here? 
I am not at all sure why function we are cloning should have a node attached given
that it is an abstract function. Or do I miss something?

Honza
>  	  base_dtor_node->same_comdat_group = deleting_dtor_node;
diff mbox

Patch

Index: src/gcc/cp/class.c
===================================================================
--- src.orig/gcc/cp/class.c	2011-03-18 19:34:07.000000000 +0100
+++ src/gcc/cp/class.c	2011-03-18 19:34:38.000000000 +0100
@@ -8405,7 +8405,7 @@  cp_fold_obj_type_ref (tree ref, tree kno
 				  DECL_VINDEX (fndecl)));
 #endif
 
-  cgraph_get_create_node (fndecl)->local.vtable_method = true;
+  cgraph_get_node (fndecl)->local.vtable_method = true;
 
   return build_address (fndecl);
 }
Index: src/gcc/cp/decl2.c
===================================================================
--- src.orig/gcc/cp/decl2.c	2011-03-18 19:34:07.000000000 +0100
+++ src/gcc/cp/decl2.c	2011-03-18 19:34:39.000000000 +0100
@@ -3375,12 +3375,12 @@  cxx_callgraph_analyze_expr (tree *tp, in
     case PTRMEM_CST:
       if (TYPE_PTRMEMFUNC_P (TREE_TYPE (t)))
 	cgraph_mark_address_taken_node (
-			      cgraph_get_create_node (PTRMEM_CST_MEMBER (t)));
+				  cgraph_do_get_node (PTRMEM_CST_MEMBER (t)));
       break;
     case BASELINK:
       if (TREE_CODE (BASELINK_FUNCTIONS (t)) == FUNCTION_DECL)
 	cgraph_mark_address_taken_node (
-			      cgraph_get_create_node (BASELINK_FUNCTIONS (t)));
+				  cgraph_do_get_node (BASELINK_FUNCTIONS (t)));
       break;
     case VAR_DECL:
       if (DECL_CONTEXT (t)
@@ -3893,7 +3893,7 @@  cp_write_global_declarations (void)
 	  if (!DECL_EXTERNAL (decl)
 	      && decl_needed_p (decl)
 	      && !TREE_ASM_WRITTEN (decl)
-	      && !cgraph_get_create_node (decl)->local.finalized)
+	      && !cgraph_get_node (decl)->local.finalized)
 	    {
 	      /* We will output the function; no longer consider it in this
 		 loop.  */
Index: src/gcc/cp/method.c
===================================================================
--- src.orig/gcc/cp/method.c	2011-03-18 19:34:07.000000000 +0100
+++ src/gcc/cp/method.c	2011-03-18 19:34:40.000000000 +0100
@@ -260,7 +260,7 @@  make_alias_for_thunk (tree function)
   if (!flag_syntax_only)
     {
       struct cgraph_node *aliasn;
-      aliasn = cgraph_same_body_alias (cgraph_get_create_node (function),
+      aliasn = cgraph_same_body_alias (cgraph_do_get_node (function),
 				       alias, function);
       DECL_ASSEMBLER_NAME (function);
       gcc_assert (aliasn != NULL);
@@ -379,7 +379,7 @@  use_thunk (tree thunk_fndecl, bool emit_
   a = nreverse (t);
   DECL_ARGUMENTS (thunk_fndecl) = a;
   TREE_ASM_WRITTEN (thunk_fndecl) = 1;
-  cgraph_add_thunk (cgraph_get_create_node (function), thunk_fndecl, function,
+  cgraph_add_thunk (cgraph_do_get_node (function), thunk_fndecl, function,
 		    this_adjusting, fixed_offset, virtual_value,
 		    virtual_offset, alias);
 
Index: src/gcc/cp/optimize.c
===================================================================
--- src.orig/gcc/cp/optimize.c	2011-03-18 19:34:07.000000000 +0100
+++ src/gcc/cp/optimize.c	2011-03-18 19:34:40.000000000 +0100
@@ -309,8 +309,9 @@  maybe_clone_body (tree fn)
 	  && (!DECL_ONE_ONLY (fns[0])
 	      || (HAVE_COMDAT_GROUP
 		  && DECL_WEAK (fns[0])))
-	  && cgraph_same_body_alias (cgraph_get_create_node (fns[0]), clone,
-				     fns[0]))
+	  && (flag_syntax_only
+	      || cgraph_same_body_alias (cgraph_do_get_node (fns[0]), clone,
+					 fns[0])))
 	{
 	  alias = true;
 	  if (DECL_ONE_ONLY (fns[0]))
@@ -424,8 +425,8 @@  maybe_clone_body (tree fn)
 	  /* If *[CD][12]* dtors go into the *[CD]5* comdat group and dtor is
 	     virtual, it goes into the same comdat group as well.  */
 	  DECL_COMDAT_GROUP (fns[2]) = comdat_group;
-	  base_dtor_node = cgraph_get_create_node (fns[0]);
-	  deleting_dtor_node = cgraph_get_create_node (fns[2]);
+	  base_dtor_node = cgraph_get_node (fns[0]);
+	  deleting_dtor_node = cgraph_get_node (fns[2]);
 	  gcc_assert (base_dtor_node->same_comdat_group == NULL);
 	  gcc_assert (deleting_dtor_node->same_comdat_group == NULL);
 	  base_dtor_node->same_comdat_group = deleting_dtor_node;