diff mbox series

[3/3] c++/modules: Support decloned cdtors

Message ID 66c529af.170a0220.62c84.d316@mx.google.com
State New
Headers show
Series [1/3] c++: Handle ABI for non-polymorphic dynamic classes | expand

Commit Message

Nathaniel Shead Aug. 20, 2024, 11:41 p.m. UTC 
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

When compiling with '-fdeclone-ctor-dtor' (enabled by default with -Os),
we run into issues where we don't correctly emit the underlying
functions.  We also need to ensure that COMDAT constructors are marked
as such before 'maybe_clone_body' attempts to propagate COMDAT groups to
the new thunks.

gcc/cp/ChangeLog:

	* module.cc (post_load_processing): Mark COMDAT as needed, emit
	declarations if maybe_clone_body fails.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/clone-2_a.C: New test.
	* g++.dg/modules/clone-2_b.C: New test.
	* g++.dg/modules/clone-3_a.C: New test.
	* g++.dg/modules/clone-3_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                         | 20 ++++++++++++++++----
 gcc/testsuite/g++.dg/modules/clone-2_a.C |  7 +++++++
 gcc/testsuite/g++.dg/modules/clone-2_b.C |  5 +++++
 gcc/testsuite/g++.dg/modules/clone-3_a.C |  9 +++++++++
 gcc/testsuite/g++.dg/modules/clone-3_b.C |  8 ++++++++
 5 files changed, 45 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_b.C

Comments

Nathaniel Shead Sept. 2, 2024, 11:45 a.m. UTC | #1 
Ping.

