[committed] libstdc++: Disable cacheline alignment for DJGPP [PR109741]

[committed] libstdc++: Disable cacheline alignment for DJGPP [PR109741]

[Jonathan Wakely]

Tested powerpc64le-linux. Builds OK on djgpp too.

Pushed to trunk.

-- >8 --

DJGPP (and maybe other targets) uses MAX_OFILE_ALIGNMENT=16 which means
that globals (and static objects) can't have alignment greater than 16.
This causes an error for the locks defined in src/c++11/shared_ptr.cc
because we try to align them to the cacheline size, to avoid false
sharing.

Add a configure check for the increased alignment, and live with false
sharing where we can't increase the alignment.

libstdc++-v3/ChangeLog:

	PR libstdc++/109741
	* acinclude.m4 (GLIBCXX_CHECK_ALIGNAS_CACHELINE): Define.
	* config.h.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Use GLIBCXX_CHECK_ALIGNAS_CACHELINE.
	* src/c++11/shared_ptr.cc (__gnu_internal::get_mutex): Do not
	align lock table if not supported. use __GCC_DESTRUCTIVE_SIZE
	instead of hardcoded 64.
---
 libstdc++-v3/acinclude.m4            | 25 +++++++++++++++
 libstdc++-v3/config.h.in             |  4 +++
 libstdc++-v3/configure               | 48 ++++++++++++++++++++++++++++
 libstdc++-v3/configure.ac            |  3 ++
 libstdc++-v3/src/c++11/shared_ptr.cc |  8 +++--
 5 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 988c532c4e2..8129373e9dd 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -5471,6 +5471,31 @@ AC_DEFUN([GLIBCXX_ZONEINFO_DIR], [
   fi
 ])
 
+dnl
+dnl Check whether lock tables can be aligned to avoid false sharing.
+dnl
+dnl Defines:
+dnl  _GLIBCXX_CAN_ALIGNAS_DESTRUCTIVE_SIZE if objects with static storage
+dnl    duration can be aligned to std::hardware_destructive_interference_size.
+dnl
+AC_DEFUN([GLIBCXX_CHECK_ALIGNAS_CACHELINE], [
+  AC_LANG_SAVE
+  AC_LANG_CPLUSPLUS
+
+  AC_MSG_CHECKING([whether static objects can be aligned to the cacheline size])
+  AC_TRY_COMPILE(, [struct alignas(__GCC_DESTRUCTIVE_SIZE) Aligned { };
+		    alignas(Aligned) static char buf[sizeof(Aligned) * 16];
+		 ], [ac_alignas_cacheline=yes], [ac_alignas_cacheline=no])
+  if test "$ac_alignas_cacheline" = yes; then
+    AC_DEFINE_UNQUOTED(_GLIBCXX_CAN_ALIGNAS_DESTRUCTIVE_SIZE, 1,
+      [Define if global objects can be aligned to
+       std::hardware_destructive_interference_size.])
+  fi
+  AC_MSG_RESULT($ac_alignas_cacheline)
+
+  AC_LANG_RESTORE
+])
+
 # Macros from the top-level gcc directory.
 m4_include([../config/gc++filt.m4])
 m4_include([../config/tls.m4])
diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
index f91f7eb9097..bbb2613ff69 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -819,6 +819,10 @@
 /* Define if the compiler supports C++11 atomics. */
 #undef _GLIBCXX_ATOMIC_BUILTINS
 
+/* Define if global objects can be aligned to
+   std::hardware_destructive_interference_size. */
+#undef _GLIBCXX_CAN_ALIGNAS_DESTRUCTIVE_SIZE
+
 /* Define to use concept checking code from the boost libraries. */
 #undef _GLIBCXX_CONCEPT_CHECKS
 
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index a9589d882e6..188be08d716 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -71957,6 +71957,54 @@ _ACEOF
   fi
 
 
+
+
+  ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether static objects can be aligned to the cacheline size" >&5
+$as_echo_n "checking whether static objects can be aligned to the cacheline size... " >&6; }
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main ()
+{
+struct alignas(__GCC_DESTRUCTIVE_SIZE) Aligned { };
+		    alignas(Aligned) static char buf[sizeof(Aligned) * 16];
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  ac_alignas_cacheline=yes
+else
+  ac_alignas_cacheline=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+  if test "$ac_alignas_cacheline" = yes; then
+
+cat >>confdefs.h <<_ACEOF
+#define _GLIBCXX_CAN_ALIGNAS_DESTRUCTIVE_SIZE 1
+_ACEOF
+
+  fi
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_alignas_cacheline" >&5
+$as_echo "$ac_alignas_cacheline" >&6; }
+
+  ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+
+
 # Define documentation rules conditionally.
 
 # See if makeinfo has been installed and is modern enough
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 0dd550a4b4b..df01f58bd83 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -538,6 +538,9 @@ GLIBCXX_EMERGENCY_EH_ALLOC
 # For src/c++20/tzdb.cc defaults.
 GLIBCXX_ZONEINFO_DIR
 
+# For src/c++11/shared_ptr.cc alignment.
+GLIBCXX_CHECK_ALIGNAS_CACHELINE
+
 # Define documentation rules conditionally.
 
 # See if makeinfo has been installed and is modern enough
