From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 1C5663858C83 for ; Wed, 15 Mar 2023 17:12:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1C5663858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail 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 688851FD87; Wed, 15 Mar 2023 17:12:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1678900348; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=e2wuFT+IIPm929oJRJNcZ4xoI+Mu6IPDh978QTOz9HY=; b=dxvRxo+h8zF7YZWgeF8BYKWDTu6Z5xkGiHNcYuzF0IlGzafM+BlmA/HFED3+kXjOYk483A V6L7vj4DuWSw5jPNFFk2E6JCA5ZOxUhg0mGsK75g7syK3ROQvjk1VKudwN8LBphI206vsg 7LTjwnt0RVepn37pucruSyLWy8DWKWI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1678900348; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=e2wuFT+IIPm929oJRJNcZ4xoI+Mu6IPDh978QTOz9HY=; b=f9d7FyBccO46sJxbfO1oMsa80ObrcQUPYKqhx43Eia6x451PtTQP8ThOWo9B1bvqM41Fi4 UkzBvGd+VfHBtpBA== 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 5B13713A00; Wed, 15 Mar 2023 17:12:28 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id o78wFnz8EWQlIQAAMHmgww (envelope-from ); Wed, 15 Mar 2023 17:12:28 +0000 From: Martin Jambor To: Arsen =?utf-8?Q?Arsenovi=C4=87?= Cc: GCC Mailing List , Jan Hubicka , Martin Liska Subject: Re: cgraph: does node->inlined_to imply node->clones is non-empty? In-Reply-To: <86bkkwude3.fsf@aarsen.me> References: <86bkkwude3.fsf@aarsen.me> User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/28.2 (x86_64-suse-linux-gnu) Date: Wed, 15 Mar 2023 18:12:27 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_SOFTFAIL,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello, I had been aware of your message even before Martin Li=C5=A1ka pointed to i= t, but I could not answer the questions without looking into the problem myself. On Mon, Mar 13 2023, Arsen Arsenovi=C4=87 via Gcc wrote: > Hi, > > I was debugging PR96059 and ran into a question which does not seem > obvious from the code. Thanks for looking into old bugs, it really is appreciated! > When the original inline > happens, ipa-inline-transform.cc:clone_inlined_nodes decides not to make > a clone, since the function being cloned is a master clone but with no > non-inline clones. The reason is rather that cloning can simply be avoided if you know that you do not need an offline copy, for anything, be it other normal calls, calls from outside of the compilation unit, indirect calls, virtual calls, calls through aliases, thunks... that you do not need the intact body of the function to create other inline copies, other specialized clones... and maybe I forgot about something. But this is an efficiency thing. > > For the test case in the PR, in ipa.cc:remove_unreachable_nodes, GCC > seems to try to remove an unreachable function that was already inlined > into a different unreachable function. No, it fails to remove it. It is still there but should not have been, that is the problem. > > This decision later trips up the gcc_assert in: > > /* Inline clones might be kept around so their materializing allows fur= ther > cloning. If the function the clone is inlined into is removed, we n= eed > to turn it into normal cone. */ > FOR_EACH_FUNCTION (node) > { > if (node->inlined_to > && !node->callers) > { > gcc_assert (node->clones); > node->inlined_to =3D NULL; > update_inlined_to_pointer (node, node); > } > node->aux =3D NULL; > } > > .. because it is expecting that an inlined function without callers > (which is necessarily true here as this function is unreachable and so > was ->remove ()'d earlier) has clones. The assert makes sure that if we encounter an inlined-to node without any caller, that it merely holds as the holder of the function body for its other specialized (think IPA-CP) or inline clones. If node->clones is false, there are no such clones and it was a bug to mark the node as required during the removal of unreachable nodes. > > Either removing the assertion or making clone_inline_nodes clone when > there are no existing clones 'fixes' (suppresses, but I haven't verified > whether the results are correct) the problem. > > Is this gcc_assert correct in that an inlined function without callers > necessarily must have clones? It is correct. An inlined function without a caller is even a logical oxymoron and can only exist if it has the purpose described above (and even then probably only in a fairly special circumstances). > > And as a side question, do all clone nodes have a ->clones pointing to > the same list of all clones, or are they in a tree-ish arrangement, > where clones of clones end up forming a tree, with the clone_of pointer > being a pointer to the parent? The latter, they form a tree. > (in this instance, the node that trips > the assert has a nullptr clone_of and clones value, which would AIUI > imply that it is the original) Yes. > This train of thinking doesn't end up involving any devirtualization > code, which the PR was originally reproduced with, but my current theory > is that devirtualizing here just exposed an edge case that is decently > well hidden, rather than it playing a crucial role. The inlined function is - I believe erroneously - marked as reachable by walk_polymorphic_call_targets() within the unreachable analysis - so devirtualizing is somewhat crucial. I believe the real question - to which I don't have an answer yet - is why does possible_polymorphic_call_targets return a method that is not virtual? (gdb) p n->decl->decl_common.virtual_flag $19 =3D 0 Or even (gdb) p referenced_from_vtable_p(n) $24 =3D false Time to dig into ipa-devirt.cc, I guess.... Martin