public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Nathaniel Shead <nathanieloshead@gmail.com>
To: gcc-patches@gcc.gnu.org
Cc: Jason Merrill <jason@redhat.com>, Nathan Sidwell <nathan@acm.org>,
	Patrick Palka <ppalka@redhat.com>
Subject: [PATCH v2] c++/modules: Emit definitions of ODR-used static members imported from modules [PR112899]
Date: Wed, 3 Jan 2024 23:42:43 +1100	[thread overview]
Message-ID: <65955649.170a0220.e1f37.25c3@mx.google.com> (raw)
In-Reply-To: <65949213.a70a0220.2947.d85b@mx.google.com>

Linaro CI tells me that this patch caused regressions on ARM. I don't
have an ARM machine available to test on, but it appears to have been
caused by attempting to stream vtables as static data members, and ARM
having different behaviour with regards to when DECL_INTERFACE_KNOWN is
marked on vtables.

I don't think treating vtables as static members here is desirable
anyway, though, so this version of the patch explicitly ignores them.

Manually checking the assembly output for some of the noted regressions
on ARM with a cross-compiler, this seems to have worked. Otherwise
bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk if it
passes Linaro CI on ARM this time?

-- >8 --

Static data members marked 'inline' should be emitted in TUs where they
are ODR-used.  We need to make sure that statics imported from modules
are correctly added to the 'pending_statics' map so that they get
emitted if needed, otherwise the attached testcase fails to link.

	PR c++/112899

gcc/cp/ChangeLog:

	* cp-tree.h (note_variable_template_instantiation): Rename to...
	(note_static_storage_variable): ...this.
	* decl2.cc (note_variable_template_instantiation): Rename to...
	(note_static_storage_variable): ...this.
	* pt.cc (instantiate_decl): Rename usage of above function.
	* module.cc (trees_in::read_var_def): Remember pending statics
	that we stream in.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/init-4_a.C: New test.
	* g++.dg/modules/init-4_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/cp-tree.h                        |  2 +-
 gcc/cp/decl2.cc                         |  4 ++--
 gcc/cp/module.cc                        |  5 +++++
 gcc/cp/pt.cc                            |  2 +-
 gcc/testsuite/g++.dg/modules/init-4_a.C |  9 +++++++++
 gcc/testsuite/g++.dg/modules/init-4_b.C | 11 +++++++++++
 6 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/init-4_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/init-4_b.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1979572c365..ebd2850599a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7113,7 +7113,7 @@ extern tree maybe_get_tls_wrapper_call		(tree);
 extern void mark_needed				(tree);
 extern bool decl_needed_p			(tree);
 extern void note_vague_linkage_fn		(tree);
-extern void note_variable_template_instantiation (tree);
+extern void note_static_storage_variable	(tree);
 extern tree build_artificial_parm		(tree, tree, tree);
 extern bool possibly_inlined_p			(tree);
 extern int parm_index                           (tree);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 0850d3f5bce..241216b0dfe 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -910,10 +910,10 @@ note_vague_linkage_fn (tree decl)
   vec_safe_push (deferred_fns, decl);
 }
 
-/* As above, but for variable template instantiations.  */
+/* As above, but for variables with static storage duration.  */
 
 void
-note_variable_template_instantiation (tree decl)
+note_static_storage_variable (tree decl)
 {
   vec_safe_push (pending_statics, decl);
 }
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 0bd46414da9..7edc0e37af0 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11752,6 +11752,11 @@ trees_in::read_var_def (tree decl, tree maybe_template)
 	  DECL_INITIALIZED_P (decl) = true;
 	  if (maybe_dup && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (maybe_dup))
 	    DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = true;
+	  if (DECL_CONTEXT (decl)
+	      && RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (decl))
+	      && !DECL_TEMPLATE_INFO (decl)
+	      && !DECL_VTABLE_OR_VTT_P (decl))
+	    note_static_storage_variable (decl);
 	}
       DECL_INITIAL (decl) = init;
       if (!dyn_init)
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index f7063e09581..ce498750758 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -27150,7 +27150,7 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
     {
       set_instantiating_module (d);
       if (variable_template_p (gen_tmpl))
-	note_variable_template_instantiation (d);
+	note_static_storage_variable (d);
       instantiate_body (td, args, d, false);
     }
 
diff --git a/gcc/testsuite/g++.dg/modules/init-4_a.C b/gcc/testsuite/g++.dg/modules/init-4_a.C
new file mode 100644
index 00000000000..e0eb97b474e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-4_a.C
@@ -0,0 +1,9 @@
+// PR c++/112899
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+
+export struct A {
+  static constexpr int x = -1;
+};
diff --git a/gcc/testsuite/g++.dg/modules/init-4_b.C b/gcc/testsuite/g++.dg/modules/init-4_b.C
new file mode 100644
index 00000000000..d28017a1d14
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-4_b.C
@@ -0,0 +1,11 @@
+// PR c++/112899
+// { dg-module-do run }
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+int main() {
+  const int& x = A::x;
+  if (x != -1)
+    __builtin_abort();
+}
-- 
2.43.0


  reply	other threads:[~2024-01-03 12:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-02 22:40 Nathaniel Shead
2024-01-02 22:45 ` [PATCH] " Nathaniel Shead
2024-01-03 12:42   ` Nathaniel Shead [this message]
2024-01-04 19:16     ` :Re: [PATCH v2] " Patrick Palka
2024-01-04 20:31 ` Jason Merrill
2024-01-04 22:24   ` Nathaniel Shead
2024-01-04 22:42     ` Jason Merrill
2024-01-04 23:02       ` Nathaniel Shead
2024-01-05  2:06         ` Jason Merrill
2024-01-06 22:30           ` Nathan Sidwell
2024-01-08  9:21             ` Iain Sandoe
2024-01-08 22:38               ` Jason Merrill
2024-01-19 18:57 ` Patrick Palka
2024-01-20 10:45   ` [PATCH v3] " Nathaniel Shead
2024-01-24 20:24     ` Jason Merrill
2024-01-26  2:28       ` [PATCH v4] " Nathaniel Shead
2024-01-26  3:35         ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=65955649.170a0220.e1f37.25c3@mx.google.com \
    --to=nathanieloshead@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=nathan@acm.org \
    --cc=ppalka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).