diff mbox series

[1/2] libstdc++: Simplify <ext/aligned_buffer.h> class templates

Message ID 20240627090040.95665-1-jwakely@redhat.com
State New
Headers show
Series [1/2] libstdc++: Simplify <ext/aligned_buffer.h> class templates | expand

Commit Message

Jonathan Wakely June 27, 2024, 8:55 a.m. UTC
I'm planning to push this, although arguably the first change isn't
worth doing if we can't use it everywhere. If we need to keep the old
code for EDG, maybe we should just keep using that? The new version
probably compiles faster though.

Removing the dependency on std::aligned_storage and adding the test is
surely useful though.

Tested x86_64-linux.

-- >8 --

As noted in a comment, the __gnu_cxx::__aligned_membuf class template
can be simplified, because alignof(T) and alignas(T) use the correct
alignment for a data member. That's true since GCC 8 and Clang 8. The
EDG front end (as used by Intel icc, aka "Intel C++ Compiler Classic")
does not implement the PR c++/69560 change, so keep using the old
implementation when __EDG__ is defined, to avoid an ABI change for icc.

For __gnu_cxx::__aligned_buffer<T> all supported compilers agree on the
value of __alignof__(T), but we can still simplify it by removing the
dependency on std::aligned_storage<sizeof(T), __alignof__(T)>.

Add a test that checks that the aligned buffer types have the expected
alignment, so that we can tell if changes like this affect their ABI
properties.

libstdc++-v3/ChangeLog:

	* include/ext/aligned_buffer.h (__aligned_membuf): Use
	alignas(T) directly instead of defining a struct and using 9its
	alignment.
	(__aligned_buffer): Remove use of std::aligned_storage.
	* testsuite/abi/aligned_buffers.cc: New test.
---
 libstdc++-v3/include/ext/aligned_buffer.h     | 20 ++++-----
 libstdc++-v3/testsuite/abi/aligned_buffers.cc | 42 +++++++++++++++++++
 2 files changed, 52 insertions(+), 10 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/abi/aligned_buffers.cc

Comments

Jonathan Wakely June 28, 2024, 7:23 p.m. UTC | #1
Pushed to trunk.

