Closed Bug 1113367 Opened 7 years ago Closed 7 years ago

SIMD.int32x4(0, 0) gets TypeError

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: sunfish, Assigned: bbouvier)

Details

Attachments

(2 files, 1 obsolete file)

Invoking the SIMD.int32x4 constructor with fewer than 4 arguments:

  SIMD.int32x4(0, 0)

gets this error:

  typein:1:0 TypeError: SIMD requires more than 3 arguments

This behavior differs from the polyfill, which takes any number of arguments and coerces 4 of them to integers with |0.

I don't currently have a strong opinion on which is right, although we do already have code in the ecmascript_simd testsuite which happens to do this, which is how I noticed this.
Attached patch Proposed patchSplinter Review
Attachment #8539679 - Flags: review?(benj)
Comment on attachment 8539679 [details] [diff] [review]
Proposed patch

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

Thanks for jumping in. However, I'm sorry the patch is merely a hack, plus it isn't generic (float32x4 have the same behavior). It is so trivial I'll just do it, but thanks anyways :)
Attachment #8539679 - Flags: review?(benj)
Attachment #8540247 - Flags: review?(sunfish)
/r/1629 - Bug 1113367: SIMD (interpreter): make constructors arguments facultative;

Pull down this commit:

hg pull review -r daba7703abd9445dd9a2750383572845f7f48305
https://reviewboard.mozilla.org/r/1629/#review1039

::: js/src/tests/ecma_7/SIMD/constructors.js
(Diff revision 1)
> +    assertEqX4(int32x4(1, 2, 3, 4), [1,2,3,4]);

Please add a few cases of constructors with more than 4 arguments too.

FYI, I filed bug https://github.com/johnmccutchan/ecmascript_simd/issues/102 on the topic of the special case of one argument.
Comment on attachment 8540247 [details]
MozReview Request: bz://1113367/bbouvier

I assumed a "Ship It" meant r+, but I guess I shouldn't assume I know what to expect from MozReview yet. Here's the r+.
Attachment #8540247 - Flags: review?(sunfish) → review+
https://hg.mozilla.org/mozilla-central/rev/d3cb86ccd8a8
Assignee: nobody → benj
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Attachment #8540247 - Attachment is obsolete: true
Attachment #8618936 - Flags: review+
You need to log in before you can comment on or make changes to this bug.