Closed Bug 1256945 Opened 5 years ago Closed 5 years ago
SIMD: Coerce lane indexes with To
58 bytes, text/x-review-board-request
The SIMDToLane() function in the SIMD.js specification requires that lane index arguments are coerced with ToNumber(). See <http://tc39.github.io/ecmascript_simd/#simd-to-lane>. Currently, SpiderMonkey requires that lane indexes are integers. It does not apply the coercion.
The SIMDToLane() function in the SIMD.js specification uses ToNumber() to coerce lane index arguments to a number before checking the the index is an integer in range. Add an ArgumentToLaneIndex() function to SIMD.cpp that implements the same semantics. This function throws a RangeError if the coerced argument is not integral or out of range. Update tests to match the new semantics. The differences are: - More values are accepted as lane indexes (null, true, false, "5.0", ...). - A TypeError is only thrown if ToNumber fails. If the argument can be coerced to a number, a RangeError is thrown if other checks fail. Fix the testShuffleForType() test in ests/ecma_7/SIMD/swizzle-shuffle.js which should have been calling `shuffle` but was calling `swizzle`. Review commit: https://reviewboard.mozilla.org/r/40207/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40207/
Attachment #8730869 - Flags: review?(bbouvier)
Attachment #8730869 - Flags: review?(bbouvier) → review+
https://reviewboard.mozilla.org/r/40207/#review36891 Thanks! I guess, technically, JS doesn't have an integer type. Everything is a double. The reasoning in the spec is something like: 1. If `ToNumber(arg)` can't even convert your argument to a number, throw a `TypeError`. This happens with symbols and SIMD values and a few other things. 2. If you can coerce to a number, but it's not a number you like, throw a `RangeError`. This includes fractional numbers, out-of-range integers, and things like NaN. The discussion in [the Github issue](https://github.com/tc39/ecmascript_simd/issues/328) is interesting. Indexes in `typedArray[index]` are treated differently. > Is it that we need to update ion to make sure that it throws the same error as well? As I understand it, Ion doesn't throw exceptions. It just bails out and lets the VM code throw the exception. (Maybe I am wrong?). In this case, we used to throw a TypeError for any kind of lane index we didn't like, but now we sometimes throw a RangeError instead. The `uglyDuckling` values tested here cover all three kinds of bailout: 1. Bail to throw a `TypeError`. 2. Bail to throw a `RangeError`. 3. Bail to coerce a weird argument like `" +3.0e0 "`. Ion doesn't handle this argument, but it is still valid. I could check that the exception is either a `RangeError` or a `TypeError`.
Comment on attachment 8730869 [details] MozReview Request: Bug 1256945 - Coerce SIMD lane indexes with ToNumber(). r=bbouvier Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40207/diff/1-2/
Attachment #8730869 - Attachment description: MozReview Request: Bug 1256945 - Coerce SIMD lane indexes with ToNumber(). r?benj → MozReview Request: Bug 1256945 - Coerce SIMD lane indexes with ToNumber(). r=bbouvier
You need to log in before you can comment on or make changes to this bug.