On Thu, 27 Jun 2024 at 10:03, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> I'm planning to push this, although arguably the first change isn't
> worth doing if we can't use it everywhere. If we need to keep the old
> code for EDG, maybe we should just keep using that? The new version
> probably compiles faster though.
>
> Removing the dependency on std::aligned_storage and adding the test is
> surely useful though.
>
> Tested x86_64-linux.
>
> -- >8 --
>
> As noted in a comment, the __gnu_cxx::__aligned_membuf class template
> can be simplified, because alignof(T) and alignas(T) use the correct
> alignment for a data member. That's true since GCC 8 and Clang 8. The
> EDG front end (as used by Intel icc, aka "Intel C++ Compiler Classic")
> does not implement the PR c++/69560 change, so keep using the old
> implementation when __EDG__ is defined, to avoid an ABI change for icc.
>
> For __gnu_cxx::__aligned_buffer<T> all supported compilers agree on the
> value of __alignof__(T), but we can still simplify it by removing the
> dependency on std::aligned_storage<sizeof(T), __alignof__(T)>.
>
> Add a test that checks that the aligned buffer types have the expected
> alignment, so that we can tell if changes like this affect their ABI
> properties.
>
> libstdc++-v3/ChangeLog:
>
>         * include/ext/aligned_buffer.h (__aligned_membuf): Use
>         alignas(T) directly instead of defining a struct and using 9its
>         alignment.
>         (__aligned_buffer): Remove use of std::aligned_storage.
>         * testsuite/abi/aligned_buffers.cc: New test.
> ---
>  libstdc++-v3/include/ext/aligned_buffer.h     | 20 ++++-----
>  libstdc++-v3/testsuite/abi/aligned_buffers.cc | 42 +++++++++++++++++++
>  2 files changed, 52 insertions(+), 10 deletions(-)
>  create mode 100644 libstdc++-v3/testsuite/abi/aligned_buffers.cc
>
> diff --git a/libstdc++-v3/include/ext/aligned_buffer.h b/libstdc++-v3/include/ext/aligned_buffer.h
> index 26b36609fa5..9c2c628e54a 100644
> --- a/libstdc++-v3/include/ext/aligned_buffer.h
> +++ b/libstdc++-v3/include/ext/aligned_buffer.h
> @@ -49,11 +49,15 @@ namespace __gnu_cxx
>        // Target macro ADJUST_FIELD_ALIGN can produce different alignment for
>        // types when used as class members. __aligned_membuf is intended
>        // for use as a class member, so align the buffer as for a class member.
> -      // Since GCC 8 we could just use alignof(_Tp) instead, but older
> -      // versions of non-GNU compilers might still need this trick.
> +      // Since GCC 8 we can just use alignas(_Tp) to get the right alignment.
> +#ifdef __EDG__
> +      // The EDG front end does not implement the PR c++/69560 alignof change.
>        struct _Tp2 { _Tp _M_t; };
> -
> -      alignas(__alignof__(_Tp2::_M_t)) unsigned char _M_storage[sizeof(_Tp)];
> +      alignas(__alignof__(_Tp2::_M_t))
> +#else
> +      alignas(_Tp)
> +#endif
> +       unsigned char _M_storage[sizeof(_Tp)];
>
>        __aligned_membuf() = default;
>
> @@ -81,8 +85,6 @@ namespace __gnu_cxx
>    template<typename _Tp>
>      using __aligned_buffer = __aligned_membuf<_Tp>;
>  #else
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>    // Similar to __aligned_membuf but aligned for complete objects, not members.
>    // This type is used in <forward_list>, <future>, <bits/shared_ptr_base.h>
>    // and <bits/hashtable_policy.h>, but ideally they would use __aligned_membuf
> @@ -90,10 +92,9 @@ namespace __gnu_cxx
>    // This type is still used to avoid an ABI change.
>    template<typename _Tp>
>      struct __aligned_buffer
> -    : std::aligned_storage<sizeof(_Tp), __alignof__(_Tp)>
>      {
> -      typename
> -       std::aligned_storage<sizeof(_Tp), __alignof__(_Tp)>::type _M_storage;
> +      // Using __alignof__ gives the alignment for a complete object.
> +      alignas(__alignof__(_Tp)) unsigned char _M_storage[sizeof(_Tp)];
>
>        __aligned_buffer() = default;
>
> @@ -120,7 +121,6 @@ namespace __gnu_cxx
>        _M_ptr() const noexcept
>        { return static_cast<const _Tp*>(_M_addr()); }
>      };
> -#pragma GCC diagnostic pop
>  #endif
>
>  } // namespace
> diff --git a/libstdc++-v3/testsuite/abi/aligned_buffers.cc b/libstdc++-v3/testsuite/abi/aligned_buffers.cc
> new file mode 100644
> index 00000000000..b4b8ea13970
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/abi/aligned_buffers.cc
> @@ -0,0 +1,42 @@
> +// { dg-do compile { target c++11 } }
> +
> +// Check alignment of the buffer types used for uninitialized storage.
> +
> +#include <ext/aligned_buffer.h>
> +
> +template<typename T> using membuf = __gnu_cxx::__aligned_membuf<T>;
> +template<typename T> using objbuf = __gnu_cxx::__aligned_buffer<T>;
> +
> +template<typename T>
> +constexpr bool
> +check_alignof_membuf()
> +{
> +  return alignof(membuf<T>) == alignof(T)
> +    && __alignof__(membuf<T>) == alignof(T);
> +}
> +
> +template<typename T>
> +constexpr bool
> +check_alignof_objbuf()
> +{
> +#if _GLIBCXX_INLINE_VERSION
> +  // For the gnu-versioned-namespace ABI __aligned_buffer == __aligned_membuf.
> +  return check_alignof_membuf<T>();
> +#else
> +  return alignof(objbuf<T>) == __alignof__(T)
> +    && __alignof__(objbuf<T>) == __alignof__(T);
> +#endif
> +}
> +
> +struct S { long long l; };
> +struct alignas(128) X { char x; };
> +static_assert( check_alignof_membuf<int>(), "membuf<int>" );
> +static_assert( check_alignof_membuf<long long>(), "membuf<long long>" );
> +static_assert( check_alignof_membuf<void*>(), "membuf<void*>" );
> +static_assert( check_alignof_membuf<S>(), "membuf<S>" );
> +static_assert( check_alignof_membuf<X>(), "membuf<X>" );
> +static_assert( check_alignof_objbuf<int>(), "objbuf<int>" );
> +static_assert( check_alignof_objbuf<long long>(), "objbuf<long long>" );
> +static_assert( check_alignof_objbuf<void*>(), "objbuf<void*>" );
> +static_assert( check_alignof_objbuf<S>(), "objbuf<S>" );
> +static_assert( check_alignof_objbuf<X>(), "objbuf<X>" );
> --
> 2.45.2
>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/ext/aligned_buffer.h b/libstdc++-v3/include/ext/aligned_buffer.h
index 26b36609fa5..9c2c628e54a 100644
--- a/libstdc++-v3/include/ext/aligned_buffer.h
+++ b/libstdc++-v3/include/ext/aligned_buffer.h
@@ -49,11 +49,15 @@  namespace __gnu_cxx
       // Target macro ADJUST_FIELD_ALIGN can produce different alignment for
       // types when used as class members. __aligned_membuf is intended
       // for use as a class member, so align the buffer as for a class member.
