[Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Ramana Radhakrishnan]

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

Hi,

While writing atomic_word.h for the ARM backend to fix PR target/66200 I 
thought it would make more sense to write it all up with atomic 
primitives instead of providing various fragile bits of inline 
asssembler. Thus this patch came about. I intend to ask for a 
specialized version of this to be applied for the ARM backend in any 
case after testing completes.

If this goes in as a cleanup I expect all the ports to be deleting their 
atomic_word.h implementation in the various ports directories and I'll 
follow up with patches asking port maintainers to test and apply for 
each of the individual cases if this is deemed to be good enough.

Currently tested on AArch64 (without the other patch for PR 
target/66200) and tested in my tree as part of testing for the ARM 
backend on arm-linux-gnueabihf with no regressions.

Ok to apply ?


regards
Ramana



* config/cpu/generic/atomic_word.h: Rewrite using atomics.

[-- Attachment #2: rewrite-generic.txt --]
[-- Type: text/plain, Size: 2313 bytes --]

commit a360fdf84683777db764ba313354da92691aeb17
Author: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
Date:   Fri May 22 08:00:10 2015 +0000

    rewrite as atomics.

diff --git a/libstdc++-v3/config/cpu/generic/atomic_word.h b/libstdc++-v3/config/cpu/generic/atomic_word.h
index 19038bb..bedce31 100644
--- a/libstdc++-v3/config/cpu/generic/atomic_word.h
+++ b/libstdc++-v3/config/cpu/generic/atomic_word.h
@@ -29,19 +29,46 @@
 #ifndef _GLIBCXX_ATOMIC_WORD_H
 #define _GLIBCXX_ATOMIC_WORD_H	1
 
+#include <bits/cxxabi_tweaks.h>
+
 typedef int _Atomic_word;
 
-// Define these two macros using the appropriate memory barrier for the target.
-// The commented out versions below are the defaults.
-// See ia64/atomic_word.h for an alternative approach.
+
+namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
+{
+  // Test the first byte of __g and ensure that no loads are hoisted across
+  // the test.
+  inline bool
+  __test_and_acquire (__cxxabiv1::__guard *__g)
+  {
+    unsigned char __c;
+    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
+    __atomic_load (__p, &__c,  __ATOMIC_ACQUIRE);
+    return __c != 0;
+  }
+
+  // Set the first byte of __g to 1 and ensure that no stores are sunk
+  // across the store.
+  inline void
+  __set_and_release (__cxxabiv1::__guard *__g)
+  {
+    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
+    unsigned char val = 1;
+    __atomic_store (__p, &val, __ATOMIC_RELEASE);
+  }
+
+#define _GLIBCXX_GUARD_TEST_AND_ACQUIRE(G) __gnu_cxx::__test_and_acquire (G)
+#define _GLIBCXX_GUARD_SET_AND_RELEASE(G) __gnu_cxx::__set_and_release (G)
 
 // This one prevents loads from being hoisted across the barrier;
 // in other words, this is a Load-Load acquire barrier.
-// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.  
-// #define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("":::"memory")
+// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.
+#define _GLIBCXX_READ_MEM_BARRIER __atomic_thread_fence (__ATOMIC_ACQUIRE)
 
 // This one prevents stores from being sunk across the barrier; in other
 // words, a Store-Store release barrier.
-// #define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("":::"memory")
+#define _GLIBCXX_WRITE_MEM_BARRIER __atomic_thread_fence (__ATOMIC_RELEASE)
+
+}
 
 #endif 

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Torvald Riegel]

On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
> Hi,
> 
> While writing atomic_word.h for the ARM backend to fix PR target/66200
> I 
> thought it would make more sense to write it all up with atomic 
> primitives instead of providing various fragile bits of inline 
> asssembler. Thus this patch came about. I intend to ask for a 
> specialized version of this to be applied for the ARM backend in any 
> case after testing completes.
> 
> If this goes in as a cleanup I expect all the ports to be deleting
> their 
> atomic_word.h implementation in the various ports directories and
> I'll 
> follow up with patches asking port maintainers to test and apply for 
> each of the individual cases if this is deemed to be good enough.

Could you use C++11 atomics right away instead of adapting the wrappers?

I think the more non-C++11 atomic operations/wrappers we can remove the
better.

> diff --git a/libstdc++-v3/config/cpu/generic/atomic_word.h b/libstdc
> ++-v3/config/cpu/generic/atomic_word.h
> index 19038bb..bedce31 100644
> --- a/libstdc++-v3/config/cpu/generic/atomic_word.h
> +++ b/libstdc++-v3/config/cpu/generic/atomic_word.h
> @@ -29,19 +29,46 @@
>  #ifndef _GLIBCXX_ATOMIC_WORD_H
>  #define _GLIBCXX_ATOMIC_WORD_H 1
>  
> +#include <bits/cxxabi_tweaks.h>
> +
>  typedef int _Atomic_word;
>  
> -// Define these two macros using the appropriate memory barrier for
> the target.
> -// The commented out versions below are the defaults.
> -// See ia64/atomic_word.h for an alternative approach.
> +
> +namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
> +{
> +  // Test the first byte of __g and ensure that no loads are hoisted
> across
> +  // the test.

That comment is not quite correct.  I'd just say that this is a
memory_order_acquire load and a comparison.

> +  inline bool
> +  __test_and_acquire (__cxxabiv1::__guard *__g)
> +  {
> +    unsigned char __c;
> +    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
> +    __atomic_load (__p, &__c,  __ATOMIC_ACQUIRE);
> +    return __c != 0;
> +  }
> +
> +  // Set the first byte of __g to 1 and ensure that no stores are
> sunk
> +  // across the store.

Likewise; I'd just say this is a memory_order_release store with 1 as
value.

> +  inline void
> +  __set_and_release (__cxxabiv1::__guard *__g)
> +  {
> +    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
> +    unsigned char val = 1;
> +    __atomic_store (__p, &val, __ATOMIC_RELEASE);
> +  }
> +
> +#define _GLIBCXX_GUARD_TEST_AND_ACQUIRE(G)
> __gnu_cxx::__test_and_acquire (G)
> +#define _GLIBCXX_GUARD_SET_AND_RELEASE(G)
> __gnu_cxx::__set_and_release (G)
>  
>  // This one prevents loads from being hoisted across the barrier;
>  // in other words, this is a Load-Load acquire barrier.

Likewise; I'd just say that this is an mo_acquire fence.

> -// This is necessary iff TARGET_RELAXED_ORDERING is defined in
> tm.h.  
> -// #define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("":::"memory")
> +// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.
> +#define _GLIBCXX_READ_MEM_BARRIER __atomic_thread_fence
> (__ATOMIC_ACQUIRE)
>  
>  // This one prevents stores from being sunk across the barrier; in
> other
>  // words, a Store-Store release barrier.

Likewise; mo_release fence.

> -// #define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile
> ("":::"memory")
> +#define _GLIBCXX_WRITE_MEM_BARRIER __atomic_thread_fence
> (__ATOMIC_RELEASE)
> +
> +}
>  
>  #endif 

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Ramana Radhakrishnan]

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



