[webkit-changes] [211129] trunk

Tue, 24 Jan 2017 18:52:19 -0800

Title: [211129] trunk
Revision
211129
Author
[email protected]
Date
2017-01-24 18:52:51 -0800 (Tue, 24 Jan 2017)

Log Message

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/_javascript_Core:

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

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (211128 => 211129)


--- trunk/JSTests/ChangeLog	2017-01-25 02:40:52 UTC (rev 211128)
+++ trunk/JSTests/ChangeLog	2017-01-25 02:52:51 UTC (rev 211129)
@@ -1,3 +1,13 @@
+2017-01-24  Filip Pizlo  <[email protected]>
+
+        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.
+
 2017-01-24  Yusuke Suzuki  <[email protected]>
 
         [JSC] Optimize Number#toString with Int52

Added: trunk/JSTests/stress/atomics-add-uint32.js (0 => 211129)


--- 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)
@@ -0,0 +1,13 @@
+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]);
+

Modified: trunk/JSTests/stress/atomics-store-return.js (211128 => 211129)


--- 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)
@@ -2,10 +2,25 @@
 var a = new Int8Array(sab);
 var result = Atomics.store(a, 0, 1000);
 if (result != 1000)
-    throw "Error: bad result: " + result;
+    throw new Error("bad result: " + result);
 
 sab = new SharedArrayBuffer(4);
 a = new Uint32Array(sab);
-var result = Atomics.store(a, 0, 4000000000);
+result = Atomics.store(a, 0, 4000000000);
 if (result != 4000000000)
-    throw "Error: bad result: " + result;
+    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);
+

Modified: trunk/Source/_javascript_Core/ChangeLog (211128 => 211129)


--- trunk/Source/_javascript_Core/ChangeLog	2017-01-25 02:40:52 UTC (rev 211128)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-01-25 02:52:51 UTC (rev 211129)
@@ -1,3 +1,29 @@
+2017-01-24  Filip Pizlo  <[email protected]>
+
+        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):
+
 2017-01-24  Yusuke Suzuki  <[email protected]>
 
         [JSC] Optimize Number#toString with Int52

Modified: trunk/Source/_javascript_Core/runtime/AtomicsObject.cpp (211128 => 211129)


--- trunk/Source/_javascript_Core/runtime/AtomicsObject.cpp	2017-01-25 02:40:52 UTC (rev 211128)
+++ trunk/Source/_javascript_Core/runtime/AtomicsObject.cpp	2017-01-25 02:52:51 UTC (rev 211129)
@@ -92,9 +92,9 @@
 {
     JSGenericTypedArrayView<Adaptor>* typedArray = jsCast<JSGenericTypedArrayView<Adaptor>*>(typedArrayView);
     
-    int32_t extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
+    double extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
     for (unsigned i = 0; i < numExtraArgs; ++i) {
-        int32_t value = exec->argument(2 + i).toInt32(exec);
+        double value = exec->argument(2 + i).toInteger(exec);
         RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
         extraArgs[i] = value;
     }
@@ -191,8 +191,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncAdd(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const int32_t* args) {
-            return jsNumber(WTF::atomicExchangeAdd(ptr, args[0]));
+        exec, [&] (auto* ptr, const double* args) {
+            return jsNumber(WTF::atomicExchangeAdd(ptr, toInt32(args[0])));
         });
 }
 
@@ -199,8 +199,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncAnd(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const int32_t* args) {
-            return jsNumber(WTF::atomicExchangeAnd(ptr, args[0]));
+        exec, [&] (auto* ptr, const double* args) {
+            return jsNumber(WTF::atomicExchangeAnd(ptr, toInt32(args[0])));
         });
 }
 
@@ -207,7 +207,7 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncCompareExchange(ExecState* exec)
 {
     return atomicOperationWithArgs<2>(
-        exec, [&] (auto* ptr, const int32_t* args) {
+        exec, [&] (auto* ptr, const double* args) {
             typedef typename std::remove_pointer<decltype(ptr)>::type T;
             T expected = static_cast<T>(args[0]);
             T newValue = static_cast<T>(args[1]);
@@ -218,7 +218,7 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncExchange(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const int32_t* args) {
+        exec, [&] (auto* ptr, const double* args) {
             typedef typename std::remove_pointer<decltype(ptr)>::type T;
             return jsNumber(WTF::atomicExchange(ptr, static_cast<T>(args[0])));
         });
@@ -249,7 +249,7 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncLoad(ExecState* exec)
 {
     return atomicOperationWithArgs<0>(
-        exec, [&] (auto* ptr, const int32_t*) {
+        exec, [&] (auto* ptr, const double*) {
             return jsNumber(WTF::atomicLoad(ptr));
         });
 }
@@ -257,8 +257,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncOr(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const int32_t* args) {
-            return jsNumber(WTF::atomicExchangeOr(ptr, args[0]));
+        exec, [&] (auto* ptr, const double* args) {
+            return jsNumber(WTF::atomicExchangeOr(ptr, toInt32(args[0])));
         });
 }
 
@@ -265,14 +265,11 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncStore(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const int32_t* args) {
+        exec, [&] (auto* ptr, const double* args) {
             typedef typename std::remove_pointer<decltype(ptr)>::type T;
-            int32_t valueAsInt = args[0];
+            double 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);
         });
 }
@@ -280,8 +277,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncSub(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const int32_t* args) {
-            return jsNumber(WTF::atomicExchangeSub(ptr, args[0]));
+        exec, [&] (auto* ptr, const double* args) {
+            return jsNumber(WTF::atomicExchangeSub(ptr, toInt32(args[0])));
         });
 }
 
@@ -390,8 +387,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncXor(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const int32_t* args) {
-            return jsNumber(WTF::atomicExchangeXor(ptr, args[0]));
+        exec, [&] (auto* ptr, const double* args) {
+            return jsNumber(WTF::atomicExchangeXor(ptr, toInt32(args[0])));
         });
 }
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to