<!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>[211129] trunk</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/211129">211129</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2017-01-24 18:52:51 -0800 (Tue, 24 Jan 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Atomics.store should return the int-converted value according to toInteger
https://bugs.webkit.org/show_bug.cgi?id=167399
Reviewed by Saam Barati.
JSTests:
* stress/atomics-add-uint32.js: Added.
* stress/atomics-store-return.js: Fix the test to match what the spec wants.
Source/JavaScriptCore:
I keep getting this wrong, but I think I've finally done it right. What we want is for
Atomics.store to return the value it was passed after toInteger, which doesn't clip the value to
any kind of range. It does get truncated to double.
This changes the code to pass those "integers" as doubles. It doesn't matter that this is slow,
since all of these code paths are slow due to their need to check everything. We'll take care of
that by making them intrinsic later.
* runtime/AtomicsObject.cpp:
(JSC::atomicsFuncAdd):
(JSC::atomicsFuncAnd):
(JSC::atomicsFuncCompareExchange):
(JSC::atomicsFuncExchange):
(JSC::atomicsFuncLoad):
(JSC::atomicsFuncOr):
(JSC::atomicsFuncStore):
(JSC::atomicsFuncSub):
(JSC::atomicsFuncXor):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJSTestsChangeLog">trunk/JSTests/ChangeLog</a></li>
<li><a href="#trunkJSTestsstressatomicsstorereturnjs">trunk/JSTests/stress/atomics-store-return.js</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeAtomicsObjectcpp">trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkJSTestsstressatomicsadduint32js">trunk/JSTests/stress/atomics-add-uint32.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/ChangeLog (211128 => 211129)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/ChangeLog 2017-01-25 02:40:52 UTC (rev 211128)
+++ trunk/JSTests/ChangeLog 2017-01-25 02:52:51 UTC (rev 211129)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2017-01-24 Filip Pizlo <fpizlo@apple.com>
+
+ Atomics.store should return the int-converted value according to toInteger
+ https://bugs.webkit.org/show_bug.cgi?id=167399
+
+ Reviewed by Saam Barati.
+
+ * stress/atomics-add-uint32.js: Added.
+ * stress/atomics-store-return.js: Fix the test to match what the spec wants.
+
</ins><span class="cx"> 2017-01-24 Yusuke Suzuki <utatane.tea@gmail.com>
</span><span class="cx">
</span><span class="cx"> [JSC] Optimize Number#toString with Int52
</span></span></pre></div>
<a id="trunkJSTestsstressatomicsadduint32js"></a>
<div class="addfile"><h4>Added: trunk/JSTests/stress/atomics-add-uint32.js (0 => 211129)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/atomics-add-uint32.js (rev 0)
+++ trunk/JSTests/stress/atomics-add-uint32.js 2017-01-25 02:52:51 UTC (rev 211129)
</span><span class="lines">@@ -0,0 +1,13 @@
</span><ins>+var sab = new SharedArrayBuffer(4);
+var a = new Uint32Array(sab);
+var result = Atomics.add(a, 0, 4000000000);
+if (result != 0)
+ throw new Error("bad result: " + result);
+if (a[0] != 4000000000)
+ throw new Error("bad value read back: " + a[0]);
+result = Atomics.sub(a, 0, 4000000000);
+if (result != 4000000000)
+ throw new Error("bad result: " + result);
+if (a[0] != 0)
+ throw new Error("bad value read back: " + a[0]);
+
</ins></span></pre></div>
<a id="trunkJSTestsstressatomicsstorereturnjs"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/stress/atomics-store-return.js (211128 => 211129)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/atomics-store-return.js 2017-01-25 02:40:52 UTC (rev 211128)
+++ trunk/JSTests/stress/atomics-store-return.js 2017-01-25 02:52:51 UTC (rev 211129)
</span><span class="lines">@@ -2,10 +2,25 @@
</span><span class="cx"> var a = new Int8Array(sab);
</span><span class="cx"> var result = Atomics.store(a, 0, 1000);
</span><span class="cx"> if (result != 1000)
</span><del>- throw "Error: bad result: " + result;
</del><ins>+ throw new Error("bad result: " + result);
</ins><span class="cx">
</span><span class="cx"> sab = new SharedArrayBuffer(4);
</span><span class="cx"> a = new Uint32Array(sab);
</span><del>-var result = Atomics.store(a, 0, 4000000000);
</del><ins>+result = Atomics.store(a, 0, 4000000000);
</ins><span class="cx"> if (result != 4000000000)
</span><del>- throw "Error: bad result: " + result;
</del><ins>+ throw new Error("bad result: " + result);
+if (a[0] != 4000000000)
+ throw new Error("bad value read back: " + a[0]);
+result = Atomics.store(a, 0, -4000000000);
+if (result != -4000000000)
+ throw new Error("bad result: " + result);
+if (a[0] != 294967296)
+ throw new Error("bad value read back: " + a[0]);
+
+var count = 0;
+result = Atomics.store(a, 0, { valueOf() { count++; return 42; } });
+if (result != 42)
+ throw new Error("bad result: " + result);
+if (count != 1)
+ throw new Error("bad count: " + count);
+
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (211128 => 211129)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2017-01-25 02:40:52 UTC (rev 211128)
+++ trunk/Source/JavaScriptCore/ChangeLog 2017-01-25 02:52:51 UTC (rev 211129)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2017-01-24 Filip Pizlo <fpizlo@apple.com>
+
+ Atomics.store should return the int-converted value according to toInteger
+ https://bugs.webkit.org/show_bug.cgi?id=167399
+
+ Reviewed by Saam Barati.
+
+ I keep getting this wrong, but I think I've finally done it right. What we want is for
+ Atomics.store to return the value it was passed after toInteger, which doesn't clip the value to
+ any kind of range. It does get truncated to double.
+
+ This changes the code to pass those "integers" as doubles. It doesn't matter that this is slow,
+ since all of these code paths are slow due to their need to check everything. We'll take care of
+ that by making them intrinsic later.
+
+ * runtime/AtomicsObject.cpp:
+ (JSC::atomicsFuncAdd):
+ (JSC::atomicsFuncAnd):
+ (JSC::atomicsFuncCompareExchange):
+ (JSC::atomicsFuncExchange):
+ (JSC::atomicsFuncLoad):
+ (JSC::atomicsFuncOr):
+ (JSC::atomicsFuncStore):
+ (JSC::atomicsFuncSub):
+ (JSC::atomicsFuncXor):
+
</ins><span class="cx"> 2017-01-24 Yusuke Suzuki <utatane.tea@gmail.com>
</span><span class="cx">
</span><span class="cx"> [JSC] Optimize Number#toString with Int52
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeAtomicsObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp (211128 => 211129)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp 2017-01-25 02:40:52 UTC (rev 211128)
+++ trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp 2017-01-25 02:52:51 UTC (rev 211129)
</span><span class="lines">@@ -92,9 +92,9 @@
</span><span class="cx"> {
</span><span class="cx"> JSGenericTypedArrayView<Adaptor>* typedArray = jsCast<JSGenericTypedArrayView<Adaptor>*>(typedArrayView);
</span><span class="cx">
</span><del>- int32_t extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
</del><ins>+ double extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
</ins><span class="cx"> for (unsigned i = 0; i < numExtraArgs; ++i) {
</span><del>- int32_t value = exec->argument(2 + i).toInt32(exec);
</del><ins>+ double value = exec->argument(2 + i).toInteger(exec);
</ins><span class="cx"> RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
</span><span class="cx"> extraArgs[i] = value;
</span><span class="cx"> }
</span><span class="lines">@@ -191,8 +191,8 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncAdd(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<1>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
- return jsNumber(WTF::atomicExchangeAdd(ptr, args[0]));
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
+ return jsNumber(WTF::atomicExchangeAdd(ptr, toInt32(args[0])));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -199,8 +199,8 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncAnd(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<1>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
- return jsNumber(WTF::atomicExchangeAnd(ptr, args[0]));
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
+ return jsNumber(WTF::atomicExchangeAnd(ptr, toInt32(args[0])));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -207,7 +207,7 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncCompareExchange(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<2>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
</ins><span class="cx"> typedef typename std::remove_pointer<decltype(ptr)>::type T;
</span><span class="cx"> T expected = static_cast<T>(args[0]);
</span><span class="cx"> T newValue = static_cast<T>(args[1]);
</span><span class="lines">@@ -218,7 +218,7 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncExchange(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<1>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
</ins><span class="cx"> typedef typename std::remove_pointer<decltype(ptr)>::type T;
</span><span class="cx"> return jsNumber(WTF::atomicExchange(ptr, static_cast<T>(args[0])));
</span><span class="cx"> });
</span><span class="lines">@@ -249,7 +249,7 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncLoad(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<0>(
</span><del>- exec, [&] (auto* ptr, const int32_t*) {
</del><ins>+ exec, [&] (auto* ptr, const double*) {
</ins><span class="cx"> return jsNumber(WTF::atomicLoad(ptr));
</span><span class="cx"> });
</span><span class="cx"> }
</span><span class="lines">@@ -257,8 +257,8 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncOr(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<1>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
- return jsNumber(WTF::atomicExchangeOr(ptr, args[0]));
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
+ return jsNumber(WTF::atomicExchangeOr(ptr, toInt32(args[0])));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -265,14 +265,11 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncStore(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<1>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
</ins><span class="cx"> typedef typename std::remove_pointer<decltype(ptr)>::type T;
</span><del>- int32_t valueAsInt = args[0];
</del><ins>+ double valueAsInt = args[0];
</ins><span class="cx"> T valueAsT = static_cast<T>(valueAsInt);
</span><span class="cx"> WTF::atomicStore(ptr, valueAsT);
</span><del>-
- if (static_cast<int32_t>(valueAsT) == valueAsInt)
- return jsNumber(valueAsT);
</del><span class="cx"> return jsNumber(valueAsInt);
</span><span class="cx"> });
</span><span class="cx"> }
</span><span class="lines">@@ -280,8 +277,8 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncSub(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<1>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
- return jsNumber(WTF::atomicExchangeSub(ptr, args[0]));
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
+ return jsNumber(WTF::atomicExchangeSub(ptr, toInt32(args[0])));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -390,8 +387,8 @@
</span><span class="cx"> EncodedJSValue JSC_HOST_CALL atomicsFuncXor(ExecState* exec)
</span><span class="cx"> {
</span><span class="cx"> return atomicOperationWithArgs<1>(
</span><del>- exec, [&] (auto* ptr, const int32_t* args) {
- return jsNumber(WTF::atomicExchangeXor(ptr, args[0]));
</del><ins>+ exec, [&] (auto* ptr, const double* args) {
+ return jsNumber(WTF::atomicExchangeXor(ptr, toInt32(args[0])));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>