On 22/05/15 17:56, Torvald Riegel wrote:
> On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
>> Hi,
>>
>> While writing atomic_word.h for the ARM backend to fix PR target/66200
>> I
>> thought it would make more sense to write it all up with atomic
>> primitives instead of providing various fragile bits of inline
>> asssembler. Thus this patch came about. I intend to ask for a
>> specialized version of this to be applied for the ARM backend in any
>> case after testing completes.
>>
>> If this goes in as a cleanup I expect all the ports to be deleting
>> their
>> atomic_word.h implementation in the various ports directories and
>> I'll
>> follow up with patches asking port maintainers to test and apply for
>> each of the individual cases if this is deemed to be good enough.
>
> Could you use C++11 atomics right away instead of adapting the wrappers?


I spent some time trying to massage guard.cc into using C++11 atomics 
but it was just easier to write it with the builtins - I consider this 
to be a step improvement compared to where we are currently.

Rewritten to use the builtins in guard.cc instead of std::atomic as that 
appears to be a bigger project than moving forward compared to where we 
are. I prefer small steps rather than big steps in these areas. Further 
I do not believe you can use the C++11 language features in the headers 
as they were not necessarily part of the standard for tr1 etc. and thus 
it's better to remain with the builtins, after all I am also continuing 
with existing practice in the headers.


>
> I think the more non-C++11 atomic operations/wrappers we can remove the
> better.
>
>> diff --git a/libstdc++-v3/config/cpu/generic/atomic_word.h b/libstdc
>> ++-v3/config/cpu/generic/atomic_word.h
>> index 19038bb..bedce31 100644
>> --- a/libstdc++-v3/config/cpu/generic/atomic_word.h
>> +++ b/libstdc++-v3/config/cpu/generic/atomic_word.h
>> @@ -29,19 +29,46 @@
>>   #ifndef _GLIBCXX_ATOMIC_WORD_H
>>   #define _GLIBCXX_ATOMIC_WORD_H 1
>>
>> +#include <bits/cxxabi_tweaks.h>
>> +
>>   typedef int _Atomic_word;
>>
>> -// Define these two macros using the appropriate memory barrier for
>> the target.
>> -// The commented out versions below are the defaults.
>> -// See ia64/atomic_word.h for an alternative approach.
>> +
>> +namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
>> +{
>> +  // Test the first byte of __g and ensure that no loads are hoisted
>> across
>> +  // the test.

>
> That comment is not quite correct.  I'd just say that this is a
> memory_order_acquire load and a comparison.
>

Done.


>> +  inline bool
>> +  __test_and_acquire (__cxxabiv1::__guard *__g)
>> +  {
>> +    unsigned char __c;
>> +    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
>> +    __atomic_load (__p, &__c,  __ATOMIC_ACQUIRE);
>> +    return __c != 0;
>> +  }
>> +
>> +  // Set the first byte of __g to 1 and ensure that no stores are
>> sunk
>> +  // across the store.
>
> Likewise; I'd just say this is a memory_order_release store with 1 as
> value.

Ok. I've now realized that this is superfluous and better to fix the one 
definition in guard.cc to do the right thing instead of something like this.


>
>> +  inline void
>> +  __set_and_release (__cxxabiv1::__guard *__g)
>> +  {
>> +    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
>> +    unsigned char val = 1;
>> +    __atomic_store (__p, &val, __ATOMIC_RELEASE);
>> +  }
>> +
>> +#define _GLIBCXX_GUARD_TEST_AND_ACQUIRE(G)
>> __gnu_cxx::__test_and_acquire (G)
>> +#define _GLIBCXX_GUARD_SET_AND_RELEASE(G)
>> __gnu_cxx::__set_and_release (G)
>>
>>   // This one prevents loads from being hoisted across the barrier;
>>   // in other words, this is a Load-Load acquire barrier.
>
> Likewise; I'd just say that this is an mo_acquire fence.
>
>> -// This is necessary iff TARGET_RELAXED_ORDERING is defined in
>> tm.h.
>> -// #define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("":::"memory")
>> +// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.
>> +#define _GLIBCXX_READ_MEM_BARRIER __atomic_thread_fence
>> (__ATOMIC_ACQUIRE)
>>
>>   // This one prevents stores from being sunk across the barrier; in
>> other
>>   // words, a Store-Store release barrier.
>
> Likewise; mo_release fence.
>
>> -// #define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile
>> ("":::"memory")
>> +#define _GLIBCXX_WRITE_MEM_BARRIER __atomic_thread_fence
>> (__ATOMIC_RELEASE)
>> +
>> +}



I don't think we can remove _GLIBCXX_READ_MEM_BARRIER and 
_GLIBCXX_WRITE_MEM_BARRIER from atomic_word.h even though they are 
superseded by the atomics as it is published in the documentation as 
available macros.

