Alwasys inline atomic operations
authorjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Sep 2016 17:11:57 +0000 (17:11 +0000)
committerjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Sep 2016 17:11:57 +0000 (17:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155371

Reviewed by Geoffrey Garen.

Fixes "build fails with memory model cannot be stronger than
success memory model for ‘__atomic_compare_exchange’".

Pre-5 revisions of GCC at Os only generated an error message
related to invalid failure memory ordering. The reason is that
libstdc++ tries to be clever about enforcing the C++ standard's
clause [atomics.types.operations.req] ¶21 which states:

    Requires: The failure argument shall not be
    `memory_order_release` nor `memory_order_acq_rel`. The failure
    argument shall be no stronger than the success argument.

It fails at doing this because its inlining heuristics are
modified by Os, and they're not quite as dumb as O0 but not smart
enough to get to the good code at O1. Adding ALWAYS_INLINE fixes
the silliness at Os, leaves O1 great, and makes O0 slightly less
bad but still pretty bad.

The other good news is that I'm going to get this particular
problem fixed in the version of C++ which will come after C++17:

https://github.com/jfbastien/papers/blob/master/source/P0418r1.bs

While we're at it we should always inline all of these wrapped
functions because the generated code is horrendous if the memory
order isn't known at compile time.

* wtf/Atomics.h:
(WTF::Atomic::load):
(WTF::Atomic::store):
(WTF::Atomic::compareExchangeWeak):
(WTF::Atomic::compareExchangeStrong):
(WTF::Atomic::exchangeAndAdd):
(WTF::Atomic::exchange):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@205914 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/Atomics.h

index ae47336..1a11b73 100644 (file)
@@ -1,3 +1,45 @@
+2016-09-14  JF Bastien  <jfbastien@apple.com>
+
+        Alwasys inline atomic operations
+        https://bugs.webkit.org/show_bug.cgi?id=155371
+
+        Reviewed by Geoffrey Garen.
+
+        Fixes "build fails with memory model cannot be stronger than
+        success memory model for ‘__atomic_compare_exchange’".
+
+        Pre-5 revisions of GCC at Os only generated an error message
+        related to invalid failure memory ordering. The reason is that
+        libstdc++ tries to be clever about enforcing the C++ standard's
+        clause [atomics.types.operations.req] ¶21 which states:
+
+            Requires: The failure argument shall not be
+            `memory_order_release` nor `memory_order_acq_rel`. The failure
+            argument shall be no stronger than the success argument.
+
+        It fails at doing this because its inlining heuristics are
+        modified by Os, and they're not quite as dumb as O0 but not smart
+        enough to get to the good code at O1. Adding ALWAYS_INLINE fixes
+        the silliness at Os, leaves O1 great, and makes O0 slightly less
+        bad but still pretty bad.
+
+        The other good news is that I'm going to get this particular
+        problem fixed in the version of C++ which will come after C++17:
+
+        https://github.com/jfbastien/papers/blob/master/source/P0418r1.bs
+
+        While we're at it we should always inline all of these wrapped
+        functions because the generated code is horrendous if the memory
+        order isn't known at compile time.
+
+        * wtf/Atomics.h:
+        (WTF::Atomic::load):
+        (WTF::Atomic::store):
+        (WTF::Atomic::compareExchangeWeak):
+        (WTF::Atomic::compareExchangeStrong):
+        (WTF::Atomic::exchangeAndAdd):
+        (WTF::Atomic::exchange):
+
 2016-09-13  Michael Saboff  <msaboff@apple.com>
 
         Promises aren't resolved properly when making a ObjC API callback
index ce4d5da..7f76bdf 100644 (file)
@@ -52,11 +52,11 @@ struct Atomic {
     // what you are doing and have thought about it very hard. The cost of seq_cst
     // is usually not high enough to justify the risk.
 
-    T load(std::memory_order order = std::memory_order_seq_cst) const { return value.load(order); }
+    ALWAYS_INLINE T load(std::memory_order order = std::memory_order_seq_cst) const { return value.load(order); }
 
-    void store(T desired, std::memory_order order = std::memory_order_seq_cst) { value.store(desired, order); }
+    ALWAYS_INLINE void store(T desired, std::memory_order order = std::memory_order_seq_cst) { value.store(desired, order); }
 
-    bool compareExchangeWeak(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
+    ALWAYS_INLINE bool compareExchangeWeak(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
     {
 #if OS(WINDOWS)
         // Windows makes strange assertions about the argument to compare_exchange_weak, and anyway,
@@ -67,7 +67,7 @@ struct Atomic {
         return value.compare_exchange_weak(expectedOrActual, desired, order);
     }
 
-    bool compareExchangeStrong(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
+    ALWAYS_INLINE bool compareExchangeStrong(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
     {
 #if OS(WINDOWS)
         // See above.
@@ -78,7 +78,7 @@ struct Atomic {
     }
     
     template<typename U>
-    T exchangeAndAdd(U addend, std::memory_order order = std::memory_order_seq_cst)
+    ALWAYS_INLINE T exchangeAndAdd(U addend, std::memory_order order = std::memory_order_seq_cst)
     {
 #if OS(WINDOWS)
         // See above.
@@ -87,7 +87,7 @@ struct Atomic {
         return value.fetch_add(addend, order);
     }
     
-    T exchange(T newValue, std::memory_order order = std::memory_order_seq_cst)
+    ALWAYS_INLINE T exchange(T newValue, std::memory_order order = std::memory_order_seq_cst)
     {
 #if OS(WINDOWS)
         // See above.