From patchwork Thu Mar 7 12:49:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 86926 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3745B3857830 for ; Thu, 7 Mar 2024 12:50:30 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by sourceware.org (Postfix) with ESMTPS id CEEC53858401 for ; Thu, 7 Mar 2024 12:50:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CEEC53858401 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 CEEC53858401 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::436 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709815808; cv=none; b=r17BIr9gfHfAXQQhN3ctkvDtF0HCqnleSSXWOuj0t3NS/wIrbTxQS7BIxJ6gNXzTE9jJDoHvSPO3CDYGVj0qsEVJChnMm7sumveFWhuX0QQn4jqSFODsLeZHYECbYyPLWMbI+3fKqhuamDAuJqJir9/+/7wA09N9MNGFGfbok+A= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709815808; c=relaxed/simple; bh=91p+rlInNqGBHUsgys0ULCzeR2+94V40tBFycJea8u4=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=W8n8taMkqRcrOknr70SxkzRz5tv3CDv3i4kPb7sGBTagZrb/XLW1MA05DKdaw6afADr0jLHCNl9rmrtqQn+evT60EFtNH0eRpo9owJ67/RlgoW5tNm19thJUmDcJz++sphMy4JkI6Ngg1wxQ2tdJUw1VytV9Z3R0nl92ztHTcbg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-6e4d48a5823so609254b3a.1 for ; Thu, 07 Mar 2024 04:50:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709815805; x=1710420605; 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=UNAF2kUHOdKT0S9C1NhB5baDTRdG4RyHdSyAMxa8JfQ=; b=OaRdX/b1102XU+I9sd1vWehMGwG8agBN4vXKyIDgmsssT0hQ3m17ugCRxKgxBS810X RdOQZE4s8XXRXkbTpjqv93BsHQZob7NrB+d3+aHkvToaRF4HTvSl0perzqGPpEq75PpK TjxShWRg8uQ0aVtHOy9TWtUJx3GSsXb9jSnq4EvxwJEkpYDVzNhnOd3fXexCQLP8QhOt wps9hBMHq8LTWQhrIshthYRQgsCzoA8BBISwEIfnxizeEkgtXlcEYKtgv5E+Simey1AL mBLurwPTSpDWx968EMwpvLOsqEuo2dREfQ7O0+DqtBUT4abPxqMtk2twaCHUfatAAPXM nXdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709815805; x=1710420605; 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=UNAF2kUHOdKT0S9C1NhB5baDTRdG4RyHdSyAMxa8JfQ=; b=K95Pr5RMqputvTyGRZBzDKWaYkFe4lKPbstBhXAHP7NjlReOLSH1Zy4YMPJhCV/2xe prTKHZw38N+e/OBsl3dwTt2Cfy6yuDBt4oq1tyQ+S77SSthhoZ8dg9zcqyp3C1yuqA8I thVv/jlFPH645mdKms+cRTG7NA8w0/MfB6W+PYq+UJ8KdVJ3PtU+2LTxlgMnFEynnn7n XLJEJBqcadYk1bshVWnXCTxo0Rq4T1/FnPlR6m6w35+t/aM3sc3azXh5FxEhHeCEG3It 95aeQbGhaM7TR5Vg0rBgwm2V7YXihcptgmdStEDK1Au+B4ZJVQcN1OUXJ2JGGUp88DsI SI6Q== X-Gm-Message-State: AOJu0YxACWrOVckTsjIk2hlG+tQryXDYWzaiIZlLhMTnFou0LhXvZZkj 29amjtsY1jd9fCeMX1g1zFlpELS2FTLXuKmi7pP4BHl6DZCV4KXC8V8ls5Bs X-Google-Smtp-Source: AGHT+IHWUfzUNciTRw0LpBng9gFRl6Pwj7Y5yT87f+Sc83JqkDmMZie3vUUn7k6Hf3eBJ4hbqvkaZg== X-Received: by 2002:a05:6a21:6da4:b0:1a1:4d8b:6f2c with SMTP id wl36-20020a056a216da400b001a14d8b6f2cmr8823960pzb.2.1709815804682; Thu, 07 Mar 2024 04:50:04 -0800 (PST) Received: from Thaum. (202-161-100-107.tpgi.com.au. [202.161.100.107]) by smtp.gmail.com with ESMTPSA id w14-20020aa7858e000000b006e5736e9e46sm12419431pfn.42.2024.03.07.04.50.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Mar 2024 04:50:03 -0800 (PST) Message-ID: <65e9b7fb.a70a0220.8763e.f61e@mx.google.com> X-Google-Original-Message-ID: Date: Thu, 7 Mar 2024 23:49:58 +1100 From: Nathaniel Shead To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Nathan Sidwell , Patrick Palka Subject: [PATCH v2] c++: Redetermine whether to write vtables on stream-in [PR114229] References: <65e7ddc4.170a0220.d647a.c749@mx.google.com> <79dad146-1efb-433c-97f8-83997cc4b51f@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <79dad146-1efb-433c-97f8-83997cc4b51f@redhat.com> X-Spam-Status: No, score=-11.2 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, T_SCC_BODY_TEXT_LINE, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org On Wed, Mar 06, 2024 at 08:59:16AM -0500, Jason Merrill wrote: > On 3/5/24 22:06, Nathaniel Shead wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > Currently, reading a variable definition always marks that decl as > > DECL_NOT_REALLY_EXTERN, with anything else imported still being > > considered external. This is not sufficient for vtables, however; for an > > extern template, a vtable may be generated (and its definition emitted) > > but nonetheless the vtable should only be emitted in the TU where that > > template is actually instantiated. > > Does the vtable go through import_export_decl? I've been thinking that that > function (and import_export_class) need to be more module-aware. Would it > make sense to do that rather than stream DECL_NOT_REALLY_EXTERN? > > Jason > Right. It doesn't go through 'import_export_decl' because when it's imported, DECL_INTERFACE_KNOWN is already set. So it seems an obvious fix here is to just ensure that we clear that flag on stream-in for vtables (we can't do it generally as it seems to be needed to be kept on various other kinds of declarations). Linaro complained about the last version of this patch too on ARM; hopefully this version is friendlier. I might also spend some time messing around to see if I can implement https://github.com/itanium-cxx-abi/cxx-abi/issues/170 later, but that will probably have to be a GCC 15 change. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk if Linaro doesn't complain about this patch? -- >8 -- We currently always stream DECL_INTERFACE_KNOWN, which is needed since many kinds of declarations already have their interface determined at parse time. But for vtables and type-info declarations we need to re-evaluate on stream-in, as whether they need to be emitted or not changes in each TU, so this patch clears DECL_INTERFACE_KNOWN on these kinds of declarations so that they can go through 'import_export_decl' again. Note that the precise details of the virt-2 tests will need to change when we implement the resolution of [1]; for now I just updated the test to not fail with the new (current) semantics. [1]: https://github.com/itanium-cxx-abi/cxx-abi/pull/171 PR c++/114229 gcc/cp/ChangeLog: * module.cc (trees_out::core_bools): Redetermine DECL_INTERFACE_KNOWN on stream-in for vtables and tinfo. gcc/testsuite/ChangeLog: * g++.dg/modules/virt-2_b.C: Update test to acknowledge that we now emit vtables here too. * g++.dg/modules/virt-3_a.C: New test. * g++.dg/modules/virt-3_b.C: New test. * g++.dg/modules/virt-3_c.C: New test. * g++.dg/modules/virt-3_d.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/module.cc | 12 +++++++++++- gcc/testsuite/g++.dg/modules/virt-2_b.C | 5 ++--- gcc/testsuite/g++.dg/modules/virt-3_a.C | 9 +++++++++ gcc/testsuite/g++.dg/modules/virt-3_b.C | 6 ++++++ gcc/testsuite/g++.dg/modules/virt-3_c.C | 3 +++ gcc/testsuite/g++.dg/modules/virt-3_d.C | 7 +++++++ 6 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_a.C create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_b.C create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_c.C create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_d.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index f7e8b357fc2..d77286328f5 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5390,7 +5390,17 @@ trees_out::core_bools (tree t) WB (t->decl_common.lang_flag_2); WB (t->decl_common.lang_flag_3); WB (t->decl_common.lang_flag_4); - WB (t->decl_common.lang_flag_5); + + { + /* This is DECL_INTERFACE_KNOWN: We should redetermine whether + we need to import or export any vtables or typeinfo objects + on stream-in. */ + bool interface_known = t->decl_common.lang_flag_5; + if (VAR_P (t) && (DECL_VTABLE_OR_VTT_P (t) || DECL_TINFO_P (t))) + interface_known = false; + WB (interface_known); + } + WB (t->decl_common.lang_flag_6); WB (t->decl_common.lang_flag_7); WB (t->decl_common.lang_flag_8); diff --git a/gcc/testsuite/g++.dg/modules/virt-2_b.C b/gcc/testsuite/g++.dg/modules/virt-2_b.C index e041f0721f9..2bc5eced013 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_b.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_b.C @@ -21,8 +21,7 @@ int main () return !(Visit (&me) == 1); } -// We do not emit Visitor vtable -// but we do emit rtti here -// { dg-final { scan-assembler-not {_ZTVW3foo7Visitor:} } } +// Again, we emit Visitor vtable and rtti here +// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } // { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } // { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } diff --git a/gcc/testsuite/g++.dg/modules/virt-3_a.C b/gcc/testsuite/g++.dg/modules/virt-3_a.C new file mode 100644 index 00000000000..a7eae7f9d35 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_a.C @@ -0,0 +1,9 @@ +// PR c++/114229 +// { dg-additional-options "-fmodules-ts -Wno-global-module" } +// { dg-module-cmi modA } + +module; +template struct basic_streambuf { virtual void overflow() { } }; +extern template struct basic_streambuf; +export module modA; +export basic_streambuf *p; diff --git a/gcc/testsuite/g++.dg/modules/virt-3_b.C b/gcc/testsuite/g++.dg/modules/virt-3_b.C new file mode 100644 index 00000000000..4d87b965bbf --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_b.C @@ -0,0 +1,6 @@ +// PR c++/114229 +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } +// { dg-module-cmi modB } + +export module modB; +import modA; diff --git a/gcc/testsuite/g++.dg/modules/virt-3_c.C b/gcc/testsuite/g++.dg/modules/virt-3_c.C new file mode 100644 index 00000000000..224bb4aed49 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_c.C @@ -0,0 +1,3 @@ +// PR c++/114229 +template struct basic_streambuf { virtual void overflow() { } }; +template struct basic_streambuf; diff --git a/gcc/testsuite/g++.dg/modules/virt-3_d.C b/gcc/testsuite/g++.dg/modules/virt-3_d.C new file mode 100644 index 00000000000..b3a0aad7abe --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_d.C @@ -0,0 +1,7 @@ +// PR c++/114229 +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } + +import modA; +import modB; +int main() { }