From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125100 invoked by alias); 3 Sep 2018 13:15:45 -0000 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 Received: (qmail 125085 invoked by uid 89); 3 Sep 2018 13:15:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy=topo, formal, va_gc, mildly X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Sep 2018 13:15:42 +0000 X-Amavis-Alert: BAD HEADER SECTION, Duplicate header field: "Cc" Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 8D21DAF5B; Mon, 3 Sep 2018 13:15:40 +0000 (UTC) From: Martin Jambor To: Richard Biener Cc: Michael Ploujnikov , GCC Patches Cc: Subject: Re: [PING v2][PATCH] Make function clone name numbering independent. In-Reply-To: References: <6A1E6E88-63F7-43E4-8A53-78C7694D34BE@gmail.com> <069cb2a6-2e37-58fb-2009-35e0300270f1@oracle.com> <618e046e-3f28-f008-237d-497bcff2531e@oracle.com> <121c01f4-c1a2-249e-2cac-c6d5ec250fcb@oracle.com> <702ab3f1-fe25-9013-7a1e-f5e1615c9396@oracle.com> <85bd8119-0bce-a06d-df3f-1a1a6ed88187@oracle.com> <3167e521-aa5b-e47c-6d9b-956a1af2a886@oracle.com> <078bf036-f02b-3bdb-545d-3f5ad67bc786@oracle.com> User-Agent: Notmuch/0.26 (https://notmuchmail.org) Emacs/26.1 (x86_64-suse-linux-gnu) Date: Mon, 03 Sep 2018 13:15:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00111.txt.bz2 Hi, On Mon, Sep 03 2018, Richard Biener wrote: > On Mon, Sep 3, 2018 at 12:02 PM Martin Jambor wrote: >> >> Hi, >> >> On Fri, Aug 31 2018, Michael Ploujnikov wrote: >> > I've done some more digging into the current uses of >> > numbered_clone_function_name and checked if any tests fail if I change >> > it to suffixed_function_name: >> > >> > - gcc/cgraphclones.c: DECL_NAME (new_decl) = numbered_clone_function_name (thunk->decl, "artificial_thunk"); >> > - no new tests fail, inconclusive >> > - and despite the comment on redirect_callee_duplicating_thunks >> > about "one or more" duplicates it doesn't seem like >> > duplicate_thunk_for_node would be called more than once for each >> > node, assuming each node is named uniquely, but I'm far from an >> > expert in this area >> >> The comment means that if there is a chain of thunks, the method clones >> all of them. Nevertheless, you need name numbering here for the same >> reason why you need them for constprop. > > The remaining question I had with the patch was if maybe all callers > can handle assigning > the numbering themselves, thus do sth like > > for-each-clone-of (i, fn) > DECL_NAME (...) = numbered_clone_function_name (..., > "artificial_thunk", i); > > which would make the map of name -> number unnecessary. > That is what I would prefer too but it also has downsides. Callers would have to do their book keeping and the various cloning interfaces would get another parameter when already they have quite a few (introducing some cloning context classes seems like an overkill to me :-). If we want to get rid of .constprop discrepancies, something like the following (only very mildly tested) patch should be enough (note that IPA-CP currently does not really need the new has because it creates clones in only one pass through the call graph, but that is something likely to change). We could then adjust other cloners that we care about. However, if clones of thunks are one of them, then the optional parameter will also proliferate to cgraph_node::create_clone(), cgraph_edge::redirect_callee_duplicating_thunks() and duplicate_thunk_for_node(). create_version_clone_with_body would almost certainly need it because of IPA-SRA too (IPA-SRA can of course always pass zero). Martin diff --git a/gcc/cgraph.h b/gcc/cgraph.h index 2b00f0165fa..703c3cfea7b 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -964,11 +964,15 @@ public: cgraph_node *new_inlined_to, bitmap args_to_skip, const char *suffix = NULL); - /* Create callgraph node clone with new declaration. The actual body will - be copied later at compilation stage. */ + /* Create callgraph node clone with new declaration. The actual body will be + copied later at compilation stage. The name of the new clone will be + constructed from the name of the original name, SUFFIX and a number which + can either be NUM_SUFFIX if non-negative or a unique incremented integer + otherwise. */ cgraph_node *create_virtual_clone (vec redirect_callers, vec *tree_map, - bitmap args_to_skip, const char * suffix); + bitmap args_to_skip, const char * suffix, + int num_suffix = -1); /* cgraph node being removed from symbol table; see if its entry can be replaced by other inline clone. */ @@ -2376,8 +2380,9 @@ basic_block init_lowered_empty_function (tree, bool, profile_count); tree thunk_adjust (gimple_stmt_iterator *, tree, bool, HOST_WIDE_INT, tree); /* In cgraphclones.c */ -tree clone_function_name_1 (const char *, const char *); -tree clone_function_name (tree decl, const char *); +tree clone_function_name_1 (const char *name, const char *suffix, + int num_suffix = -1); +tree clone_function_name (tree decl, const char *suffix, int num_suffix = -1); void tree_function_versioning (tree, tree, vec *, bool, bitmap, bool, bitmap, basic_block); diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index 0c0a94b04a3..865c3fa1ad0 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -513,11 +513,13 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count, static GTY(()) unsigned int clone_fn_id_num; -/* Return a new assembler name for a clone with SUFFIX of a decl named - NAME. */ +/* Return a new assembler name for a clone with SUFFIX of a decl named NAME + Apart from, string SUFFIX, the new name will end with either NUM_SIFFIX, if + non-negative, or a unique unspecified number otherwise. . */ tree -clone_function_name_1 (const char *name, const char *suffix) +clone_function_name_1 (const char *name, const char *suffix, + int num_suffix) { size_t len = strlen (name); char *tmp_name, *prefix; @@ -526,22 +528,32 @@ clone_function_name_1 (const char *name, const char *suffix) memcpy (prefix, name, len); strcpy (prefix + len + 1, suffix); prefix[len] = symbol_table::symbol_suffix_separator (); - ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, clone_fn_id_num++); + unsigned int num; + if (num_suffix < 0) + num = clone_fn_id_num++; + else + num = (unsigned) num_suffix; + ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, num); return get_identifier (tmp_name); } -/* Return a new assembler name for a clone of DECL with SUFFIX. */ +/* Return a new assembler name for a clone of DECL with SUFFIX. Apart from, + string SUFFIX, the new name will end with either NUM_SIFFIX, if + non-negative, or a unique unspecified number otherwise. */ tree -clone_function_name (tree decl, const char *suffix) +clone_function_name (tree decl, const char *suffix, int num_suffix) { tree name = DECL_ASSEMBLER_NAME (decl); - return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix); + return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix, num_suffix); } -/* Create callgraph node clone with new declaration. The actual body will - be copied later at compilation stage. +/* Create callgraph node clone with new declaration. The actual body will be + copied later at compilation stage. The name of the new clone will be + constructed from the name of the original name, SUFFIX and a number which + can either be NUM_SUFFIX if non-negative or a unique incremented integer + otherwise. TODO: after merging in ipa-sra use function call notes instead of args_to_skip bitmap interface. @@ -549,7 +561,8 @@ clone_function_name (tree decl, const char *suffix) cgraph_node * cgraph_node::create_virtual_clone (vec redirect_callers, vec *tree_map, - bitmap args_to_skip, const char * suffix) + bitmap args_to_skip, const char * suffix, + int num_suffix) { tree old_decl = decl; cgraph_node *new_node = NULL; @@ -584,7 +597,8 @@ cgraph_node::create_virtual_clone (vec redirect_callers, strcpy (name + len + 1, suffix); name[len] = '.'; DECL_NAME (new_decl) = get_identifier (name); - SET_DECL_ASSEMBLER_NAME (new_decl, clone_function_name (old_decl, suffix)); + SET_DECL_ASSEMBLER_NAME (new_decl, clone_function_name (old_decl, suffix, + num_suffix)); SET_DECL_RTL (new_decl, NULL); new_node = create_clone (new_decl, count, false, diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index afc45969558..52690996419 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -376,6 +376,9 @@ static profile_count max_count; static long overall_size, max_new_size; +/* Hash to give different UID suffixes */ +static hash_map *clone_num_suffixes; + /* Return the param lattices structure corresponding to the Ith formal parameter of the function described by INFO. */ static inline struct ipcp_param_lattices * @@ -3850,8 +3853,11 @@ create_specialized_node (struct cgraph_node *node, } } + int &suffix_counter = clone_num_suffixes->get_or_insert(node); new_node = node->create_virtual_clone (callers, replace_trees, - args_to_skip, "constprop"); + args_to_skip, "constprop", + suffix_counter); + suffix_counter++; bool have_self_recursive_calls = !self_recursive_calls.is_empty (); for (unsigned j = 0; j < self_recursive_calls.length (); j++) @@ -5065,6 +5071,7 @@ ipcp_driver (void) ipa_check_create_node_params (); ipa_check_create_edge_args (); + clone_num_suffixes = new hash_map; if (dump_file) { @@ -5086,6 +5093,7 @@ ipcp_driver (void) ipcp_store_vr_results (); /* Free all IPCP structures. */ + delete clone_num_suffixes; free_toporder_info (&topo); delete edge_clone_summaries; edge_clone_summaries = NULL;