It was obvious that alpha, powerpc, aix, ia64 could just fall back to 
the standard implementations. The cris and sparc implementations have 
different types for _Atomic_word from generic and the rest - my guess is 
sparc can remove the _GLIBCXX_READ_MEM_BARRIER and 
_GLIBCXX_WRITE_MEM_BARRIER but I have no way of testing this is sane.

I'll leave that to the respective target maintainers for sparc and cris 
to deal as appropriate.


2015-06-09  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

         PR c++/66192
         PR target/66200
         * config/cpu/generic/atomic_word.h (_GLIBCXX_READ_MEM_BARRIER): 
Define
         (_GLIBCXX_WRITE_MEM_BARRIER): Likewise
         * include/bits/shared_ptr_base.h: Use ACQ_REL barrier.
         * include/ext/atomicity.h: Likewise.
         * include/tr1/shared_ptr.h: Likewise.
         * libsupc++/guard.cc (__test_and_acquire): Rewrite with atomics.
         Update comment.
         (__set_and_release): Likewise.
	* testsuite/20_util/shared_ptr/cons/43820_neg.cc (void test01): Adjust 
for line numbers.
	* testsuite/20_util/shared_ptr/cons/void_neg.cc: Likewise.
	* testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc: Likewise.


2015-06-09  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

         * config/cpu/alpha/atomic_word.h: Remove.
         * config/cpu/ia64/atomic_word.h: Remove.
         * config/cpu/powerpc/atomic_word.h: Remove.
         * config/os/aix/atomic_word.h: Remove.
         * configure.host (atomic_word_dir) [ia64, aix*, powerpc, alpha]:
         Use generic definition.


Bootstrap and regression tested on aarch64-none-linux-gnu, 
arm-none-linux-gnueabihf and arm-none-eabi.


regards
Ramana

>>
>>   #endif
>

[-- Attachment #2: 0001-libstdc-TLC-for-guards-and-atomic-operations.patch --]
[-- Type: text/x-diff, Size: 8671 bytes --]

From 78f3aac42047b8e1f44d589d33600b27110b164f Mon Sep 17 00:00:00 2001
From: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
Date: Thu, 4 Jun 2015 12:35:07 +0100
Subject: [PATCH 1/2] libstdc++ TLC for guards and atomic operations.

This provides proper definitions for _GLIBCXX_READ_MEM_BARRIER and _GLIBCXX_WRITE_MEM_BARRIER,
rewrites the guards in terms of proper atomic extensions and removes internal uses of
_GLIBCXX_READ_MEM_BARRIER and _GLIBCXX_WRITE_MEM_BARRIER and replaces them with equivalent
atomics.

2015-06-04  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	PR c++/66192
	PR target/66200
	* config/cpu/generic/atomic_word.h (_GLIBCXX_READ_MEM_BARRIER): Define
	(_GLIBCXX_WRITE_MEM_BARRIER): Likewise
	* include/bits/shared_ptr_base.h: Use ACQ_REL barrier.
	* include/ext/atomicity.h: Likewise.
	* include/tr1/shared_ptr.h: Likewise.
	* libsupc++/guard.cc (__test_and_acquire): Rewrite with atomics.
	Update comment.
	(__set_and_release): Likewise.

libstdc++-v3.

	* testsuite/20_util/shared_ptr/cons/43820_neg.cc (void test01): Adjust for line numbers.
	* testsuite/20_util/shared_ptr/cons/void_neg.cc: Likewise.
	* testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc: Likewise.
---
 libstdc++-v3/config/cpu/generic/atomic_word.h         | 17 +++++------------
 libstdc++-v3/include/bits/shared_ptr_base.h           |  6 ++----
 libstdc++-v3/include/ext/atomicity.h                  |  4 ++--
 libstdc++-v3/include/tr1/shared_ptr.h                 |  6 ++----
 libstdc++-v3/libsupc++/guard.cc                       | 19 ++++++++++++++-----
 .../testsuite/20_util/shared_ptr/cons/43820_neg.cc    |  2 +-
 .../testsuite/20_util/shared_ptr/cons/void_neg.cc     |  2 +-
 .../2_general_utilities/shared_ptr/cons/43820_neg.cc  |  4 ++--
 8 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/libstdc++-v3/config/cpu/generic/atomic_word.h b/libstdc++-v3/config/cpu/generic/atomic_word.h
index 19038bb..ccf1e5a 100644
--- a/libstdc++-v3/config/cpu/generic/atomic_word.h
+++ b/libstdc++-v3/config/cpu/generic/atomic_word.h
@@ -31,17 +31,10 @@
 
 typedef int _Atomic_word;
 
-// Define these two macros using the appropriate memory barrier for the target.
-// The commented out versions below are the defaults.
-// See ia64/atomic_word.h for an alternative approach.
-
-// This one prevents loads from being hoisted across the barrier;
-// in other words, this is a Load-Load acquire barrier.
-// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.  
-// #define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("":::"memory")
-
-// This one prevents stores from being sunk across the barrier; in other
-// words, a Store-Store release barrier.
-// #define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("":::"memory")
+
+// This is a memory order acquire fence.
+#define _GLIBCXX_READ_MEM_BARRIER __atomic_thread_fence (__ATOMIC_ACQUIRE)
+// This is a memory order release fence.
+#define _GLIBCXX_WRITE_MEM_BARRIER __atomic_thread_fence (__ATOMIC_RELEASE)
 
 #endif 
diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h
index 081df87..aec10fe 100644
--- a/libstdc++-v3/include/bits/shared_ptr_base.h
+++ b/libstdc++-v3/include/bits/shared_ptr_base.h
@@ -154,8 +154,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    // See http://gcc.gnu.org/ml/libstdc++/2005-11/msg00136.html
 	    if (_Mutex_base<_Lp>::_S_need_barriers)
 	      {
-	        _GLIBCXX_READ_MEM_BARRIER;
-	        _GLIBCXX_WRITE_MEM_BARRIER;
+		__atomic_thread_fence (__ATOMIC_ACQ_REL);
 	      }
 
             // Be race-detector-friendly.  For more info see bits/c++config.
@@ -185,8 +184,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      {
 	        // See _M_release(),
 	        // destroy() must observe results of dispose()
-	        _GLIBCXX_READ_MEM_BARRIER;
-	        _GLIBCXX_WRITE_MEM_BARRIER;
+		__atomic_thread_fence (__ATOMIC_ACQ_REL);
 	      }
 	    _M_destroy();
 	  }
