Re: [PATCH 1/4] Remove cgraph_node function and fixup all callers

[Jan Hubicka <hubicka at ucw dot cz>]

> 2011-03-18  Martin Jambor  <mjambor@suse.cz>
> 
> 	* cgraph.h (cgraph_node): Remove function declaration.
> 	(cgraph_create_node): Declare.
> 	(cgraph_get_create_node): Likewise.
> 	(cgraph_do_get_node): New function.
> 
> 	* 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_do_get_node instead of cgraph_node.
> 	(cgraph_update_edges_for_call_stmt): Likewise.
> 	(cgraph_clone_edge): Likewise.
> 	(cgraph_create_virtual_clone): Likewise.
> 	(cgraph_set_call_stmt): Call cgraph_get_create_node instead of
> 	cgraph_node.
> 	(cgraph_local_info): Call cgraph_get_node instead of cgraph_node,
> 	handle NULL return value.
> 	(cgraph_global_info): Likewise.
> 	(cgraph_rtl_info): Likewise.
> 	(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_node and cgraph_get_create_node
> 	instead of cgraph_node.
> 	(rebuild_cgraph_edges): Likewise.
> 	(cgraph_rebuild_references): Call cgraph_get_node instead of
> 	cgraph_node.
> 	(remove_cgraph_callee_edges): Likewise.
> 
> 	* cgraphunit.c (cgraph_finalize_function): Call cgraph_get_create_node
> 	instead of cgraph_node.
> 	(cgraph_mark_if_needed): Call cgraph_get_node instead of cgraph_node.
> 	(verify_cgraph_node): Likewise.
> 	(cgraph_analyze_functions): Likewise.
> 	(cgraph_preserve_function_body_p): Likewise.
> 	(save_inline_function_body): Likewise.
> 	(save_inline_function_body): Likewise.
> 	(cgraph_copy_node_for_versioning): Call cgraph_create_node instead of
> 	cgraph_node.
> 
> 	* tree-inline.c (copy_bb): Call cgraph_get_node instead of cgraph_node.
> 	(estimate_num_insns): Likewise and handle returned NULL.
> 	(optimize_inline_calls): Call cgraph_get_node instead of cgraph_node.
> 	(tree_function_versioning): Call cgraph_do_get_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.
> 	* c-family/c-gimplify.c (c_genericize): Likewise.
> 	* lto-cgraph.c (input_node): Likewise.
> 	* lto-streamer-in.c (input_function): Likewise.
> 
> 	* except.c (set_nothrow_function_flags): Call cgraph_get_node instead
> 	of cgraph_node.
> 	* final.c (rest_of_clean_state): Likewise.
> 	* gimple-iterator.c (update_call_edge_frequencies): Likewise.
> 	* passes.c (pass_init_dump_file): Likewise.
> 	(execute_all_ipa_transforms): Likewise.
> 	(function_called_by_processed_nodes_p): Likewise.
> 	* predict.c (maybe_hot_frequency_p): Likewise.
> 	(probably_never_executed_bb_p): Likewise.
> 	(compute_function_frequency): Likewise.
> 	* tree-nested.c (check_for_nested_with_variably_modified): Likewise.
> 	(unnest_nesting_tree_1): Likewise.
> 	(lower_nested_functions): Likewise.
> 	* tree-optimize.c (execute_fixup_cfg): Likewise.
> 	(tree_rest_of_compilation): Likewise.
> 	* tree-optimize.c (execute_fixup_cfg): Likewise.
> 	(tree_rest_of_compilation): Likewise.
> 	* tree-profile.c (gimple_gen_ic_func_profiler): Likewise.
> 	* tree-sra.c (ipa_early_sra): Likewise.
> 	* tree-ssa-loop-ivopts.c (computation_cost): Likewise.
> 	* config/i386/i386.c (ix86_compute_frame_layout): Likewise.
> 	* ipa.c (record_cdtor_fn): Likewise.
> 	* ipa-inline.c (cgraph_early_inlining): Likewise.
> 	(compute_inline_parameters_for_current): Likewise.
> 	* ipa-prop.c (ipa_make_edge_direct_to_target): Likewise.
> 	* ipa-pure-const.c (local_pure_const): Likewise.
> 	* ipa-split.c (split_function): Likewise.
> 	(split_function): Likewise.
> 	(execute_split_functions): Likewise.
> 
> 	* lto-streamer-in.c (lto_read_body): Call cgraph_do_get_node instead
> 	of cgraph_node.
> 	* omp-low.c (new_omp_context): Likewise.
> 	(create_task_copyfn): Likewise.
> 	* tree-emutls.c (lower_emutls_function_body): Likewise.
> 	* ipa-struct-reorg.c (update_cgraph_with_malloc_call): Likewise.
> 	* ipa-type-escape.c (check_call): Likewise.
> 	* matrix-reorg.c (transform_allocation_sites): Likewise.
> 
> 	* gimplify.c (unshare_body): Call cgraph_get_node instead of
> 	cgraph_node, handle NULL return value.
> 	(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): Call cgraph_get_node instead of
> 	cgraph_node and handle returned NULL.
> 	(assemble_start_function): Likewise.
> 	(mark_decl_referenced): Call cgraph_get_create_node instead of
> 	cgraph_node.
> 	(assemble_alias): Likewise.
> 
> gcc/c-family/
> 	* c-gimplify.c (c_genericize): Call cgraph_get_create_node instead
> 
> 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.
> 	(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/java/
> 	* decl.c (java_mark_decl_local): Call cgraph_get_node instead of
> 	cgraph_node and handle returned NULL.
> 
> gcc/objc/
> 	* objc-act.c (mark_referenced_methods): Call cgraph_get_create_node
> 	instead of cgraph_node.

Seems OK, however..
> Index: src/gcc/gimplify.c
> ===================================================================
> --- src.orig/gcc/gimplify.c	2011-03-19 01:16:24.000000000 +0100
> +++ src/gcc/gimplify.c	2011-03-19 01:54:42.000000000 +0100
> @@ -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))

