From patchwork Thu Nov 11 15:00:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 47479 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 4659E3857C50 for ; Thu, 11 Nov 2021 15:00:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4659E3857C50 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636642855; bh=hpepCZaPe3KZO/8VBWMFb9Hmkasj8F0dVIYz3ro/jRU=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=h37aXwoZWRvltKZWN0HouY3QhMTfiORo7rGcKuq1/vLYs/vZ+AgT00tdFTyDuniqj OR6Cbl8D+hNy0uqFsQhDJmtdSH2BcPOrO13fpFLZ6qMo7mUaqEdFpTVsfrv7g+PEei qNSK0xzJWAzQdTx5rj+V29U2Xr4KL36rxMVMgROA= 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 D4ADE385840A for ; Thu, 11 Nov 2021 15:00:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D4ADE385840A Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 8FA032827E3; Thu, 11 Nov 2021 16:00:23 +0100 (CET) Date: Thu, 11 Nov 2021 16:00:23 +0100 To: gcc-patches@gcc.gnu.org Subject: Fix some side cases of side effects analysis Message-ID: <20211111150023.GH17431@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, I wrote script comparing modref pure/const discovery with ipa-pure-const and found mistakes on both ends. I fixed ipa-pure-const in previous two patches. This plugs the case where modref was too optimistic in handling looping pure consts which were previously missed due to early exits on ECF_CONST | ECF_PURE. Those early exists are bit anoying and I think as a cleanup I may just drop some of them as premature optimizations coming from time modref was very simplistic on what it propagates. Bootstrapped/regtested x86_64-linux, will commit it shortly. gcc/ChangeLog: 2021-11-11 Jan Hubicka * ipa-modref.c (modref_summary::useful_p): Check also for side-effects with looping const/pure. (modref_summary_lto::useful_p): Likewise. (merge_call_side_effects): Merge side effects before early exit for pure/const. (process_fnspec): Also handle pure functions. (analyze_call): Do not early exit on looping pure const. (propagate_unknown_call): Also handle nontrivial SCC as side-effect. (modref_propagate_in_scc): diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index f8b7b900527..45b391a565e 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -331,11 +331,11 @@ modref_summary::useful_p (int ecf_flags, bool check_flags) && remove_useless_eaf_flags (static_chain_flags, ecf_flags, false)) return true; if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) - return false; + return (!side_effects && (ecf_flags & ECF_LOOPING_CONST_OR_PURE)); if (loads && !loads->every_base) return true; if (ecf_flags & ECF_PURE) - return false; + return (!side_effects && (ecf_flags & ECF_LOOPING_CONST_OR_PURE)); return stores && !stores->every_base; } @@ -416,11 +416,11 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags) && remove_useless_eaf_flags (static_chain_flags, ecf_flags, false)) return true; if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) - return false; + return (!side_effects && (ecf_flags & ECF_LOOPING_CONST_OR_PURE)); if (loads && !loads->every_base) return true; if (ecf_flags & ECF_PURE) - return false; + return (!side_effects && (ecf_flags & ECF_LOOPING_CONST_OR_PURE)); return stores && !stores->every_base; } @@ -925,6 +925,18 @@ merge_call_side_effects (modref_summary *cur_summary, auto_vec parm_map; modref_parm_map chain_map; bool changed = false; + int flags = gimple_call_flags (stmt); + + if (!cur_summary->side_effects && callee_summary->side_effects) + { + if (dump_file) + fprintf (dump_file, " - merging side effects.\n"); + cur_summary->side_effects = true; + changed = true; + } + + if (flags & (ECF_CONST | ECF_NOVOPS)) + return changed; /* We can not safely optimize based on summary of callee if it does not always bind to current def: it is possible that memory load @@ -988,12 +1000,6 @@ merge_call_side_effects (modref_summary *cur_summary, changed = true; } } - if (!cur_summary->side_effects - && callee_summary->side_effects) - { - cur_summary->side_effects = true; - changed = true; - } return changed; } @@ -1091,7 +1097,7 @@ process_fnspec (modref_summary *cur_summary, attr_fnspec fnspec = gimple_call_fnspec (call); int flags = gimple_call_flags (call); - if (!(flags & (ECF_CONST | ECF_NOVOPS)) + if (!(flags & (ECF_CONST | ECF_NOVOPS | ECF_PURE)) || (flags & ECF_LOOPING_CONST_OR_PURE) || (cfun->can_throw_non_call_exceptions && stmt_could_throw_p (cfun, call))) @@ -1101,6 +1107,8 @@ process_fnspec (modref_summary *cur_summary, if (cur_summary_lto) cur_summary_lto->side_effects = true; } + if (flags & (ECF_CONST | ECF_NOVOPS)) + return true; if (!fnspec.known_p ()) { if (dump_file && gimple_call_builtin_p (call, BUILT_IN_NORMAL)) @@ -1203,7 +1211,8 @@ analyze_call (modref_summary *cur_summary, modref_summary_lto *cur_summary_lto, /* Check flags on the function call. In certain cases, analysis can be simplified. */ int flags = gimple_call_flags (stmt); - if (flags & (ECF_CONST | ECF_NOVOPS)) + if ((flags & (ECF_CONST | ECF_NOVOPS)) + && !(flags & ECF_LOOPING_CONST_OR_PURE)) { if (dump_file) fprintf (dump_file, @@ -3963,7 +3972,8 @@ static bool propagate_unknown_call (cgraph_node *node, cgraph_edge *e, int ecf_flags, modref_summary *cur_summary, - modref_summary_lto *cur_summary_lto) + modref_summary_lto *cur_summary_lto, + bool nontrivial_scc) { bool changed = false; class fnspec_summary *fnspec_sum = fnspec_summaries->get (e); @@ -3973,12 +3983,12 @@ propagate_unknown_call (cgraph_node *node, if (e->callee && builtin_safe_for_const_function_p (&looping, e->callee->decl)) { - if (cur_summary && !cur_summary->side_effects) + if (looping && cur_summary && !cur_summary->side_effects) { cur_summary->side_effects = true; changed = true; } - if (cur_summary_lto && !cur_summary_lto->side_effects) + if (looping && cur_summary_lto && !cur_summary_lto->side_effects) { cur_summary_lto->side_effects = true; changed = true; @@ -3986,8 +3996,9 @@ propagate_unknown_call (cgraph_node *node, return changed; } - if (!(ecf_flags & (ECF_CONST | ECF_NOVOPS)) - || (ecf_flags & ECF_LOOPING_CONST_OR_PURE)) + if (!(ecf_flags & (ECF_CONST | ECF_NOVOPS | ECF_PURE)) + || (ecf_flags & ECF_LOOPING_CONST_OR_PURE) + || nontrivial_scc) { if (cur_summary && !cur_summary->side_effects) { @@ -4000,6 +4011,8 @@ propagate_unknown_call (cgraph_node *node, changed = true; } } + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) + return changed; if (fnspec_sum && compute_parm_map (e, &parm_map)) @@ -4126,6 +4139,8 @@ modref_propagate_in_scc (cgraph_node *component_node) while (changed) { + bool nontrivial_scc + = ((struct ipa_dfs_info *) component_node->aux)->next_cycle; changed = false; for (struct cgraph_node *cur = component_node; cur; cur = ((struct ipa_dfs_info *) cur->aux)->next_cycle) @@ -4151,14 +4166,12 @@ modref_propagate_in_scc (cgraph_node *component_node) for (cgraph_edge *e = cur->indirect_calls; e; e = e->next_callee) { - if (e->indirect_info->ecf_flags & (ECF_CONST | ECF_NOVOPS)) - continue; if (dump_file) - fprintf (dump_file, " Indirect call" - "collapsing loads\n"); + fprintf (dump_file, " Indirect call\n"); if (propagate_unknown_call (node, e, e->indirect_info->ecf_flags, - cur_summary, cur_summary_lto)) + cur_summary, cur_summary_lto, + nontrivial_scc)) { changed = true; remove_useless_summaries (node, &cur_summary, @@ -4180,8 +4193,9 @@ modref_propagate_in_scc (cgraph_node *component_node) modref_summary_lto *callee_summary_lto = NULL; struct cgraph_node *callee; - if (flags & (ECF_CONST | ECF_NOVOPS) - || !callee_edge->inline_failed) + if (!callee_edge->inline_failed + || ((flags & (ECF_CONST | ECF_NOVOPS)) + && !(flags & ECF_LOOPING_CONST_OR_PURE))) continue; /* Get the callee and its summary. */ @@ -4210,7 +4224,8 @@ modref_propagate_in_scc (cgraph_node *component_node) " or not available\n"); changed |= propagate_unknown_call (node, callee_edge, flags, - cur_summary, cur_summary_lto); + cur_summary, cur_summary_lto, + nontrivial_scc); if (!cur_summary && !cur_summary_lto) break; continue; @@ -4226,7 +4241,8 @@ modref_propagate_in_scc (cgraph_node *component_node) fprintf (dump_file, " No call target summary\n"); changed |= propagate_unknown_call (node, callee_edge, flags, - cur_summary, NULL); + cur_summary, NULL, + nontrivial_scc); } if (cur_summary_lto && !(callee_summary_lto = summaries_lto->get (callee))) @@ -4235,9 +4251,27 @@ modref_propagate_in_scc (cgraph_node *component_node) fprintf (dump_file, " No call target summary\n"); changed |= propagate_unknown_call (node, callee_edge, flags, - NULL, cur_summary_lto); + NULL, cur_summary_lto, + nontrivial_scc); } + if (callee_summary && !cur_summary->side_effects + && (callee_summary->side_effects + || callee_edge->recursive_p ())) + { + cur_summary->side_effects = true; + changed = true; + } + if (callee_summary_lto && !cur_summary_lto->side_effects + && (callee_summary_lto->side_effects + || callee_edge->recursive_p ())) + { + cur_summary_lto->side_effects = true; + changed = true; + } + if (flags & (ECF_CONST | ECF_NOVOPS)) + continue; + /* We can not safely optimize based on summary of callee if it does not always bind to current def: it is possible that memory load was optimized out earlier which may not happen in @@ -4265,12 +4299,6 @@ modref_propagate_in_scc (cgraph_node *component_node) changed |= cur_summary->loads->merge (callee_summary->loads, &parm_map, &chain_map, !first); - if (!cur_summary->side_effects - && callee_summary->side_effects) - { - cur_summary->side_effects = true; - changed = true; - } if (!ignore_stores) { changed |= cur_summary->stores->merge @@ -4289,12 +4317,6 @@ modref_propagate_in_scc (cgraph_node *component_node) changed |= cur_summary_lto->loads->merge (callee_summary_lto->loads, &parm_map, &chain_map, !first); - if (!cur_summary_lto->side_effects - && callee_summary_lto->side_effects) - { - cur_summary_lto->side_effects = true; - changed = true; - } if (!ignore_stores) { changed |= cur_summary_lto->stores->merge