From patchwork Fri Nov 22 16:27:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1199549 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=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-514413-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="KN8Uz1h+"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47KMKF0Hp3z9sPK for ; Sat, 23 Nov 2019 03:27:26 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type; q=dns; s=default; b=aW/46aOQbXlOBRskwQPLA77+8+BWgV8bkHk8jk614q0myWXYbh UPiE5nGmHuNOeAYGkfnK4EV8xUzwQ/MwIQ/G/9vj8NZtYn5lJig/IMRSs2s70sWn SDWFO2yRPu/YWitVY1/4H1o+5XEFzXgA2e54HUiWjS+U5h07AOdeyKzdU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type; s= default; bh=Pr3WjMJ1T7IjZipwc3KJhVqD1xQ=; b=KN8Uz1h+4JpncJO+XLH8 V7fFJNctiak7BfVtoeCXEHctkNT1StjXv/EbEoNan2jXRhN3EKkKuw738UnJaQAG i1tc0G8QjlddZ2RuYHSrYxi+2W0IZXz6nEDR00dJLCU14z8df5dPbkeLbYdbiHnH 9q/NS1hBrB8iJF1HgmkEMfA= Received: (qmail 62872 invoked by alias); 22 Nov 2019 16:27:19 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 62864 invoked by uid 89); 22 Nov 2019 16:27:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy=symtab_node, sk:bitmap_, symbol_table X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 22 Nov 2019 16:27:17 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 46DCCABE9 for ; Fri, 22 Nov 2019 16:27:15 +0000 (UTC) From: Martin Jambor To: GCC Patches Cc: Jan Hubicka Subject: [PATCH] ipa: Prevent materialization of clones with removed bodies (PR 92109) User-Agent: Notmuch/0.29.1 (https://notmuchmail.org) Emacs/26.3 (x86_64-suse-linux-gnu) Date: Fri, 22 Nov 2019 17:27:14 +0100 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes Hi, this bug is an interesting one. The immediate cause of the ICE is that IPA-SRA modification phase baked into clone materialization was expecting to see a body already changed by IPA-CP while it was actually looking at the original function. That was because we were materializing the (offline) clone in an ltrans from which it was called but where it did not belonged at all. remove_unreachable_nodes realized this and wanted to keep just the declaration and remove the body - and therefor also all traces of the IPA-CP clone which was necessary just for materialization. But materialization code still attempted to resurrect the clone. Honza proposed immediately removing any node which only needs to have a declaration from the tree of clones to prevent materialization code from even considering them which is what the following patch does. Unfortunately, I was not able to come up with a small testcase, the bug is triggered by fragile partitioning decisions. I have bootstrapped, LTO-bootstrapped and tested the patch on an x86_64-linux, OK for trunk? Thanks, Martin 2019-11-21 Martin Jambor PR ipa/92109 * cgraph.h (cgraph_node::remove_from_clone_tree): Declare. * cgraphclones.c (cgraph_node::remove_from_clone_tree): New method. (cgraph_materialize_clone): Move removel from clone tree to the the new method and use it instead. * ipa.c (symbol_table::remove_unreachable_nodes): When removing bodies of clones, also remove it from the clone tree. --- gcc/cgraph.h | 4 ++++ gcc/cgraphclones.c | 35 ++++++++++++++++++++++------------- gcc/ipa.c | 11 ++++++++--- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/gcc/cgraph.h b/gcc/cgraph.h index a4f14743f00..0d2442c997c 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -967,6 +967,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node ipa_param_adjustments *param_adjustments, const char * suffix, unsigned num_suffix); + /* Remove the node from the tree of virtual and inline clones and make it a + standalone node - not a clone any more. */ + void remove_from_clone_tree (); + /* cgraph node being removed from symbol table; see if its entry can be replaced by other inline clone. */ cgraph_node *find_replacement (void); diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index bfcebb20495..ac5c57a47aa 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -1013,6 +1013,22 @@ cgraph_node::create_version_clone_with_body return new_version_node; } +/* Remove the node from the tree of virtual and inline clones and make it a + standalone node - not a clone any more. */ + +void cgraph_node::remove_from_clone_tree () +{ + if (next_sibling_clone) + next_sibling_clone->prev_sibling_clone = prev_sibling_clone; + if (prev_sibling_clone) + prev_sibling_clone->next_sibling_clone = next_sibling_clone; + else + clone_of->clones = next_sibling_clone; + next_sibling_clone = NULL; + prev_sibling_clone = NULL; + clone_of = NULL; +} + /* Given virtual clone, turn it into actual clone. */ static void @@ -1033,22 +1049,15 @@ cgraph_materialize_clone (cgraph_node *node) dump_function_to_file (node->decl, symtab->dump_file, dump_flags); } + cgraph_node *clone_of = node->clone_of; /* Function is no longer clone. */ - if (node->next_sibling_clone) - node->next_sibling_clone->prev_sibling_clone = node->prev_sibling_clone; - if (node->prev_sibling_clone) - node->prev_sibling_clone->next_sibling_clone = node->next_sibling_clone; - else - node->clone_of->clones = node->next_sibling_clone; - node->next_sibling_clone = NULL; - node->prev_sibling_clone = NULL; - if (!node->clone_of->analyzed && !node->clone_of->clones) + node->remove_from_clone_tree (); + if (!clone_of->analyzed && !clone_of->clones) { - node->clone_of->release_body (); - node->clone_of->remove_callees (); - node->clone_of->remove_all_references (); + clone_of->release_body (); + clone_of->remove_callees (); + clone_of->remove_all_references (); } - node->clone_of = NULL; bitmap_obstack_release (NULL); } diff --git a/gcc/ipa.c b/gcc/ipa.c index 0c92980db46..2404024d722 100644 --- a/gcc/ipa.c +++ b/gcc/ipa.c @@ -520,9 +520,14 @@ symbol_table::remove_unreachable_nodes (FILE *file) reliably. */ if (node->alias || node->thunk.thunk_p) ; - else if (!body_needed_for_clonning.contains (node->decl) - && !node->alias && !node->thunk.thunk_p) - node->release_body (); + else if (!body_needed_for_clonning.contains (node->decl)) + { + /* Make the node a non-clone so that we do not attempt to + materialize it later. */ + if (node->clone_of) + node->remove_from_clone_tree (); + node->release_body (); + } else if (!node->clone_of) gcc_assert (in_lto_p || DECL_RESULT (node->decl)); if (node->definition && !node->alias && !node->thunk.thunk_p)