SIMD cmov code runs in node but fails in SpiderMonkey.

[juj]

Here is an interesting small test case that works as expected in node.js and native, but fails in SpiderMonkey/Firefox:

#include <xmmintrin.h>
#include <inttypes.h>
#include <stdio.h>
#include <math.h>
#include <assert.h>

union FloatIntReinterpret { float f; uint32_t i; };
float ReinterpretAsFloat(uint32_t i) { FloatIntReinterpret fi; fi.i = i; return fi.f; }
const __m128 sseMaskXYZ = _mm_set_ps(0.f, ReinterpretAsFloat(0xFFFFFFFFU), ReinterpretAsFloat(0xFFFFFFFFU), ReinterpretAsFloat(0xFFFFFFFFU));
#define shuffle1_ps(reg, shuffle) _mm_shuffle_ps((__m128)(reg), (__m128)(reg), (shuffle))
__m128 sum_xyz_ps3(__m128 m) { return _mm_add_ps(m, _mm_add_ps(shuffle1_ps(m, _MM_SHUFFLE(3,0,2,1)), shuffle1_ps(m, _MM_SHUFFLE(3,1,0,2)))); }
__m128 dot3_ps3(__m128 a, __m128 b) { return sum_xyz_ps3(_mm_mul_ps(a, b)); }
__m128 vec3_length_ps3(__m128 vec) { return _mm_sqrt_ps(dot3_ps3(vec, vec)); }
__m128 cmov_ps(__m128 a, __m128 b, __m128 mask) { return _mm_or_ps(_mm_andnot_ps(mask, a), _mm_and_ps(mask, b)); }

__m128 vec4_safe_normalize3(__m128 vec, __m128 &outLength)
{
    outLength = vec3_length_ps3(vec);
    return cmov_ps(vec, cmov_ps(_mm_div_ps(vec, outLength), _mm_set_ps(0,0,0,1), _mm_cmplt_ps(outLength, _mm_set1_ps(1e-4f))), sseMaskXYZ);
}

void tostr(__m128 m, char *outstr)
{
    union { __m128 m; float val[4]; } u; u.m = m;
    sprintf(outstr, "x: %f, y: %f, z: %f, w: %f", u.val[0], u.val[1], u.val[2], u.val[3]);
}

int main()
{
    __m128 f = _mm_set_ps(1000.f, 3.f, 2.f, -1.f);
    __m128 outLength;
    __m128 normalized = vec4_safe_normalize3(f, outLength);
    char str[256];
    tostr(f, str); printf("f: %s\n", str);
    tostr(outLength, str); printf("outLength: %s\n", str);
    float length = _mm_cvtss_f32(vec3_length_ps3(normalized));
    tostr(normalized, str); printf("normalized: %s, length: %f\n", str, length);
    assert(fabs(length - 1.f) < 1e-5f);
}

In native/node.js, it prints:

f: x: -1.000000, y: 2.000000, z: 3.000000, w: 1000.000000
outLength: x: 3.741657, y: 3.741657, z: 3.741657, w: 1732.050781
normalized: x: -0.267261, y: 0.534522, z: 0.801784, w: 1000.000000, length: 1.000000

whereas in SpiderMonkey/Firefox, it prints:

f: x: -1.000000, y: 2.000000, z: 3.000000, w: 1000.000000
outLength: x: 3.741657, y: 3.741657, z: 3.741657, w: 1732.050781
normalized: x: -0.250000, y: 0.500000, z: 0.750000, w: 1000.000000, length: 0.935414 
uncaught exception: Assertion failed: fabs(length - 1.f) < 1e-5f, at: a.cpp,38,main at jsStackTrace@file:///C:/code/MathGeoLib/emcc_sse/a.js:5621:13
stackTrace@file:///C:/code/MathGeoLib/emcc_sse/a.js:5638:22
___assert_fail@file:///C:/code/MathGeoLib/emcc_sse/a.js:6256:210
_main@file:///C:/code/MathGeoLib/emcc_sse/a.js:11545:3
asm._main@file:///C:/code/MathGeoLib/emcc_sse/a.js:19058:8
callMain@file:///C:/code/MathGeoLib/emcc_sse/a.js:20818:15
doRun@file:///C:/code/MathGeoLib/emcc_sse/a.js:20876:42
run/<@file:///C:/code/MathGeoLib/emcc_sse/a.js:20887:7

This might be related to NaN canonicalization, and I recall @sunfishcode saying that the SIMD.js polyfill (what node.js is running) does canonicalize, but browser doesn't, but here the effect is the opposite(?).

[kripken]

I get the same uncaught exception in node as in spidermonkey, same output in both.

[juj]

I'm also getting the same error in node now, not sure if I tested it wrong in node way back. In any case, the PR to emscripten-fastcomp fixes this in both.

[sunfishcode]

After talking with @kripken and looking at the testcases, it looks like NaN bit patterns in scalar values are a lost cause in JS and therefore also asm.js, however NaN bit patterns in SIMD values are doable. I wrote up this patch:

sunfishcode/emscripten-fastcomp@7644927

which implements a fix just for vector constants, in a very simple way. I plan to clean this code up a bit, and it's not really tested yet, but I'm posting it here to see what people think. Is this a feasible way forward?

[juj]

Filed a related issue #3787 that also hits the test case in this bug. The problem depicted in this issue now works correctly (except for having to work around #3787), so closing as fixed.