diff mbox

[libstdc++/65033] Give alignment info to libatomic

Message ID alpine.BSF.2.02.1504030518160.69548@arjuna.pair.com
State New
Headers show

Commit Message

Hans-Peter Nilsson April 3, 2015, 9:24 a.m. UTC 
On Thu, 2 Apr 2015, Hans-Peter Nilsson wrote:
> Why then use __alignof(_M_i) (the object-alignment)
> instead of _S_alignment (the deduced alas insufficiently
> increased type-alignment)?

(The immediate reason is that _S_alignment wasn't there until a
later revision, but the gist of the question remains. :-)

> > making sure that atomic_is_lock_free returns the same
> > value for all objects of a given type,
>
> (That would work but it doesn't seem to be the case.)
>
> > we probably should have changed the
> > interface so that we would pass size and alignment rather than size and object
> > pointer.
> >
> > Instead, we decided that passing null for the object pointer would be
> > sufficient.  But as this PR shows, we really do need to take alignment into
> > account.
>
> Regarding what's actually needed, alignment of an atomic type
> should always be *forced to be at least the natural alignment of
> of the object* (with non-power-of-two sized-objects rounded up)
> and until then atomic types won't work for targets where the
> non-atomic equivalents have less alignment (as straddling a
> page-boundary won't be lock-less-atomic anywhere where
> straddling a page-boundary may cause a non-atomic-access...) So,
> not target-specific except for targets that require even
> higher-than-natural alignment.

So, can we do something like this instead for gcc5?
(Completely untested and may be syntactically, namespacingly and
cxxstandardversionly flawed.)


brgds, H-P
diff mbox

Patch

Index: include/std/atomic
===================================================================
--- include/std/atomic	(revision 221849)
+++ include/std/atomic	(working copy)
@@ -165,16 +165,16 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct atomic
     {
     private:
-      // Align 1/2/4/8/16-byte types the same as integer types of that size.
+      // Align 1/2/4/8/16-byte types to the natural alignment of that size.
       // This matches the alignment effects of the C11 _Atomic qualifier.
       static constexpr int _S_min_alignment
-	= sizeof(_Tp) == sizeof(char)	   ? alignof(char)
-	: sizeof(_Tp) == sizeof(short)	   ? alignof(short)
-	: sizeof(_Tp) == sizeof(int)	   ? alignof(int)
-	: sizeof(_Tp) == sizeof(long)	   ? alignof(long)
-	: sizeof(_Tp) == sizeof(long long) ? alignof(long long)
+      = sizeof(_Tp) == sizeof(char)	   ? max(sizeof(char), alignof(char))
+	: sizeof(_Tp) == sizeof(short)	   ? max(sizeof(short), alignof(short))
+	: sizeof(_Tp) == sizeof(int)	   ? max(sizeof(int), alignof(int))
+	: sizeof(_Tp) == sizeof(long)	   ? max(sizeof(long), alignof(long))
+	: sizeof(_Tp) == sizeof(long long) ? max(sizeof(long long), alignof(long long))
 #ifdef _GLIBCXX_USE_INT128
-	: sizeof(_Tp) == sizeof(__int128)  ? alignof(__int128)
+	: sizeof(_Tp) == sizeof(__int128)  ? max(sizeof(__int128), alignof(__int128))
 #endif
 	: 0;

@@ -216,7 +216,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       is_lock_free() const noexcept
       {
 	// Produce a fake, minimally aligned pointer.
-	void *__a = reinterpret_cast<void *>(-__alignof(_M_i));
+	void *__a = reinterpret_cast<void *>(-_S_alignment);
 	return __atomic_is_lock_free(sizeof(_M_i), __a);
       }

@@ -224,7 +224,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       is_lock_free() const volatile noexcept
       {
 	// Produce a fake, minimally aligned pointer.
-	void *__a = reinterpret_cast<void *>(-__alignof(_M_i));
+	void *__a = reinterpret_cast<void *>(-_S_alignment);
 	return __atomic_is_lock_free(sizeof(_M_i), __a);
       }

Index: include/bits/atomic_base.h
===================================================================
--- include/bits/atomic_base.h	(revision 221849)
+++ include/bits/atomic_base.h	(working copy)
@@ -240,7 +240,23 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     private:
       typedef _ITp 	__int_type;

-      __int_type 	_M_i;
+      // Align 1/2/4/8/16-byte types to the natural alignment of that size.
+      // This matches the alignment effects of the C11 _Atomic qualifier.
+      static constexpr int _S_min_alignment
+      = sizeof(_Tp) == sizeof(char)	   ? max(sizeof(char), __alignof(char))
+	: sizeof(_Tp) == sizeof(short)	   ? max(sizeof(short), __alignof(short))
+	: sizeof(_Tp) == sizeof(int)	   ? max(sizeof(int), __alignof(int))
+	: sizeof(_Tp) == sizeof(long)	   ? max(sizeof(long), __alignof(long))
+	: sizeof(_Tp) == sizeof(long long) ? max(sizeof(long long), __alignof(long long))
+#ifdef _GLIBCXX_USE_INT128
+	: sizeof(_Tp) == sizeof(__int128)  ? max(sizeof(__int128), __alignof(__int128))
+#endif
+	: 0;
+
+      static constexpr int _S_alignment
+        = _S_min_alignment > alignof(_Tp) ? _S_min_alignment : __alignof(_Tp);
+
+      __int_type 	_M_i __attribute ((__aligned(_S_alignment)));

     public:
       __atomic_base() noexcept = default;
@@ -348,7 +364,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       is_lock_free() const noexcept
       {
 	// Produce a fake, minimally aligned pointer.
-	void *__a = reinterpret_cast<void *>(-__alignof(_M_i));
+	void *__a = reinterpret_cast<void *>(-_S_alignment);
 	return __atomic_is_lock_free(sizeof(_M_i), __a);
       }

@@ -356,7 +372,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       is_lock_free() const volatile noexcept
       {
 	// Produce a fake, minimally aligned pointer.
-	void *__a = reinterpret_cast<void *>(-__alignof(_M_i));
+	void *__a = reinterpret_cast<void *>(-_S_alignment);
 	return __atomic_is_lock_free(sizeof(_M_i), __a);
       }