diff --git a/libstdc++-v3/include/ext/atomicity.h b/libstdc++-v3/include/ext/atomicity.h
index aff33f8..6d32cc4 100644
--- a/libstdc++-v3/include/ext/atomicity.h
+++ b/libstdc++-v3/include/ext/atomicity.h
@@ -108,10 +108,10 @@ _GLIBCXX_END_NAMESPACE_VERSION
 // that the compiler doesn't reorder memory accesses across the
 // barriers.
 #ifndef _GLIBCXX_READ_MEM_BARRIER
-#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("":::"memory")
+#define _GLIBCXX_READ_MEM_BARRIER __atomic_thread_fence (__ATOMIC_ACQUIRE)
 #endif
 #ifndef _GLIBCXX_WRITE_MEM_BARRIER
-#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("":::"memory")
+#define _GLIBCXX_WRITE_MEM_BARRIER __atomic_thread_fence (__ATOMIC_RELEASE)
 #endif
 
 #endif 
diff --git a/libstdc++-v3/include/tr1/shared_ptr.h b/libstdc++-v3/include/tr1/shared_ptr.h
index ebb9d36..a1c7d72 100644
--- a/libstdc++-v3/include/tr1/shared_ptr.h
+++ b/libstdc++-v3/include/tr1/shared_ptr.h
@@ -145,8 +145,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    // See http://gcc.gnu.org/ml/libstdc++/2005-11/msg00136.html
 	    if (_Mutex_base<_Lp>::_S_need_barriers)
 	      {
-	        _GLIBCXX_READ_MEM_BARRIER;
-	        _GLIBCXX_WRITE_MEM_BARRIER;
+		__atomic_thread_fence (__ATOMIC_ACQ_REL);
 	      }
 
             // Be race-detector-friendly.  For more info see bits/c++config.
@@ -176,8 +175,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      {
 	        // See _M_release(),
 	        // destroy() must observe results of dispose()
-	        _GLIBCXX_READ_MEM_BARRIER;
-	        _GLIBCXX_WRITE_MEM_BARRIER;
+		__atomic_thread_fence (__ATOMIC_ACQ_REL);
 	      }
 	    _M_destroy();
 	  }
diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc
index 9f19fd4..4a2cfe9 100644
--- a/libstdc++-v3/libsupc++/guard.cc
+++ b/libstdc++-v3/libsupc++/guard.cc
@@ -107,22 +107,31 @@ namespace
 # endif
 
 # ifndef _GLIBCXX_GUARD_TEST_AND_ACQUIRE
+
+// Test the guard variable with a memory load with
+// acquire semantics.
+
 inline bool
 __test_and_acquire (__cxxabiv1::__guard *g)
 {
-  bool b = _GLIBCXX_GUARD_TEST (g);
-  _GLIBCXX_READ_MEM_BARRIER;
-  return b;
+  unsigned char __c;
+  unsigned char *__p = reinterpret_cast<unsigned char *>(g);
+  __atomic_load (__p, &__c,  __ATOMIC_ACQUIRE);
+  return _GLIBCXX_GUARD_TEST(&__c);
 }
 #  define _GLIBCXX_GUARD_TEST_AND_ACQUIRE(G) __test_and_acquire (G)
 # endif
 
 # ifndef _GLIBCXX_GUARD_SET_AND_RELEASE
+
+// Set the guard variable to 1 with memory order release semantics.
+
 inline void
 __set_and_release (__cxxabiv1::__guard *g)
 {
-  _GLIBCXX_WRITE_MEM_BARRIER;
-  _GLIBCXX_GUARD_SET (g);
+  unsigned char *__p = reinterpret_cast<unsigned char *>(g);
+  unsigned char val = 1;
+  __atomic_store (__p, &val, __ATOMIC_RELEASE);
 }
 #  define _GLIBCXX_GUARD_SET_AND_RELEASE(G) __set_and_release (G)
 # endif
diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc
index 6255522..fdfb7d9 100644
--- a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc
@@ -32,7 +32,7 @@ void test01()
 {
   X* px = 0;
   std::shared_ptr<X> p1(px);   // { dg-error "here" }
-  // { dg-error "incomplete" "" { target *-*-* } 891 }
+  // { dg-error "incomplete" "" { target *-*-* } 889 }
 
   std::shared_ptr<X> p9(ap());  // { dg-error "here" }
   // { dg-error "incomplete" "" { target *-*-* } 307 }
diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/void_neg.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/void_neg.cc
index 3f6c4ae..10074c1 100644
--- a/libstdc++-v3/testsuite/20_util/shared_ptr/cons/void_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/shared_ptr/cons/void_neg.cc
@@ -25,5 +25,5 @@
 void test01()
 {
   std::shared_ptr<void> p((void*)nullptr);   // { dg-error "here" }
-  // { dg-error "incomplete" "" { target *-*-* } 890 }
+  // { dg-error "incomplete" "" { target *-*-* } 888 }
 }
diff --git a/libstdc++-v3/testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc b/libstdc++-v3/testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc
index 276c6a5..3f44eb7 100644
--- a/libstdc++-v3/testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc
+++ b/libstdc++-v3/testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc
@@ -32,8 +32,8 @@ void test01()
 {
   X* px = 0;
   std::tr1::shared_ptr<X> p1(px);   // { dg-error "here" }
-  // { dg-error "incomplete" "" { target *-*-* } 556 }
+  // { dg-error "incomplete" "" { target *-*-* } 554 }
 
   std::tr1::shared_ptr<X> p9(ap());  // { dg-error "here" }
-  // { dg-error "incomplete" "" { target *-*-* } 595 }
+  // { dg-error "incomplete" "" { target *-*-* } 593 }
 }
-- 
1.9.1


[-- Attachment #3: 0002-libstdc-TLC-Ports-PowerPC-Alpha-AIX-IA64-Use-generic.patch --]
[-- Type: text/x-diff, Size: 10494 bytes --]

From 207fd0ab76ecc0b9b45da62a6fed90569f7b2362 Mon Sep 17 00:00:00 2001
From: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
Date: Thu, 4 Jun 2015 13:16:30 +0100
Subject: [PATCH 2/2] libstdc++ TLC [Ports, PowerPC, Alpha, AIX, IA64] - Use
 generic implementation for ports for atomic_word.h

The PowerPC, AIX, Alpha, IA64  implementations of atomic_word.h are in no
way different from what can be achieved with the generic rewrite in
Patch 1 of this series - delete these.

2015-06-04  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	* config/cpu/alpha/atomic_word.h: Remove.
	* config/cpu/ia64/atomic_word.h: Remove.
	* config/cpu/powerpc/atomic_word.h: Remove.
	* config/os/aix/atomic_word.h: Remove.
	* configure.host (atomic_word_dir) [ia64, aix*, powerpc, alpha]:
	Use generic definition.
---
 libstdc++-v3/config/cpu/alpha/atomic_word.h   | 33 --------------
 libstdc++-v3/config/cpu/ia64/atomic_word.h    | 64 ---------------------------
 libstdc++-v3/config/cpu/powerpc/atomic_word.h | 38 ----------------
 libstdc++-v3/config/os/aix/atomic_word.h      | 43 ------------------
 libstdc++-v3/configure.host                   | 11 -----
 5 files changed, 189 deletions(-)
 delete mode 100644 libstdc++-v3/config/cpu/alpha/atomic_word.h
 delete mode 100644 libstdc++-v3/config/cpu/ia64/atomic_word.h
 delete mode 100644 libstdc++-v3/config/cpu/powerpc/atomic_word.h
 delete mode 100644 libstdc++-v3/config/os/aix/atomic_word.h

diff --git a/libstdc++-v3/config/cpu/alpha/atomic_word.h b/libstdc++-v3/config/cpu/alpha/atomic_word.h
deleted file mode 100644
index 9d3d708..0000000
--- a/libstdc++-v3/config/cpu/alpha/atomic_word.h
+++ /dev/null
@@ -1,33 +0,0 @@
-// Low-level type for atomic operations -*- C++ -*-
-
-// Copyright (C) 2004-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.
-
-// Under Section 7 of GPL version 3, you are granted additional
-// permissions described in the GCC Runtime Library Exception, version
-// 3.1, as published by the Free Software Foundation.
-
-// You should have received a copy of the GNU General Public License and
-// a copy of the GCC Runtime Library Exception along with this program;
-// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
-// <http://www.gnu.org/licenses/>.
-
-#ifndef _GLIBCXX_ATOMIC_WORD_H
-#define _GLIBCXX_ATOMIC_WORD_H	1
-
-typedef int _Atomic_word;
-
-#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("mb":::"memory")
-#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("wmb":::"memory")
-
-#endif 
diff --git a/libstdc++-v3/config/cpu/ia64/atomic_word.h b/libstdc++-v3/config/cpu/ia64/atomic_word.h
deleted file mode 100644
index 6aeeb64..0000000
--- a/libstdc++-v3/config/cpu/ia64/atomic_word.h
+++ /dev/null
@@ -1,64 +0,0 @@
-// Low-level type for atomic operations -*- C++ -*-
-
-// Copyright (C) 2004-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.
-
-// Under Section 7 of GPL version 3, you are granted additional
-// permissions described in the GCC Runtime Library Exception, version
-// 3.1, as published by the Free Software Foundation.
-
-// You should have received a copy of the GNU General Public License and
-// a copy of the GCC Runtime Library Exception along with this program;
-// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
-// <http://www.gnu.org/licenses/>.
-
-#ifndef _GLIBCXX_ATOMIC_WORD_H
-#define _GLIBCXX_ATOMIC_WORD_H	1
-
-#include <bits/cxxabi_tweaks.h>
-
-typedef int _Atomic_word;
-
-namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
-{
-  // Test the first byte of __g and ensure that no loads are hoisted across
-  // the test.
-  inline bool
-  __test_and_acquire (__cxxabiv1::__guard *__g)
-  {
-    unsigned char __c;
-    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
-    // ldN.acq is a load with an implied hoist barrier.
-    // would ld8+mask be faster than just doing an ld1?
-    __asm __volatile ("ld1.acq %0 = %1" : "=r"(__c) : "m"(*__p) : "memory");
-    return __c != 0;
-  }
-
-  // Set the first byte of __g to 1 and ensure that no stores are sunk
-  // across the store.
-  inline void
-  __set_and_release (__cxxabiv1::__guard *__g)
-  {
-    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
-    // stN.rel is a store with an implied sink barrier.
-    // could load word, set flag, and CAS it back
-    __asm __volatile ("st1.rel %0 = %1" : "=m"(*__p) : "r"(1) : "memory");
-  }
-
-  // We don't define the _BARRIER macros on ia64 because the barriers are
-  // included in the test and set, above.
-#define _GLIBCXX_GUARD_TEST_AND_ACQUIRE(G) __gnu_cxx::__test_and_acquire (G)
-#define _GLIBCXX_GUARD_SET_AND_RELEASE(G) __gnu_cxx::__set_and_release (G)
-}
-
-#endif 
diff --git a/libstdc++-v3/config/cpu/powerpc/atomic_word.h b/libstdc++-v3/config/cpu/powerpc/atomic_word.h
deleted file mode 100644
index 1ceb02c..0000000
--- a/libstdc++-v3/config/cpu/powerpc/atomic_word.h
+++ /dev/null
@@ -1,38 +0,0 @@
-// Low-level type for atomic operations -*- C++ -*-
-
-// Copyright (C) 2004-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.
-
-// Under Section 7 of GPL version 3, you are granted additional
-// permissions described in the GCC Runtime Library Exception, version
-// 3.1, as published by the Free Software Foundation.
-
-// You should have received a copy of the GNU General Public License and
-// a copy of the GCC Runtime Library Exception along with this program;
-// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
-// <http://www.gnu.org/licenses/>.
-
-#ifndef _GLIBCXX_ATOMIC_WORD_H
-#define _GLIBCXX_ATOMIC_WORD_H	1
-
-typedef int _Atomic_word;
-
-#ifdef __NO_LWSYNC__
-#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("sync":::"memory")
-#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("sync":::"memory")
-#else
-#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("lwsync":::"memory")
-#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("lwsync":::"memory")
-#endif
-
-#endif 
diff --git a/libstdc++-v3/config/os/aix/atomic_word.h b/libstdc++-v3/config/os/aix/atomic_word.h
deleted file mode 100644
index 2b2647a..0000000
--- a/libstdc++-v3/config/os/aix/atomic_word.h
+++ /dev/null
@@ -1,43 +0,0 @@
-// Low-level type for atomic operations -*- C++ -*-
-
-// Copyright (C) 2004-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.
-
-// Under Section 7 of GPL version 3, you are granted additional
-// permissions described in the GCC Runtime Library Exception, version
-// 3.1, as published by the Free Software Foundation.
-
-// You should have received a copy of the GNU General Public License and
-// a copy of the GCC Runtime Library Exception along with this program;
-// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
-// <http://www.gnu.org/licenses/>.
-
-/** @file bits/atomic_word.h
- *  This is an internal header file, included by other library headers.
- *  Do not attempt to use it directly. @headername{ext/atomicity.h}
- */
-
-#ifndef _GLIBCXX_ATOMIC_WORD_H
-#define _GLIBCXX_ATOMIC_WORD_H	1
-
-typedef int _Atomic_word;
-
-#ifdef _ARCH_PPC
-#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("isync":::"memory")
-#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("sync":::"memory")
-#else
-#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("":::"memory")
-#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile ("":::"memory")
-#endif
-
-#endif
diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host
index 8892f31..c93db1e 100644
--- a/libstdc++-v3/configure.host
+++ b/libstdc++-v3/configure.host
@@ -156,21 +156,12 @@ esac
 # Most can just use generic.
 # THIS TABLE IS SORTED.  KEEP IT THAT WAY.
 case "${host_cpu}" in
-  alpha*)
-    atomic_word_dir=cpu/alpha
-    ;;
   cris*)
     atomic_word_dir=cpu/cris
     ;;