On Wed, Aug 21, 2024 at 09:41:31AM +1000, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> When compiling with '-fdeclone-ctor-dtor' (enabled by default with -Os),
> we run into issues where we don't correctly emit the underlying
> functions.  We also need to ensure that COMDAT constructors are marked
> as such before 'maybe_clone_body' attempts to propagate COMDAT groups to
> the new thunks.
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (post_load_processing): Mark COMDAT as needed, emit
> 	declarations if maybe_clone_body fails.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/clone-2_a.C: New test.
> 	* g++.dg/modules/clone-2_b.C: New test.
> 	* g++.dg/modules/clone-3_a.C: New test.
> 	* g++.dg/modules/clone-3_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/module.cc                         | 20 ++++++++++++++++----
>  gcc/testsuite/g++.dg/modules/clone-2_a.C |  7 +++++++
>  gcc/testsuite/g++.dg/modules/clone-2_b.C |  5 +++++
>  gcc/testsuite/g++.dg/modules/clone-3_a.C |  9 +++++++++
>  gcc/testsuite/g++.dg/modules/clone-3_b.C |  8 ++++++++
>  5 files changed, 45 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 5cd4f313933..9a9c0fdfe81 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -17948,10 +17948,22 @@ post_load_processing ()
>        dump () && dump ("Post-load processing of %N", decl);
>  
>        gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl));
> -      /* Cloning can cause loading -- specifically operator delete for
> -	 the deleting dtor.  */
> -      if (!TREE_ASM_WRITTEN (decl) && maybe_clone_body (decl))
> -	TREE_ASM_WRITTEN (decl) = 1;
> +
> +      if (DECL_COMDAT (decl))
> +	comdat_linkage (decl);
> +      if (!TREE_ASM_WRITTEN (decl))
> +	{
> +	  /* Cloning can cause loading -- specifically operator delete for
> +	     the deleting dtor.  */
> +	  if (maybe_clone_body (decl))
> +	    TREE_ASM_WRITTEN (decl) = 1;
> +	  else
> +	    {
> +	      /* We didn't clone the cdtor, make sure we emit it.  */
> +	      note_vague_linkage_fn (decl);
> +	      cgraph_node::finalize_function (decl, true);
> +	    }
> +	}
>      }
>  
>    cfun = old_cfun;
> diff --git a/gcc/testsuite/g++.dg/modules/clone-2_a.C b/gcc/testsuite/g++.dg/modules/clone-2_a.C
> new file mode 100644
> index 00000000000..47e21581fdc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-2_a.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +export struct S {
> +  inline S(int) {}
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/clone-2_b.C b/gcc/testsuite/g++.dg/modules/clone-2_b.C
> new file mode 100644
> index 00000000000..80c1e149518
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-2_b.C
> @@ -0,0 +1,5 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +
> +import M;
> +
> +S s(0);
> diff --git a/gcc/testsuite/g++.dg/modules/clone-3_a.C b/gcc/testsuite/g++.dg/modules/clone-3_a.C
> new file mode 100644
> index 00000000000..87de746f5c2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-3_a.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +struct A {};
> +export struct B : virtual A {
> +  inline B (int) {}
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/clone-3_b.C b/gcc/testsuite/g++.dg/modules/clone-3_b.C
> new file mode 100644
> index 00000000000..23c9ac4a804
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-3_b.C
> @@ -0,0 +1,8 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +
> +import M;
> +
> +int main() {
> +  B b(0);
> +}
> -- 
> 2.43.2
>
Nathaniel Shead Oct. 4, 2024, 12:04 p.m. UTC | #2 
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660958.html

On Wed, Aug 21, 2024 at 09:41:31AM +1000, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> When compiling with '-fdeclone-ctor-dtor' (enabled by default with -Os),
> we run into issues where we don't correctly emit the underlying
> functions.  We also need to ensure that COMDAT constructors are marked
> as such before 'maybe_clone_body' attempts to propagate COMDAT groups to
> the new thunks.
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (post_load_processing): Mark COMDAT as needed, emit
> 	declarations if maybe_clone_body fails.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/clone-2_a.C: New test.
> 	* g++.dg/modules/clone-2_b.C: New test.
> 	* g++.dg/modules/clone-3_a.C: New test.
> 	* g++.dg/modules/clone-3_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/module.cc                         | 20 ++++++++++++++++----
>  gcc/testsuite/g++.dg/modules/clone-2_a.C |  7 +++++++
>  gcc/testsuite/g++.dg/modules/clone-2_b.C |  5 +++++
>  gcc/testsuite/g++.dg/modules/clone-3_a.C |  9 +++++++++
>  gcc/testsuite/g++.dg/modules/clone-3_b.C |  8 ++++++++
>  5 files changed, 45 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 5cd4f313933..9a9c0fdfe81 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -17948,10 +17948,22 @@ post_load_processing ()
>        dump () && dump ("Post-load processing of %N", decl);
>  
>        gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl));
> -      /* Cloning can cause loading -- specifically operator delete for
> -	 the deleting dtor.  */
> -      if (!TREE_ASM_WRITTEN (decl) && maybe_clone_body (decl))
> -	TREE_ASM_WRITTEN (decl) = 1;
> +
> +      if (DECL_COMDAT (decl))
> +	comdat_linkage (decl);
> +      if (!TREE_ASM_WRITTEN (decl))
> +	{
> +	  /* Cloning can cause loading -- specifically operator delete for
> +	     the deleting dtor.  */
> +	  if (maybe_clone_body (decl))
> +	    TREE_ASM_WRITTEN (decl) = 1;
> +	  else
> +	    {
> +	      /* We didn't clone the cdtor, make sure we emit it.  */
> +	      note_vague_linkage_fn (decl);
> +	      cgraph_node::finalize_function (decl, true);
> +	    }
> +	}
>      }
>  
>    cfun = old_cfun;
> diff --git a/gcc/testsuite/g++.dg/modules/clone-2_a.C b/gcc/testsuite/g++.dg/modules/clone-2_a.C
> new file mode 100644
> index 00000000000..47e21581fdc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-2_a.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +export struct S {
> +  inline S(int) {}
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/clone-2_b.C b/gcc/testsuite/g++.dg/modules/clone-2_b.C
> new file mode 100644
> index 00000000000..80c1e149518
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-2_b.C
> @@ -0,0 +1,5 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +
> +import M;
> +
> +S s(0);
> diff --git a/gcc/testsuite/g++.dg/modules/clone-3_a.C b/gcc/testsuite/g++.dg/modules/clone-3_a.C
> new file mode 100644
> index 00000000000..87de746f5c2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-3_a.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +struct A {};
> +export struct B : virtual A {
> +  inline B (int) {}
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/clone-3_b.C b/gcc/testsuite/g++.dg/modules/clone-3_b.C
> new file mode 100644
> index 00000000000..23c9ac4a804
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-3_b.C
> @@ -0,0 +1,8 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +
> +import M;
> +
> +int main() {
> +  B b(0);
> +}
> -- 
> 2.43.2
>
Jason Merrill Oct. 24, 2024, 4:12 p.m. UTC | #3 
On 8/20/24 7:41 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> When compiling with '-fdeclone-ctor-dtor' (enabled by default with -Os),
> we run into issues where we don't correctly emit the underlying
> functions.  We also need to ensure that COMDAT constructors are marked
> as such before 'maybe_clone_body' attempts to propagate COMDAT groups to
> the new thunks.
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (post_load_processing): Mark COMDAT as needed, emit
> 	declarations if maybe_clone_body fails.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/clone-2_a.C: New test.
> 	* g++.dg/modules/clone-2_b.C: New test.
> 	* g++.dg/modules/clone-3_a.C: New test.
> 	* g++.dg/modules/clone-3_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                         | 20 ++++++++++++++++----
>   gcc/testsuite/g++.dg/modules/clone-2_a.C |  7 +++++++
>   gcc/testsuite/g++.dg/modules/clone-2_b.C |  5 +++++
>   gcc/testsuite/g++.dg/modules/clone-3_a.C |  9 +++++++++
>   gcc/testsuite/g++.dg/modules/clone-3_b.C |  8 ++++++++
>   5 files changed, 45 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/clone-2_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/clone-3_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 5cd4f313933..9a9c0fdfe81 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -17948,10 +17948,22 @@ post_load_processing ()
>         dump () && dump ("Post-load processing of %N", decl);
>   
>         gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl));
> -      /* Cloning can cause loading -- specifically operator delete for
> -	 the deleting dtor.  */
> -      if (!TREE_ASM_WRITTEN (decl) && maybe_clone_body (decl))
> -	TREE_ASM_WRITTEN (decl) = 1;
> +
> +      if (DECL_COMDAT (decl))
> +	comdat_linkage (decl);
> +      if (!TREE_ASM_WRITTEN (decl))
> +	{
> +	  /* Cloning can cause loading -- specifically operator delete for
> +	     the deleting dtor.  */
> +	  if (maybe_clone_body (decl))
> +	    TREE_ASM_WRITTEN (decl) = 1;
> +	  else
> +	    {
> +	      /* We didn't clone the cdtor, make sure we emit it.  */
> +	      note_vague_linkage_fn (decl);
> +	      cgraph_node::finalize_function (decl, true);
> +	    }
> +	}
>       }
>   
>     cfun = old_cfun;
> diff --git a/gcc/testsuite/g++.dg/modules/clone-2_a.C b/gcc/testsuite/g++.dg/modules/clone-2_a.C
> new file mode 100644
> index 00000000000..47e21581fdc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-2_a.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +export struct S {
> +  inline S(int) {}
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/clone-2_b.C b/gcc/testsuite/g++.dg/modules/clone-2_b.C
> new file mode 100644
> index 00000000000..80c1e149518
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-2_b.C
> @@ -0,0 +1,5 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +
> +import M;
> +
> +S s(0);
> diff --git a/gcc/testsuite/g++.dg/modules/clone-3_a.C b/gcc/testsuite/g++.dg/modules/clone-3_a.C
> new file mode 100644
> index 00000000000..87de746f5c2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-3_a.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +struct A {};
> +export struct B : virtual A {
> +  inline B (int) {}
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/clone-3_b.C b/gcc/testsuite/g++.dg/modules/clone-3_b.C
> new file mode 100644
> index 00000000000..23c9ac4a804
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/clone-3_b.C
> @@ -0,0 +1,8 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
> +
> +import M;
> +
> +int main() {
> +  B b(0);
> +}
diff mbox series

Patch

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 5cd4f313933..9a9c0fdfe81 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -17948,10 +17948,22 @@  post_load_processing ()
       dump () && dump ("Post-load processing of %N", decl);
 
       gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl));
