From patchwork Thu Apr 9 18:09:19 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1268738 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.cz Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48yq0m0v7Nz9sPF for ; Fri, 10 Apr 2020 04:09:26 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A489F385BF81; Thu, 9 Apr 2020 18:09:23 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 95030385BF81 for ; Thu, 9 Apr 2020 18:09:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 95030385BF81 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mjambor@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id DC236AC19 for ; Thu, 9 Apr 2020 18:09:18 +0000 (UTC) From: Martin Jambor To: GCC Patches Subject: [PATCH] ipa: Make call redirection detect already adjusted calls (PR 93621) User-Agent: Notmuch/0.29.3 (https://notmuchmail.org) Emacs/26.3 (x86_64-suse-linux-gnu) Date: Thu, 09 Apr 2020 20:09:19 +0200 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-3054.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: , Cc: Jan Hubicka Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, PR 93621 testcase makes redirect_call_stmt_to_callee wrongly assume that a call statement needs redirecting but then rightly fails an assert ensuring the call statement parameters have not already been adjusted because they were already created adjusted as part of thunk expansion. The test fails because the decl in the call call statement is different than the decl of the callee, because the latter was created in save_inline_function_body. This patch adds a way to link these two and detect the situation in redirect_call_stmt_to_callee. I tend to think the deallocation bit is important for JIT, althoug in the course of normal compilation it happens only just before the compiler shuts down. This patch passes bootstrap and LTO bootstrap and testing on x86_64-linux, LTO profiledbootstrap is underway. Is it OK the way it is or do we for example want to move the new summary to ipa-utils so that we don't have to include ipa-inline in two new files? Should the deallocation be someplace else? Any other comments, preferences, constructive criticism? :-) Thanks, Martin 2020-04-09 Martin Jambor PR ipa/93621 * ipa-inline.h (ipa_saved_clone_sources): Declare. * ipa-inline-transform.c (ipa_saved_clone_sources): New variable. (save_inline_function_body): Link the new body holder with the previous one. * cgraph.c: Include ipa-inline.h. (cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl from the statement in ipa_saved_clone_sources. * cgraphunit.c: Include ipa-inline.h. (expand_all_functions): Free ipa_saved_clone_sources. --- gcc/ChangeLog | 13 +++++++++++++ gcc/cgraph.c | 11 +++++++++++ gcc/cgraphunit.c | 4 +++- gcc/ipa-inline-transform.c | 19 +++++++++++++++++++ gcc/ipa-inline.h | 1 + gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/g++.dg/ipa/pr93621.C | 29 +++++++++++++++++++++++++++++ 7 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr93621.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index e10fb251c16..d68f5a68f3e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,16 @@ +2020-04-09 Martin Jambor + + PR ipa/93621 + * ipa-inline.h (ipa_saved_clone_sources): Declare. + * ipa-inline-transform.c (ipa_saved_clone_sources): New variable. + (save_inline_function_body): Link the new body holder with the + previous one. + * cgraph.c: Include ipa-inline.h. + (cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl from + the statement in ipa_saved_clone_sources. + * cgraphunit.c: Include ipa-inline.h. + (expand_all_functions): Free ipa_saved_clone_sources. + 2020-04-05 Zachary Spytz * extend.texi: Add free to list of ISO C90 functions that diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 6b780f80eb3..b4b00b3b1c1 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see #include "attribs.h" #include "selftest.h" #include "tree-into-ssa.h" +#include "ipa-inline.h" /* FIXME: Only for PROP_loops, but cgraph shouldn't have to know about this. */ #include "tree-pass.h" @@ -1470,6 +1471,16 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) || decl == e->callee->decl) return e->call_stmt; + if (decl && ipa_saved_clone_sources) + { + cgraph_node **p = ipa_saved_clone_sources->get (e->callee); + if (p && decl == (*p)->decl) + { + gimple_call_set_fndecl (e->call_stmt, e->callee->decl); + return e->call_stmt; + } + } + if (flag_checking && decl) { cgraph_node *node = cgraph_node::get (decl); diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 0e255f25b7d..a1ace95879a 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -205,6 +205,7 @@ along with GCC; see the file COPYING3. If not see #include "lto-section-names.h" #include "stringpool.h" #include "attribs.h" +#include "ipa-inline.h" /* Queue of cgraph nodes scheduled to be added into cgraph. This is a secondary queue used during optimization to accommodate passes that @@ -2481,7 +2482,8 @@ expand_all_functions (void) symtab->process_new_functions (); free_gimplify_stack (); - + delete ipa_saved_clone_sources; + ipa_saved_clone_sources = NULL; free (order); } diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c index eed992d314d..cd64a643506 100644 --- a/gcc/ipa-inline-transform.c +++ b/gcc/ipa-inline-transform.c @@ -531,6 +531,11 @@ inline_call (struct cgraph_edge *e, bool update_original, return new_edges_found; } +/* For each node that was made the holder of function body by + save_inline_function_body, this summary contains pointer to the previous + holder of the body. */ + +function_summary *ipa_saved_clone_sources; /* Copy function body of NODE and redirect all inline clones to it. This is done before inline plan is applied to NODE when there are @@ -588,6 +593,20 @@ save_inline_function_body (struct cgraph_node *node) first_clone->next_sibling_clone = NULL; gcc_assert (!first_clone->prev_sibling_clone); } + + cgraph_node *prev_body_holder = node; + if (!ipa_saved_clone_sources) + ipa_saved_clone_sources = new function_summary (symtab); + else + { + cgraph_node **p = ipa_saved_clone_sources->get (node); + if (p) + { + prev_body_holder = *p; + gcc_assert (prev_body_holder); + } + } + *ipa_saved_clone_sources->get_create (first_clone) = prev_body_holder; first_clone->clone_of = NULL; /* Now node in question has no clones. */ diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h index 5025b6045fc..c596f77d0e7 100644 --- a/gcc/ipa-inline.h +++ b/gcc/ipa-inline.h @@ -65,6 +65,7 @@ void clone_inlined_nodes (struct cgraph_edge *e, bool, bool, int *); extern int ncalls_inlined; extern int nfunctions_inlined; +extern function_summary *ipa_saved_clone_sources; /* Return estimated size of the inline sequence of EDGE. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 88bab5d3d19..88f33ab2b90 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-04-04 Martin Jambor + + PR ipa/93621 + * g++.dg/ipa/pr93621.C: New test. + 2020-04-05 Iain Sandoe * g++.dg/coroutines/torture/co-await-14-template-traits.C: Rename... diff --git a/gcc/testsuite/g++.dg/ipa/pr93621.C b/gcc/testsuite/g++.dg/ipa/pr93621.C new file mode 100644 index 00000000000..ffe6bbdae2e --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr93621.C @@ -0,0 +1,29 @@ +// PR ipa/93621 +// { dg-do compile } +// { dg-options "-O3 --param ipa-cp-eval-threshold=100 --param large-function-growth=60 --param large-function-insns=10 --param uninlined-thunk-insns=1000" } + +typedef enum { X } E; +struct A { + virtual void bar (); +}; +struct B { + virtual E fn (const char *, int, int *) = 0; +}; +struct C : A, B { + E fn (const char *, int, int *); + void fn2 (); + B *foo; +}; +void C::fn2 () { + if (!foo) + return; + foo->fn (0, 0, 0); +} +E +C::fn (const char *, int, int *) +{ + fn2 (); + foo = 0; + fn (0, 0, 0); + return X; +}