diff mbox

[v3] RFC: rename __calculate_memory_order

Message ID CAH6eHdRSC_kzCOA2xqNfLrv56OnafcwUmzYvCxv+F1xP=eB7QA@mail.gmail.com
State New
Headers show

Commit Message

Jonathan Wakely Dec. 3, 2011, 12:58 p.m. UTC 
Are there any objections to this patch?

I find the function easier to parse in this form and it allows it to
be constexpr. Maybe more importantly, it determines the memory order
to be used by compare_exchange_xxx on failure so I think
__cmpexch_failure_order is a more descriptive name than
__calculate_memory_order.

        * include/bits/atomic_base.h (__calculate_memory_order): Rename to...
        (__cmpexch_failure_order): This, and rewrite as constexpr function.
        (compare_exchange_strong, compare_exchange_weak): Use it.
        * include/std/atomic (compare_exchange_strong, compare_exchange_weak):
        Likewise.

Tested x86_64-linux.

Comments

Benjamin Kosnik Dec. 7, 2011, 11:58 p.m. UTC | #1 
>         * include/bits/atomic_base.h (__calculate_memory_order):
> Rename to... (__cmpexch_failure_order): This, and rewrite as
> constexpr function. (compare_exchange_strong, compare_exchange_weak):
> Use it.
>         * include/std/atomic (compare_exchange_strong,
> compare_exchange_weak): Likewise.
> 
> Tested x86_64-linux.

looks great to me. More constexpr, what's not to like?

-benjamin
Jonathan Wakely Dec. 8, 2011, 9:45 a.m. UTC | #2 
On 7 December 2011 23:58, Benjamin Kosnik wrote:
>
>>         * include/bits/atomic_base.h (__calculate_memory_order):
>> Rename to... (__cmpexch_failure_order): This, and rewrite as
>> constexpr function. (compare_exchange_strong, compare_exchange_weak):
>> Use it.
>>         * include/std/atomic (compare_exchange_strong,
>> compare_exchange_weak): Likewise.
>>
>> Tested x86_64-linux.
>
> looks great to me. More constexpr, what's not to like?

OK, thanks - I checked it in.
diff mbox

Patch

Index: include/bits/atomic_base.h
===================================================================
--- include/bits/atomic_base.h	(revision 181967)
+++ include/bits/atomic_base.h	(working copy)
@@ -59,14 +59,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       memory_order_seq_cst
     } memory_order;
 
-  inline memory_order
-  __calculate_memory_order(memory_order __m) noexcept
+  // Drop release ordering as per [atomics.types.operations.req]/21
+  constexpr memory_order
+  __cmpexch_failure_order(memory_order __m) noexcept
   {
-    const bool __cond1 = __m == memory_order_release;
-    const bool __cond2 = __m == memory_order_acq_rel;
-    memory_order __mo1(__cond1 ? memory_order_relaxed : __m);
-    memory_order __mo2(__cond2 ? memory_order_acquire : __mo1);
-    return __mo2;
+    return __m == memory_order_acq_rel ? memory_order_acquire
+      : __m == memory_order_release ? memory_order_relaxed : __m;
   }
 
   inline void
@@ -505,7 +503,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			    memory_order __m = memory_order_seq_cst) noexcept
       {
 	return compare_exchange_weak(__i1, __i2, __m,
-				     __calculate_memory_order(__m));
+				     __cmpexch_failure_order(__m));
       }
 
       bool
@@ -513,7 +511,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		   memory_order __m = memory_order_seq_cst) volatile noexcept
       {
 	return compare_exchange_weak(__i1, __i2, __m,
-				     __calculate_memory_order(__m));
+				     __cmpexch_failure_order(__m));
       }
 
       bool
@@ -544,7 +542,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			      memory_order __m = memory_order_seq_cst) noexcept
       {
 	return compare_exchange_strong(__i1, __i2, __m,
-				       __calculate_memory_order(__m));
+				       __cmpexch_failure_order(__m));
       }
 
       bool
@@ -552,7 +550,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		 memory_order __m = memory_order_seq_cst) volatile noexcept
       {
 	return compare_exchange_strong(__i1, __i2, __m,
-				       __calculate_memory_order(__m));
+				       __cmpexch_failure_order(__m));
       }
 
       __int_type
Index: include/std/atomic
===================================================================
--- include/std/atomic	(revision 181967)
+++ include/std/atomic	(working copy)
@@ -408,7 +408,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			    memory_order __m = memory_order_seq_cst) noexcept
       {
 	return compare_exchange_weak(__p1, __p2, __m,
-				     __calculate_memory_order(__m));
+				     __cmpexch_failure_order(__m));
       }
 
       bool
@@ -416,7 +416,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		    memory_order __m = memory_order_seq_cst) volatile noexcept
       {
 	return compare_exchange_weak(__p1, __p2, __m,
-				     __calculate_memory_order(__m));
+				     __cmpexch_failure_order(__m));
       }
 
       bool
@@ -435,7 +435,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			      memory_order __m = memory_order_seq_cst) noexcept
       {
 	return _M_b.compare_exchange_strong(__p1, __p2, __m,
-					    __calculate_memory_order(__m));
+					    __cmpexch_failure_order(__m));
       }
 
       bool
@@ -443,7 +443,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		    memory_order __m = memory_order_seq_cst) volatile noexcept
       {
 	return _M_b.compare_exchange_strong(__p1, __p2, __m,
-					    __calculate_memory_order(__m));
+					    __cmpexch_failure_order(__m));
       }
 
       __pointer_type