-  ia64)
-    atomic_word_dir=cpu/ia64
-    ;;
   i[4567]86 | x86_64)
     atomic_flags="-march=native"
     ;;
-  powerpc* | rs6000)
-    atomic_word_dir=cpu/powerpc
-    ;;
   sparc* | ultrasparc)
     atomic_word_dir=cpu/sparc
     atomic_flags="-mcpu=v9"
@@ -226,12 +217,10 @@ case "${host_os}" in
     # explicitly duplicate the directory for 4.[<3].
     os_include_dir="os/aix"
     atomicity_dir="os/aix"
-    atomic_word_dir="os/aix"
     ;;
   aix4.*)
     os_include_dir="os/generic"
     atomicity_dir="os/aix"
-    atomic_word_dir="os/aix"
     ;;
   aix*)
     os_include_dir="os/generic"
-- 
1.9.1

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[David Edelsohn]

On Tue, Jun 9, 2015 at 5:50 AM, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:
>
>
> On 22/05/15 17:56, Torvald Riegel wrote:
>>
>> On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
>>>
>>> Hi,
>>>
>>> While writing atomic_word.h for the ARM backend to fix PR target/66200
>>> I
>>> thought it would make more sense to write it all up with atomic
>>> primitives instead of providing various fragile bits of inline
>>> asssembler. Thus this patch came about. I intend to ask for a
>>> specialized version of this to be applied for the ARM backend in any
>>> case after testing completes.
>>>
>>> If this goes in as a cleanup I expect all the ports to be deleting
>>> their
>>> atomic_word.h implementation in the various ports directories and
>>> I'll
>>> follow up with patches asking port maintainers to test and apply for
>>> each of the individual cases if this is deemed to be good enough.
>>
>>
>> Could you use C++11 atomics right away instead of adapting the wrappers?
>
>
>
> I spent some time trying to massage guard.cc into using C++11 atomics but it
> was just easier to write it with the builtins - I consider this to be a step
> improvement compared to where we are currently.
>
> Rewritten to use the builtins in guard.cc instead of std::atomic as that
> appears to be a bigger project than moving forward compared to where we are.
> I prefer small steps rather than big steps in these areas. Further I do not
> believe you can use the C++11 language features in the headers as they were
> not necessarily part of the standard for tr1 etc. and thus it's better to
> remain with the builtins, after all I am also continuing with existing
> practice in the headers.
>
>
>>
>> I think the more non-C++11 atomic operations/wrappers we can remove the
>> better.
>>
>>> diff --git a/libstdc++-v3/config/cpu/generic/atomic_word.h b/libstdc
>>> ++-v3/config/cpu/generic/atomic_word.h
>>> index 19038bb..bedce31 100644
>>> --- a/libstdc++-v3/config/cpu/generic/atomic_word.h
>>> +++ b/libstdc++-v3/config/cpu/generic/atomic_word.h
>>> @@ -29,19 +29,46 @@
>>>   #ifndef _GLIBCXX_ATOMIC_WORD_H
>>>   #define _GLIBCXX_ATOMIC_WORD_H 1
>>>
>>> +#include <bits/cxxabi_tweaks.h>
>>> +
>>>   typedef int _Atomic_word;
>>>
>>> -// Define these two macros using the appropriate memory barrier for
>>> the target.
>>> -// The commented out versions below are the defaults.
>>> -// See ia64/atomic_word.h for an alternative approach.
>>> +
>>> +namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
>>> +{
>>> +  // Test the first byte of __g and ensure that no loads are hoisted
>>> across
>>> +  // the test.
>
>
>>
>> That comment is not quite correct.  I'd just say that this is a
>> memory_order_acquire load and a comparison.
>>
>
> Done.
>
>
>>> +  inline bool
>>> +  __test_and_acquire (__cxxabiv1::__guard *__g)
>>> +  {
>>> +    unsigned char __c;
>>> +    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
>>> +    __atomic_load (__p, &__c,  __ATOMIC_ACQUIRE);
>>> +    return __c != 0;
>>> +  }
>>> +
>>> +  // Set the first byte of __g to 1 and ensure that no stores are
>>> sunk
>>> +  // across the store.
>>
>>
>> Likewise; I'd just say this is a memory_order_release store with 1 as
>> value.
>
>
> Ok. I've now realized that this is superfluous and better to fix the one
> definition in guard.cc to do the right thing instead of something like this.
>
>
>>
>>> +  inline void
>>> +  __set_and_release (__cxxabiv1::__guard *__g)
>>> +  {
>>> +    unsigned char *__p = reinterpret_cast<unsigned char *>(__g);
>>> +    unsigned char val = 1;
>>> +    __atomic_store (__p, &val, __ATOMIC_RELEASE);
>>> +  }
>>> +
>>> +#define _GLIBCXX_GUARD_TEST_AND_ACQUIRE(G)
>>> __gnu_cxx::__test_and_acquire (G)
>>> +#define _GLIBCXX_GUARD_SET_AND_RELEASE(G)
>>> __gnu_cxx::__set_and_release (G)
>>>
>>>   // This one prevents loads from being hoisted across the barrier;
>>>   // in other words, this is a Load-Load acquire barrier.
>>
>>
>> Likewise; I'd just say that this is an mo_acquire fence.
>>
>>> -// This is necessary iff TARGET_RELAXED_ORDERING is defined in
>>> tm.h.
>>> -// #define _GLIBCXX_READ_MEM_BARRIER __asm __volatile ("":::"memory")
>>> +// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h.
>>> +#define _GLIBCXX_READ_MEM_BARRIER __atomic_thread_fence
>>> (__ATOMIC_ACQUIRE)
>>>
>>>   // This one prevents stores from being sunk across the barrier; in
>>> other
>>>   // words, a Store-Store release barrier.
>>
>>
>> Likewise; mo_release fence.
>>
>>> -// #define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile
>>> ("":::"memory")
>>> +#define _GLIBCXX_WRITE_MEM_BARRIER __atomic_thread_fence
>>> (__ATOMIC_RELEASE)
>>> +
>>> +}
>
>
>
>
> I don't think we can remove _GLIBCXX_READ_MEM_BARRIER and
> _GLIBCXX_WRITE_MEM_BARRIER from atomic_word.h even though they are
> superseded by the atomics as it is published in the documentation as
> available macros.
>
> It was obvious that alpha, powerpc, aix, ia64 could just fall back to the
> standard implementations. The cris and sparc implementations have different
> types for _Atomic_word from generic and the rest - my guess is sparc can
> remove the _GLIBCXX_READ_MEM_BARRIER and _GLIBCXX_WRITE_MEM_BARRIER but I
> have no way of testing this is sane.
>
> I'll leave that to the respective target maintainers for sparc and cris to
> deal as appropriate.
>
>
> 2015-06-09  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>
>
>         PR c++/66192
>         PR target/66200
>         * config/cpu/generic/atomic_word.h (_GLIBCXX_READ_MEM_BARRIER):
> Define
>         (_GLIBCXX_WRITE_MEM_BARRIER): Likewise
>         * include/bits/shared_ptr_base.h: Use ACQ_REL barrier.
>         * include/ext/atomicity.h: Likewise.
>         * include/tr1/shared_ptr.h: Likewise.
>         * libsupc++/guard.cc (__test_and_acquire): Rewrite with atomics.
>         Update comment.
>         (__set_and_release): Likewise.
>         * testsuite/20_util/shared_ptr/cons/43820_neg.cc (void test01):
> Adjust for line numbers.
>         * testsuite/20_util/shared_ptr/cons/void_neg.cc: Likewise.
>         * testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc:
> Likewise.
>
>
> 2015-06-09  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>
>
>         * config/cpu/alpha/atomic_word.h: Remove.
>         * config/cpu/ia64/atomic_word.h: Remove.
>         * config/cpu/powerpc/atomic_word.h: Remove.
>         * config/os/aix/atomic_word.h: Remove.
>         * configure.host (atomic_word_dir) [ia64, aix*, powerpc, alpha]:
>         Use generic definition.
>
>
> Bootstrap and regression tested on aarch64-none-linux-gnu,
> arm-none-linux-gnueabihf and arm-none-eabi.

