From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 89D00385AC33 for ; Thu, 22 Jul 2021 12:43:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 89D00385AC33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 33F7322646; Thu, 22 Jul 2021 12:43:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1626957808; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Nort4GGqeQ3kmj//qtAZcUMCUJ0UDpVtAwtKjMGhwUQ=; b=QQG0Cagi2vUo5uvspoi68qvZB9UK4jvnSopDiOqCbdRVswPe2UeEJc7rJwC0yns77/VR7n 6Qgt0s/D7DxaqWJ642mwwXVJqEuxxS9MW6KS4VLjG5HHDQ0IcoMiXJfm5Q30vgSBS0HARu Jb7LYteISTCdSdzGOkm7KAKTWB0Q0gE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1626957808; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Nort4GGqeQ3kmj//qtAZcUMCUJ0UDpVtAwtKjMGhwUQ=; b=eE/niWIPsP0zJag6BRF3QcRHwpQXxXBVv2rROQLuhQIPGN1ym/bhdf0Jqxj9ftLvlNNLnJ hZEseK9SRQydLlCA== Received: from suse.cz (virgil.suse.cz [10.100.13.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 1F80EA3B9C; Thu, 22 Jul 2021 12:43:28 +0000 (UTC) From: Martin Jambor To: Richard Biener , JiangNing OS Cc: GCC Patches , Jan Hubicka Subject: Re: [RFC] ipa: Adjust references to identify read-only globals In-Reply-To: References: User-Agent: Notmuch/0.32.2 (https://notmuchmail.org) Emacs/27.2 (x86_64-suse-linux-gnu) Date: Thu, 22 Jul 2021 14:43:28 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Jul 2021 12:43:33 -0000 Hi, On Tue, Jul 20 2021, Martin Jambor wrote: > On Tue, Jul 20 2021, Richard Biener wrote: >> On Tue, Jul 20, 2021 at 10:54 AM JiangNing OS via Gcc-patches >> wrote: [...] >>> > this patch has been motivated by SPEC 2017's 544.nab_r in which there is a >>> > static variable which is never written to and so zero throughout the run-time >>> > of the benchmark. However, it is passed by reference to a function in which >>> > it is read and (after some multiplications) passed into __builtin_exp which in >>> > turn unnecessarily consumes almost 10% of the total benchmark run-time. >>> >>> I do see ~8.5% runtime reduction on aarch64. >>> >>> > The situation is illustrated by the added testcase remref-3.c. >>> > >>> > The patch adds a flag to ipa-prop descriptor of each parameter to mark such >>> > parameters. IPA-CP and inling then take the effort to remove IPA_REF_ADDR >>> > references in the caller and only add IPA_REF_LOAD reference to the >>> > clone/overall inlined function. This is sufficient for subsequent symbol table >>> > analysis code to identify the read-only variable as such and optimize the code. >>> > >>> > I plan to compile a number of packages with the patch to test it some more >>> > and get a bit better idea of its impact. But it has passed bootstrap, >>> > LTObootstrap and testing on x86_64-linux and i686-linux and so unless I find >>> > any problem, I would like to commit it at some point next month without any >>> > major changes, so I'd be grateful for any feedback even now. >>> >>> I see 3 cases in SPEC2017 failed to compile on aarch64, i.e. 521.wrf_r, 527.cam4_r, 554.roms_r. For example, >>> [...] > > At least after I fixed a bug pointed out in Honza's review, I cannot > replicate any ICE building any of 521.wrf_r, 527.cam4_r, 554.roms_r on > x86_64, at least without LTO. But with LTO, I get an undefined symbol > link error building 527.cam4_r which is of course certainly a bug in the > patch. I will investigate and hopefully fix it and re-post the patch > but then I would appreciate if you checked it on aarch64 for me. > I have fixed that bug and another one in an assert condition and the result is below. I have re-tested the updated patch including bootstrap and testing, LTO-bootstrap and LTO-profiledbootstrap on x86_64 and bootstrap and testing on aarch64. I have also used it to build SPEC2017 on x86_64 with and without LTO and on aarch64 without LTO (LTO build is underway, but my arch64 machine is an old AMD Seattle one) and I have also built a few random packages with it. Since it has been pre-approved by Honza, I would like to commit it to master soon. Nevertheless, Jiangning, I am OK to wait a day or so if you can give it another test on your setup. Thanks, Martin ipa: Adjust references to identify read-only globals this patch has been motivated by SPEC 2017's 544.nab_r in which there is a static variable which is never written to and so zero throughout the run-time of the benchmark. However, it is passed by reference to a function in which it is read and (after some multiplications) passed into __builtin_exp which in turn unnecessarily consumes almost 10% of the total benchmark run-time. The situation is illustrated by the added testcase remref-3.c. The patch adds a flag to ipa-prop descriptor of each parameter to mark such parameters. IPA-CP and inling then take the effort to remove IPA_REF_ADDR references in the caller and only add IPA_REF_LOAD reference to the clone/overall inlined function. This is sufficient for subsequent symbol table analysis code to identify the read-only variable as such and optimize the code. There are two changes from the RFC version posted to the list earlier. First, three missing calls to get_base_address were added (there was another one in an assert). Second, references are not stripped off the callers if the cloned function cannot change the signature. The second change reveals a real shortcoming stemming from the fact we cannot adjust function prototypes with fnspecs. But that is a more general problem. gcc/ChangeLog: 2021-07-20 Martin Jambor * cgraph.h (ipa_replace_map): New field force_load_ref. * ipa-prop.h (ipa_param_descriptor): Reduce precision of move_cost, aded new flag load_dereferenced, adjusted comments. (ipa_get_param_dereferenced): New function. (ipa_set_param_dereferenced): Likewise. * cgraphclones.c (cgraph_node::create_virtual_clone): Follow it. * ipa-cp.c: Include gimple.h. (ipcp_discover_new_direct_edges): Take into account dereferenced flag. (get_replacement_map): New parameter force_load_ref, set the appropriate flag in ipa_replace_map if set. (struct symbol_and_index_together): New type. (adjust_refs_in_act_callers): New function. (adjust_references_in_caller): Likewise. (create_specialized_node): When appropriate, call adjust_references_in_caller and force only load references. * ipa-prop.c (load_from_dereferenced_name): New function. (ipa_analyze_controlled_uses): Also detect loads from a dereference, harden testing of call statements. (ipa_write_node_info): Stream the dereferenced flag. (ipa_read_node_info): Likewise. (ipa_set_jf_constant): Also create refdesc when jump function references a variable. (cgraph_node_for_jfunc): Rename to symtab_node_for_jfunc, work also on references of variables and return a symtab_node. Adjust all callers. (propagate_controlled_uses): Also remove references to VAR_DECLs. gcc/testsuite/ChangeLog: 2021-06-29 Martin Jambor * gcc.dg/ipa/remref-3.c: New test. * gcc.dg/ipa/remref-4.c: Likewise. * gcc.dg/ipa/remref-5.c: Likewise. * gcc.dg/ipa/remref-6.c: Likewise. --- gcc/cgraph.h | 4 + gcc/cgraphclones.c | 10 +- gcc/ipa-cp.c | 156 ++++++++++++++++++++++++-- gcc/ipa-prop.c | 166 ++++++++++++++++++++++------ gcc/ipa-prop.h | 29 ++++- gcc/testsuite/gcc.dg/ipa/remref-3.c | 23 ++++ gcc/testsuite/gcc.dg/ipa/remref-4.c | 31 ++++++ gcc/testsuite/gcc.dg/ipa/remref-5.c | 38 +++++++ gcc/testsuite/gcc.dg/ipa/remref-6.c | 24 ++++ 9 files changed, 431 insertions(+), 50 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-3.c create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-4.c create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-5.c create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-6.c diff --git a/gcc/cgraph.h b/gcc/cgraph.h index 9f4338fdf87..eb32962e240 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -700,6 +700,10 @@ struct GTY(()) ipa_replace_map tree new_tree; /* Parameter number to replace, when old_tree is NULL. */ int parm_num; + /* Set if the newly added reference should not be an address one, but a load + one from the operand of the ADDR_EXPR in NEW_TREE. This is for cases when + the corresponding parameter p is used only as *p. */ + unsigned force_load_ref : 1; }; enum cgraph_simd_clone_arg_type diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index 7e463acab91..086e13a8371 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -633,7 +633,15 @@ cgraph_node::create_virtual_clone (vec redirect_callers, || in_lto_p) new_node->unique_name = true; FOR_EACH_VEC_SAFE_ELT (tree_map, i, map) - new_node->maybe_create_reference (map->new_tree, NULL); + { + tree repl = map->new_tree; + if (map->force_load_ref) + { + gcc_assert (TREE_CODE (repl) == ADDR_EXPR); + repl = get_base_address (TREE_OPERAND (repl, 0)); + } + new_node->maybe_create_reference (repl, NULL); + } if (ipa_transforms_to_apply.exists ()) new_node->ipa_transforms_to_apply diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 57c18af2bab..c81cbd0e2b6 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -106,6 +106,7 @@ along with GCC; see the file COPYING3. If not see #include "backend.h" #include "tree.h" #include "gimple-expr.h" +#include "gimple.h" #include "predict.h" #include "alloc-pool.h" #include "tree-pass.h" @@ -4008,7 +4009,8 @@ ipcp_discover_new_direct_edges (struct cgraph_node *node, { ipa_node_params *info = ipa_node_params_sum->get (node); int c = ipa_get_controlled_uses (info, param_index); - if (c != IPA_UNDESCRIBED_USE) + if (c != IPA_UNDESCRIBED_USE + && !ipa_get_param_load_dereferenced (info, param_index)) { struct ipa_ref *to_del; @@ -4317,10 +4319,14 @@ gather_edges_for_value (ipcp_value *val, cgraph_node *dest, } /* Construct a replacement map for a know VALUE for a formal parameter PARAM. - Return it or NULL if for some reason it cannot be created. */ + Return it or NULL if for some reason it cannot be created. FORCE_LOAD_REF + should be set to true when the reference created for the constant should be + a load one and not an address one because the corresponding parameter p is + only used as *p. */ static struct ipa_replace_map * -get_replacement_map (class ipa_node_params *info, tree value, int parm_num) +get_replacement_map (class ipa_node_params *info, tree value, int parm_num, + bool force_load_ref) { struct ipa_replace_map *replace_map; @@ -4332,10 +4338,15 @@ get_replacement_map (class ipa_node_params *info, tree value, int parm_num) fprintf (dump_file, " with const "); print_generic_expr (dump_file, value); - fprintf (dump_file, "\n"); + + if (force_load_ref) + fprintf (dump_file, " - forcing load reference\n"); + else + fprintf (dump_file, "\n"); } replace_map->parm_num = parm_num; replace_map->new_tree = value; + replace_map->force_load_ref = force_load_ref; return replace_map; } @@ -4488,6 +4499,113 @@ update_specialized_profile (struct cgraph_node *new_node, dump_profile_updates (orig_node, new_node); } +static void adjust_references_in_caller (cgraph_edge *cs, + symtab_node *symbol, int index); + +/* Simple structure to pass a symbol and index (with same meaning as parameters + of adjust_references_in_caller) through a void* parameter of a + call_for_symbol_thunks_and_aliases callback. */ +struct symbol_and_index_together +{ + symtab_node *symbol; + int index; +}; + +/* Worker callback of call_for_symbol_thunks_and_aliases to recursively call + adjust_references_in_caller on edges up in the call-graph, if necessary. */ +static bool +adjust_refs_in_act_callers (struct cgraph_node *node, void *data) +{ + symbol_and_index_together *pack = (symbol_and_index_together *) data; + for (cgraph_edge *cs = node->callers; cs; cs = cs->next_caller) + if (!cs->caller->thunk) + adjust_references_in_caller (cs, pack->symbol, pack->index); + return false; +} + +/* At INDEX of a function being called by CS there is an ADDR_EXPR of a + variable which is only dereferenced and which is represented by SYMBOL. See + if we can remove ADDR reference in callers assosiated witht the call. */ + +static void +adjust_references_in_caller (cgraph_edge *cs, symtab_node *symbol, int index) +{ + ipa_edge_args *args = ipa_edge_args_sum->get (cs); + ipa_jump_func *jfunc = ipa_get_ith_jump_func (args, index); + if (jfunc->type == IPA_JF_CONST) + { + ipa_ref *to_del = cs->caller->find_reference (symbol, cs->call_stmt, + cs->lto_stmt_uid); + if (!to_del) + return; + to_del->remove_reference (); + if (dump_file) + fprintf (dump_file, " Removed a reference from %s to %s.\n", + cs->caller->dump_name (), symbol->dump_name ()); + return; + } + + if (jfunc->type != IPA_JF_PASS_THROUGH + || ipa_get_jf_pass_through_operation (jfunc) != NOP_EXPR) + return; + + int fidx = ipa_get_jf_pass_through_formal_id (jfunc); + cgraph_node *caller = cs->caller; + ipa_node_params *caller_info = ipa_node_params_sum->get (caller); + /* TODO: This consistency check may be too big and not really + that useful. Consider removing it. */ + tree cst; + if (caller_info->ipcp_orig_node) + cst = caller_info->known_csts[fidx]; + else + { + ipcp_lattice *lat = ipa_get_scalar_lat (caller_info, fidx); + gcc_assert (lat->is_single_const ()); + cst = lat->values->value; + } + gcc_assert (TREE_CODE (cst) == ADDR_EXPR + && (symtab_node::get (get_base_address (TREE_OPERAND (cst, 0))) + == symbol)); + + int cuses = ipa_get_controlled_uses (caller_info, fidx); + if (cuses == IPA_UNDESCRIBED_USE) + return; + gcc_assert (cuses > 0); + cuses--; + ipa_set_controlled_uses (caller_info, fidx, cuses); + if (cuses) + return; + + if (caller_info->ipcp_orig_node) + { + /* Cloning machinery has created a reference here, we need to either + remove it or change it to a read one. */ + ipa_ref *to_del = caller->find_reference (symbol, NULL, 0); + if (to_del && to_del->use == IPA_REF_ADDR) + { + to_del->remove_reference (); + if (dump_file) + fprintf (dump_file, " Removed a reference from %s to %s.\n", + cs->caller->dump_name (), symbol->dump_name ()); + if (ipa_get_param_load_dereferenced (caller_info, fidx)) + { + caller->create_reference (symbol, IPA_REF_LOAD, NULL); + if (dump_file) + fprintf (dump_file, + " ...and replaced it with LOAD one.\n"); + } + } + } + + symbol_and_index_together pack; + pack.symbol = symbol; + pack.index = fidx; + if (caller->can_change_signature) + caller->call_for_symbol_thunks_and_aliases (adjust_refs_in_act_callers, + &pack, true); +} + + /* Return true if we would like to remove a parameter from NODE when cloning it with KNOWN_CSTS scalar constants. */ @@ -4595,15 +4713,31 @@ create_specialized_node (struct cgraph_node *node, for (i = 0; i < count; i++) { tree t = known_csts[i]; - if (t) - { - struct ipa_replace_map *replace_map; + if (!t) + continue; - gcc_checking_assert (TREE_CODE (t) != TREE_BINFO); - replace_map = get_replacement_map (info, t, i); - if (replace_map) - vec_safe_push (replace_trees, replace_map); + gcc_checking_assert (TREE_CODE (t) != TREE_BINFO); + + bool load_ref = false; + symtab_node *ref_symbol; + if (TREE_CODE (t) == ADDR_EXPR) + { + tree base = get_base_address (TREE_OPERAND (t, 0)); + if (TREE_CODE (base) == VAR_DECL + && ipa_get_controlled_uses (info, i) == 0 + && ipa_get_param_load_dereferenced (info, i) + && (ref_symbol = symtab_node::get (base))) + { + load_ref = true; + if (node->can_change_signature) + for (cgraph_edge *caller : callers) + adjust_references_in_caller (caller, ref_symbol, i); + } } + + ipa_replace_map *replace_map = get_replacement_map (info, t, i, load_ref); + if (replace_map) + vec_safe_push (replace_trees, replace_map); } auto_vec self_recursive_calls; for (i = callers.length () - 1; i >= 0; i--) diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index f74d2e17b69..a9b1b3c8aef 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -544,7 +544,9 @@ ipa_set_jf_constant (struct ipa_jump_func *jfunc, tree constant, jfunc->value.constant.value = unshare_expr_without_location (constant); if (TREE_CODE (constant) == ADDR_EXPR - && TREE_CODE (TREE_OPERAND (constant, 0)) == FUNCTION_DECL) + && (TREE_CODE (TREE_OPERAND (constant, 0)) == FUNCTION_DECL + || (TREE_CODE (TREE_OPERAND (constant, 0)) == VAR_DECL + && TREE_STATIC (TREE_OPERAND (constant, 0))))) { struct ipa_cst_ref_desc *rdesc; @@ -2876,6 +2878,16 @@ ipa_analyze_params_uses_in_bb (struct ipa_func_body_info *fbi, basic_block bb) visit_ref_for_mod_analysis); } +/* Return true EXPR is a load from a dereference of SSA_NAME NAME. */ + +static bool +load_from_dereferenced_name (tree expr, tree name) +{ + tree base = get_base_address (expr); + return (TREE_CODE (base) == MEM_REF + && TREE_OPERAND (base, 0) == name); +} + /* Calculate controlled uses of parameters of NODE. */ static void @@ -2886,7 +2898,8 @@ ipa_analyze_controlled_uses (struct cgraph_node *node) for (int i = 0; i < ipa_get_param_count (info); i++) { tree parm = ipa_get_param (info, i); - int controlled_uses = 0; + int call_uses = 0; + bool load_dereferenced = false; /* For SSA regs see if parameter is used. For non-SSA we compute the flag during modification analysis. */ @@ -2897,27 +2910,77 @@ ipa_analyze_controlled_uses (struct cgraph_node *node) if (ddef && !has_zero_uses (ddef)) { imm_use_iterator imm_iter; - use_operand_p use_p; + gimple *stmt; ipa_set_param_used (info, i, true); - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, ddef) - if (!is_gimple_call (USE_STMT (use_p))) - { - if (!is_gimple_debug (USE_STMT (use_p))) - { - controlled_uses = IPA_UNDESCRIBED_USE; - break; - } - } - else - controlled_uses++; + FOR_EACH_IMM_USE_STMT (stmt, imm_iter, ddef) + { + if (is_gimple_debug (stmt)) + continue; + + int all_stmt_uses = 0; + use_operand_p use_p; + FOR_EACH_IMM_USE_ON_STMT (use_p, imm_iter) + all_stmt_uses++; + + if (is_gimple_call (stmt)) + { + if (gimple_call_internal_p (stmt)) + { + call_uses = IPA_UNDESCRIBED_USE; + break; + } + int recognized_stmt_uses; + if (gimple_call_fn (stmt) == ddef) + recognized_stmt_uses = 1; + else + recognized_stmt_uses = 0; + unsigned arg_count = gimple_call_num_args (stmt); + for (unsigned i = 0; i < arg_count; i++) + { + tree arg = gimple_call_arg (stmt, i); + if (arg == ddef) + recognized_stmt_uses++; + else if (load_from_dereferenced_name (arg, ddef)) + { + load_dereferenced = true; + recognized_stmt_uses++; + } + } + + if (recognized_stmt_uses != all_stmt_uses) + { + call_uses = IPA_UNDESCRIBED_USE; + break; + } + if (call_uses >= 0) + call_uses += all_stmt_uses; + } + else if (gimple_assign_single_p (stmt)) + { + tree rhs = gimple_assign_rhs1 (stmt); + if (all_stmt_uses != 1 + || !load_from_dereferenced_name (rhs, ddef)) + { + call_uses = IPA_UNDESCRIBED_USE; + break; + } + load_dereferenced = true; + } + else + { + call_uses = IPA_UNDESCRIBED_USE; + break; + } + } } else - controlled_uses = 0; + call_uses = 0; } else - controlled_uses = IPA_UNDESCRIBED_USE; - ipa_set_controlled_uses (info, i, controlled_uses); + call_uses = IPA_UNDESCRIBED_USE; + ipa_set_controlled_uses (info, i, call_uses); + ipa_set_param_load_dereferenced (info, i, load_dereferenced); } } @@ -3640,16 +3703,17 @@ jfunc_rdesc_usable (struct ipa_jump_func *jfunc) declaration, return the associated call graph node. Otherwise return NULL. */ -static cgraph_node * -cgraph_node_for_jfunc (struct ipa_jump_func *jfunc) +static symtab_node * +symtab_node_for_jfunc (struct ipa_jump_func *jfunc) { gcc_checking_assert (jfunc->type == IPA_JF_CONST); tree cst = ipa_get_jf_constant (jfunc); if (TREE_CODE (cst) != ADDR_EXPR - || TREE_CODE (TREE_OPERAND (cst, 0)) != FUNCTION_DECL) + || (TREE_CODE (TREE_OPERAND (cst, 0)) != FUNCTION_DECL + && TREE_CODE (TREE_OPERAND (cst, 0)) != VAR_DECL)) return NULL; - return cgraph_node::get (TREE_OPERAND (cst, 0)); + return symtab_node::get (TREE_OPERAND (cst, 0)); } @@ -3666,7 +3730,7 @@ try_decrement_rdesc_refcount (struct ipa_jump_func *jfunc) && (rdesc = jfunc_rdesc_usable (jfunc)) && --rdesc->refcount == 0) { - symtab_node *symbol = cgraph_node_for_jfunc (jfunc); + symtab_node *symbol = symtab_node_for_jfunc (jfunc); if (!symbol) return false; @@ -3721,8 +3785,8 @@ try_make_edge_direct_simple_call (struct cgraph_edge *ie, gcc_checking_assert (cs->callee && (cs != ie || jfunc->type != IPA_JF_CONST - || !cgraph_node_for_jfunc (jfunc) - || cs->callee == cgraph_node_for_jfunc (jfunc))); + || !symtab_node_for_jfunc (jfunc) + || cs->callee == symtab_node_for_jfunc (jfunc))); ok = try_decrement_rdesc_refcount (jfunc); gcc_checking_assert (ok); } @@ -4085,7 +4149,15 @@ propagate_controlled_uses (struct cgraph_edge *cs) == NOP_EXPR || c == IPA_UNDESCRIBED_USE); c = combine_controlled_uses_counters (c, d); ipa_set_controlled_uses (new_root_info, src_idx, c); - if (c == 0 && new_root_info->ipcp_orig_node) + bool lderef = true; + if (c != IPA_UNDESCRIBED_USE) + { + lderef = (ipa_get_param_load_dereferenced (new_root_info, src_idx) + || ipa_get_param_load_dereferenced (old_root_info, i)); + ipa_set_param_load_dereferenced (new_root_info, src_idx, lderef); + } + + if (c == 0 && !lderef && new_root_info->ipcp_orig_node) { struct cgraph_node *n; struct ipa_ref *ref; @@ -4114,17 +4186,27 @@ propagate_controlled_uses (struct cgraph_edge *cs) if (rdesc->refcount == 0) { tree cst = ipa_get_jf_constant (jf); - struct cgraph_node *n; gcc_checking_assert (TREE_CODE (cst) == ADDR_EXPR - && TREE_CODE (TREE_OPERAND (cst, 0)) - == FUNCTION_DECL); - n = cgraph_node::get (TREE_OPERAND (cst, 0)); + && ((TREE_CODE (TREE_OPERAND (cst, 0)) + == FUNCTION_DECL) + || (TREE_CODE (TREE_OPERAND (cst, 0)) + == VAR_DECL))); + + symtab_node *n = symtab_node::get (TREE_OPERAND (cst, 0)); if (n) { struct cgraph_node *clone; - bool ok; - ok = remove_described_reference (n, rdesc); - gcc_checking_assert (ok); + bool removed = remove_described_reference (n, rdesc); + /* The reference might have been removed by IPA-CP. */ + if (removed + && ipa_get_param_load_dereferenced (old_root_info, i)) + { + new_root->create_reference (n, IPA_REF_LOAD, NULL); + if (dump_file) + fprintf (dump_file, "ipa-prop: ...replaced it with " + " LOAD one from %s to %s.\n", + new_root->dump_name (), n->dump_name ()); + } clone = cs->caller; while (clone->inlined_to @@ -4347,7 +4429,7 @@ ipa_edge_args_sum_t::duplicate (cgraph_edge *src, cgraph_edge *dst, else if (src->caller == dst->caller) { struct ipa_ref *ref; - symtab_node *n = cgraph_node_for_jfunc (src_jf); + symtab_node *n = symtab_node_for_jfunc (src_jf); gcc_checking_assert (n); ref = src->caller->find_reference (n, src->call_stmt, src->lto_stmt_uid); @@ -4574,7 +4656,9 @@ ipa_print_node_params (FILE *f, struct cgraph_node *node) if (c == IPA_UNDESCRIBED_USE) fprintf (f, " undescribed_use"); else - fprintf (f, " controlled_uses=%i", c); + fprintf (f, " controlled_uses=%i %s", c, + ipa_get_param_load_dereferenced (info, i) + ? "(load_dereferenced)" : ""); fprintf (f, "\n"); } } @@ -4969,7 +5053,13 @@ ipa_write_node_info (struct output_block *ob, struct cgraph_node *node) gcc_assert (!info->node_enqueued); gcc_assert (!info->ipcp_orig_node); for (j = 0; j < ipa_get_param_count (info); j++) - bp_pack_value (&bp, ipa_is_param_used (info, j), 1); + { + /* TODO: We could just not stream the bit in the undescribed case. */ + bool d = (ipa_get_controlled_uses (info, j) != IPA_UNDESCRIBED_USE) + ? ipa_get_param_load_dereferenced (info, j) : true; + bp_pack_value (&bp, d, 1); + bp_pack_value (&bp, ipa_is_param_used (info, j), 1); + } streamer_write_bitpack (&bp); for (j = 0; j < ipa_get_param_count (info); j++) { @@ -5095,10 +5185,14 @@ ipa_read_node_info (class lto_input_block *ib, struct cgraph_node *node, bp = streamer_read_bitpack (ib); for (k = 0; k < param_count; k++) { + bool load_dereferenced = bp_unpack_value (&bp, 1); bool used = bp_unpack_value (&bp, 1); if (prevails) - ipa_set_param_used (info, k, used); + { + ipa_set_param_load_dereferenced (info, k, load_dereferenced); + ipa_set_param_used (info, k, used); + } } for (k = 0; k < param_count; k++) { diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 3d28a6e8640..de35e3969f4 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -550,14 +550,21 @@ struct GTY(()) ipa_param_descriptor tree decl_or_type; /* If all uses of the parameter are described by ipa-prop structures, this says how many there are. If any use could not be described by means of - ipa-prop structures, this is IPA_UNDESCRIBED_USE. */ + ipa-prop structures (which include flag dereferenced below), this is + IPA_UNDESCRIBED_USE. */ int controlled_uses; - unsigned int move_cost : 28; + unsigned int move_cost : 27; /* The parameter is used. */ unsigned used : 1; unsigned used_by_ipa_predicates : 1; unsigned used_by_indirect_call : 1; unsigned used_by_polymorphic_call : 1; + /* Set to true when in addition to being used in call statements, the + parameter has also been used for loads (but not for writes, does not + escape, etc.). This allows us to identify parameters p which are only + used as *p, and so when we propagate a constant to them, we can generate a + LOAD and not ADDR reference to them. */ + unsigned load_dereferenced : 1; }; /* ipa_node_params stores information related to formal parameters of functions @@ -795,6 +802,24 @@ ipa_set_controlled_uses (class ipa_node_params *info, int i, int val) (*info->descriptors)[i].controlled_uses = val; } +/* Assuming a parameter does not have IPA_UNDESCRIBED_USE controlled uses, + return flag which indicates it has been dereferenced but only in a load. */ +static inline int +ipa_get_param_load_dereferenced (class ipa_node_params *info, int i) +{ + gcc_assert (ipa_get_controlled_uses (info, i) != IPA_UNDESCRIBED_USE); + return (*info->descriptors)[i].load_dereferenced; +} + +/* Set the load_dereferenced flag of a given parameter. */ + +static inline void +ipa_set_param_load_dereferenced (class ipa_node_params *info, int i, bool val) +{ + gcc_checking_assert (info->descriptors); + (*info->descriptors)[i].load_dereferenced = val; +} + /* Return the used flag corresponding to the Ith formal parameter of the function associated with INFO. */ diff --git a/gcc/testsuite/gcc.dg/ipa/remref-3.c b/gcc/testsuite/gcc.dg/ipa/remref-3.c new file mode 100644 index 00000000000..8a82ca65c68 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/remref-3.c @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-early-inlining -fdump-ipa-cp-details -fdump-tree-optimized" } */ + +static double global = 0.0; + +double foo_temp5; + +static void foo(double *ptr) { + static double abcd; + double v, exp_res; + v = *ptr; + exp_res = __builtin_exp(v); + foo_temp5 = exp_res * abcd; + abcd += foo_temp5; +} + +void entry() +{ + foo(&global); +} + +/* { dg-final { scan-ipa-dump "Removed a reference" "cp" } } */ +/* { dg-final { scan-tree-dump-not "builtin_exp" "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/ipa/remref-4.c b/gcc/testsuite/gcc.dg/ipa/remref-4.c new file mode 100644 index 00000000000..5a55437e971 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/remref-4.c @@ -0,0 +1,31 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-early-inlining -fdump-ipa-cp-details -fdump-tree-optimized" } */ + +static double global = 0.0; +double foo_temp5; + +static void foo(double *ptr) { + static double abcd; + double v, exp_res; + v = *ptr; + exp_res = __builtin_exp(v); + foo_temp5 = exp_res * abcd; + abcd += foo_temp5; +} + +double last_value; + +static void bar(double *ptr) +{ + last_value = *ptr; + foo (ptr); +} + +void entry() +{ + bar (&global); +} + +/* { dg-final { scan-ipa-dump "Removed a reference" "cp" } } */ +/* { dg-final { scan-ipa-dump "replaced it with LOAD" "cp" } } */ +/* { dg-final { scan-tree-dump-not "builtin_exp" "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/ipa/remref-5.c b/gcc/testsuite/gcc.dg/ipa/remref-5.c new file mode 100644 index 00000000000..c520e34d809 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/remref-5.c @@ -0,0 +1,38 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -fno-early-inlining -fdump-ipa-cp-details -fdump-tree-optimized" } */ + +static double global = 0.0; +double foo_temp5; + +static void foo(double *ptr) { + static double abcd; + double v, exp_res; + v = *ptr; + exp_res = __builtin_exp(v); + foo_temp5 = exp_res * abcd; + abcd += foo_temp5; +} + +double last_value; + +static void bar(double *ptr) +{ + last_value = *ptr; + for (unsigned i = 0; i < 200; i++) + foo (ptr); +} + +void entry() +{ + bar (&global); +} + +void decoy(double *ptr) +{ + bar (ptr); +} + + +/* { dg-final { scan-ipa-dump "Removed a reference" "cp" } } */ +/* { dg-final { scan-ipa-dump "replaced it with LOAD" "cp" } } */ +/* { dg-final { scan-tree-dump-times "builtin_exp" 1 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/ipa/remref-6.c b/gcc/testsuite/gcc.dg/ipa/remref-6.c new file mode 100644 index 00000000000..de3649315c2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/remref-6.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-cp -fdump-tree-optimized" } */ + +static double global = 0.0; + +double foo_temp5; + +static void foo(double *ptr) { + static double abcd; + double v, exp_res; + v = *ptr; + exp_res = __builtin_exp(v); + foo_temp5 = exp_res * abcd; + abcd += foo_temp5; +} + +void entry() +{ + foo(&global); +} + +/* { dg-final { scan-ipa-dump "Removed a reference" "inline" } } */ +/* { dg-final { scan-ipa-dump "replaced it with LOAD" "inline" } } */ +/* { dg-final { scan-tree-dump-not "builtin_exp" "optimized" } } */ -- 2.32.0