From: Jonathan Wakely <jwakely@redhat.com>
To: Richard Henderson <rth@redhat.com>
Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org,
Andrew MacLeod <amacleod@redhat.com>
Subject: Re: [libstdc++/65033] Give alignment info to libatomic
Date: Thu, 26 Mar 2015 13:21:00 -0000 [thread overview]
Message-ID: <20150326132147.GL9755@redhat.com> (raw)
In-Reply-To: <551306C1.6060702@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]
On 25/03/15 12:04 -0700, Richard Henderson wrote:
>On 03/25/2015 11:49 AM, Jonathan Wakely wrote:
>> On 25/03/15 11:36 -0700, Richard Henderson wrote:
>>> On 03/25/2015 09:22 AM, Jonathan Wakely wrote:
>> On 25/03/15 11:39 -0700, Richard Henderson wrote:
>>> On 03/25/2015 09:22 AM, Jonathan Wakely wrote:
>>>> +static_assert( alignof(std::atomic<twoints>) > alignof(int),
>>>> + "std::atomic not suitably aligned" );
>>>
>>> This is only true if int64_t has alignment larger than int32_t,
>>> which is unfortunately not always the case.
>>
>> Huh, didn't realise that. I could change the tests to check it's
>> alignof(std::int64_t) as the next assertion does, but is it safe to
>> assume that struct twoints { int a; int b; } is exactly 64 bits
>> everywhere?
>
>Certainly not. But if you're going to explicitly use int64_t elsewhere, you
>might as well explicitly use int32_t as well. Then I believe you can
>reasonably assert
>
> alignof(twoint32) == alignof(int64_t)
Yes, that makes sense, thanks. Here's what I plan to commit then.
This includes your fix to avoid decreasing alignment, but I didn't add
a test for that as I couldn't make it fail on any of the targets I
test on.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 3923 bytes --]
commit f796769ad20c0353490b9f1a7e019e2f0c1771fb
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Wed Sep 3 15:39:53 2014 +0100
PR libstdc++/62259
PR libstdc++/65147
* include/std/atomic (atomic<T>): Increase alignment for types with
the same size as one of the integral types.
* testsuite/29_atomics/atomic/60695.cc: Adjust dg-error line number.
* testsuite/29_atomics/atomic/62259.cc: New.
diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index 1a17427..42244bd 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -165,7 +165,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct atomic
{
private:
- _Tp _M_i;
+ // Align 1/2/4/8/16-byte types the same as integer types 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)
+#ifdef _GLIBCXX_USE_INT128
+ : sizeof(_Tp) == sizeof(__int128) ? alignof(__int128)
+#endif
+ : 0;
+
+ static constexpr int _S_alignment
+ = _S_min_alignment > alignof(_Tp) ? _S_min_alignment : alignof(_Tp);
+
+ alignas(_S_alignment) _Tp _M_i;
static_assert(__is_trivially_copyable(_Tp),
"std::atomic requires a trivially copyable type");
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc b/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc
index b59c6ba..6f618a0 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/60695.cc
@@ -27,4 +27,4 @@ struct X {
char stuff[0]; // GNU extension, type has zero size
};
-std::atomic<X> a; // { dg-error "not supported" "" { target *-*-* } 173 }
+std::atomic<X> a; // { dg-error "not supported" "" { target *-*-* } 189 }
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/62259.cc b/libstdc++-v3/testsuite/29_atomics/atomic/62259.cc
new file mode 100644
index 0000000..cf5423a
--- /dev/null
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/62259.cc
@@ -0,0 +1,58 @@
+// Copyright (C) 2015 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-require-atomic-builtins "" }
+// { dg-require-cstdint "" }
+// { dg-options "-std=gnu++11" }
+// { dg-do compile }
+
+#include <atomic>
+#include <cstdint>
+
+using std::int32_t;
+using std::int64_t;
+
+// libstdc++/62259
+
+struct twoints {
+ int32_t a;
+ int32_t b;
+};
+
+static_assert( alignof(std::atomic<twoints>) == alignof(int64_t),
+ "std::atomic not suitably aligned" );
+
+// libstdc++/65147
+
+struct power_of_two_obj {
+ char c [8];
+};
+
+std::atomic<power_of_two_obj> obj1;
+
+static_assert( alignof(obj1) == alignof(int64_t),
+ "std::atomic not suitably aligned" );
+
+struct container_struct {
+ char c[1];
+ std::atomic<power_of_two_obj> ao;
+};
+
+container_struct obj2;
+
+static_assert( alignof(obj2.ao) == alignof(int64_t),
+ "std::atomic not suitably aligned" );
next prev parent reply other threads:[~2015-03-26 13:21 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-12 21:23 Richard Henderson
2015-02-18 12:15 ` Jonathan Wakely
2015-03-25 16:22 ` Jonathan Wakely
2015-03-25 18:36 ` Richard Henderson
2015-03-25 18:49 ` Jonathan Wakely
2015-03-25 19:04 ` Richard Henderson
2015-03-26 13:21 ` Jonathan Wakely [this message]
2015-03-31 13:41 ` Jonathan Wakely
2015-03-31 14:54 ` Richard Henderson
2015-03-31 15:03 ` Jonathan Wakely
2015-03-31 15:13 ` Richard Henderson
2015-03-31 15:41 ` Jonathan Wakely
2015-04-06 22:59 ` Hans-Peter Nilsson
2015-04-13 4:45 ` patch fix issue 1 with "[libstdc++/65033] Give alignment info to libatomic" Hans-Peter Nilsson
2015-04-13 11:59 ` Jonathan Wakely
2015-04-13 5:59 ` Issue 2 " Hans-Peter Nilsson
2015-04-13 17:53 ` Joseph Myers
2015-03-25 18:39 ` [libstdc++/65033] Give alignment info to libatomic Richard Henderson
2015-04-03 3:04 ` Hans-Peter Nilsson
2015-03-26 11:54 ` Jonathan Wakely
2015-04-03 3:57 ` Hans-Peter Nilsson
2015-04-03 9:25 ` Hans-Peter Nilsson
2015-04-03 14:13 ` Jonathan Wakely
2015-04-03 19:13 ` Richard Henderson
2015-04-07 13:14 ` Jonathan Wakely
2015-04-09 11:17 ` Jonathan Wakely
2015-04-06 1:07 ` Hans-Peter Nilsson
2015-04-07 9:45 ` Jonathan Wakely
2015-04-07 10:52 ` Hans-Peter Nilsson
2015-04-07 13:12 ` Jonathan Wakely
2015-04-07 14:51 ` Hans-Peter Nilsson
2015-04-07 15:06 ` Jonathan Wakely
2015-04-08 3:58 ` Hans-Peter Nilsson
2015-04-08 9:35 ` Jonathan Wakely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150326132147.GL9755@redhat.com \
--to=jwakely@redhat.com \
--cc=amacleod@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
--cc=rth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).