From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id B78203858D20 for ; Wed, 1 May 2024 14:27:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B78203858D20 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 B78203858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714573638; cv=none; b=iJVLEQSRsq8qHrd08WKAVCbShbEUuKXmg62O7Mzst3fVR8qyca4x2gQtM4ix1My6Ga/fEjsWE4ggamgNYJpIC9SX+GMyckFz7FGY+m5ZeRPBI3Lx6jmYYtRT575KXQ1/VlNgorSIJ5upN8XGz+aBMVR1yT/gIh4+kZSfPgH421E= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714573638; c=relaxed/simple; bh=mIFDnYuWM6OovT8QAAIyUeyZMhXfwTzNYexkCj4jZk4=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=AKKSQOCeLQRP4/iz2Z40FsyTWRZ90DJSS27aqlBId7qeu7iMWuoony8wUoZnmetdgywA7FhLmAKwAwDbPsvjpd/mpv7I5+jEP3+RCPT5I0KX15ihZa+dShW1IgknoP390sHbGkQbKpR5oCRkdIx/0Hoz3KIgY3oTYNWBzh+AG1s= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-6ecee1f325bso6045192b3a.2 for ; Wed, 01 May 2024 07:27:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1714573635; x=1715178435; 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=Zqdck5RkztdIsBeS9hqKXgUBOr9VSR+wmpgEfc13hfA=; b=l9JlnKqN6iyOxBZAg69HFhwCIPblCCZsfs91wsps8fwzK1AGmzt+mn8WVZneAia85/ JHi3VP3cxs+KMsscY2ClZ5RZOCZzEyUzd4etfW4dC1jPbHPwy73fmjKhhnidCYqC21iK W7yeU3OnzrzEuNedYnXl8W00CZAgDrON1CzCQ8pyGN+khdlEnrM1tVQ9wfMfrE5oKD/i TSuPxaYelWt9XHb4jWFJ/dVv1JpcNYYc9i2WPvqcpdIiTxb07rWw8A33I+jsDEz20gFM aOb7SLYPQYMiU0gKvR4xaVb49mKIbYsLKgp9sVkTiPJR6T6y6/4Anqb5DoGiNJgknEXe AJzw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714573635; x=1715178435; 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=Zqdck5RkztdIsBeS9hqKXgUBOr9VSR+wmpgEfc13hfA=; b=MMu1E9ydSdVwuucceR0f9s7e2WfnPPef473121jq8tQM5KEiQWjvsNlphLBz76H6D5 3McNpCkAlVc0vOyewZ5xpScpKFUVDvwRhZlccrnpJzKTuvD1iCh1LrGA7L9rK5IqnWNE U/v4zHJmsmUjMtnt9D+Ddapt/D4COkwnKwoV/jS90AMhBpbGbIOyo141htue7eLwt0OE 58qe8ppLh4hI4oy7GoVrH8k7vVZDezAdJOOOxeJZPtChPIl2fcnNU036mkoypNBIrXGx TanDS0VrMGOWcqOkWszcXkzE0gEtwqpC2Zh9kSuNZDBE/6Tlp2JBz1XFvceMGs/sh+nU iRCQ== X-Gm-Message-State: AOJu0YwRQc3vQcWGFL1SGJkAL1RsIp04HKxgu7R89sQD2TlEzbYimEC8 ytM6XvsqH1w3S/hJ1rCGMLWg9JD5j3dU0Ydph196DlasGc9HQ/x1UpGlKw== X-Google-Smtp-Source: AGHT+IGJJsVju6sSvQd7+4UPQxDE1OWrQEEgzNMpjP/3fyB1MfxdBPIIL/ZaYZnVoyap8NwlHNNFIg== X-Received: by 2002:a05:6a20:842a:b0:1aa:590a:96d7 with SMTP id c42-20020a056a20842a00b001aa590a96d7mr4025623pzd.53.1714573633570; Wed, 01 May 2024 07:27:13 -0700 (PDT) Received: from Thaum. (121-44-11-123.tpgi.com.au. [121.44.11.123]) by smtp.gmail.com with ESMTPSA id fn10-20020a056a002fca00b006e72c8ece23sm22707568pfb.191.2024.05.01.07.27.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 May 2024 07:27:13 -0700 (PDT) Message-ID: <66325141.050a0220.452b5.4921@mx.google.com> X-Google-Original-Message-ID: Date: Thu, 2 May 2024 00:27:08 +1000 From: Nathaniel Shead To: Patrick Palka Cc: gcc-patches@gcc.gnu.org, Jason Merrill , Nathan Sidwell Subject: Re: [PATCH 2/4] c++/modules: Track module purview for deferred instantiations [PR114630] References: <66321257.170a0220.326c5.1f68@mx.google.com> <663212bb.050a0220.9a96f.94c6@mx.google.com> <770b32b3-85a5-14aa-f179-f8f44790d532@idea> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <770b32b3-85a5-14aa-f179-f8f44790d532@idea> X-Spam-Status: No, score=-11.9 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, May 01, 2024 at 10:11:20AM -0400, Patrick Palka wrote: > On Wed, 1 May 2024, Nathaniel Shead wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > When calling instantiate_pending_templates at end of parsing, any new > > functions that are instantiated from this point have their module > > purview set based on the current value of module_kind. > > > > This is unideal, however, as the modules code will then treat these > > instantiations as reachable and cause large swathes of the GMF to be > > emitted into the module CMI, despite no code in the actual module > > purview referencing it. > > > > This patch fixes this by also remembering the value of module_kind when > > the instantiation was deferred, and restoring it when doing this > > deferred instantiation. That way newly instantiated declarations > > appropriately get a DECL_MODULE_PURVIEW_P appropriate for where the > > instantiation was required, meaning that GMF entities won't be counted > > as reachable unless referenced by an actually reachable entity. > > > > Note that purviewness and attachment etc. is generally only determined > > by the base template: this is purely for determining whether a > > specialisation was declared in the module purview and hence whether it > > should be streamed out. See the comment on 'set_instantiating_module'. > > > > PR c++/114630 > > PR c++/114795 > > > > gcc/cp/ChangeLog: > > > > * cp-tree.h (struct tinst_level): Add field for tracking > > module_kind. > > * pt.cc (push_tinst_level_loc): Cache module_kind in new_level. > > (reopen_tinst_level): Restore module_kind from level. > > (instantiate_pending_templates): Save and restore module_kind so > > it isn't affected by reopen_tinst_level. > > LGTM. Another approach is to instantiate all so-far deferred > instantiations and vtables once we reach the start of the module > purview, but your approach is much cleaner. > > Note that deferred instantiation can induce more deferred instantiation, > but your approach will do the right thing here (module_kind will be > inherited from the point of instantiation). > > What if an instantiation is needed from both the GMF and the module > purview? Then IIUC it'll be instantiated as if from the GMF, which > seems right to me. > Hm..., so I believe it'll be marked according to whichever instantiates it "first", so if e.g. deferred from GMF and then instantiated non-deferred from purview it'll be marked purview (and the deferred instantiation will be skipped as it's already instantiated). This could mean it gets unnecessarily emitted if it only got instantiated because it got used in e.g. a non-inline function body, I suppose; but that's true in general actually, at the moment. Maybe then a better approach would be to instead always use the DECL_MODULE_PURVIEW_P of the instantiating template instead of the global 'module_purview_p' function in 'set_instantiating_module'? I think that should still make sure that instantiations are emitted when they need to be (because they'll be reachable from a declaration in the purview): I might try to experiment with something like this. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/gmf-3.C: New test. > > > > Signed-off-by: Nathaniel Shead > > --- > > gcc/cp/cp-tree.h | 3 +++ > > gcc/cp/pt.cc | 4 ++++ > > gcc/testsuite/g++.dg/modules/gmf-3.C | 13 +++++++++++++ > > 3 files changed, 20 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/modules/gmf-3.C > > > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > > index 1938ada0268..0e619120ccc 100644 > > --- a/gcc/cp/cp-tree.h > > +++ b/gcc/cp/cp-tree.h > > @@ -6626,6 +6626,9 @@ struct GTY((chain_next ("%h.next"))) tinst_level { > > /* The location where the template is instantiated. */ > > location_t locus; > > > > + /* The module kind where the template is instantiated. */ > > + unsigned module_kind; > > + > > /* errorcount + sorrycount when we pushed this level. */ > > unsigned short errors; > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index 1c3eef60c06..401aa92bc3e 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -11277,6 +11277,7 @@ push_tinst_level_loc (tree tldcl, tree targs, location_t loc) > > new_level->tldcl = tldcl; > > new_level->targs = targs; > > new_level->locus = loc; > > + new_level->module_kind = module_kind; > > new_level->errors = errorcount + sorrycount; > > new_level->next = NULL; > > new_level->refcount = 0; > > @@ -11345,6 +11346,7 @@ reopen_tinst_level (struct tinst_level *level) > > for (t = level; t; t = t->next) > > ++tinst_depth; > > > > + module_kind = level->module_kind; > > set_refcount_ptr (current_tinst_level, level); > > pop_tinst_level (); > > if (current_tinst_level) > > @@ -27442,6 +27444,7 @@ instantiate_pending_templates (int retries) > > { > > int reconsider; > > location_t saved_loc = input_location; > > + unsigned saved_module_kind = module_kind; > > > > /* Instantiating templates may trigger vtable generation. This in turn > > may require further template instantiations. We place a limit here > > @@ -27532,6 +27535,7 @@ instantiate_pending_templates (int retries) > > while (reconsider); > > > > input_location = saved_loc; > > + module_kind = saved_module_kind; > > } > > > > /* Substitute ARGVEC into T, which is a list of initializers for > > diff --git a/gcc/testsuite/g++.dg/modules/gmf-3.C b/gcc/testsuite/g++.dg/modules/gmf-3.C > > new file mode 100644 > > index 00000000000..e52ae904ea9 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/gmf-3.C > > @@ -0,0 +1,13 @@ > > +// PR c++/114630 > > +// { dg-additional-options "-fmodules-ts -Wno-global-module -fdump-lang-module" } > > +// { dg-module-cmi M } > > + > > +module; > > +template struct allocator { > > + allocator() {} > > +}; > > +template class allocator; > > +export module M; > > + > > +// The whole GMF should be discarded here > > +// { dg-final { scan-lang-dump "Wrote 0 clusters" module } } > > -- > > 2.43.2 > > > > >