From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1075) id 072CC3858401; Sun, 7 Nov 2021 17:21:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 072CC3858401 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jan Hubicka To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-4978] Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p X-Act-Checkin: gcc X-Git-Author: Jan Hubicka X-Git-Refname: refs/heads/master X-Git-Oldrev: a28cfe49203705ff9675b79fce88d6087b11d098 X-Git-Newrev: f6f704fd104b79fc88914978772737cd05423059 Message-Id: <20211107172133.072CC3858401@sourceware.org> Date: Sun, 7 Nov 2021 17:21:33 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Nov 2021 17:21:33 -0000 https://gcc.gnu.org/g:f6f704fd104b79fc88914978772737cd05423059 commit r12-4978-gf6f704fd104b79fc88914978772737cd05423059 Author: Jan Hubicka Date: Sun Nov 7 18:20:45 2021 +0100 Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p While proofreading the code for handling EAF flags of !binds_to_current_def_p I noticed that the interprocedural dataflow actually ignores the flag possibly introducing wrong code on quite complex interposable functions in non-trivial recursion cycles (or at ltrans partition boundary). This patch unifies the flags changes to single place (remove_useless_eaf_flags) and does extend modref_merge_call_site_flags to do the right thing. lto-bootstrapped/regtested x86_64-linux. Plan to commit it today after bit more testing (firefox/clang build). gcc/ChangeLog: * gimple.c (gimple_call_arg_flags): Use interposable_eaf_flags. (gimple_call_retslot_flags): Likewise. (gimple_call_static_chain_flags): Likewise. * ipa-modref.c (remove_useless_eaf_flags): Do not remove everything for NOVOPS. (modref_summary::useful_p): Likewise. (modref_summary_lto::useful_p): Likewise. (analyze_parms): Do not give up on NOVOPS. (analyze_function): When dumping report chnages in EAF flags between IPA and local pass. (modref_merge_call_site_flags): Compute implicit eaf flags based on callee ecf_flags and fnspec; if the function does not bind to current defs use interposable_eaf_flags. (modref_propagate_flags_in_scc): Update. * ipa-modref.h (interposable_eaf_flags): New function. Diff: --- gcc/gimple.c | 34 ++-------- gcc/ipa-modref.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++------- gcc/ipa-modref.h | 29 +++++++++ 3 files changed, 203 insertions(+), 55 deletions(-) diff --git a/gcc/gimple.c b/gcc/gimple.c index 7a578f5113e..3d1d3a15b2c 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -1595,17 +1595,7 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg) /* We have possibly optimized out load. Be conservative here. */ if (!node->binds_to_current_def_p ()) - { - if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED)) - { - modref_flags &= ~EAF_UNUSED; - modref_flags |= EAF_NOESCAPE; - } - if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD)) - modref_flags &= ~EAF_NOREAD; - if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT)) - modref_flags &= ~EAF_DIRECT; - } + modref_flags = interposable_eaf_flags (modref_flags, flags); if (dbg_cnt (ipa_mod_ref_pta)) flags |= modref_flags; } @@ -1633,13 +1623,7 @@ gimple_call_retslot_flags (const gcall *stmt) /* We have possibly optimized out load. Be conservative here. */ if (!node->binds_to_current_def_p ()) - { - if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED)) - { - modref_flags &= ~EAF_UNUSED; - modref_flags |= EAF_NOESCAPE; - } - } + modref_flags = interposable_eaf_flags (modref_flags, flags); if (dbg_cnt (ipa_mod_ref_pta)) flags |= modref_flags; } @@ -1665,19 +1649,9 @@ gimple_call_static_chain_flags (const gcall *stmt) { int modref_flags = summary->static_chain_flags; - /* We have possibly optimized out load. Be conservative here. */ + /* ??? Nested functions should always bind to current def. */ if (!node->binds_to_current_def_p ()) - { - if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED)) - { - modref_flags &= ~EAF_UNUSED; - modref_flags |= EAF_NOESCAPE; - } - if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD)) - modref_flags &= ~EAF_NOREAD; - if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT)) - modref_flags &= ~EAF_DIRECT; - } + modref_flags = interposable_eaf_flags (modref_flags, flags); if (dbg_cnt (ipa_mod_ref_pta)) flags |= modref_flags; } diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index 5209fbdfbf4..4e64ee5d9fd 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -291,9 +291,7 @@ modref_summary::~modref_summary () static int remove_useless_eaf_flags (int eaf_flags, int ecf_flags, bool returns_void) { - if (ecf_flags & ECF_NOVOPS) - return 0; - if (ecf_flags & ECF_CONST) + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) eaf_flags &= ~implicit_const_eaf_flags; else if (ecf_flags & ECF_PURE) eaf_flags &= ~implicit_pure_eaf_flags; @@ -319,8 +317,6 @@ eaf_flags_useful_p (vec &flags, int ecf_flags) bool modref_summary::useful_p (int ecf_flags, bool check_flags) { - if (ecf_flags & ECF_NOVOPS) - return false; if (arg_flags.length () && !check_flags) return true; if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags)) @@ -331,7 +327,7 @@ modref_summary::useful_p (int ecf_flags, bool check_flags) if (check_flags && remove_useless_eaf_flags (static_chain_flags, ecf_flags, false)) return true; - if (ecf_flags & ECF_CONST) + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) return false; if (loads && !loads->every_base) return true; @@ -405,8 +401,6 @@ modref_summary_lto::~modref_summary_lto () bool modref_summary_lto::useful_p (int ecf_flags, bool check_flags) { - if (ecf_flags & ECF_NOVOPS) - return false; if (arg_flags.length () && !check_flags) return true; if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags)) @@ -417,7 +411,7 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags) if (check_flags && remove_useless_eaf_flags (static_chain_flags, ecf_flags, false)) return true; - if (ecf_flags & ECF_CONST) + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) return false; if (loads && !loads->every_base) return true; @@ -2321,10 +2315,6 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, tree retslot = NULL; tree static_chain = NULL; - /* For novops functions we have nothing to gain by EAF flags. */ - if (ecf_flags & ECF_NOVOPS) - return; - /* If there is return slot, look up its SSA name. */ if (DECL_RESULT (current_function_decl) && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl))) @@ -2471,6 +2461,11 @@ analyze_function (function *f, bool ipa) modref_summary *summary = NULL; modref_summary_lto *summary_lto = NULL; + bool past_flags_known = false; + auto_vec past_flags; + int past_retslot_flags = 0; + int past_static_chain_flags = 0; + /* Initialize the summary. If we run in local mode there is possibly pre-existing summary from IPA pass. Dump it so it is easy to compare if mod-ref info has @@ -2490,6 +2485,11 @@ analyze_function (function *f, bool ipa) fprintf (dump_file, "Past summary:\n"); optimization_summaries->get (cgraph_node::get (f->decl))->dump (dump_file); + past_flags.reserve_exact (summary->arg_flags.length ()); + past_flags.splice (summary->arg_flags); + past_retslot_flags = summary->retslot_flags; + past_static_chain_flags = summary->static_chain_flags; + past_flags_known = true; } optimization_summaries->remove (cgraph_node::get (f->decl)); } @@ -2630,6 +2630,108 @@ analyze_function (function *f, bool ipa) if (summary_lto) summary_lto->dump (dump_file); dump_modref_edge_summaries (dump_file, fnode, 2); + /* To simplify debugging, compare IPA and local solutions. */ + if (past_flags_known && summary) + { + size_t len = summary->arg_flags.length (); + + if (past_flags.length () > len) + len = past_flags.length (); + for (size_t i = 0; i < len; i++) + { + int old_flags = i < past_flags.length () ? past_flags[i] : 0; + int new_flags = i < summary->arg_flags.length () + ? summary->arg_flags[i] : 0; + old_flags = remove_useless_eaf_flags + (old_flags, flags_from_decl_or_type (current_function_decl), + VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))); + if (old_flags != new_flags) + { + if ((old_flags & ~new_flags) == 0) + fprintf (dump_file, " Flags for param %i improved:", + (int)i); + else if ((new_flags & ~old_flags) == 0) + fprintf (dump_file, " Flags for param %i worsened:", + (int)i); + else + fprintf (dump_file, " Flags for param %i changed:", + (int)i); + dump_eaf_flags (dump_file, old_flags, false); + fprintf (dump_file, " -> "); + dump_eaf_flags (dump_file, new_flags, true); + } + } + past_retslot_flags = remove_useless_eaf_flags + (past_retslot_flags, + flags_from_decl_or_type (current_function_decl), + VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))); + if (past_retslot_flags != summary->retslot_flags) + { + if ((past_retslot_flags & ~summary->retslot_flags) == 0) + fprintf (dump_file, " Flags for retslot improved:"); + else if ((summary->retslot_flags & ~past_retslot_flags) == 0) + fprintf (dump_file, " Flags for retslot worsened:"); + else + fprintf (dump_file, " Flags for retslot changed:"); + dump_eaf_flags (dump_file, past_retslot_flags, false); + fprintf (dump_file, " -> "); + dump_eaf_flags (dump_file, summary->retslot_flags, true); + } + past_static_chain_flags = remove_useless_eaf_flags + (past_static_chain_flags, + flags_from_decl_or_type (current_function_decl), + VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))); + if (past_static_chain_flags != summary->static_chain_flags) + { + if ((past_static_chain_flags & ~summary->static_chain_flags) == 0) + fprintf (dump_file, " Flags for static chain improved:"); + else if ((summary->static_chain_flags + & ~past_static_chain_flags) == 0) + fprintf (dump_file, " Flags for static chain worsened:"); + else + fprintf (dump_file, " Flags for static chain changed:"); + dump_eaf_flags (dump_file, past_static_chain_flags, false); + fprintf (dump_file, " -> "); + dump_eaf_flags (dump_file, summary->static_chain_flags, true); + } + } + else if (past_flags_known && !summary) + { + for (size_t i = 0; i < past_flags.length (); i++) + { + int old_flags = past_flags[i]; + old_flags = remove_useless_eaf_flags + (old_flags, flags_from_decl_or_type (current_function_decl), + VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))); + if (old_flags) + { + fprintf (dump_file, " Flags for param %i worsened:", + (int)i); + dump_eaf_flags (dump_file, old_flags, false); + fprintf (dump_file, " -> \n"); + } + } + past_retslot_flags = remove_useless_eaf_flags + (past_retslot_flags, + flags_from_decl_or_type (current_function_decl), + VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))); + if (past_retslot_flags) + { + fprintf (dump_file, " Flags for retslot worsened:"); + dump_eaf_flags (dump_file, past_retslot_flags, false); + fprintf (dump_file, " ->\n"); + } + past_static_chain_flags = remove_useless_eaf_flags + (past_static_chain_flags, + flags_from_decl_or_type (current_function_decl), + VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))); + if (past_static_chain_flags) + { + fprintf (dump_file, " Flags for static chain worsened:"); + dump_eaf_flags (dump_file, past_static_chain_flags, false); + fprintf (dump_file, " ->\n"); + } + } } } @@ -4039,12 +4141,15 @@ modref_merge_call_site_flags (escape_summary *sum, modref_summary *summary, modref_summary_lto *summary_lto, tree caller, - int ecf_flags) + cgraph_edge *e, + int caller_ecf_flags, + int callee_ecf_flags, + bool binds_to_current_def) { escape_entry *ee; unsigned int i; bool changed = false; - bool ignore_stores = ignore_stores_p (caller, ecf_flags); + bool ignore_stores = ignore_stores_p (caller, callee_ecf_flags); /* If we have no useful info to propagate. */ if ((!cur_summary || !cur_summary->arg_flags.length ()) @@ -4055,6 +4160,8 @@ modref_merge_call_site_flags (escape_summary *sum, { int flags = 0; int flags_lto = 0; + /* Returning the value is already accounted to at local propagation. */ + int implicit_flags = EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY; if (summary && ee->arg < summary->arg_flags.length ()) flags = summary->arg_flags[ee->arg]; @@ -4066,14 +4173,43 @@ modref_merge_call_site_flags (escape_summary *sum, flags = deref_flags (flags, ignore_stores); flags_lto = deref_flags (flags_lto, ignore_stores); } - else if (ignore_stores) + if (ignore_stores) + implicit_flags |= ignore_stores_eaf_flags; + if (callee_ecf_flags & ECF_PURE) + implicit_flags |= implicit_pure_eaf_flags; + if (callee_ecf_flags & (ECF_CONST | ECF_NOVOPS)) + implicit_flags |= implicit_const_eaf_flags; + class fnspec_summary *fnspec_sum = fnspec_summaries->get (e); + if (fnspec_sum) { - flags |= ignore_stores_eaf_flags; - flags_lto |= ignore_stores_eaf_flags; + attr_fnspec fnspec (fnspec_sum->fnspec); + int fnspec_flags = 0; + + if (fnspec.arg_specified_p (ee->arg)) + { + if (!fnspec.arg_used_p (ee->arg)) + fnspec_flags = EAF_UNUSED; + else + { + if (fnspec.arg_direct_p (ee->arg)) + fnspec_flags |= EAF_DIRECT; + if (fnspec.arg_noescape_p (ee->arg)) + fnspec_flags |= EAF_NOESCAPE | EAF_NODIRECTESCAPE; + if (fnspec.arg_readonly_p (ee->arg)) + fnspec_flags |= EAF_NOCLOBBER; + } + } + implicit_flags |= fnspec_flags; + } + if (!ee->direct) + implicit_flags = deref_flags (implicit_flags, ignore_stores); + flags |= implicit_flags; + flags_lto |= implicit_flags; + if (!binds_to_current_def && (flags || flags_lto)) + { + flags = interposable_eaf_flags (flags, implicit_flags); + flags_lto = interposable_eaf_flags (flags_lto, implicit_flags); } - /* Returning the value is already accounted to at local propagation. */ - flags |= ee->min_flags | EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY; - flags_lto |= ee->min_flags | EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY; /* Noescape implies that value also does not escape directly. Fnspec machinery does set both so compensate for this. */ if (flags & EAF_NOESCAPE) @@ -4095,7 +4231,7 @@ modref_merge_call_site_flags (escape_summary *sum, if ((f & flags) != f) { f = remove_useless_eaf_flags - (f & flags, ecf_flags, + (f & flags, caller_ecf_flags, VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller)))); changed = true; } @@ -4112,7 +4248,7 @@ modref_merge_call_site_flags (escape_summary *sum, if ((f & flags_lto) != f) { f = remove_useless_eaf_flags - (f & flags_lto, ecf_flags, + (f & flags_lto, caller_ecf_flags, VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller)))); changed = true; } @@ -4146,6 +4282,7 @@ modref_propagate_flags_in_scc (cgraph_node *component_node) if (!cur_summary && !cur_summary_lto) continue; + int caller_ecf_flags = flags_from_decl_or_type (cur->decl); if (dump_file) fprintf (dump_file, " Processing %s%s%s\n", @@ -4164,7 +4301,11 @@ modref_propagate_flags_in_scc (cgraph_node *component_node) changed |= modref_merge_call_site_flags (sum, cur_summary, cur_summary_lto, NULL, NULL, - node->decl, e->indirect_info->ecf_flags); + node->decl, + e, + caller_ecf_flags, + e->indirect_info->ecf_flags, + false); } if (!cur_summary && !cur_summary_lto) @@ -4216,7 +4357,11 @@ modref_propagate_flags_in_scc (cgraph_node *component_node) changed |= modref_merge_call_site_flags (sum, cur_summary, cur_summary_lto, callee_summary, callee_summary_lto, - node->decl, ecf_flags); + node->decl, + callee_edge, + caller_ecf_flags, + ecf_flags, + callee->binds_to_current_def_p ()); if (dump_file && changed) { if (cur_summary) diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h index ddc86869069..20170a65ded 100644 --- a/gcc/ipa-modref.h +++ b/gcc/ipa-modref.h @@ -58,4 +58,33 @@ static const int implicit_pure_eaf_flags = EAF_NOCLOBBER | EAF_NOESCAPE static const int ignore_stores_eaf_flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_NODIRECTESCAPE; +/* If function does not bind to current def (i.e. it is inline in comdat + section), the modref analysis may not match the behaviour of function + which will be later symbol interposed to. All side effects must match + however it is possible that the other function body contains more loads + which may trap. + MODREF_FLAGS are flags determined by analysis of function body while + FLAGS are flags known otherwise (i.e. by fnspec, pure/const attributes + etc.) */ +static inline int +interposable_eaf_flags (int modref_flags, int flags) +{ + /* If parameter was previously unused, we know it is only read + and its value is not used. */ + if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED)) + { + modref_flags &= ~EAF_UNUSED; + modref_flags |= EAF_NOESCAPE | EAF_NOT_RETURNED + | EAF_NOT_RETURNED_DIRECTLY | EAF_NOCLOBBER; + } + /* We can not deterine that value is not read at all. */ + if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD)) + modref_flags &= ~EAF_NOREAD; + /* Clear direct flags so we also know that value is possibly read + indirectly. */ + if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT)) + modref_flags &= ~EAF_DIRECT; + return modref_flags; +} + #endif