[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