-      /* Cloning can cause loading -- specifically operator delete for
-	 the deleting dtor.  */
-      if (!TREE_ASM_WRITTEN (decl) && maybe_clone_body (decl))
-	TREE_ASM_WRITTEN (decl) = 1;
+
+      if (DECL_COMDAT (decl))
+	comdat_linkage (decl);
+      if (!TREE_ASM_WRITTEN (decl))
+	{
+	  /* Cloning can cause loading -- specifically operator delete for
+	     the deleting dtor.  */
+	  if (maybe_clone_body (decl))
+	    TREE_ASM_WRITTEN (decl) = 1;
+	  else
+	    {
+	      /* We didn't clone the cdtor, make sure we emit it.  */
+	      note_vague_linkage_fn (decl);
+	      cgraph_node::finalize_function (decl, true);
+	    }
+	}
     }
 
   cfun = old_cfun;
diff --git a/gcc/testsuite/g++.dg/modules/clone-2_a.C b/gcc/testsuite/g++.dg/modules/clone-2_a.C
new file mode 100644
index 00000000000..47e21581fdc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/clone-2_a.C
@@ -0,0 +1,7 @@ 
+// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
+// { dg-module-cmi M }
+
+export module M;
+export struct S {
+  inline S(int) {}
+};
diff --git a/gcc/testsuite/g++.dg/modules/clone-2_b.C b/gcc/testsuite/g++.dg/modules/clone-2_b.C
new file mode 100644
index 00000000000..80c1e149518
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/clone-2_b.C
@@ -0,0 +1,5 @@ 
+// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
+
+import M;
+
+S s(0);
diff --git a/gcc/testsuite/g++.dg/modules/clone-3_a.C b/gcc/testsuite/g++.dg/modules/clone-3_a.C
new file mode 100644
index 00000000000..87de746f5c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/clone-3_a.C
@@ -0,0 +1,9 @@ 
+// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
+// { dg-module-cmi M }
+
+export module M;
+
+struct A {};
+export struct B : virtual A {
+  inline B (int) {}
+};
diff --git a/gcc/testsuite/g++.dg/modules/clone-3_b.C b/gcc/testsuite/g++.dg/modules/clone-3_b.C
new file mode 100644
index 00000000000..23c9ac4a804
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/clone-3_b.C
@@ -0,0 +1,8 @@ 
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts -fdeclone-ctor-dtor" }
+
+import M;
+
+int main() {
+  B b(0);
+}