Patch to apply atop of those in the dependent bugs

Note that there's general agreement to modify the spec here to support a fully general two-input shuffle instead of the current shuffleMix. See [0] for my proposed realization of this idea. [0] https://github.com/johnmccutchan/ecmascript_simd/issues/74

Now looking at swizzle/general shuffle. A few questions: - would it make sense that Odin only accepts constant lane indexes? Or do we need variable lane indexes as well? - how should we behave if a constant or variable lane index is out of range? clamp it in [0, 4] for swizzle, [0, 8] for shuffle?

Rebased, but using shuffle/shuffleMix, so not flagging for review yet.

(note that in comment 2, ranges should be [0, 3] and [0, 7], obviously)

I think asm.js can require constant swizzle masks and constant shuffle lane indices. And then it can also require them to be in bounds :-). Variable values are something that non-asm.js will have to deal with. There's an open question in issue #74 about what to do about out-of-range values. I'd kind of prefer a wraparound to a clamp, but I don't have a strong opinion yet.

This adds support for swizzle and shuffle. As these functions take 4 literal integers, I've extracted the logic out of CheckSimdCallArgs to avoid emitting some unused MConstant for each lane selector, which would be thrown away by GVN later anyways. In tests, this patch tests all possible combination of selector lanes, so that all specific optimized cases get covered as well. Logging shows it takes 5 seconds locally, which is pretty big considering testSIMD ran in 2 seconds beforehands. Is this ok? Alternatives are to test a few generic cases and edge cases, but that seems more prone to error. Otherwise, I could split the cases in testSIMD-shuffle.js in asm.js, or even make a subdir with simd/ test cases (and start splitting test cases which are spec-only from the ones which are impl-dependent).

Comment on attachment 8505643 [details] [diff] [review] Odin SIMD: Add support for swizzle and shuffle Review of attachment 8505643 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit-test/tests/asm.js/testSIMD.js @@ +870,5 @@ > + (lanes[3] < 4 ? lhs : rhs)[lanes[3] % 4]]; > +} > + > +before = Date.now(); > +for (var i = 0; i < Math.pow(4, 4); i++) { this should be Math.pow(8, 8), which makes the test way longer :( I'll update tests so as to run only interesting / representative cases.

Comment on attachment 8505643 [details] [diff] [review] Odin SIMD: Add support for swizzle and shuffle Review of attachment 8505643 [details] [diff] [review]: ----------------------------------------------------------------- Very nice! ::: js/src/asmjs/AsmJSValidate.cpp @@ +5126,5 @@ > +{ > + for (unsigned i = 0; i < 4; i++, lane = NextNode(lane)) { > + uint32_t u32; > + if (!IsLiteralInt(f.m(), lane, &u32)) > + return f.failf(lane, "lane selector should be a constant int32 literal"); I'd say "integer literal" instead of "int32" (since IsLiteralInt accepts BigUnsigned). @@ +5168,5 @@ > + > + ParseNode *arg = CallArgList(call); > + MDefinition *vecs[2]; > + > + for (unsigned i = 0; i < 2; i++, arg = NextNode(arg)) { Can you put the \n before 'vecs', not after (or you can remove it altogether)?

Try was green per bug 1021716 comment 62: https://hg.mozilla.org/integration/mozilla-inbound/rev/8e3bacf8cebd