From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16415 invoked by alias); 13 Feb 2010 18:03:49 -0000 Received: (qmail 16403 invoked by uid 22791); 13 Feb 2010 18:03:48 -0000 X-SWARE-Spam-Status: No, hits=-4.3 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 13 Feb 2010 18:03:43 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) by mx2.suse.de (Postfix) with ESMTP id 4163A88B6C for ; Sat, 13 Feb 2010 19:03:41 +0100 (CET) Resent-From: Martin Jambor Resent-Date: Sat, 13 Feb 2010 19:04:35 +0100 Resent-Message-ID: <20100213180435.GB6441@alvy.suse.cz> Resent-To: GCC Patches Message-Id: <20100213180156.993252867@alvy.suse.cz> User-Agent: quilt/0.48-1 Date: Sat, 13 Feb 2010 18:03:00 -0000 From: Martin Jambor To: GCC Patches Cc: Jan Hubicka Subject: [PATCH 1/6] Clarify edge redirection for inline clones References: <20100213180136.555197900@alvy.suse.cz> Content-Disposition: inline; filename=improve_cgraph_edge_redirection.diff X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-02/txt/msg00513.txt.bz2 Hi, this patch a new one in the series and it changes how we set declarations in call statements to reflect the callee of the corresponding call graph edge. Currently we do this for all nodes in cgraph_materialize_all_clones and since at this point inline clones still share their bodies, we do it only for one of them, the one which passes through condition (!node->clone_of || node->clone_of->decl != node->decl) For the same reason, it is also never done for call statements for which gimple_call_fndecl returns NULL - so that we don't change statements of all inline clones because of an indirect-inlining created edge out of one of them. However, in order to use this mechanism for IPA-CP devirtualization we need to be able to redirect even these statements. Therefore this patch keeps the current redirection only for nodes which are not inlined and deals with these in copy_bb in tree-inline.c. I have put this change to the beginning of the series because the current mechanism is quite fragile and I suspect that PR 41290 is caused by an original node (with inline clones) somehow ceasing to exist which would lead exactly to these kinds of cgraph verification issues. I have asked the reporter to verify this hypothesis and if it fixes the issue, I will request an approval to commit to trunk now. Otherwise I will wait with this until stage1. I have bootstrapped and regression tested this patch separately on x86_64-linux. Thanks, Martin 2010-02-12 Martin Jambor * cgraph.h (cgraph_redirect_edge_call_stmt_to_callee): Declare. * cgraphunit.c (cgraph_materialize_all_clones): Moved call redirection... (cgraph_redirect_edge_call_stmt_to_callee): ...to this new function. (verify_cgraph_node): Do not check for edges pointing to wrong nodes in inline clones. * tree-inline.c (copy_bb): Call cgraph_redirect_edge_call_stmt_to_callee. Index: icln/gcc/cgraph.h =================================================================== --- icln.orig/gcc/cgraph.h +++ icln/gcc/cgraph.h @@ -530,7 +530,7 @@ void cgraph_remove_edge_duplication_hook struct cgraph_2node_hook_list *cgraph_add_node_duplication_hook (cgraph_2node_hook, void *); void cgraph_remove_node_duplication_hook (struct cgraph_2node_hook_list *); void cgraph_materialize_all_clones (void); - +gimple cgraph_redirect_edge_call_stmt_to_callee (struct cgraph_edge *); /* In cgraphbuild.c */ unsigned int rebuild_cgraph_edges (void); void reset_inline_failed (struct cgraph_node *); Index: icln/gcc/cgraphunit.c =================================================================== --- icln.orig/gcc/cgraphunit.c +++ icln/gcc/cgraphunit.c @@ -751,8 +751,9 @@ verify_cgraph_node (struct cgraph_node * debug_tree (e->callee->decl); error_found = true; } - else if (!clone_of_p (cgraph_node (decl), e->callee) - && !e->callee->global.inlined_to) + else if (!node->global.inlined_to + && !e->callee->global.inlined_to + && !clone_of_p (cgraph_node (decl), e->callee)) { error ("edge points to wrong declaration:"); debug_tree (e->callee->decl); @@ -2222,11 +2223,60 @@ cgraph_materialize_clone (struct cgraph_ bitmap_obstack_release (NULL); } +/* If necessary, change the function declaration in the call statement + associated with E so that it corresponds to the edge callee. */ + +gimple +cgraph_redirect_edge_call_stmt_to_callee (struct cgraph_edge *e) +{ + tree decl = gimple_call_fndecl (e->call_stmt); + gimple new_stmt; + gimple_stmt_iterator gsi; + + if (!decl || decl == e->callee->decl + /* Don't update call from same body alias to the real function. */ + || cgraph_get_node (decl) == cgraph_get_node (e->callee->decl)) + return e->call_stmt; + + if (cgraph_dump_file) + { + fprintf (cgraph_dump_file, "updating call of %s in %s:", + cgraph_node_name (e->caller), + cgraph_node_name (e->callee)); + print_gimple_stmt (cgraph_dump_file, e->call_stmt, 0, dump_flags); + } + + if (e->callee->clone.combined_args_to_skip) + new_stmt = gimple_call_copy_skip_args (e->call_stmt, + e->callee->clone.combined_args_to_skip); + else + new_stmt = e->call_stmt; + if (gimple_vdef (new_stmt) + && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME) + SSA_NAME_DEF_STMT (gimple_vdef (new_stmt)) = new_stmt; + gimple_call_set_fndecl (new_stmt, e->callee->decl); + + gsi = gsi_for_stmt (e->call_stmt); + gsi_replace (&gsi, new_stmt, true); + + /* Update EH information too, just in case. */ + maybe_clean_or_replace_eh_stmt (e->call_stmt, new_stmt); + + cgraph_set_call_stmt_including_clones (e->caller, e->call_stmt, new_stmt); + + if (cgraph_dump_file) + { + fprintf (cgraph_dump_file, " updated to:"); + print_gimple_stmt (cgraph_dump_file, e->call_stmt, 0, dump_flags); + } + return new_stmt; +} + /* Once all functions from compilation unit are in memory, produce all clones - and update all calls. - We might also do this on demand if we don't want to bring all functions to - memory prior compilation, but current WHOPR implementation does that and it is - is bit easier to keep everything right in this order. */ + and update all calls. We might also do this on demand if we don't want to + bring all functions to memory prior compilation, but current WHOPR + implementation does that and it is is bit easier to keep everything right in + this order. */ void cgraph_materialize_all_clones (void) { @@ -2302,63 +2352,15 @@ cgraph_materialize_all_clones (void) if (cgraph_dump_file) fprintf (cgraph_dump_file, "Updating call sites\n"); for (node = cgraph_nodes; node; node = node->next) - if (node->analyzed && gimple_has_body_p (node->decl) - && (!node->clone_of || node->clone_of->decl != node->decl)) + if (node->analyzed && !node->global.inlined_to + && gimple_has_body_p (node->decl)) { struct cgraph_edge *e; current_function_decl = node->decl; push_cfun (DECL_STRUCT_FUNCTION (node->decl)); for (e = node->callees; e; e = e->next_callee) - { - tree decl = gimple_call_fndecl (e->call_stmt); - /* When function gets inlined, indirect inlining might've invented - new edge for orginally indirect stmt. Since we are not - preserving clones in the original form, we must not update here - since other inline clones don't need to contain call to the same - call. Inliner will do the substitution for us later. */ - if (decl && decl != e->callee->decl) - { - gimple new_stmt; - gimple_stmt_iterator gsi; - - if (cgraph_get_node (decl) == cgraph_get_node (e->callee->decl)) - /* Don't update call from same body alias to the real function. */ - continue; - - if (cgraph_dump_file) - { - fprintf (cgraph_dump_file, "updating call of %s in %s:", - cgraph_node_name (node), - cgraph_node_name (e->callee)); - print_gimple_stmt (cgraph_dump_file, e->call_stmt, 0, dump_flags); - } - - if (e->callee->clone.combined_args_to_skip) - new_stmt = gimple_call_copy_skip_args (e->call_stmt, - e->callee->clone.combined_args_to_skip); - else - new_stmt = e->call_stmt; - if (gimple_vdef (new_stmt) - && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME) - SSA_NAME_DEF_STMT (gimple_vdef (new_stmt)) = new_stmt; - gimple_call_set_fndecl (new_stmt, e->callee->decl); - - gsi = gsi_for_stmt (e->call_stmt); - gsi_replace (&gsi, new_stmt, true); - - /* Update EH information too, just in case. */ - maybe_clean_or_replace_eh_stmt (e->call_stmt, new_stmt); - - cgraph_set_call_stmt_including_clones (node, e->call_stmt, new_stmt); - - if (cgraph_dump_file) - { - fprintf (cgraph_dump_file, " updated to:"); - print_gimple_stmt (cgraph_dump_file, e->call_stmt, 0, dump_flags); - } - } - } + cgraph_redirect_edge_call_stmt_to_callee (e); pop_cfun (); current_function_decl = NULL; #ifdef ENABLE_CHECKING Index: icln/gcc/tree-inline.c =================================================================== --- icln.orig/gcc/tree-inline.c +++ icln/gcc/tree-inline.c @@ -1651,6 +1651,7 @@ copy_bb (copy_body_data *id, basic_block bb->frequency, copy_basic_block->frequency); } + stmt = cgraph_redirect_edge_call_stmt_to_callee (edge); } break;