The non-NULL check is here because of gimplification happening before cgraph construction?
> Index: src/gcc/passes.c
> ===================================================================
> --- src.orig/gcc/passes.c	2011-03-19 01:16:23.000000000 +0100
> +++ src/gcc/passes.c	2011-03-19 01:54:42.000000000 +0100
> @@ -1344,7 +1344,7 @@ pass_init_dump_file (struct opt_pass *pa
>        if (dump_file && current_function_decl)
>  	{
>  	  const char *dname, *aname;
> -	  struct cgraph_node *node = cgraph_node (current_function_decl);
> +	  struct cgraph_node *node = cgraph_get_node (current_function_decl);

Don't you want cgraph_do_get_node on those places that will ICE anyway?
> Index: src/gcc/tree-ssa-alias.c
> ===================================================================
> --- src.orig/gcc/tree-ssa-alias.c	2011-03-19 01:16:23.000000000 +0100
> +++ src/gcc/tree-ssa-alias.c	2011-03-19 01:54:42.000000000 +0100
> @@ -1246,14 +1246,15 @@ 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

Why non-NULL tests are needed here?  It seems that at the time cgraph is created we should have
nodes for all accessible functions.

What I wonder about is that we have similar API for annotations.  Here we have
var_ann_t for type name
var_ann (var) for what cgraph_get_node is
get_var_ann (var) for what cgraph_get_create_node is
create_var_ann (var) for hwat cgraph_create_node is.

So we may want to be consistent here. I never had problem with overlaping
struct and function obvoiusly, but if people disagree, what about:

cgraph_create_node = crate node and aborts if already exist
cgraph_get_node = return node or create noe
cgraph_maybe_get_node = return node or NULL
cgraph_do_get_node = return node and abort if NULL.

We may later even want to introduce our popular cgraph_node_d/cgraph_node_t and rename back cgraph_do_get_node to cgraph_node,
since it is is the most common accestor.
I would preffer to do that incrementally so we can merge ipa-inline changes.

Honza