tc39 / proposal-ecmascript-sharedmem

Shared memory and atomics for ECMAscript
Mozilla Public License 2.0
375 stars 32 forks source link

Several bugs in llvm-like branch #150

Closed lars-t-hansen closed 8 years ago

lars-t-hansen commented 8 years ago

@syg, I think what's on gh-pages seems to be built from changes you have not pushed, so I'm filing these bugs here until current code is in the repo:

syg commented 8 years ago

@lars-t-hansen Regarding the last bullet, the intention was that conversion was to be done as part of the compareExchange semantic function (it's really a closure, unlike all the other semantic functions that are passed to AtomicReadModifyWrite). I'll reinstate the manual conversion and make the compareExchange close over the converted value.

syg commented 8 years ago

@lars-t-hansen Please close the issue if you think of the fix for your last bullet in the linked commit is satisfactory.

lars-t-hansen commented 8 years ago

@syg, the fix for compareExchange is not satisfactory because now compareExchange has different observable argument processing order than the other functions. Consider the original spec of this function:

1. Let _buffer_ be ? ValidateSharedIntegerTypedArray(_typedArray_).
1. Let _i_ be ? ValidateAtomicAccess(_typedArray_, _index_).
1. Let _expected_ be ? ToInteger(_expectedValue_).
1. Let _replacement_ be ? ToInteger(_replacedValue_).
1. Let _arrayTypeName_ be _typedArray_.[[TypedArrayName]].
1. Let _convOp_ be the conversion operation specified in Table 49 for _arrayTypeName_.
1. Let _x_ be _convOp_ (_expected_).
1. Let _elementSize_ be the Number value of the Element Size value specified in Table 49 for _arrayTypeName_.
1. Let _elementType_ be the String value of the Element Type value in Table 49 for _arrayTypeName_.
1. Let _offset_ be _typedArray_.[[ByteOffset]].
1. Let _indexedPosition_ be (_i_ × _elementSize_) + _offset_.
1. With atomic access to shared memory, do:
    1. Let _r_ be GetValueFromBuffer(_buffer_, _indexedPosition_, _elementType_).
    1. If _r_ is the same as _x_ then:
        1. Perform SetValueInBuffer(_buffer_, _indexedPosition_, _elementType_, _replacement_).
    1. Return _r_.

Here, if the index is not convertable to number or is out of range that will be signaled in step 2 before we convert any of the other arguments. The old spec for this algorithm was written the way it was written to be compatible in its observable side effect order with the other algorithms in the proposal, all of which were designed to be compatible in behavior with existing routines in ES, sadly at a cost in spec elegance for compareExchange.

syg commented 8 years ago

@lars-t-hansen Okay, I basically resurrected the old version.

lars-t-hansen commented 8 years ago

Looks good.