From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id A6E813858C62 for ; Thu, 19 Jan 2023 17:41:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A6E813858C62 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id D94E75D135; Thu, 19 Jan 2023 17:41:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1674150065; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=lAdbPC7UiBKvk4H3KSpUHFH2oMD06cJTlWvdHC72Opg=; b=dzej4EQdQqjSXVuqmQ3Jk6dHtV8J+2sj1v4bl9xds9wgMzFzHwk2ba9/PB8bB/4Yn0wRAd ErXrmj9PxXdQnSDkwS3ht9VAdUKB6xm3nUeT2/AmbCbS6AGsaXYxdARFAkS9YaBScRneJU 8gNRHLWFTAfxTOWp6zwaghngA07KEPk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1674150065; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=lAdbPC7UiBKvk4H3KSpUHFH2oMD06cJTlWvdHC72Opg=; b=JLNwhKUxDiVHnjNuT1H1ACEmOxHCz0UXk3JLQ9/DuhE0ggpTLwfP8kY45gtk6QP6bEOGUt gzvc9iVv+Mh2A3Aw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id CB8E7134F5; Thu, 19 Jan 2023 17:41:05 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id FuusMbGAyWOIDAAAMHmgww (envelope-from ); Thu, 19 Jan 2023 17:41:05 +0000 From: Martin Jambor To: Jan Hubicka Cc: Martin =?utf-8?Q?Li=C5=A1ka?= , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] IPA: do not release body if still needed In-Reply-To: References: <9070c8aa-3496-7424-6c4b-33d1e5606b31@suse.cz> User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/28.1 (x86_64-suse-linux-gnu) Date: Thu, 19 Jan 2023 18:41:05 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, On Wed, Jan 18 2023, Jan Hubicka wrote: >> The code removing function bodies when the last call graph clone of a >> node is removed is too aggressive when there are nodes up the >> clone_of chain which still need them. Fixed by expanding the check. >> >> gcc/ChangeLog: >> >> 2023-01-18 Martin Jambor >> >> PR ipa/107944 >> * cgraph.cc (cgraph_node::remove): Check whether nodes up the >> lcone_of chain also do not need the body. >> --- >> gcc/cgraph.cc | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc >> index 5e60c2b73db..5f72ace9b57 100644 >> --- a/gcc/cgraph.cc >> +++ b/gcc/cgraph.cc >> @@ -1893,8 +1893,18 @@ cgraph_node::remove (void) >> else if (clone_of) >> { >> clone_of->clones = next_sibling_clone; >> - if (!clone_of->analyzed && !clone_of->clones && !clones) >> - clone_of->release_body (); >> + if (!clones) >> + { >> + bool need_body = false; >> + for (cgraph_node *n = clone_of; n; n = n->clone_of) >> + if (n->analyzed || n->clones) >> + { >> + need_body = true; > If you want to walk immediate clones and see if any of them is needed, I > wonder why you don't also walk recursively clones of clones? The intent is to avoid PR 100413. When a node is being removed, we need to figure out if it is the last one needing the body. If a (possibly indirect) clone_of has a clone, they are still to be materialized and so the body is necessary. If those other clones are all also going to be removed as unreachable rather than materialized, then the last one will release the body. > > Original idea was that the clones should be materialized and removed one > by one (or proved unreachable and just removed) and once we remove last > one, we should figure out that body is not needed. For that one does not > not need the walk at all. So if you have clones of F like this F / \ A B / \ C D / \ M R And then A and C are removed as unreachable or materialized, M is materialized, and afterwards R is removed as unreachable then the removal of R also has to trigger releasing the body. In order not to trigger the bug we are fixing, it needs to check that neither of D, B or F need the body themselves or have any clones which need it. Thus the walk. Now, the method as an alternative point where it releases the body a few lines below, and having two looks a bit clumsy. But it is not entirely straightforward how to combine the conditions guarding the two places. > > How exactly we end up with clones that are not analyzed? I hope I am not misremembering but analyzed gets cleared when a node is there just to hold body for its clones and is no longer necessary for any other reason, no? Martin > > Honza >> + break; >> + } >> + if (!need_body) >> + clone_of->release_body (); >> + } >> } >> if (next_sibling_clone) >> next_sibling_clone->prev_sibling_clone = prev_sibling_clone; >> -- >> 2.39.0 >>