diff --git a/libstdc++-v3/src/c++11/shared_ptr.cc b/libstdc++-v3/src/c++11/shared_ptr.cc
index 74e879e5828..ae47ca03301 100644
--- a/libstdc++-v3/src/c++11/shared_ptr.cc
+++ b/libstdc++-v3/src/c++11/shared_ptr.cc
@@ -34,8 +34,12 @@ namespace __gnu_internal _GLIBCXX_VISIBILITY(hidden)
   __gnu_cxx::__mutex&
   get_mutex(unsigned char i)
   {
-    // increase alignment to put each lock on a separate cache line
-    struct alignas(64) M : __gnu_cxx::__mutex { };
+#ifdef _GLIBCXX_CAN_ALIGNAS_DESTRUCTIVE_SIZE
+    // Increase alignment to put each lock on a separate cache line.
+    struct alignas(__GCC_DESTRUCTIVE_SIZE) M : __gnu_cxx::__mutex { };
+#else
+    using M = __gnu_cxx::__mutex;
+#endif
     // Use a static buffer, so that the mutexes are not destructed
     // before potential users (or at all)
     static __attribute__ ((aligned(__alignof__(M))))
-- 
2.40.1

Re: [committed] libstdc++: Disable cacheline alignment for DJGPP [PR109741]

[Martin Jambor]

Hello,

On Tue, May 16 2023, Jonathan Wakely via Gcc-patches wrote:
> Tested powerpc64le-linux. Builds OK on djgpp too.
>
> Pushed to trunk.
>
> -- >8 --
>
> DJGPP (and maybe other targets) uses MAX_OFILE_ALIGNMENT=16 which means
> that globals (and static objects) can't have alignment greater than 16.
> This causes an error for the locks defined in src/c++11/shared_ptr.cc
> because we try to align them to the cacheline size, to avoid false
> sharing.
>
> Add a configure check for the increased alignment, and live with false
> sharing where we can't increase the alignment.
>
> libstdc++-v3/ChangeLog:
>
> 	PR libstdc++/109741
> 	* acinclude.m4 (GLIBCXX_CHECK_ALIGNAS_CACHELINE): Define.
> 	* config.h.in: Regenerate.
> 	* configure: Regenerate.
> 	* configure.ac: Use GLIBCXX_CHECK_ALIGNAS_CACHELINE.
> 	* src/c++11/shared_ptr.cc (__gnu_internal::get_mutex): Do not
> 	align lock table if not supported. use __GCC_DESTRUCTIVE_SIZE
> 	instead of hardcoded 64.

The periodic tests that Martin Liška left behind for me (for now) to
look at are now complaining that the configure and configure.ac in
libstdc++ are not in sync, the difference being (drum roll):

diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 188be08d716..412c4bf0e85 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -71957,6 +71957,7 @@ _ACEOF
   fi
 
 
+# For src/c++11/shared_ptr.cc alignment.
 
 
   ac_ext=cpp


Can I commit this change or do I need to attempt to adjust the script to
ignore comments in configure or what would be the correct way to deal
with this? (Option requiring work on my side may take some time during
which the otherwise useful tests would not work, I am afraid).

Thanks,

Martin

Re: [committed] libstdc++: Disable cacheline alignment for DJGPP [PR109741]

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

On Wed, 17 May 2023 at 10:32, Martin Jambor wrote:

> Hello,
>
> On Tue, May 16 2023, Jonathan Wakely via Gcc-patches wrote:
> > Tested powerpc64le-linux. Builds OK on djgpp too.
> >
> > Pushed to trunk.
> >
> > -- >8 --
> >
> > DJGPP (and maybe other targets) uses MAX_OFILE_ALIGNMENT=16 which means
> > that globals (and static objects) can't have alignment greater than 16.
> > This causes an error for the locks defined in src/c++11/shared_ptr.cc
> > because we try to align them to the cacheline size, to avoid false
> > sharing.
> >
> > Add a configure check for the increased alignment, and live with false
> > sharing where we can't increase the alignment.
> >
> > libstdc++-v3/ChangeLog:
> >
> >       PR libstdc++/109741
> >       * acinclude.m4 (GLIBCXX_CHECK_ALIGNAS_CACHELINE): Define.
> >       * config.h.in: Regenerate.
> >       * configure: Regenerate.
> >       * configure.ac: Use GLIBCXX_CHECK_ALIGNAS_CACHELINE.
> >       * src/c++11/shared_ptr.cc (__gnu_internal::get_mutex): Do not
> >       align lock table if not supported. use __GCC_DESTRUCTIVE_SIZE
> >       instead of hardcoded 64.
>
> The periodic tests that Martin Liška left behind for me (for now) to
> look at are now complaining that the configure and configure.ac in
> libstdc++ are not in sync, the difference being (drum roll):
>
> diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
> index 188be08d716..412c4bf0e85 100755
> --- a/libstdc++-v3/configure
> +++ b/libstdc++-v3/configure
> @@ -71957,6 +71957,7 @@ _ACEOF
>    fi
>
>
> +# For src/c++11/shared_ptr.cc alignment.
>
>
>    ac_ext=cpp
>


Oh no!  I was worried I'd actually broken something when I saw the email
land in my inbox :-)

Looks like I added that comment to configure.ac and then forgot to re-run
autoreconf.



> Can I commit this change or do I need to attempt to adjust the script to
> ignore comments in configure or what would be the correct way to deal
> with this? (Option requiring work on my side may take some time during
> which the otherwise useful tests would not work, I am afraid).
>
>
The "correct" fix is to run autoreconf in the libstdc++-v3 dir to regen the
configure script, but for something this trivial it would also be fine to
just make the change manually.

I've pushed the regenerated file now, as r14-930-g7ddbc6171b383c

Thanks for running those checks!