Title: [211122] trunk
Revision
211122
Author
[email protected]
Date
2017-01-24 16:53:48 -0800 (Tue, 24 Jan 2017)

Log Message

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

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

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (211121 => 211122)


--- trunk/JSTests/ChangeLog	2017-01-25 00:49:33 UTC (rev 211121)
+++ trunk/JSTests/ChangeLog	2017-01-25 00:53:48 UTC (rev 211122)
@@ -1,5 +1,14 @@
 2017-01-24  Filip Pizlo  <[email protected]>
 
+        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  <[email protected]>
+
         -0 is a valid array index and AtomicsObject should know this
         https://bugs.webkit.org/show_bug.cgi?id=167386
 

Added: trunk/JSTests/stress/atomics-store-return.js (0 => 211122)


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

Modified: trunk/Source/_javascript_Core/ChangeLog (211121 => 211122)


--- trunk/Source/_javascript_Core/ChangeLog	2017-01-25 00:49:33 UTC (rev 211121)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-01-25 00:53:48 UTC (rev 211122)
@@ -1,5 +1,31 @@
 2017-01-24  Filip Pizlo  <[email protected]>
 
+        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  <[email protected]>
+
         -0 is a valid array index and AtomicsObject should know this
         https://bugs.webkit.org/show_bug.cgi?id=167386
 

Modified: trunk/Source/_javascript_Core/runtime/AtomicsObject.cpp (211121 => 211122)


--- trunk/Source/_javascript_Core/runtime/AtomicsObject.cpp	2017-01-25 00:49:33 UTC (rev 211121)
+++ trunk/Source/_javascript_Core/runtime/AtomicsObject.cpp	2017-01-25 00:53:48 UTC (rev 211122)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -92,15 +92,14 @@
 {
     JSGenericTypedArrayView<Adaptor>* typedArray = jsCast<JSGenericTypedArrayView<Adaptor>*>(typedArrayView);
     
-    typename Adaptor::Type extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
+    int32_t 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);
         RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
-        extraArgs[i] = Adaptor::toNativeFromInt32(value);
+        extraArgs[i] = value;
     }
 
-    typename Adaptor::Type result = func(typedArray->typedVector() + accessIndex, extraArgs);
-    return JSValue::encode(Adaptor::toJSValue(result));
+    return JSValue::encode(func(typedArray->typedVector() + accessIndex, extraArgs));
 }
 
 unsigned validatedAccessIndex(VM& vm, ExecState* exec, JSArrayBufferView* typedArrayView)
@@ -192,8 +191,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncAdd(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const auto* args) {
-            return WTF::atomicExchangeAdd(ptr, args[0]);
+        exec, [&] (auto* ptr, const int32_t* args) {
+            return jsNumber(WTF::atomicExchangeAdd(ptr, args[0]));
         });
 }
 
@@ -200,8 +199,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncAnd(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const auto* args) {
-            return WTF::atomicExchangeAnd(ptr, args[0]);
+        exec, [&] (auto* ptr, const int32_t* args) {
+            return jsNumber(WTF::atomicExchangeAnd(ptr, args[0]));
         });
 }
 
@@ -208,8 +207,11 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncCompareExchange(ExecState* exec)
 {
     return atomicOperationWithArgs<2>(
-        exec, [&] (auto* ptr, const auto* args) {
-            return WTF::atomicCompareExchangeStrong(ptr, args[0], args[1]);
+        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));
         });
 }
 
@@ -216,8 +218,9 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncExchange(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const auto* args) {
-            return WTF::atomicExchange(ptr, args[0]);
+        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])));
         });
 }
 
@@ -246,8 +249,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncLoad(ExecState* exec)
 {
     return atomicOperationWithArgs<0>(
-        exec, [&] (auto* ptr, const auto*) {
-            return WTF::atomicLoad(ptr);
+        exec, [&] (auto* ptr, const int32_t*) {
+            return jsNumber(WTF::atomicLoad(ptr));
         });
 }
 
@@ -254,8 +257,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncOr(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const auto* args) {
-            return WTF::atomicExchangeOr(ptr, args[0]);
+        exec, [&] (auto* ptr, const int32_t* args) {
+            return jsNumber(WTF::atomicExchangeOr(ptr, args[0]));
         });
 }
 
@@ -262,10 +265,15 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncStore(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const auto* args) {
-            auto value = args[0];
-            WTF::atomicStore(ptr, value);
-            return value;
+        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);
         });
 }
 
@@ -272,8 +280,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncSub(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const auto* args) {
-            return WTF::atomicExchangeSub(ptr, args[0]);
+        exec, [&] (auto* ptr, const int32_t* args) {
+            return jsNumber(WTF::atomicExchangeSub(ptr, args[0]));
         });
 }
 
@@ -382,8 +390,8 @@
 EncodedJSValue JSC_HOST_CALL atomicsFuncXor(ExecState* exec)
 {
     return atomicOperationWithArgs<1>(
-        exec, [&] (auto* ptr, const auto* args) {
-            return WTF::atomicExchangeXor(ptr, args[0]);
+        exec, [&] (auto* ptr, const int32_t* args) {
+            return jsNumber(WTF::atomicExchangeXor(ptr, args[0]));
         });
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to