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
