Closed
Bug 1133859
Opened 10 years ago
Closed 10 years ago
SIMD-JS operations are not available on SharedArrayViews.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: jujjyl, Assigned: bbouvier)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS])
Attachments
(1 file)
8.62 KB,
patch
|
lth
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
Blocks: shared-array-buffer
Assignee | ||
Comment 1•10 years ago
|
||
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
Assignee | ||
Comment 2•10 years ago
|
||
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;
}
Assignee | ||
Comment 3•10 years ago
|
||
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•10 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+
Assignee | ||
Comment 5•10 years ago
|
||
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
Assignee | ||
Comment 6•10 years ago
|
||
Reporter | ||
Comment 7•10 years ago
|
||
Thanks for the patch Benjamin, tested it out and it works flawlessly!
Comment 8•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•10 years ago
|
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Comment 9•10 years ago
|
||
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
Comment 10•10 years ago
|
||
(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•10 years ago
|
||
(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.
Description
•