The revised patch looks good on PowerPC.

Thanks, David

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Torvald Riegel]

On Tue, 2015-06-09 at 10:50 +0100, Ramana Radhakrishnan wrote:
> 
> On 22/05/15 17:56, Torvald Riegel wrote:
> > On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
> >> Hi,
> >>
> >> While writing atomic_word.h for the ARM backend to fix PR target/66200
> >> I
> >> thought it would make more sense to write it all up with atomic
> >> primitives instead of providing various fragile bits of inline
> >> asssembler. Thus this patch came about. I intend to ask for a
> >> specialized version of this to be applied for the ARM backend in any
> >> case after testing completes.
> >>
> >> If this goes in as a cleanup I expect all the ports to be deleting
> >> their
> >> atomic_word.h implementation in the various ports directories and
> >> I'll
> >> follow up with patches asking port maintainers to test and apply for
> >> each of the individual cases if this is deemed to be good enough.
> >
> > Could you use C++11 atomics right away instead of adapting the wrappers?
> 
> 
> I spent some time trying to massage guard.cc into using C++11 atomics 
> but it was just easier to write it with the builtins - I consider this 
> to be a step improvement compared to where we are currently.

Fair enough.

> I don't think we can remove _GLIBCXX_READ_MEM_BARRIER and 
> _GLIBCXX_WRITE_MEM_BARRIER from atomic_word.h even though they are 
> superseded by the atomics as it is published in the documentation as 
> available macros.

