SIMD-JS operations are not available on SharedArrayViews.

RESOLVED FIXED in Firefox 38

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Jukka Jylänki, Assigned: bbouvier)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla38
x86_64
Windows 8.1
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed)

Details

(Whiteboard: [DocArea=JS])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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?

Updated

3 years ago
Blocks: 1054841
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.
Assignee: nobody → benj
Status: NEW → ASSIGNED
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;
}
Created attachment 8565905 [details] [diff] [review]
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).
Attachment #8565905 - Flags: review?(lhansen)

Comment 4

3 years ago
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.
Attachment #8565905 - Flags: review?(lhansen) → review+
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
(Reporter)

Comment 7

3 years ago
Thanks for the patch Benjamin, tested it out and it works flawlessly!
https://hg.mozilla.org/mozilla-central/rev/161af2aba1e9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
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
Keywords: dev-doc-needed → dev-doc-complete
(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.
(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!
You need to log in before you can comment on or make changes to this bug.