From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id 57EA13858C98 for ; Thu, 4 Apr 2024 11:28:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 57EA13858C98 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 57EA13858C98 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::633 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712230088; cv=none; b=p+BzNbMpihPpD8IiMfEu+pZHpALtwmjigktF2Qr1KXhZ/9CJxRjYhVkdlzkFeaV7XBLnaTz3LKhjD+44g9NIs8d8V52ISDmjnsPdmjZITkkSuDFPSmneIjqyQH4odkjHHx5e2Cxq2xpESTd/zUY3pYJSSgI+gLTvKQy8YdT8cuA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712230088; c=relaxed/simple; bh=LjlGlLfGcSttyXAEZ2vNv6IvtzvnVwHBDFuak6oI3Ng=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=aWv8vUbxPLXeBJwL9Uu+C/p6Z1vmyVvaPUFauxHjwTLS2HwTfmGT3WUZaJ2kqHd4cKmnqiX6E4XWA23KRtNja/EgpvYBbx6CjpEz7f7qARAxNaX/IfBBMFzG+kLwcxnU8TX7DTd2xK/ZjuORgrdb7mad+9yMEdS1gQthEp9XjyY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x633.google.com with SMTP id d9443c01a7336-1e252f2bf23so7094895ad.2 for ; Thu, 04 Apr 2024 04:28:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712230082; x=1712834882; darn=gcc.gnu.org; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=qi0PTc8402EIFyehBYt/IJKc+NQDaxpwUUhYDYg0NhQ=; b=fwJU/CO55Bpcf2yN0A4N4SzBPOlnCuI2dZNsLmY6jY8KX9Fo13O+ix8gYf5i0cCrOd f9rM+8z188HkpjzGZPQIQ8PMVvUe9lBpaVEWtBBd2hDCyrD6bzQZqO7ZE/NKOVyHjTqI iQyhIERRNDYnOicl6R50p6GlhWJR3/0FGa6k4ZTF4BmfTZHx+duzRPEIv8vLPCHLwlGv C2Cm5GdNCWhlJpHgQYn+3pmMDDlzvC1SDpekt/2GDuuLPYvHok7wik05qBKtAPs++hvt VKOc9gInKQe74erExrizdASaYNly6YXdDxzTB0BW3eAJXDX4gzojhN16NSEpyguVuUfC p/vA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712230082; x=1712834882; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=qi0PTc8402EIFyehBYt/IJKc+NQDaxpwUUhYDYg0NhQ=; b=AsUYFPma/cshClQIPg5S7+CfobP9PnajLKsF7RKkPt3Wvq7m1BLdhoyKxCWKWjvLCF dXr0FUXAm2dnPVceX7HeNibcq9nvjdnRw5L+y9bJAl7Ury/24ErPHZ2Vt9X3to7+k+AZ a7LLvG6B7wUIzwFU9rYcX89BQyxbhKO0WN9QP6Ig5pPKER1mYwOBtkISaYjuiDHCx45p +mjAoIDFPW5BMJKOkiGgn/8g+4EwzMIpkNbi8eViMjAdxVtvMidacbLKS8MEqOrYKBGE 9CmchBtFpoBFAT/xU8ZwnltMcCSH6HHy6OGKUcEjKh9lUrdNUJGzQRx3AgM5IorkCE8a AoJA== X-Gm-Message-State: AOJu0YzRmAZQBQ3WIt+cUkYqgXIWrgwgp3fP0kdWl1T3kR80GyKa+BdW lg2Z0F/vJFugdmNrzAuoo21RqRwTO4Wu/ZbBn2TugFNnkmuiu5kqF1h3n94g X-Google-Smtp-Source: AGHT+IFwMozo8pAx/liJrNgqdOEh9E9jz0hvI+nTjU4I9TEZQr3MrZdX0Ghc8yRDqQZgGnd/XVkFCw== X-Received: by 2002:a17:902:b08f:b0:1e2:9ddc:f72d with SMTP id p15-20020a170902b08f00b001e29ddcf72dmr1797942plr.26.1712230082011; Thu, 04 Apr 2024 04:28:02 -0700 (PDT) Received: from Thaum. (193-119-89-230.tpgi.com.au. [193.119.89.230]) by smtp.gmail.com with ESMTPSA id w13-20020a170902d3cd00b001e2ad73b15bsm1363324plb.203.2024.04.04.04.27.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Apr 2024 04:28:01 -0700 (PDT) Message-ID: <660e8ec1.170a0220.2a0c4.3501@mx.google.com> X-Google-Original-Message-ID: Date: Thu, 4 Apr 2024 22:27:56 +1100 From: Nathaniel Shead To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Nathan Sidwell Subject: Re: [PATCH] c++: Keep DECL_SAVED_TREE of destructor instantiations in modules [PR104040] References: <660633b1.170a0220.717f3.817f@mx.google.com> <660ca996.170a0220.60d95.0377@mx.google.com> <6ea38b8a-4146-4c02-b28a-80f1de178e01@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6ea38b8a-4146-4c02-b28a-80f1de178e01@redhat.com> X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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: On Wed, Apr 03, 2024 at 11:18:01AM -0400, Jason Merrill wrote: > On 4/2/24 20:57, Nathaniel Shead wrote: > > On Tue, Apr 02, 2024 at 01:18:17PM -0400, Jason Merrill wrote: > > > On 3/28/24 23:21, Nathaniel Shead wrote: > > > > - && !(modules_p () && DECL_DECLARED_INLINE_P (fn))) > > > > + && !(modules_p () > > > > + && (DECL_DECLARED_INLINE_P (fn) > > > > + || DECL_TEMPLATE_INSTANTIATION (fn)))) > > > > > > How about using vague_linkage_p? > > > > > > > Right, of course. How about this? > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > A template instantiation still needs to have its DECL_SAVED_TREE so that > > its definition is emitted into the CMI. This way it can be emitted in > > the object file of any importers that use it, in case it doesn't end up > > getting emitted in this TU. > > > > PR c++/104040 > > > > gcc/cp/ChangeLog: > > > > * semantics.cc (expand_or_defer_fn_1): Keep DECL_SAVED_TREE for > > all vague linkage functions. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/pr104040_a.C: New test. > > * g++.dg/modules/pr104040_b.C: New test. > > > > Signed-off-by: Nathaniel Shead > > Reviewed-by: Jason Merrill > > --- > > gcc/cp/semantics.cc | 5 +++-- > > gcc/testsuite/g++.dg/modules/pr104040_a.C | 14 ++++++++++++++ > > gcc/testsuite/g++.dg/modules/pr104040_b.C | 8 ++++++++ > > 3 files changed, 25 insertions(+), 2 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/modules/pr104040_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/pr104040_b.C > > > > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > > index adb1ba48d29..03800a20b26 100644 > > --- a/gcc/cp/semantics.cc > > +++ b/gcc/cp/semantics.cc > > @@ -5033,9 +5033,10 @@ expand_or_defer_fn_1 (tree fn) > > /* We don't want to process FN again, so pretend we've written > > it out, even though we haven't. */ > > TREE_ASM_WRITTEN (fn) = 1; > > - /* If this is a constexpr function, keep DECL_SAVED_TREE. */ > > + /* If this is a constexpr function, or the body might need to be > > + exported from a module CMI, keep DECL_SAVED_TREE. */ > > if (!DECL_DECLARED_CONSTEXPR_P (fn) > > - && !(modules_p () && DECL_DECLARED_INLINE_P (fn))) > > + && !(modules_p () && vague_linkage_p (fn))) > > Also, how about module_maybe_has_cmi_p? OK with that change. > > Jason > Using 'module_maybe_has_cmi_p' doesn't seem to work. This is for two reasons, one of them fixable and one of them not (easily): - It seems that header modules don't count for 'module_maybe_has_cmi_p'; I didn't notice this initially, and maybe they should for the no-linkage decls too? But even accounting for this, - For some reason only clearing it if the module might have a CMI causes crashes in importers for some testcases. I'm not 100% sure why yet, but I suspect it might be some duplicate-decls thing where the type inconsistently has DECL_SAVED_TREE applied, since this is also called on streamed-in declarations. Out of interest, what was the reason that it was cleared at all in the first place? I wasn't able to find anything with git blame; is it just for performance reasons in avoiding excess lowering later?