<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[205914] trunk/Source/WTF</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/205914">205914</a></dd>
<dt>Author</dt> <dd>jfbastien@apple.com</dd>
<dt>Date</dt> <dd>2016-09-14 10:11:57 -0700 (Wed, 14 Sep 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Alwasys inline atomic operations
https://bugs.webkit.org/show_bug.cgi?id=155371

Reviewed by Geoffrey Garen.

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

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

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfAtomicsh">trunk/Source/WTF/wtf/Atomics.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (205913 => 205914)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/ChangeLog        2016-09-14 16:56:34 UTC (rev 205913)
+++ trunk/Source/WTF/ChangeLog        2016-09-14 17:11:57 UTC (rev 205914)
</span><span class="lines">@@ -1,3 +1,45 @@
</span><ins>+2016-09-14  JF Bastien  &lt;jfbastien@apple.com&gt;
+
+        Alwasys inline atomic operations
+        https://bugs.webkit.org/show_bug.cgi?id=155371
+
+        Reviewed by Geoffrey Garen.
+
+        Fixes &quot;build fails with memory model cannot be stronger than
+        success memory model for ‘__atomic_compare_exchange’&quot;.
+
+        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):
+
</ins><span class="cx"> 2016-09-13  Michael Saboff  &lt;msaboff@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Promises aren't resolved properly when making a ObjC API callback
</span></span></pre></div>
<a id="trunkSourceWTFwtfAtomicsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/Atomics.h (205913 => 205914)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/Atomics.h        2016-09-14 16:56:34 UTC (rev 205913)
+++ trunk/Source/WTF/wtf/Atomics.h        2016-09-14 17:11:57 UTC (rev 205914)
</span><span class="lines">@@ -52,11 +52,11 @@
</span><span class="cx">     // what you are doing and have thought about it very hard. The cost of seq_cst
</span><span class="cx">     // is usually not high enough to justify the risk.
</span><span class="cx"> 
</span><del>-    T load(std::memory_order order = std::memory_order_seq_cst) const { return value.load(order); }
</del><ins>+    ALWAYS_INLINE T load(std::memory_order order = std::memory_order_seq_cst) const { return value.load(order); }
</ins><span class="cx"> 
</span><del>-    void store(T desired, std::memory_order order = std::memory_order_seq_cst) { value.store(desired, order); }
</del><ins>+    ALWAYS_INLINE void store(T desired, std::memory_order order = std::memory_order_seq_cst) { value.store(desired, order); }
</ins><span class="cx"> 
</span><del>-    bool compareExchangeWeak(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
</del><ins>+    ALWAYS_INLINE bool compareExchangeWeak(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
</ins><span class="cx">     {
</span><span class="cx"> #if OS(WINDOWS)
</span><span class="cx">         // Windows makes strange assertions about the argument to compare_exchange_weak, and anyway,
</span><span class="lines">@@ -67,7 +67,7 @@
</span><span class="cx">         return value.compare_exchange_weak(expectedOrActual, desired, order);
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    bool compareExchangeStrong(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
</del><ins>+    ALWAYS_INLINE bool compareExchangeStrong(T expected, T desired, std::memory_order order = std::memory_order_seq_cst)
</ins><span class="cx">     {
</span><span class="cx"> #if OS(WINDOWS)
</span><span class="cx">         // See above.
</span><span class="lines">@@ -78,7 +78,7 @@
</span><span class="cx">     }
</span><span class="cx">     
</span><span class="cx">     template&lt;typename U&gt;
</span><del>-    T exchangeAndAdd(U addend, std::memory_order order = std::memory_order_seq_cst)
</del><ins>+    ALWAYS_INLINE T exchangeAndAdd(U addend, std::memory_order order = std::memory_order_seq_cst)
</ins><span class="cx">     {
</span><span class="cx"> #if OS(WINDOWS)
</span><span class="cx">         // See above.
</span><span class="lines">@@ -87,7 +87,7 @@
</span><span class="cx">         return value.fetch_add(addend, order);
</span><span class="cx">     }
</span><span class="cx">     
</span><del>-    T exchange(T newValue, std::memory_order order = std::memory_order_seq_cst)
</del><ins>+    ALWAYS_INLINE T exchange(T newValue, std::memory_order order = std::memory_order_seq_cst)
</ins><span class="cx">     {
</span><span class="cx"> #if OS(WINDOWS)
</span><span class="cx">         // See above.
</span></span></pre>
</div>
</div>

</body>
</html>