[committed] libstdc++: Disable cacheline alignment for DJGPP [PR109741]
Checks
Commit Message
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(-)
Comments
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
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!
@@ -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])
@@ -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
@@ -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
@@ -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
@@ -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))))