1133859 - SIMD-JS operations are not available on SharedArrayViews.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jukka Jylänki]

Attempting to execute SIMD.js operations in Emscripten in conjunction with Lars Hansen's SharedArrayBuffer work, it looks like all SIMD memory operations will fail with an error "TypeError: invalid arguments" when one of the operands is a Shared*Array instead of a regular typed array view.

For the SSE+pthreads demo, this is a bit critical and blocking. Is it easy to lift the restriction and allow Shared*Arrays to be used for SIMD operations?

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

After discussion with Lars, it appears it is just about accepting shared views as inputs of SIMD.load/store.  In that case, the changes should be pretty small.  Taking.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

For AsmJS, there's nothing to change: if you use the SharedUint8Array view, things work fine.  Note that is the only array view size for which it will work, as SIMD.load/store (in asm.js) expects an uint8arrayview (so as not to have to implicitly do the pointer shift).


function f(glob, ffi, heap) {
    "use asm";
    var su8 = new glob.SharedUint8Array(heap);
    var f4 = glob.SIMD.float32x4;
    var load = f4.load;
    function g() {
        return load(su8, 0);
    }
    return g;
}

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Allow shared array views as SIMD.load/store arguments

Nothing needs to be changed in asm.js as stated in the previous comment.  This
implements support in the interpreter and adds basic test cases (the code paths
are entirely shared with non-shared views, so adding tests wouldn't have a lot
of worth, unless i am missing something).

Comment 4

[Lars T Hansen [:lth]]

Comment on attachment 8565905 [details] [diff] [review]
Allow shared array views as SIMD.load/store arguments

Review of attachment 8565905 [details] [diff] [review]:
-----------------------------------------------------------------

Nit: In load.js I would initialize the storage to something predictable (I know, tricky the way the test is written) and test that the values read are the values expected, not zero - since zero is an easy default for a broken implementation to supply.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Thanks, I could adapt the framework contained in this test file to handle properly SAB, and could test at a few positions all load kinds.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=14b0ae5fbda5

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/161af2aba1e9

Comment 7

[Jukka Jylänki]

Thanks for the patch Benjamin, tested it out and it works flawlessly!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/161af2aba1e9

Comment 9

[Florian Scholz (Open Web Docs)]

Shared*Arrays are not yet documented, but for SIMD I added them as inputs to the load/store pages:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SIMD/load
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SIMD/store

Comment 10

[Lars T Hansen [:lth]]

(In reply to Florian Scholz [:fscholz] (MDN) from comment #9)
> Shared*Arrays are not yet documented, but for SIMD I added them as inputs to
> the load/store pages:
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> Global_Objects/SIMD/load
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> Global_Objects/SIMD/store

I don't have any strong opinions either way, but the reason those arrays are not documented is that there is not full agreement on the design yet (including not just names but whether they exist at all).  So you want to be a little careful about creating references to them.

Comment 11

[Florian Scholz (Open Web Docs)]

(In reply to Lars T Hansen [:lth] from comment #10)
> I don't have any strong opinions either way, but the reason those arrays are
> not documented is that there is not full agreement on the design yet
> (including not just names but whether they exist at all).  So you want to be
> a little careful about creating references to them.

Thanks, I am still on hold there :-) I am trying to follow discussions, implementation status in engines etc., but putting the "dev-doc-needed" keyword on bugs with API changes (and providing some notes like what you just wrote) is always a big help!