This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [libstdc++/65033] Give alignment info to libatomic
- From: Hans-Peter Nilsson <hp at bitrange dot com>
- To: Richard Henderson <rth at redhat dot com>
- Cc: libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
- Date: Fri, 3 Apr 2015 05:24:58 -0400 (EDT)
- Subject: Re: [libstdc++/65033] Give alignment info to libatomic
- Authentication-results: sourceware.org; auth=none
- References: <54DD19B7 dot 6060401 at redhat dot com> <alpine dot BSF dot 2 dot 02 dot 1504022240580 dot 40679 at arjuna dot pair dot com>
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.)
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);
}
brgds, H-P