Closed nmostafa closed 8 years ago
My understanding is that "only accepting number literals as indices" is an asm.js optimization requirement - there's no reason the functions can't or shouldn't accept numbers via a variable. It's very typical for builtin methods that take numbers to perform ToNumber
on the item, and I think the SIMD methods should do that too - even though it wouldn't be necessary in the asm.js subset.
The builtin should accept a Number variable outside Asm.js. That part is fine. The concern is allowing non-Numbers to be passed as lane indices. Simd.js builtins is all about performance, so whenever possible, we should make the API strict to avoid hidden perf cliffs (especially if the flexibility is not justified by existing use-cases).
In this case JS allows it, and we should throw TypeError. Otherwise, the type-specializer will give up on these cases, and either bailout or call a helper, either of which is very slow.
And by the way, we allowed lane indices to be non-literals, just because there is no way in JS to enforce that outside Asm.js. SIMD.js documentation should encourage use of literals, otherwise perf is not guaranteed.
I agree with @nmostafa: simd.js is solely a performance feature, the API should IMO reflect this as much as possible and avoid "it just works" behavior whenever possible.
We changed the throwing to a cast at the direct request of TC39. I don't think this changes performance at all; it just makes it more consistent with the rest of the language. ToNumber is all over JavaScript, and accordingly, VMs already optimize it well. I think we discussed this change in a phone call (but of course it's fine to revisit). I don't think this will affect either asm.js or non-asm.js performance.
@nmostafa Why do you think this would make a perf cliff?
ToNumber would be optimized well, if the argument is a Number value. If given a String, Array or Bool, for example, it will require special handling via calling a helper. VMs are very unlikely to handle these cases via a fast-path, since they are very rare to happen.
Now, if a developer has a bug that inadvertently assigns a non-Number to a lane index, he will face mysterious perf degradation. It would be nicer if the code throws a type-error exposing the bug.
All SIMD ops have statically typed operands, I don't see why lane indices should be any different.
Nothing is statically checked for; everything is dynamic. Only SIMD types are checked for with a TypeError. Everywhere in the spec, when a Number is required, we call ToNumber. This is just the inheritance from being embedded in the rest of JS. Sure, it will be slower if you pass a non-Number, but this isn't really a cliff so much as a "pay-as-you-go" language feature as far as I can tell. The only cliff is that you lose asm.js validation, but this is a cross-cutting design decision for asm.js.
We are at Stage 3 in TC39. That means the committee has signed off on our aesthetic choices, and any further changes should be driven by feedback from implementation concerns, not design-level feedback. It took a lot of discussion and compromises to get here; some committee members wanted much more drastic changes which would've really hurt performance in mainline cases. I don't think this hurts anyone's performance, and it doesn't cause a cliff but rather an incremental cost which is exactly parallel to every single other place in JS where Numbers are accepted as arguments. Developers can also use systems like TypeScript to avoid this hazard, just like they would elsewhere in the language.
I don't think reopening this issue would have practical benefit. It is likely to make things more difficult in TC39 (and we need their continued buy-in to get follow-on features like Float64x2 and saturatingAbsoluteDifference), and it is unlikely to make a big difference for developers. Getting rid of some of JavaScript's implicit coercions could definitely have some benefits, but @rossberg-chromium has been arguing this for a while, and it seems the fight has been lost.
Unfortunately, JS is full of performance cliffs. This issue has already been discussed at length and at the committee's request we have the existing design. The expectation is that performance analysis tools will help developers identify this specific performance cliff if they hit it.
In #120, we agreed to accept only Numbers as lane indices.
Looking at the spec text, I don't see this being enforced.
This is reflected in couple of places in the spec.
1) SIMDToLane. It coerces any type to number. And only throws if out of range.
2) SIMDConstructor.shuffle( a, b, ...lanes ) and swizzle. Assumes 0 for missing lane indices. Missing lane indices are of Undefined value, and we should throw here.