<!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>[211122] 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/211122">211122</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2017-01-24 16:53:48 -0800 (Tue, 24 Jan 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Atomics.store should return the int-converted value, not the value that it stored
https://bugs.webkit.org/show_bug.cgi?id=167395
Reviewed by Saam Barati.
JSTests:
* stress/atomics-store-return.js: Added.
Source/JavaScriptCore:
Previously the code was based around passing a lambda that operated over the native type of the
operation (so for example int8_t if we were doing things to Int8Arrays). But to support this
behavior of store, we need it to be able to control how it converts its result to JSValue and it
needs to see its argument as an int32_t. It turns out that it's easy for all of the functions in
AtomicsObject.cpp to also adopt this protocol since the conversion to JSValue is just jsNumber()
from the native type in those cases, and the conversion from int32_t is done for free in
std::atomic.
* 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="#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="#trunkJSTestsstressatomicsstorereturnjs">trunk/JSTests/stress/atomics-store-return.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/ChangeLog (211121 => 211122)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/ChangeLog 2017-01-25 00:49:33 UTC (rev 211121)
+++ trunk/JSTests/ChangeLog 2017-01-25 00:53:48 UTC (rev 211122)
</span><span class="lines">@@ -1,5 +1,14 @@
</span><span class="cx"> 2017-01-24 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><ins>+ Atomics.store should return the int-converted value, not the value that it stored
+ https://bugs.webkit.org/show_bug.cgi?id=167395
+
+ Reviewed by Saam Barati.
+
+ * stress/atomics-store-return.js: Added.
+
+2017-01-24 Filip Pizlo <fpizlo@apple.com>
+
</ins><span class="cx"> -0 is a valid array index and AtomicsObject should know this
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=167386
</span><span class="cx">
</span></span></pre></div>
<a id="trunkJSTestsstressatomicsstorereturnjs"></a>
<div class="addfile"><h4>Added: trunk/JSTests/stress/atomics-store-return.js (0 => 211122)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/atomics-store-return.js (rev 0)
+++ trunk/JSTests/stress/atomics-store-return.js 2017-01-25 00:53:48 UTC (rev 211122)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+var sab = new SharedArrayBuffer(1);
+var a = new Int8Array(sab);
+var result = Atomics.store(a, 0, 1000);
+if (result != 1000)
+ throw "Error: bad result: " + result;
+
+sab = new SharedArrayBuffer(4);
+a = new Uint32Array(sab);
+var result = Atomics.store(a, 0, 4000000000);
+if (result != 4000000000)
+ throw "Error: bad result: " + result;
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (211121 => 211122)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2017-01-25 00:49:33 UTC (rev 211121)
+++ trunk/Source/JavaScriptCore/ChangeLog 2017-01-25 00:53:48 UTC (rev 211122)
</span><span class="lines">@@ -1,5 +1,31 @@
</span><span class="cx"> 2017-01-24 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><ins>+ Atomics.store should return the int-converted value, not the value that it stored
+ https://bugs.webkit.org/show_bug.cgi?id=167395
+
+ Reviewed by Saam Barati.
+
+ Previously the code was based around passing a lambda that operated over the native type of the
+ operation (so for example int8_t if we were doing things to Int8Arrays). But to support this
+ behavior of store, we need it to be able to control how it converts its result to JSValue and it
+ needs to see its argument as an int32_t. It turns out that it's easy for all of the functions in
+ AtomicsObject.cpp to also adopt this protocol since the conversion to JSValue is just jsNumber()
+ from the native type in those cases, and the conversion from int32_t is done for free in
+ std::atomic.
+
+ * runtime/AtomicsObject.cpp:
+ (JSC::atomicsFuncAdd):
+ (JSC::atomicsFuncAnd):
+ (JSC::atomicsFuncCompareExchange):
+ (JSC::atomicsFuncExchange):
+ (JSC::atomicsFuncLoad):
+ (JSC::atomicsFuncOr):
+ (JSC::atomicsFuncStore):
+ (JSC::atomicsFuncSub):
+ (JSC::atomicsFuncXor):
+
+2017-01-24 Filip Pizlo <fpizlo@apple.com>
+
</ins><span class="cx"> -0 is a valid array index and AtomicsObject should know this
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=167386
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeAtomicsObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp (211121 => 211122)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp 2017-01-25 00:49:33 UTC (rev 211121)
+++ trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp 2017-01-25 00:53:48 UTC (rev 211122)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2016 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -92,15 +92,14 @@
</span><span class="cx"> {
</span><span class="cx"> JSGenericTypedArrayView<Adaptor>* typedArray = jsCast<JSGenericTypedArrayView<Adaptor>*>(typedArrayView);
</span><span class="cx">
</span><del>- typename Adaptor::Type extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
</del><ins>+ int32_t 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><span class="cx"> int32_t value = exec->argument(2 + i).toInt32(exec);
</span><span class="cx"> RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
</span><del>- extraArgs[i] = Adaptor::toNativeFromInt32(value);
</del><ins>+ extraArgs[i] = value;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- typename Adaptor::Type result = func(typedArray->typedVector() + accessIndex, extraArgs);
- return JSValue::encode(Adaptor::toJSValue(result));
</del><ins>+ return JSValue::encode(func(typedArray->typedVector() + accessIndex, extraArgs));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> unsigned validatedAccessIndex(VM& vm, ExecState* exec, JSArrayBufferView* typedArrayView)
</span><span class="lines">@@ -192,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 auto* args) {
- return WTF::atomicExchangeAdd(ptr, args[0]);
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ return jsNumber(WTF::atomicExchangeAdd(ptr, args[0]));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -200,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 auto* args) {
- return WTF::atomicExchangeAnd(ptr, args[0]);
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ return jsNumber(WTF::atomicExchangeAnd(ptr, args[0]));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -208,8 +207,11 @@
</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 auto* args) {
- return WTF::atomicCompareExchangeStrong(ptr, args[0], args[1]);
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ typedef typename std::remove_pointer<decltype(ptr)>::type T;
+ T expected = static_cast<T>(args[0]);
+ T newValue = static_cast<T>(args[1]);
+ return jsNumber(WTF::atomicCompareExchangeStrong(ptr, expected, newValue));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -216,8 +218,9 @@
</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 auto* args) {
- return WTF::atomicExchange(ptr, args[0]);
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ typedef typename std::remove_pointer<decltype(ptr)>::type T;
+ return jsNumber(WTF::atomicExchange(ptr, static_cast<T>(args[0])));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -246,8 +249,8 @@
</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 auto*) {
- return WTF::atomicLoad(ptr);
</del><ins>+ exec, [&] (auto* ptr, const int32_t*) {
+ return jsNumber(WTF::atomicLoad(ptr));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -254,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 auto* args) {
- return WTF::atomicExchangeOr(ptr, args[0]);
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ return jsNumber(WTF::atomicExchangeOr(ptr, args[0]));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -262,10 +265,15 @@
</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 auto* args) {
- auto value = args[0];
- WTF::atomicStore(ptr, value);
- return value;
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ typedef typename std::remove_pointer<decltype(ptr)>::type T;
+ int32_t valueAsInt = args[0];
+ T valueAsT = static_cast<T>(valueAsInt);
+ WTF::atomicStore(ptr, valueAsT);
+
+ if (static_cast<int32_t>(valueAsT) == valueAsInt)
+ return jsNumber(valueAsT);
+ return jsNumber(valueAsInt);
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -272,8 +280,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 auto* args) {
- return WTF::atomicExchangeSub(ptr, args[0]);
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ return jsNumber(WTF::atomicExchangeSub(ptr, args[0]));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -382,8 +390,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 auto* args) {
- return WTF::atomicExchangeXor(ptr, args[0]);
</del><ins>+ exec, [&] (auto* ptr, const int32_t* args) {
+ return jsNumber(WTF::atomicExchangeXor(ptr, args[0]));
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>