Closed
Bug 1094855
Opened 10 years ago
Closed 10 years ago
SIMD.js: Add minNum/maxNum to the interpreter and update min/max
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(6 files, 1 obsolete file)
11.52 KB,
patch
|
till
:
review+
|
Details | Diff | Splinter Review |
12.76 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
4.16 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
4.16 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
6.86 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
4.97 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
See [0] for the reference impl. [0] https://github.com/johnmccutchan/ecmascript_simd/blob/master/src/ecmascript_simd.js
Assignee | ||
Comment 1•10 years ago
|
||
Should be straightforward. Contemplating the tests makes me think we could probably self-host the entire SIMD implementation...
Attachment #8518194 -
Flags: review?(till)
Comment 2•10 years ago
|
||
Comment on attachment 8518194 [details] [diff] [review] SIMD: Add minNum/maxNum and update min/max in the interpreter Review of attachment 8518194 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this works. Or, you could just self-host these SIMD functions *and* Math.min/max. I'd assume that you'd still want Ion fast-paths as these operations can probably be turned into better specific instructions. That's entirely possible to do with self-hosted versions, and would speed up baseline, IIUC.
Attachment #8518194 -
Flags: review?(till) → review+
Assignee | ||
Comment 3•10 years ago
|
||
This updates the min/max operations for float32x4, according to the assembly sequences posted on the github repo.
Attachment #8522238 -
Flags: review?(sunfish)
Assignee | ||
Comment 4•10 years ago
|
||
Not sure this even needs a review, as it just moves patches, but anyways. This moves tests that were supposed to stay close to multiply/divide.
Attachment #8522240 -
Flags: review?(sunfish)
Assignee | ||
Comment 5•10 years ago
|
||
This adds helpers for using cmpps and not having these weird const integers show up at various places in the code, plus it's more consistent with x86's doc.
Attachment #8522243 -
Flags: review?(sunfish)
Assignee | ||
Comment 6•10 years ago
|
||
(Previous patch was using ss suffix instead of ps suffix. Rather silly for packed singles.)
Attachment #8522298 -
Flags: review?(sunfish)
Assignee | ||
Updated•10 years ago
|
Attachment #8522243 -
Attachment is obsolete: true
Attachment #8522243 -
Flags: review?(sunfish)
Comment 7•10 years ago
|
||
Comment on attachment 8522238 [details] [diff] [review] Update SimdBinaryArith::Min/Max to properly handle comparisons involving -0/0 and NaNs Review of attachment 8522238 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!
Comment 8•10 years ago
|
||
Comment on attachment 8522238 [details] [diff] [review] Update SimdBinaryArith::Min/Max to properly handle comparisons involving -0/0 and NaNs Review of attachment 8522238 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!
Attachment #8522238 -
Flags: review?(sunfish) → review+
Updated•10 years ago
|
Attachment #8522240 -
Flags: review?(sunfish) → review+
Assignee | ||
Comment 9•10 years ago
|
||
Oh boy, even with pseudo code in front of the eyes, this gives headache to implement without blendvps and avx forms. Bleh.
Attachment #8522379 -
Flags: review?(sunfish)
Assignee | ||
Comment 10•10 years ago
|
||
Annnnnnnnd that will be done.
Attachment #8522381 -
Flags: review?(luke)
Comment 11•10 years ago
|
||
Comment on attachment 8522298 [details] [diff] [review] Add helpers for cmpps Review of attachment 8522298 [details] [diff] [review]: ----------------------------------------------------------------- Please define an enum in BaseAssembler-x86-shared.h for the actual constants, similar to the Condition enum. That will make it less important to make the cmpps interface private, but I'm ok either way. I do like it that the code now uses cmpltps and friends.
Attachment #8522298 -
Flags: review?(sunfish) → review+
Comment 12•10 years ago
|
||
Comment on attachment 8522379 [details] [diff] [review] Implement SIMD.float32x4.minNum/maxNum in the JITs Review of attachment 8522379 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.
Attachment #8522379 -
Flags: review?(sunfish) → review+
Updated•10 years ago
|
Attachment #8522381 -
Flags: review?(luke) → review+
Assignee | ||
Comment 13•10 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e21104ba9205 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/cecc072d44bc remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/8937c8785f74 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/dcd252c4cfad remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f7ba8aa473d3 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd68fda05a2b
Status: NEW → ASSIGNED
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/e21104ba9205 https://hg.mozilla.org/mozilla-central/rev/cecc072d44bc https://hg.mozilla.org/mozilla-central/rev/8937c8785f74 https://hg.mozilla.org/mozilla-central/rev/dcd252c4cfad https://hg.mozilla.org/mozilla-central/rev/f7ba8aa473d3 https://hg.mozilla.org/mozilla-central/rev/dd68fda05a2b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•