This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 4/7] Removal of cgraph_node function
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:
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);
}
Thanks,
Martin