diff mbox series

c++: Alignment changes to layout compatibility/common initial sequence - DR2583

Message ID Y3ImZ/fcwjmMFQyb@tucnak
State Unresolved
Headers
Series c++: Alignment changes to layout compatibility/common initial sequence - DR2583 |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Nov. 14, 2022, 11:28 a.m. UTC 
  Hi!

Working virtually out of Baker Island.

When trying to figure out what to do about alignment,
layout_compatible_type_p returns false if TYPE_ALIGN on
ENUMERAL_TYPE/CLASS_TYPE_P (but not scalar types?) differ, or if members
don't have the same positions.

What is in DR2583 doesn't say anything like that though, on the other side
it says that if the corresponding entities don't have the same alignment
requirements, they aren't part of the common initial sequence.

So, my understanding of this is we shouldn't check TYPE_ALIGN in
layout_compatible_type_p, but instead DECL_ALIGN in
next_common_initial_seqence.

Lightly tested (on is-layout*/is-corresponding*/dr2583.C only) so far,
ok if it passes full bootstrap/regtest?
Or do we need different rules?

2022-11-14  Jakub Jelinek  <jakub@redhat.com>

	* typeck.cc (next_common_initial_seqence): Return false members have
	different DECL_ALIGN.
	(layout_compatible_type_p): Don't test TYPE_ALIGN of ENUMERAL_TYPE
	or CLASS_TYPE_P.

	* g++.dg/cpp2a/is-layout-compatible3.C: Expect enums with different
	alignas to be layout compatible, while classes with different
	alignas on members layout incompatible.
	* g++.dg/DRs/dr2583.C: New test.


	Jakub

Comments

Jason Merrill Nov. 15, 2022, 10:57 p.m. UTC | #1 
On 11/14/22 01:28, Jakub Jelinek wrote:
> Hi!
> 
> Working virtually out of Baker Island.
> 
> When trying to figure out what to do about alignment,
> layout_compatible_type_p returns false if TYPE_ALIGN on
> ENUMERAL_TYPE/CLASS_TYPE_P (but not scalar types?) differ, or if members
> don't have the same positions.
> 
> What is in DR2583 doesn't say anything like that though, on the other side
> it says that if the corresponding entities don't have the same alignment
> requirements, they aren't part of the common initial sequence.
> 
> So, my understanding of this is we shouldn't check TYPE_ALIGN in
> layout_compatible_type_p, but instead DECL_ALIGN in
> next_common_initial_seqence.

Agreed.

