Changeset 205914 in webkit


Ignore:
Timestamp:
Sep 14, 2016 10:11:57 AM (8 years ago)
Author:
jfbastien@apple.com
Message:

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):

Location:
trunk/Source/WTF
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r205895 r205914  
     12016-09-14  JF Bastien  <jfbastien@apple.com>
     2
     3        Alwasys inline atomic operations
     4        https://bugs.webkit.org/show_bug.cgi?id=155371
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Fixes "build fails with memory model cannot be stronger than
     9        success memory model for ‘__atomic_compare_exchange’".
     10
     11        Pre-5 revisions of GCC at Os only generated an error message
     12        related to invalid failure memory ordering. The reason is that
     13        libstdc++ tries to be clever about enforcing the C++ standard's
     14        clause [atomics.types.operations.req] ¶21 which states:
     15
     16            Requires: The failure argument shall not be
     17            `memory_order_release` nor `memory_order_acq_rel`. The failure
     18            argument shall be no stronger than the success argument.
     19
     20        It fails at doing this because its inlining heuristics are
     21        modified by Os, and they're not quite as dumb as O0 but not smart
     22        enough to get to the good code at O1. Adding ALWAYS_INLINE fixes
     23        the silliness at Os, leaves O1 great, and makes O0 slightly less
     24        bad but still pretty bad.
     25
     26        The other good news is that I'm going to get this particular
     27        problem fixed in the version of C++ which will come after C++17:
     28
     29        https://github.com/jfbastien/papers/blob/master/source/P0418r1.bs
     30
     31        While we're at it we should always inline all of these wrapped
     32        functions because the generated code is horrendous if the memory
     33        order isn't known at compile time.
     34
     35        * wtf/Atomics.h:
     36        (WTF::Atomic::load):
     37        (WTF::Atomic::store):
     38        (WTF::Atomic::compareExchangeWeak):
     39        (WTF::Atomic::compareExchangeStrong):
     40        (WTF::Atomic::exchangeAndAdd):
     41        (WTF::Atomic::exchange):
     42
    1432016-09-13  Michael Saboff  <msaboff@apple.com>
    244

  • trunk/Source/WTF/wtf/Atomics.h

    r199760 r205914  
    5353    // is usually not high enough to justify the risk.
    5454
    55     T load(std::memory_order order = std::memory_order_seq_cst) const { return value.load(order); }
     55    ALWAYS_INLINE T load(std::memory_order order = std::memory_order_seq_cst) const { return value.load(order); }
    5656
    57     void store(T desired, std::memory_order order = std::memory_order_seq_cst) { value.store(desired, order); }
     57    ALWAYS_INLINE void store(T desired, std::memory_order order = std::memory_order_seq_cst) { value.store(desired, order); }
    5858
    59     bool compareExchangeWeak(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
     59    ALWAYS_INLINE bool compareExchangeWeak(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
    6060    {
    6161#if OS(WINDOWS)
    … …  
    6868    }
    6969
    70     bool compareExchangeStrong(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
     70    ALWAYS_INLINE bool compareExchangeStrong(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
    7171    {
    7272#if OS(WINDOWS)
    … …  
    7979   
    8080    template<typename U>
    81     T exchangeAndAdd(U addend, std::memory_order order = std::memory_order_seq_cst)
     81    ALWAYS_INLINE T exchangeAndAdd(U addend, std::memory_order order = std::memory_order_seq_cst)
    8282    {
    8383#if OS(WINDOWS)
    … …  
    8888    }
    8989   
    90     T exchange(T newValue, std::memory_order order = std::memory_order_seq_cst)
     90    ALWAYS_INLINE T exchange(T newValue, std::memory_order order = std::memory_order_seq_cst)
    9191    {
    9292#if OS(WINDOWS)
Note: See TracChangeset for help on using the changeset viewer.