From patchwork Sun Nov 7 11:42:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 47178 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4F4E43858403 for ; Sun, 7 Nov 2021 11:43:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4F4E43858403 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636285407; bh=4Xf+3MyxFqjO5wilF5nzOm825j3X0OJpWlNx0bRW/mw=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=daY06xnhJbbb3F34r/REg1XvP3F5Fw/XTOcZnubSyMduG2lYgR7FFlXcjUU9tp2Rw l1K1Hvnq5NWCVmyOb4prE0oTw1/KWox6U3+29ByaLUy5RrPdJ6eKMvd8WBOYtUb0je StA8JewR9slQcpdRYUb8RdYBGFnO4g20B+IBRYGY= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 9BE133858401 for ; Sun, 7 Nov 2021 11:42:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9BE133858401 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 3BD122809DA; Sun, 7 Nov 2021 12:42:56 +0100 (CET) Date: Sun, 7 Nov 2021 12:42:56 +0100 To: gcc-patches@gcc.gnu.org Subject: Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p Message-ID: <20211107114256.GA50485@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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-Patchwork-Original-From: Jan Hubicka via Gcc-patches From: Jan Hubicka Reply-To: Jan Hubicka Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi, 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 nterposable 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 kive up on NOVOPS. (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 --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..26a719ff40c 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))) @@ -4039,12 +4029,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 +4048,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 +4061,42 @@ 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; + } + } + if (!ee->direct) + fnspec_flags = deref_flags (fnspec_flags, ignore_stores); + implicit_flags |= fnspec_flags; + } + 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 +4118,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 +4135,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 +4169,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 +4188,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 +4244,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