> Lightly tested (on is-layout*/is-corresponding*/dr2583.C only) so far,
> ok if it passes full bootstrap/regtest?
> Or do we need different rules?
> 
> 2022-11-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* typeck.cc (next_common_initial_seqence): Return false members have
> 	different DECL_ALIGN.
> 	(layout_compatible_type_p): Don't test TYPE_ALIGN of ENUMERAL_TYPE
> 	or CLASS_TYPE_P.
> 
> 	* g++.dg/cpp2a/is-layout-compatible3.C: Expect enums with different
> 	alignas to be layout compatible, while classes with different
> 	alignas on members layout incompatible.
> 	* g++.dg/DRs/dr2583.C: New test.
> 
> --- gcc/cp/typeck.cc.jj	2022-11-13 04:53:46.010682269 -1200
> +++ gcc/cp/typeck.cc	2022-11-13 23:14:41.355180354 -1200
> @@ -1833,6 +1833,8 @@ next_common_initial_seqence (tree &memb1
>     if ((!lookup_attribute ("no_unique_address", DECL_ATTRIBUTES (memb1)))
>         != !lookup_attribute ("no_unique_address", DECL_ATTRIBUTES (memb2)))
>       return false;
> +  if (DECL_ALIGN (memb1) != DECL_ALIGN (memb2))
> +    return false;
>     if (!tree_int_cst_equal (bit_position (memb1), bit_position (memb2)))
>       return false;
>     return true;
> @@ -1854,15 +1856,13 @@ layout_compatible_type_p (tree type1, tr
>     type2 = cp_build_qualified_type (type2, TYPE_UNQUALIFIED);
>   
>     if (TREE_CODE (type1) == ENUMERAL_TYPE)
> -    return (TYPE_ALIGN (type1) == TYPE_ALIGN (type2)
> -	    && tree_int_cst_equal (TYPE_SIZE (type1), TYPE_SIZE (type2))
> +    return (tree_int_cst_equal (TYPE_SIZE (type1), TYPE_SIZE (type2))
>   	    && same_type_p (finish_underlying_type (type1),
>   			    finish_underlying_type (type2)));
>   
>     if (CLASS_TYPE_P (type1)
>         && std_layout_type_p (type1)
>         && std_layout_type_p (type2)
> -      && TYPE_ALIGN (type1) == TYPE_ALIGN (type2)
>         && tree_int_cst_equal (TYPE_SIZE (type1), TYPE_SIZE (type2)))
>       {
>         tree field1 = TYPE_FIELDS (type1);
> --- gcc/testsuite/g++.dg/cpp2a/is-layout-compatible3.C.jj	2021-08-18 21:42:27.414421719 -1200
> +++ gcc/testsuite/g++.dg/cpp2a/is-layout-compatible3.C	2022-11-13 23:20:05.008776825 -1200
> @@ -55,10 +55,10 @@ static_assert (!std::is_layout_compatibl
>   static_assert (!std::is_layout_compatible_v<M, N>);
>   static_assert (!std::is_layout_compatible_v<O, P>);
>   static_assert (!std::is_layout_compatible_v<P, D>);
> -static_assert (!std::is_layout_compatible_v<Q, R>);
> +static_assert (std::is_layout_compatible_v<Q, R>);
>   static_assert (!std::is_layout_compatible_v<U, V>);
>   static_assert (!std::is_layout_compatible_v<A, I>);
>   static_assert (!std::is_layout_compatible_v<C, I>);
> -static_assert (std::is_layout_compatible_v<E, F>);
> +static_assert (!std::is_layout_compatible_v<E, F>);
>   static_assert (std::is_layout_compatible_v<G, H>);
>   static_assert (std::is_layout_compatible_v<C1, D1>);
> --- gcc/testsuite/g++.dg/DRs/dr2583.C.jj	2022-11-13 22:58:11.977640606 -1200
> +++ gcc/testsuite/g++.dg/DRs/dr2583.C	2022-11-13 23:18:04.630414835 -1200
> @@ -0,0 +1,31 @@
> +// DR 2583 - Common initial sequence should consider over-alignment.
> +// { dg-do compile { target c++11 } }
> +
> +#include <type_traits>
> +
> +struct A {
> +  int i;
> +  char c;
> +};
> +
> +struct B {
> +  int i;
> +  alignas(8) char c;
> +};
> +
> +struct S0 {
> +  alignas(16) char x[128];
> +  int i;
> +};
> +
> +struct alignas(16) S1 {
> +  char x[128];
> +  int i;
> +};
> +
> +#if __cpp_lib_is_layout_compatible >= 201907L
> +static_assert (std::is_corresponding_member (&A::i, &B::i), "");
> +static_assert (alignof (char) == 8 || !std::is_corresponding_member (&A::c, &B::c), "");

Maybe

std_is_corresponding_member (&A::c, &B::c) == (alignof (char) == 8)

?

Could also use an alignas(alignof(type)) case to verify that it matches 
a member with no alignas.

> +static_assert (alignof (char) == 16 || !std::is_corresponding_member (&S0::x, &S1::x), "");
> +static_assert (alignof (char) == 16 || !std::is_corresponding_member (&S0::i, &S1::i), "");
> +#endif
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/cp/typeck.cc.jj	2022-11-13 04:53:46.010682269 -1200
+++ gcc/cp/typeck.cc	2022-11-13 23:14:41.355180354 -1200
@@ -1833,6 +1833,8 @@  next_common_initial_seqence (tree &memb1
   if ((!lookup_attribute ("no_unique_address", DECL_ATTRIBUTES (memb1)))
       != !lookup_attribute ("no_unique_address", DECL_ATTRIBUTES (memb2)))
     return false;
+  if (DECL_ALIGN (memb1) != DECL_ALIGN (memb2))
+    return false;
   if (!tree_int_cst_equal (bit_position (memb1), bit_position (memb2)))
     return false;
   return true;
@@ -1854,15 +1856,13 @@  layout_compatible_type_p (tree type1, tr
   type2 = cp_build_qualified_type (type2, TYPE_UNQUALIFIED);
 
   if (TREE_CODE (type1) == ENUMERAL_TYPE)
-    return (TYPE_ALIGN (type1) == TYPE_ALIGN (type2)
-	    && tree_int_cst_equal (TYPE_SIZE (type1), TYPE_SIZE (type2))
+    return (tree_int_cst_equal (TYPE_SIZE (type1), TYPE_SIZE (type2))
 	    && same_type_p (finish_underlying_type (type1),
 			    finish_underlying_type (type2)));
 
   if (CLASS_TYPE_P (type1)
       && std_layout_type_p (type1)
       && std_layout_type_p (type2)
-      && TYPE_ALIGN (type1) == TYPE_ALIGN (type2)
       && tree_int_cst_equal (TYPE_SIZE (type1), TYPE_SIZE (type2)))
     {
       tree field1 = TYPE_FIELDS (type1);
--- gcc/testsuite/g++.dg/cpp2a/is-layout-compatible3.C.jj	2021-08-18 21:42:27.414421719 -1200
+++ gcc/testsuite/g++.dg/cpp2a/is-layout-compatible3.C	2022-11-13 23:20:05.008776825 -1200
@@ -55,10 +55,10 @@  static_assert (!std::is_layout_compatibl
 static_assert (!std::is_layout_compatible_v<M, N>);
 static_assert (!std::is_layout_compatible_v<O, P>);
 static_assert (!std::is_layout_compatible_v<P, D>);
-static_assert (!std::is_layout_compatible_v<Q, R>);
+static_assert (std::is_layout_compatible_v<Q, R>);
 static_assert (!std::is_layout_compatible_v<U, V>);
 static_assert (!std::is_layout_compatible_v<A, I>);
 static_assert (!std::is_layout_compatible_v<C, I>);
-static_assert (std::is_layout_compatible_v<E, F>);
+static_assert (!std::is_layout_compatible_v<E, F>);
 static_assert (std::is_layout_compatible_v<G, H>);
 static_assert (std::is_layout_compatible_v<C1, D1>);
--- gcc/testsuite/g++.dg/DRs/dr2583.C.jj	2022-11-13 22:58:11.977640606 -1200
+++ gcc/testsuite/g++.dg/DRs/dr2583.C	2022-11-13 23:18:04.630414835 -1200
@@ -0,0 +1,31 @@ 
+// DR 2583 - Common initial sequence should consider over-alignment.
+// { dg-do compile { target c++11 } }
+
+#include <type_traits>
+
+struct A {
+  int i;
+  char c;
+};
+
+struct B {
+  int i;
+  alignas(8) char c;
+};
+
+struct S0 {
+  alignas(16) char x[128];
+  int i;
+};
+
+struct alignas(16) S1 {
+  char x[128];
+  int i;
+};
+
+#if __cpp_lib_is_layout_compatible >= 201907L
+static_assert (std::is_corresponding_member (&A::i, &B::i), "");
+static_assert (alignof (char) == 8 || !std::is_corresponding_member (&A::c, &B::c), "");
+static_assert (alignof (char) == 16 || !std::is_corresponding_member (&S0::x, &S1::x), "");
+static_assert (alignof (char) == 16 || !std::is_corresponding_member (&S0::i, &S1::i), "");
+#endif