-      // Since GCC 8 we could just use alignof(_Tp) instead, but older
-      // versions of non-GNU compilers might still need this trick.
+      // Since GCC 8 we can just use alignas(_Tp) to get the right alignment.
+#ifdef __EDG__
+      // The EDG front end does not implement the PR c++/69560 alignof change.
       struct _Tp2 { _Tp _M_t; };
-
-      alignas(__alignof__(_Tp2::_M_t)) unsigned char _M_storage[sizeof(_Tp)];
+      alignas(__alignof__(_Tp2::_M_t))
+#else
+      alignas(_Tp)
+#endif
+	unsigned char _M_storage[sizeof(_Tp)];
 
       __aligned_membuf() = default;
 
@@ -81,8 +85,6 @@  namespace __gnu_cxx
   template<typename _Tp>
     using __aligned_buffer = __aligned_membuf<_Tp>;
 #else
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   // Similar to __aligned_membuf but aligned for complete objects, not members.
   // This type is used in <forward_list>, <future>, <bits/shared_ptr_base.h>
   // and <bits/hashtable_policy.h>, but ideally they would use __aligned_membuf
@@ -90,10 +92,9 @@  namespace __gnu_cxx
   // This type is still used to avoid an ABI change.
   template<typename _Tp>
     struct __aligned_buffer
-    : std::aligned_storage<sizeof(_Tp), __alignof__(_Tp)>
     {
-      typename
-	std::aligned_storage<sizeof(_Tp), __alignof__(_Tp)>::type _M_storage;
+      // Using __alignof__ gives the alignment for a complete object.
+      alignas(__alignof__(_Tp)) unsigned char _M_storage[sizeof(_Tp)];
 
       __aligned_buffer() = default;
 
@@ -120,7 +121,6 @@  namespace __gnu_cxx
       _M_ptr() const noexcept
       { return static_cast<const _Tp*>(_M_addr()); }
     };
-#pragma GCC diagnostic pop
 #endif
 
 } // namespace
diff --git a/libstdc++-v3/testsuite/abi/aligned_buffers.cc b/libstdc++-v3/testsuite/abi/aligned_buffers.cc
new file mode 100644
index 00000000000..b4b8ea13970
--- /dev/null
+++ b/libstdc++-v3/testsuite/abi/aligned_buffers.cc
@@ -0,0 +1,42 @@ 
+// { dg-do compile { target c++11 } }
+
+// Check alignment of the buffer types used for uninitialized storage.
+
+#include <ext/aligned_buffer.h>
+
+template<typename T> using membuf = __gnu_cxx::__aligned_membuf<T>;
+template<typename T> using objbuf = __gnu_cxx::__aligned_buffer<T>;
+
+template<typename T>
+constexpr bool
+check_alignof_membuf()
+{
+  return alignof(membuf<T>) == alignof(T)
+    && __alignof__(membuf<T>) == alignof(T);
+}
+
+template<typename T>
+constexpr bool
+check_alignof_objbuf()
+{
+#if _GLIBCXX_INLINE_VERSION
+  // For the gnu-versioned-namespace ABI __aligned_buffer == __aligned_membuf.
+  return check_alignof_membuf<T>();
+#else
+  return alignof(objbuf<T>) == __alignof__(T)
+    && __alignof__(objbuf<T>) == __alignof__(T);
+#endif
+}
+
+struct S { long long l; };
+struct alignas(128) X { char x; };
+static_assert( check_alignof_membuf<int>(), "membuf<int>" );
+static_assert( check_alignof_membuf<long long>(), "membuf<long long>" );
+static_assert( check_alignof_membuf<void*>(), "membuf<void*>" );
+static_assert( check_alignof_membuf<S>(), "membuf<S>" );
+static_assert( check_alignof_membuf<X>(), "membuf<X>" );
+static_assert( check_alignof_objbuf<int>(), "objbuf<int>" );
+static_assert( check_alignof_objbuf<long long>(), "objbuf<long long>" );
+static_assert( check_alignof_objbuf<void*>(), "objbuf<void*>" );
+static_assert( check_alignof_objbuf<S>(), "objbuf<S>" );
+static_assert( check_alignof_objbuf<X>(), "objbuf<X>" );