I see.  We should at least update the documentation of those, as the
current one isn't a really portable specification.  If we can, I'd
deprecate them.  Jonathan, what do you think?

> It was obvious that alpha, powerpc, aix, ia64 could just fall back to 
> the standard implementations. The cris and sparc implementations have 
> different types for _Atomic_word from generic and the rest - my guess is 
> sparc can remove the _GLIBCXX_READ_MEM_BARRIER and 
> _GLIBCXX_WRITE_MEM_BARRIER but I have no way of testing this is sane.
> 
> I'll leave that to the respective target maintainers for sparc and cris 
> to deal as appropriate.

LGTM.

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Jonathan Wakely]

On 11/06/15 23:56 +0200, Torvald Riegel wrote:
>> > On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
>> I don't think we can remove _GLIBCXX_READ_MEM_BARRIER and
>> _GLIBCXX_WRITE_MEM_BARRIER from atomic_word.h even though they are
>> superseded by the atomics as it is published in the documentation as
>> available macros.
>
>I see.  We should at least update the documentation of those, as the
>current one isn't a really portable specification.  If we can, I'd
>deprecate them.  Jonathan, what do you think?

Yes, I'm in favour of deprecating them. They are GCC-specific anyway,
so there is no reason to prefer them to std::atomic_ or __atomic_
fences.

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Jonathan Wakely]

On 12/06/15 10:30 +0100, Ramana Radhakrishnan wrote:
>On Fri, Jun 12, 2015 at 10:06 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 11/06/15 23:56 +0200, Torvald Riegel wrote:
>>>>
>>>> > On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
>>>> I don't think we can remove _GLIBCXX_READ_MEM_BARRIER and
>>>> _GLIBCXX_WRITE_MEM_BARRIER from atomic_word.h even though they are
>>>> superseded by the atomics as it is published in the documentation as
>>>> available macros.
>>>
>>>
>>> I see.  We should at least update the documentation of those, as the
>>> current one isn't a really portable specification.  If we can, I'd
>>> deprecate them.  Jonathan, what do you think?
>>
>>
>> Yes, I'm in favour of deprecating them. They are GCC-specific anyway,
>> so there is no reason to prefer them to std::atomic_ or __atomic_
>> fences.
>
>I'll treat it as a follow-up.

Sure.

>Can I get an ack for this patch though ? I could backport this as is
>to fix the problems on ARM / AArch64 (PR target/66200) - alternatively
>I'll provide header implementations of the same for the release
>branches.

Yes, OK for trunk, thanks.

I think it's safer if the backport only changes the ARM and AArch64
implementations, at least for now. If no problems are found on trunk
we could consider backporting the whole thing for all targets, but it
may not be worth it if the other targets are working OK.

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Ramana Radhakrishnan]

On Fri, Jun 12, 2015 at 10:06 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 11/06/15 23:56 +0200, Torvald Riegel wrote:
>>>
>>> > On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
>>> I don't think we can remove _GLIBCXX_READ_MEM_BARRIER and
>>> _GLIBCXX_WRITE_MEM_BARRIER from atomic_word.h even though they are
>>> superseded by the atomics as it is published in the documentation as
>>> available macros.
>>
>>
>> I see.  We should at least update the documentation of those, as the
>> current one isn't a really portable specification.  If we can, I'd
>> deprecate them.  Jonathan, what do you think?
>
>
> Yes, I'm in favour of deprecating them. They are GCC-specific anyway,
> so there is no reason to prefer them to std::atomic_ or __atomic_
> fences.

I'll treat it as a follow-up.

Can I get an ack for this patch though ? I could backport this as is
to fix the problems on ARM / AArch64 (PR target/66200) - alternatively
I'll provide header implementations of the same for the release
branches.


regards
Ramana

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[Torvald Riegel]

On Fri, 2015-06-12 at 10:30 +0100, Ramana Radhakrishnan wrote:
> On Fri, Jun 12, 2015 at 10:06 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> > On 11/06/15 23:56 +0200, Torvald Riegel wrote:
> >>>
> >>> > On Fri, 2015-05-22 at 12:37 +0100, Ramana Radhakrishnan wrote:
> >>> I don't think we can remove _GLIBCXX_READ_MEM_BARRIER and
> >>> _GLIBCXX_WRITE_MEM_BARRIER from atomic_word.h even though they are
> >>> superseded by the atomics as it is published in the documentation as
> >>> available macros.
> >>
> >>
> >> I see.  We should at least update the documentation of those, as the
> >> current one isn't a really portable specification.  If we can, I'd
> >> deprecate them.  Jonathan, what do you think?
> >
> >
> > Yes, I'm in favour of deprecating them. They are GCC-specific anyway,
> > so there is no reason to prefer them to std::atomic_ or __atomic_
> > fences.
> 
> I'll treat it as a follow-up.

Thanks.

> Can I get an ack for this patch though ?

The above comment was of a general nature, not specifically about this
patch or meant as a precondition for this patch.  Sorry if that wasn't
clear :)
To me, your patch looks good.

Re: [Patch libstdc++] Rewrite cpu/generic/atomic_word.h

[David Edelsohn]

I bootstrapped this on powerpc-ibm-aix7.1.0.0 and my colleagues
bootstrapped this on powerpc64-linux and powerpc64le-linux.

It works and produces reasonable instruction sequences.

We can iterate on the syntax, but the core concept seems to work correctly.

Thanks, David