From patchwork Wed Nov 3 00:47:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 46977 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 9BF483857C43 for ; Wed, 3 Nov 2021 00:48:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9BF483857C43 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1635900481; bh=Ih4HO3MMgKLDJq0ZmnYcupbl3wK5S0YVv9atESu8YIs=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=g6XMjEPDhDXtANWV7UngxysCX04Gjal9QuuX+5pKA0Pafy5lm/PHOmqTiKLwtrXpe DbwGyGUXj4BIBW7KpQz8qij8tz/7qA/pLLr95ydeAqnp9eX4Vm/PgFyjS2mMTMvfFZ u/uJYv+3t0dGf8yI8HABLEi9aG0gR0o3DOMrQ5E4= 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 0A8513858C27 for ; Wed, 3 Nov 2021 00:47:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0A8513858C27 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 0235B2827E1; Wed, 3 Nov 2021 01:47:30 +0100 (CET) Date: Wed, 3 Nov 2021 01:47:30 +0100 To: gcc-patches@gcc.gnu.org Subject: Fix wrong code caused by ipa-modref retslot handling Message-ID: <20211103004730.GE30167@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.4 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, this patch fixes (quite nasty) thinko in how I propagate EAF flags from callee to caller. In this case some flags needs to be changed. In particular - EAF_NOT_RETURNED in callee does not really mean EAF_NOT_RETURNED in caller since we speak of different return values - if callee escapes the parametr, we caller may return it - for retslot the rewritting is even bit more funny, since escaping to of return slot to return slot is not really an escape, however escape of argument to itself is. This patch should correct all of the cases above and does fix the testcase from PR103040. Bootstrapped/regtested x86_64 with all languages. Also lto-bootstrapped. Comitted. gcc/ChangeLog: PR ipa/103040 * ipa-modref.c (callee_to_caller_flags): New function. (modref_eaf_analysis::analyze_ssa_name): Use it. (ipa_merge_modref_summary_after_inlining): Fix whitespace. gcc/testsuite/ChangeLog: * g++.dg/torture/pr103040.C: New test. diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index f9eafc974d5..db071d2212f 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -1694,7 +1694,8 @@ private: /* Call statements may return their parameters. Consider argument number ARG of USE_STMT and determine flags that can needs to be cleared in case pointer possibly indirectly references from ARG I is returned. - LATTICE, DEPTH and ipa are same as in analyze_ssa_name. */ + LATTICE, DEPTH and ipa are same as in analyze_ssa_name. + ARG is set to -1 for static chain. */ void modref_eaf_analysis::merge_call_lhs_flags (gcall *call, int arg, @@ -1708,30 +1709,56 @@ modref_eaf_analysis::merge_call_lhs_flags (gcall *call, int arg, /* If we know that function returns given argument and it is not ARG we can still be happy. */ - int flags = gimple_call_return_flags (call); - if ((flags & ERF_RETURNS_ARG) - && (flags & ERF_RETURN_ARG_MASK) != arg) - return; - - int eaf_flags = gimple_call_arg_flags (call, arg); - - if (eaf_flags & (EAF_NOT_RETURNED | EAF_UNUSED)) - return; + if (arg >= 0) + { + int flags = gimple_call_return_flags (call); + if ((flags & ERF_RETURNS_ARG) + && (flags & ERF_RETURN_ARG_MASK) != arg) + return; + } /* If return value is SSA name determine its flags. */ if (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME) { tree lhs = gimple_call_lhs (call); - merge_with_ssa_name (name, lhs, - (deref || (eaf_flags & EAF_NOT_RETURNED_DIRECTLY))); + merge_with_ssa_name (name, lhs, deref); } /* In the case of memory store we can do nothing. */ - else if (eaf_flags & EAF_NOT_RETURNED_DIRECTLY) + else if (deref) m_lattice[index].merge (deref_flags (0, false)); else m_lattice[index].merge (0); } +/* CALL_FLAGS are EAF_FLAGS of the argument. Turn them + into flags for caller, update LATTICE of corresponding + argument if needed. */ + +static int +callee_to_caller_flags (int call_flags, bool ignore_stores, + modref_lattice &lattice) +{ + /* call_flags is about callee returning a value + that is not the same as caller returning it. */ + call_flags |= EAF_NOT_RETURNED + | EAF_NOT_RETURNED_DIRECTLY; + /* TODO: We miss return value propagation. + Be conservative and if value escapes to memory + also mark it as escaping. */ + if (!ignore_stores && !(call_flags & EAF_UNUSED)) + { + if (!(call_flags & EAF_NOESCAPE)) + lattice.merge (~(EAF_NOT_RETURNED | EAF_UNUSED)); + if (!(call_flags & (EAF_NODIRECTESCAPE | EAF_NOESCAPE))) + lattice.merge (~(EAF_NOT_RETURNED_DIRECTLY + | EAF_NOT_RETURNED + | EAF_UNUSED)); + } + else + call_flags |= ignore_stores_eaf_flags; + return call_flags; +} + /* Analyze EAF flags for SSA name NAME and store result to LATTICE. LATTICE is an array of modref_lattices. DEPTH is a recursion depth used to make debug output prettier. @@ -1843,12 +1870,55 @@ modref_eaf_analysis::analyze_ssa_name (tree name) may make LHS to escape. See PR 98499. */ if (gimple_call_return_slot_opt_p (call) && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call)))) - m_lattice[index].merge (gimple_call_retslot_flags (call)); + { + int call_flags = gimple_call_retslot_flags (call); + bool isretslot = false; + + if (DECL_RESULT (current_function_decl) + && DECL_BY_REFERENCE + (DECL_RESULT (current_function_decl))) + isretslot = ssa_default_def + (cfun, + DECL_RESULT (current_function_decl)) + == name; + + /* Passing returnslot to return slot is special because + not_returned and escape has same meaning. + However passing arg to return slot is different. If + the callee's return slot is returned it means that + arg is written to itself which is an escape. */ + if (!isretslot) + { + if (!(call_flags & (EAF_NOT_RETURNED | EAF_UNUSED))) + m_lattice[index].merge (~(EAF_NOESCAPE + | EAF_UNUSED)); + if (!(call_flags & (EAF_NOT_RETURNED_DIRECTLY + | EAF_UNUSED + | EAF_NOT_RETURNED))) + m_lattice[index].merge (~(EAF_NODIRECTESCAPE + | EAF_NOESCAPE + | EAF_UNUSED)); + call_flags = callee_to_caller_flags + (call_flags, false, + m_lattice[index]); + } + m_lattice[index].merge (call_flags); + } } if (gimple_call_chain (call) && (gimple_call_chain (call) == name)) - m_lattice[index].merge (gimple_call_static_chain_flags (call)); + { + int call_flags = gimple_call_static_chain_flags (call); + if (!ignore_retval + && !(call_flags & (EAF_NOT_RETURNED | EAF_UNUSED))) + merge_call_lhs_flags (call, -1, name, false); + call_flags = callee_to_caller_flags + (call_flags, ignore_stores, + m_lattice[index]); + if (!(ecf_flags & (ECF_CONST | ECF_NOVOPS))) + m_lattice[index].merge (call_flags); + } /* Process internal functions and right away. */ bool record_ipa = m_ipa && !gimple_call_internal_p (call); @@ -1860,42 +1930,45 @@ modref_eaf_analysis::analyze_ssa_name (tree name) /* Name is directly passed to the callee. */ if (gimple_call_arg (call, i) == name) { + int call_flags = gimple_call_arg_flags (call, i); + if (!ignore_retval && !(call_flags + & (EAF_NOT_RETURNED | EAF_UNUSED))) + merge_call_lhs_flags + (call, i, name, + call_flags & EAF_NOT_RETURNED_DIRECTLY); if (!(ecf_flags & (ECF_CONST | ECF_NOVOPS))) { - int call_flags = gimple_call_arg_flags (call, i) - | EAF_NOT_RETURNED - | EAF_NOT_RETURNED_DIRECTLY; - if (ignore_stores) - call_flags |= ignore_stores_eaf_flags; - + call_flags = callee_to_caller_flags + (call_flags, ignore_stores, + m_lattice[index]); if (!record_ipa) m_lattice[index].merge (call_flags); else m_lattice[index].add_escape_point (call, i, call_flags, true); } - if (!ignore_retval) - merge_call_lhs_flags (call, i, name, false); } /* Name is dereferenced and passed to a callee. */ else if (memory_access_to (gimple_call_arg (call, i), name)) { + int call_flags = deref_flags + (gimple_call_arg_flags (call, i), ignore_stores); + if (!ignore_retval + && !(call_flags & (EAF_NOT_RETURNED | EAF_UNUSED))) + merge_call_lhs_flags (call, i, name, true); if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) m_lattice[index].merge_direct_load (); else { - int call_flags = deref_flags - (gimple_call_arg_flags (call, i) - | EAF_NOT_RETURNED - | EAF_NOT_RETURNED_DIRECTLY, ignore_stores); + call_flags = callee_to_caller_flags + (call_flags, ignore_stores, + m_lattice[index]); if (!record_ipa) m_lattice[index].merge (call_flags); else m_lattice[index].add_escape_point (call, i, - call_flags, false); + call_flags, false); } - if (!ignore_retval) - merge_call_lhs_flags (call, i, name, true); } } } @@ -4068,7 +4141,7 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge) needed = true; } if (to_info_lto - && (int)to_info_lto->arg_flags.length () > ee->parm_index) + && (int)to_info_lto->arg_flags.length () > ee->parm_index) { int flags = callee_info_lto && callee_info_lto->arg_flags.length () > ee->arg diff --git a/gcc/testsuite/g++.dg/torture/pr103040.C b/gcc/testsuite/g++.dg/torture/pr103040.C new file mode 100644 index 00000000000..d6348952826 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr103040.C @@ -0,0 +1,37 @@ +// { dg-do run } +// { dg-additional-options "-fno-early-inlining" } +struct S101273 +{ + int x; + S101273* impl; + S101273(int x) + { + this->x = x; + this->impl = this; + } + S101273(const S101273 &o) + { + this->x = o.x; + this->impl = this; + } + ~S101273() { } +}; + +S101273 makeS101273() +{ + return S101273(2); +} + +S101273 nrvo101273() +{ + S101273 ret = makeS101273(); + return ret; +} + +int main() +{ + auto nrvo = nrvo101273(); + if(&nrvo != nrvo.impl) __builtin_abort (); + + return 0; +}