⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 211129 in webkit


Ignore:
Timestamp:
Jan 24, 2017, 6:52:51 PM (10 years ago)
Author:
fpizlo@apple.com
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/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):

Location:
trunk
Files:
1 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r211128 r211129  
     12017-01-24  Filip Pizlo  <fpizlo@apple.com>
     2
     3        Atomics.store should return the int-converted value according to toInteger
     4        https://bugs.webkit.org/show_bug.cgi?id=167399
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/atomics-add-uint32.js: Added.
     9        * stress/atomics-store-return.js: Fix the test to match what the spec wants.
     10
    1112017-01-24  Yusuke Suzuki  <utatane.tea@gmail.com>
    212
  • trunk/JSTests/stress/atomics-store-return.js

    r211122 r211129  
    33var result = Atomics.store(a, 0, 1000);
    44if (result != 1000)
    5     throw "Error: bad result: " + result;
     5    throw new Error("bad result: " + result);
    66
    77sab = new SharedArrayBuffer(4);
    88a = new Uint32Array(sab);
    9 var result = Atomics.store(a, 0, 4000000000);
     9result = Atomics.store(a, 0, 4000000000);
    1010if (result != 4000000000)
    11     throw "Error: bad result: " + result;
     11    throw new Error("bad result: " + result);
     12if (a[0] != 4000000000)
     13    throw new Error("bad value read back: " + a[0]);
     14result = Atomics.store(a, 0, -4000000000);
     15if (result != -4000000000)
     16    throw new Error("bad result: " + result);
     17if (a[0] != 294967296)
     18    throw new Error("bad value read back: " + a[0]);
     19
     20var count = 0;
     21result = Atomics.store(a, 0, { valueOf() { count++; return 42; } });
     22if (result != 42)
     23    throw new Error("bad result: " + result);
     24if (count != 1)
     25    throw new Error("bad count: " + count);
     26
  • trunk/Source/JavaScriptCore/ChangeLog

    r211128 r211129  
     12017-01-24  Filip Pizlo  <fpizlo@apple.com>
     2
     3        Atomics.store should return the int-converted value according to toInteger
     4        https://bugs.webkit.org/show_bug.cgi?id=167399
     5
     6        Reviewed by Saam Barati.
     7       
     8        I keep getting this wrong, but I think I've finally done it right. What we want is for
     9        Atomics.store to return the value it was passed after toInteger, which doesn't clip the value to
     10        any kind of range. It does get truncated to double.
     11       
     12        This changes the code to pass those "integers" as doubles. It doesn't matter that this is slow,
     13        since all of these code paths are slow due to their need to check everything. We'll take care of
     14        that by making them intrinsic later.
     15
     16        * runtime/AtomicsObject.cpp:
     17        (JSC::atomicsFuncAdd):
     18        (JSC::atomicsFuncAnd):
     19        (JSC::atomicsFuncCompareExchange):
     20        (JSC::atomicsFuncExchange):
     21        (JSC::atomicsFuncLoad):
     22        (JSC::atomicsFuncOr):
     23        (JSC::atomicsFuncStore):
     24        (JSC::atomicsFuncSub):
     25        (JSC::atomicsFuncXor):
     26
    1272017-01-24  Yusuke Suzuki  <utatane.tea@gmail.com>
    228
  • trunk/Source/JavaScriptCore/runtime/AtomicsObject.cpp

    r211122 r211129  
    9393    JSGenericTypedArrayView<Adaptor>* typedArray = jsCast<JSGenericTypedArrayView<Adaptor>*>(typedArrayView);
    9494   
    95     int32_t extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
     95    double extraArgs[numExtraArgs + 1]; // Add 1 to avoid 0 size array error in VS.
    9696    for (unsigned i = 0; i < numExtraArgs; ++i) {
    97         int32_t value = exec->argument(2 + i).toInt32(exec);
     97        double value = exec->argument(2 + i).toInteger(exec);
    9898        RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
    9999        extraArgs[i] = value;
     
    192192{
    193193    return atomicOperationWithArgs<1>(
    194         exec, [&] (auto* ptr, const int32_t* args) {
    195             return jsNumber(WTF::atomicExchangeAdd(ptr, args[0]));
     194        exec, [&] (auto* ptr, const double* args) {
     195            return jsNumber(WTF::atomicExchangeAdd(ptr, toInt32(args[0])));
    196196        });
    197197}
     
    200200{
    201201    return atomicOperationWithArgs<1>(
    202         exec, [&] (auto* ptr, const int32_t* args) {
    203             return jsNumber(WTF::atomicExchangeAnd(ptr, args[0]));
     202        exec, [&] (auto* ptr, const double* args) {
     203            return jsNumber(WTF::atomicExchangeAnd(ptr, toInt32(args[0])));
    204204        });
    205205}
     
    208208{
    209209    return atomicOperationWithArgs<2>(
    210         exec, [&] (auto* ptr, const int32_t* args) {
     210        exec, [&] (auto* ptr, const double* args) {
    211211            typedef typename std::remove_pointer<decltype(ptr)>::type T;
    212212            T expected = static_cast<T>(args[0]);
     
    219219{
    220220    return atomicOperationWithArgs<1>(
    221         exec, [&] (auto* ptr, const int32_t* args) {
     221        exec, [&] (auto* ptr, const double* args) {
    222222            typedef typename std::remove_pointer<decltype(ptr)>::type T;
    223223            return jsNumber(WTF::atomicExchange(ptr, static_cast<T>(args[0])));
     
    250250{
    251251    return atomicOperationWithArgs<0>(
    252         exec, [&] (auto* ptr, const int32_t*) {
     252        exec, [&] (auto* ptr, const double*) {
    253253            return jsNumber(WTF::atomicLoad(ptr));
    254254        });
     
    258258{
    259259    return atomicOperationWithArgs<1>(
    260         exec, [&] (auto* ptr, const int32_t* args) {
    261             return jsNumber(WTF::atomicExchangeOr(ptr, args[0]));
     260        exec, [&] (auto* ptr, const double* args) {
     261            return jsNumber(WTF::atomicExchangeOr(ptr, toInt32(args[0])));
    262262        });
    263263}
     
    266266{
    267267    return atomicOperationWithArgs<1>(
    268         exec, [&] (auto* ptr, const int32_t* args) {
     268        exec, [&] (auto* ptr, const double* args) {
    269269            typedef typename std::remove_pointer<decltype(ptr)>::type T;
    270             int32_t valueAsInt = args[0];
     270            double valueAsInt = args[0];
    271271            T valueAsT = static_cast<T>(valueAsInt);
    272272            WTF::atomicStore(ptr, valueAsT);
    273            
    274             if (static_cast<int32_t>(valueAsT) == valueAsInt)
    275                 return jsNumber(valueAsT);
    276273            return jsNumber(valueAsInt);
    277274        });
     
    281278{
    282279    return atomicOperationWithArgs<1>(
    283         exec, [&] (auto* ptr, const int32_t* args) {
    284             return jsNumber(WTF::atomicExchangeSub(ptr, args[0]));
     280        exec, [&] (auto* ptr, const double* args) {
     281            return jsNumber(WTF::atomicExchangeSub(ptr, toInt32(args[0])));
    285282        });
    286283}
     
    391388{
    392389    return atomicOperationWithArgs<1>(
    393         exec, [&] (auto* ptr, const int32_t* args) {
    394             return jsNumber(WTF::atomicExchangeXor(ptr, args[0]));
     390        exec, [&] (auto* ptr, const double* args) {
     391            return jsNumber(WTF::atomicExchangeXor(ptr, toInt32(args[0])));
    395392        });
    396393}
Note: See TracChangeset for help on using the changeset viewer.