1108870 - SIMD: Support ?: on SIMD values

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Dan Gohman [:sunfish]]

One of the errors reported in https://github.com/kripken/emscripten/issues/3043 is that Emscripten is generating ?: expressions with SIMD values, which is not currently supported by Odin.

Here's a simple example:

function module(global) {
    "use asm";
    var fx4 = global.SIMD.float32x4;
    function e(a, y, z) {
        a = a|0;
        y = fx4(y);
        z = fx4(z);
        return a ? y : z;
    }
}

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Support ternary conditionals with SIMD operands in asm.js

Instead of copying the same condition scheme as double/float/int, we can use a
simpler form here, as SIMD types don't have the -ish equivalent (and i am not
looking forward to them :)). As a matter of fact, we won't have to modify this
piece of code when we'll add new SIMD types.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 8533727 [details] [diff] [review]
Support ternary conditionals with SIMD operands in asm.js

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

::: js/src/asmjs/AsmJSValidate.cpp
@@ +6212,5 @@
>      } else if (thenType.isDouble() && elseType.isDouble()) {
>          *type = Type::Double;
>      } else if (thenType.isFloat() && elseType.isFloat()) {
>          *type = Type::Float;
> +    } else if (elseType.isSimd() && thenType <= elseType) {

While I think correct, the asymmetry of 'thenType <= elseType' gives me pause.  Technically, since there are no SIMD -ish types, you could just use equality but this also goes against our style of never depending on exact type.  How about "&& thenType <= elseType && elseType <= thenType" (which is generally how one defines equivalence anyway)?

::: js/src/jit-test/tests/asm.js/testSIMD.js
@@ +238,5 @@
> +assertAsmTypeFail('glob', USE_ASM + F32 + I32 + "function f() {var x=f4(1,2,3,4); var y=i4(1,2,3,4); x=1?x:y;} return f");
> +assertAsmTypeFail('glob', USE_ASM + F32 + I32 + "function f() {var x=f4(1,2,3,4); var y=i4(1,2,3,4); x=1?y:y;} return f");
> +
> +CheckF4('', 'var x=f4(1,2,3,4); var y=f4(4,3,2,1); x=3?y:x', [4, 3, 2, 1]);
> +CheckI4('', 'var x=i4(1,2,3,4); var y=i4(4,3,2,1); x=(x.x|0)?y:x', [4, 3, 2, 1]);

Might be nice to have a testcase that has non-const then/else values.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Thanks! I've added the double <= operation to simulate equivalence. However, I am wondering how we could generalize this for the conditions right above (thenType.isFloat() && elseType.isFloat()): maybe (thenType <= Type::Float && elseType <= Type::Float) ? This is just a matter of consistency, so probably not a big deal.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/66151fa917a7

Comment 4

[Luke Wagner [:luke]]

I think the asymmetry results in the fact that we are using one test to handle all the SIMD types.  An alternative would have been to split your one else-if into two (and use .isInt32x4()/.isFloat32x4()), but that will be unpleasant as we add other SIMD types.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/66151fa917a7