diff mbox

[4/7] Removal of cgraph_node function

Message ID 20110411141542.GC1659@virgil.arch.suse.de
State New
Headers show

Commit Message

Martin Jambor April 11, 2011, 2:15 p.m. UTC 
Hi,

On Mon, Apr 11, 2011 at 12:28:36PM +0200, Jan Hubicka wrote:
> > 2011-04-06  Martin Jambor  <mjambor@suse.cz>
> > 
> > 	* cgraph.h (cgraph_node): Remove function declaration.
> > 	(cgraph_create_node): Declare.
> > 	(cgraph_get_create_node): Likewise.
> > 
> > 	* cgraph.c (cgraph_create_node): Renamed to cgraph_create_node_1.
> > 	Updated all callers.
> > 	(cgraph_node): Renamed to cgraph_create_node, assert that a node for
> > 	the decl does not already exist.  Call cgraph_get_create_node instead
> > 	of cgraph_node.
> > 	(cgraph_get_create_node): New function.
> > 	(cgraph_same_body_alias): Update comment.
> > 	(cgraph_set_call_stmt): Call cgraph_get_node instead of cgraph_node,
> > 	assert it does not return NULL.
> > 	(cgraph_update_edges_for_call_stmt): Likewise.
> > 	(cgraph_clone_edge): Likewise.
> > 	(cgraph_create_virtual_clone): Likewise.
> > 	(cgraph_update_edges_for_call_stmt_node): Call cgraph_get_create_node
> > 	instead of cgraph_node.
> > 	(cgraph_add_new_function): Call cgraph_create_node or
> > 	cgraph_get_create_node instead of cgraph_node.
> > 
> > 	* cgraphbuild.c (record_reference): Call cgraph_get_create_node
> > 	instead of cgraph_node.
> > 	(record_eh_tables): Likewise.
> > 	(mark_address): Likewise.
> > 	(mark_load): Likewise.
> > 	(build_cgraph_edges): Call cgraph_get_create_node instead
> > 	of cgraph_node.
> > 	(rebuild_cgraph_edges): Likewise.
> > 
> > 	* cgraphunit.c (cgraph_finalize_function): Call cgraph_get_create_node
> > 	instead of cgraph_node.
> > 	(cgraph_copy_node_for_versioning): Call cgraph_create_node instead of
> > 	cgraph_node.
> > 
> > 	* lto-symtab.c (lto_symtab_merge_cgraph_nodes_1): Call
> > 	cgraph_create_node instead of cgraph_node.
> > 
> > 	* c-decl.c (finish_function): Call cgraph_get_create_node instead
> > 	of cgraph_node.
> > 	* lto-cgraph.c (input_node): Likewise.
> > 	* lto-streamer-in.c (input_function): Likewise.
> > 	* varasm.c (mark_decl_referenced): Likewise.
> > 	(assemble_alias): Likewise.
> > 
> > gcc/c-family/
> > 	* c-gimplify.c (c_genericize): Call cgraph_get_create_node instead
> > 	of cgraph_node.
> > 
> > gcc/cp/
> > 	* cp/class.c (cp_fold_obj_type_ref): Call cgraph_get_create_node
> > 	instead of cgraph_node.
> > 	* cp/decl2.c (cxx_callgraph_analyze_expr): Likewise.
> > 	(cp_write_global_declarations): Likewise.
> > 	* cp/optimize.c (maybe_clone_body): Likewise.
> > 	* cp/semantics.c (maybe_add_lambda_conv_op): Likewise.
> > 	* cp/mangle.c (mangle_decl): Likewise.
> > 	* cp/method.c (make_alias_for_thunk): Likewise.
> > 	(use_thunk): Likewise.
> > 
> > gcc/ada/
> > 	* gcc-interface/utils.c (end_subprog_body): Call
> > 	cgraph_get_create_node instead of cgraph_node.
> > 
> > gcc/fortran/
> > 	* trans-decl.c (gfc_generate_function_code): Call
> > 	cgraph_get_create_node instead of cgraph_node.
> > 
> > gcc/objc/
> > 	* objc-act.c (mark_referenced_methods): Call cgraph_get_create_node
> > 	instead of cgraph_node.
> 
> OK.
> > Index: src/gcc/lto-cgraph.c
> > ===================================================================
> > --- src.orig/gcc/lto-cgraph.c
> > +++ src/gcc/lto-cgraph.c
> > @@ -1045,7 +1045,7 @@ input_node (struct lto_file_decl_data *f
> >  				0, CGRAPH_FREQ_BASE, 0, false, NULL);
> >      }
> >    else
> > -    node = cgraph_node (fn_decl);
> > +    node = cgraph_get_create_node (fn_decl);
> >  
> >    node->count = lto_input_sleb128 (ib);
> >    node->count_materialization_scale = lto_input_sleb128 (ib);
> > Index: src/gcc/lto-streamer-in.c
> > ===================================================================
> > --- src.orig/gcc/lto-streamer-in.c
> > +++ src/gcc/lto-streamer-in.c
> > @@ -1301,7 +1301,7 @@ input_function (tree fn_decl, struct dat
> >    DECL_INITIAL (fn_decl) = lto_input_tree (ib, data_in);
> >    gcc_assert (DECL_INITIAL (fn_decl));
> >    DECL_SAVED_TREE (fn_decl) = NULL_TREE;
> > -  node = cgraph_node (fn_decl);
> > +  node = cgraph_get_create_node (fn_decl);
> 
> I would expect those two to be cgraph_create_node and cgraph_get_node or we
> have latent bug somewhere. Did you have particular reason for the choice here?
> I guess we can handle this incrementally.

cgraph_create_node was aborting on the assert.  I don't remember on
which testcases(s) but they are in our testcase so we can have a look.

> > Index: src/gcc/cgraphbuild.c
> > ===================================================================
> > --- src.orig/gcc/cgraphbuild.c
> > +++ src/gcc/cgraphbuild.c
> > @@ -74,9 +74,9 @@ record_reference (tree *tp, int *walk_su
> >        if (TREE_CODE (decl) == FUNCTION_DECL)
> >  	{
> >  	  if (!ctx->only_vars)
> > -	  cgraph_mark_address_taken_node (cgraph_node (decl));
> > +	    cgraph_mark_address_taken_node (cgraph_get_create_node (decl));
> >  	  ipa_record_reference (NULL, ctx->varpool_node,
> > -			        cgraph_node (decl), NULL,
> > +			        cgraph_get_node (decl), NULL,
> 
> Please CSE the cgraph_get_create_node call here.  In both cases we want
> get_create_node, during the later cgraph builds the new nodes ight become needed
> as result of devirtualization and external construvctor folding.
> 

I see, I have changed the hunk into the following and will commit it
as such after re-testing:


Thanks,

Martin
diff mbox

Patch

Index: src/gcc/cgraphbuild.c
===================================================================
--- src.orig/gcc/cgraphbuild.c
+++ src/gcc/cgraphbuild.c
@@ -73,10 +73,10 @@  record_reference (tree *tp, int *walk_su
       decl = get_base_var (*tp);
       if (TREE_CODE (decl) == FUNCTION_DECL)
 	{
+	  struct cgraph_node *node = cgraph_get_create_node (decl);
 	  if (!ctx->only_vars)
-	  cgraph_mark_address_taken_node (cgraph_node (decl));
-	  ipa_record_reference (NULL, ctx->varpool_node,
-			        cgraph_node (decl), NULL,
+	    cgraph_mark_address_taken_node (node);
+	  ipa_record_reference (NULL, ctx->varpool_node, node, NULL,
 			        IPA_REF_ADDR, NULL);
 	}