diff mbox series

[1/3] c++: Handle ABI for non-polymorphic dynamic classes

Message ID 66c52908.170a0220.2c87fb.d675@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:38 p.m. UTC 
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

The Itanium ABI has specific rules for when virtual tables for dynamic
classes should be emitted.  However we didn't consider structures with
virtual inheritance but no virtual members as dynamic classes for ABI
purposes; this patch fixes this.

gcc/cp/ChangeLog:

	* decl2.cc (import_export_class): Use TYPE_CONTAINS_VPTR_P
	instead of TYPE_POLYMORPHIC_P.
	(import_export_decl): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/virt-5_a.C: New test.
	* g++.dg/modules/virt-5_b.C: New test.

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

Comments

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

On Wed, Aug 21, 2024 at 09:38:44AM +1000, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> The Itanium ABI has specific rules for when virtual tables for dynamic
> classes should be emitted.  However we didn't consider structures with
> virtual inheritance but no virtual members as dynamic classes for ABI
> purposes; this patch fixes this.
> 
> gcc/cp/ChangeLog:
> 
> 	* decl2.cc (import_export_class): Use TYPE_CONTAINS_VPTR_P
> 	instead of TYPE_POLYMORPHIC_P.
> 	(import_export_decl): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/virt-5_a.C: New test.
> 	* g++.dg/modules/virt-5_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/decl2.cc                         |  4 ++--
>  gcc/testsuite/g++.dg/modules/virt-5_a.C | 16 ++++++++++++++++
>  gcc/testsuite/g++.dg/modules/virt-5_b.C | 11 +++++++++++
>  3 files changed, 29 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_b.C
> 
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index e9ae979896c..af544f40dac 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -2431,7 +2431,7 @@ import_export_class (tree ctype)
>         translation unit, then export the class; otherwise, import
>         it.  */
>        import_export = -1;
> -  else if (TYPE_POLYMORPHIC_P (ctype))
> +  else if (TYPE_CONTAINS_VPTR_P (ctype))
>      {
>        tree cdecl = TYPE_NAME (ctype);
>        if (DECL_LANG_SPECIFIC (cdecl) && DECL_MODULE_ATTACH_P (cdecl))
> @@ -3527,7 +3527,7 @@ import_export_decl (tree decl)
>  	  class_type = type;
>  	  import_export_class (type);
>  	  if (CLASSTYPE_INTERFACE_KNOWN (type)
> -	      && TYPE_POLYMORPHIC_P (type)
> +	      && TYPE_CONTAINS_VPTR_P (type)
>  	      && CLASSTYPE_INTERFACE_ONLY (type)
>  	      /* If -fno-rtti was specified, then we cannot be sure
>  		 that RTTI information will be emitted with the
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_a.C b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> new file mode 100644
> index 00000000000..f4c6abe85ef
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> @@ -0,0 +1,16 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +struct C {};
> +struct B : virtual C {};
> +
> +// Despite no non-inline key function, this is still a dynamic class
> +// and so by the Itanium ABI 5.2.3 should be uniquely emitted in this TU
> +export struct A : B {
> +  inline A (int) {}
> +};
> +
> +// { dg-final { scan-assembler {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler {_ZTVW1M1A:} } }
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_b.C b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> new file mode 100644
> index 00000000000..785dd92ac1e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> @@ -0,0 +1,11 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +int main() {
> +  A a(0);
> +}
> +
> +// { dg-final { scan-assembler-not {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler-not {_ZTVW1M1A:} } }
> -- 
> 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/660956.html.

On Wed, Aug 21, 2024 at 09:38:44AM +1000, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> The Itanium ABI has specific rules for when virtual tables for dynamic
> classes should be emitted.  However we didn't consider structures with
> virtual inheritance but no virtual members as dynamic classes for ABI
> purposes; this patch fixes this.
> 
> gcc/cp/ChangeLog:
> 
> 	* decl2.cc (import_export_class): Use TYPE_CONTAINS_VPTR_P
> 	instead of TYPE_POLYMORPHIC_P.
> 	(import_export_decl): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/virt-5_a.C: New test.
> 	* g++.dg/modules/virt-5_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/decl2.cc                         |  4 ++--
>  gcc/testsuite/g++.dg/modules/virt-5_a.C | 16 ++++++++++++++++
>  gcc/testsuite/g++.dg/modules/virt-5_b.C | 11 +++++++++++
>  3 files changed, 29 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_b.C
> 
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index e9ae979896c..af544f40dac 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -2431,7 +2431,7 @@ import_export_class (tree ctype)
>         translation unit, then export the class; otherwise, import
>         it.  */
>        import_export = -1;
> -  else if (TYPE_POLYMORPHIC_P (ctype))
> +  else if (TYPE_CONTAINS_VPTR_P (ctype))
>      {
>        tree cdecl = TYPE_NAME (ctype);
>        if (DECL_LANG_SPECIFIC (cdecl) && DECL_MODULE_ATTACH_P (cdecl))
> @@ -3527,7 +3527,7 @@ import_export_decl (tree decl)
>  	  class_type = type;
>  	  import_export_class (type);
>  	  if (CLASSTYPE_INTERFACE_KNOWN (type)
> -	      && TYPE_POLYMORPHIC_P (type)
> +	      && TYPE_CONTAINS_VPTR_P (type)
>  	      && CLASSTYPE_INTERFACE_ONLY (type)
>  	      /* If -fno-rtti was specified, then we cannot be sure
>  		 that RTTI information will be emitted with the
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_a.C b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> new file mode 100644
> index 00000000000..f4c6abe85ef
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> @@ -0,0 +1,16 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +struct C {};
> +struct B : virtual C {};
> +
> +// Despite no non-inline key function, this is still a dynamic class
> +// and so by the Itanium ABI 5.2.3 should be uniquely emitted in this TU
> +export struct A : B {
> +  inline A (int) {}
> +};
> +
> +// { dg-final { scan-assembler {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler {_ZTVW1M1A:} } }
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_b.C b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> new file mode 100644
> index 00000000000..785dd92ac1e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> @@ -0,0 +1,11 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +int main() {
> +  A a(0);
> +}
> +
> +// { dg-final { scan-assembler-not {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler-not {_ZTVW1M1A:} } }
> -- 
> 2.43.2
>
Nathaniel Shead Oct. 23, 2024, 2:29 a.m. UTC | #3 
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660956.html
(and the rest of this series).

On Wed, Aug 21, 2024 at 09:38:44AM +1000, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> The Itanium ABI has specific rules for when virtual tables for dynamic
> classes should be emitted.  However we didn't consider structures with
> virtual inheritance but no virtual members as dynamic classes for ABI
> purposes; this patch fixes this.
> 
> gcc/cp/ChangeLog:
> 
> 	* decl2.cc (import_export_class): Use TYPE_CONTAINS_VPTR_P
> 	instead of TYPE_POLYMORPHIC_P.
> 	(import_export_decl): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/virt-5_a.C: New test.
> 	* g++.dg/modules/virt-5_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/decl2.cc                         |  4 ++--
>  gcc/testsuite/g++.dg/modules/virt-5_a.C | 16 ++++++++++++++++
>  gcc/testsuite/g++.dg/modules/virt-5_b.C | 11 +++++++++++
>  3 files changed, 29 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_b.C
> 
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index e9ae979896c..af544f40dac 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -2431,7 +2431,7 @@ import_export_class (tree ctype)
>         translation unit, then export the class; otherwise, import
>         it.  */
>        import_export = -1;
> -  else if (TYPE_POLYMORPHIC_P (ctype))
> +  else if (TYPE_CONTAINS_VPTR_P (ctype))
>      {
>        tree cdecl = TYPE_NAME (ctype);
>        if (DECL_LANG_SPECIFIC (cdecl) && DECL_MODULE_ATTACH_P (cdecl))
> @@ -3527,7 +3527,7 @@ import_export_decl (tree decl)
>  	  class_type = type;
>  	  import_export_class (type);
>  	  if (CLASSTYPE_INTERFACE_KNOWN (type)
> -	      && TYPE_POLYMORPHIC_P (type)
> +	      && TYPE_CONTAINS_VPTR_P (type)
>  	      && CLASSTYPE_INTERFACE_ONLY (type)
>  	      /* If -fno-rtti was specified, then we cannot be sure
>  		 that RTTI information will be emitted with the
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_a.C b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> new file mode 100644
> index 00000000000..f4c6abe85ef
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> @@ -0,0 +1,16 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +struct C {};
> +struct B : virtual C {};
> +
> +// Despite no non-inline key function, this is still a dynamic class
> +// and so by the Itanium ABI 5.2.3 should be uniquely emitted in this TU
> +export struct A : B {
> +  inline A (int) {}
> +};
> +
> +// { dg-final { scan-assembler {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler {_ZTVW1M1A:} } }
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_b.C b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> new file mode 100644
> index 00000000000..785dd92ac1e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> @@ -0,0 +1,11 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +int main() {
> +  A a(0);
> +}
> +
> +// { dg-final { scan-assembler-not {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler-not {_ZTVW1M1A:} } }
> -- 
> 2.43.2
>
Jason Merrill Oct. 24, 2024, 4:07 p.m. UTC | #4 
On 8/20/24 7:38 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> The Itanium ABI has specific rules for when virtual tables for dynamic
> classes should be emitted.  However we didn't consider structures with
> virtual inheritance but no virtual members as dynamic classes for ABI
> purposes; this patch fixes this.
> 
> gcc/cp/ChangeLog:
> 
> 	* decl2.cc (import_export_class): Use TYPE_CONTAINS_VPTR_P
> 	instead of TYPE_POLYMORPHIC_P.
> 	(import_export_decl): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/virt-5_a.C: New test.
> 	* g++.dg/modules/virt-5_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/decl2.cc                         |  4 ++--
>   gcc/testsuite/g++.dg/modules/virt-5_a.C | 16 ++++++++++++++++
>   gcc/testsuite/g++.dg/modules/virt-5_b.C | 11 +++++++++++
>   3 files changed, 29 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/virt-5_b.C
> 
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index e9ae979896c..af544f40dac 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -2431,7 +2431,7 @@ import_export_class (tree ctype)
>          translation unit, then export the class; otherwise, import
>          it.  */
>         import_export = -1;
> -  else if (TYPE_POLYMORPHIC_P (ctype))
> +  else if (TYPE_CONTAINS_VPTR_P (ctype))
>       {
>         tree cdecl = TYPE_NAME (ctype);
>         if (DECL_LANG_SPECIFIC (cdecl) && DECL_MODULE_ATTACH_P (cdecl))
> @@ -3527,7 +3527,7 @@ import_export_decl (tree decl)
>   	  class_type = type;
>   	  import_export_class (type);
>   	  if (CLASSTYPE_INTERFACE_KNOWN (type)
> -	      && TYPE_POLYMORPHIC_P (type)
> +	      && TYPE_CONTAINS_VPTR_P (type)
>   	      && CLASSTYPE_INTERFACE_ONLY (type)
>   	      /* If -fno-rtti was specified, then we cannot be sure
>   		 that RTTI information will be emitted with the
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_a.C b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> new file mode 100644
> index 00000000000..f4c6abe85ef
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_a.C
> @@ -0,0 +1,16 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +struct C {};
> +struct B : virtual C {};
> +
> +// Despite no non-inline key function, this is still a dynamic class
> +// and so by the Itanium ABI 5.2.3 should be uniquely emitted in this TU
> +export struct A : B {
> +  inline A (int) {}
> +};
> +
> +// { dg-final { scan-assembler {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler {_ZTVW1M1A:} } }
> diff --git a/gcc/testsuite/g++.dg/modules/virt-5_b.C b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> new file mode 100644
> index 00000000000..785dd92ac1e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/virt-5_b.C
> @@ -0,0 +1,11 @@
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +int main() {
> +  A a(0);
> +}
> +
> +// { dg-final { scan-assembler-not {_ZTTW1M1A:} } }
> +// { dg-final { scan-assembler-not {_ZTVW1M1A:} } }
diff mbox series

Patch

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index e9ae979896c..af544f40dac 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2431,7 +2431,7 @@  import_export_class (tree ctype)
        translation unit, then export the class; otherwise, import
        it.  */
       import_export = -1;
-  else if (TYPE_POLYMORPHIC_P (ctype))
+  else if (TYPE_CONTAINS_VPTR_P (ctype))
     {
       tree cdecl = TYPE_NAME (ctype);
       if (DECL_LANG_SPECIFIC (cdecl) && DECL_MODULE_ATTACH_P (cdecl))
@@ -3527,7 +3527,7 @@  import_export_decl (tree decl)
 	  class_type = type;
 	  import_export_class (type);
 	  if (CLASSTYPE_INTERFACE_KNOWN (type)
-	      && TYPE_POLYMORPHIC_P (type)
+	      && TYPE_CONTAINS_VPTR_P (type)
 	      && CLASSTYPE_INTERFACE_ONLY (type)
 	      /* If -fno-rtti was specified, then we cannot be sure
 		 that RTTI information will be emitted with the
diff --git a/gcc/testsuite/g++.dg/modules/virt-5_a.C b/gcc/testsuite/g++.dg/modules/virt-5_a.C
new file mode 100644
index 00000000000..f4c6abe85ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/virt-5_a.C
@@ -0,0 +1,16 @@ 
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+
+struct C {};
+struct B : virtual C {};
+
+// Despite no non-inline key function, this is still a dynamic class
+// and so by the Itanium ABI 5.2.3 should be uniquely emitted in this TU
+export struct A : B {
+  inline A (int) {}
+};
+
+// { dg-final { scan-assembler {_ZTTW1M1A:} } }
+// { dg-final { scan-assembler {_ZTVW1M1A:} } }
diff --git a/gcc/testsuite/g++.dg/modules/virt-5_b.C b/gcc/testsuite/g++.dg/modules/virt-5_b.C
new file mode 100644
index 00000000000..785dd92ac1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/virt-5_b.C
@@ -0,0 +1,11 @@ 
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+int main() {
+  A a(0);
+}
+
+// { dg-final { scan-assembler-not {_ZTTW1M1A:} } }
+// { dg-final { scan-assembler-not {_ZTVW1M1A:} } }