Bug 1113445 - SpiderMonkey: Handle -0 properly in SIMD.float32x4.signMask r=bbouvier,waldo
authorDan Gohman <sunfish@mozilla.com>
Tue, 23 Dec 2014 13:52:49 -0800
changeset 221117 25507fee09eba0c2ecc38551ca1877d027b36fbe
parent 221116 145cfaf3c17726c7abed34ef013816932461239f
child 221118 d8d78db85c5dd9b59edf9fd8a889b200d411482e
push id53286
push userdgohman@mozilla.com
push dateTue, 23 Dec 2014 21:53:21 +0000
treeherdermozilla-inbound@d8d78db85c5d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbouvier, waldo
bugs1113445
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1113445 - SpiderMonkey: Handle -0 properly in SIMD.float32x4.signMask r=bbouvier,waldo
js/src/builtin/SIMD.cpp
js/src/jit-test/tests/asm.js/testSIMD.js
js/src/tests/ecma_7/SIMD/signmask.js
mfbt/IntegerTypeTraits.h
--- a/js/src/builtin/SIMD.cpp
+++ b/js/src/builtin/SIMD.cpp
@@ -8,16 +8,17 @@
  * JS SIMD pseudo-module.
  * Specification matches polyfill:
  * https://github.com/johnmccutchan/ecmascript_simd/blob/master/src/ecmascript_simd.js
  * The objects float32x4 and int32x4 are installed on the SIMD pseudo-module.
  */
 
 #include "builtin/SIMD.h"
 
+#include "mozilla/IntegerTypeTraits.h"
 #include "jsapi.h"
 #include "jsfriendapi.h"
 
 #include "builtin/TypedObject.h"
 #include "js/Value.h"
 
 #include "jsobjinlines.h"
 
@@ -142,22 +143,27 @@ static bool SignMask(JSContext *cx, unsi
     TypeDescr &descr = typedObj.typeDescr();
     if (descr.kind() != type::Simd || descr.as<SimdTypeDescr>().type() != SimdType::type) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO,
                              SimdTypeDescr::class_.name, "signMask",
                              InformalValueTypeName(args.thisv()));
         return false;
     }
 
-    Elem *data = reinterpret_cast<Elem *>(typedObj.typedMem());
-    int32_t mx = data[0] < 0.0 ? 1 : 0;
-    int32_t my = data[1] < 0.0 ? 1 : 0;
-    int32_t mz = data[2] < 0.0 ? 1 : 0;
-    int32_t mw = data[3] < 0.0 ? 1 : 0;
-    int32_t result = mx | my << 1 | mz << 2 | mw << 3;
+    // Load the data as integer so that we treat the sign bit consistently,
+    // since -0.0 is not less than zero, but it still has the sign bit set.
+    typedef typename mozilla::SignedStdintTypeForSize<sizeof(Elem)>::Type Int;
+    static_assert(SimdType::lanes * sizeof(Int) <= jit::Simd128DataSize,
+                  "signMask access should respect the bounds of the type");
+    const Elem *elems = reinterpret_cast<const Elem *>(typedObj.typedMem());
+    int32_t result = 0;
+    for (unsigned i = 0; i < SimdType::lanes; ++i) {
+        Int x = mozilla::BitwiseCast<Int>(elems[i]);
+        result |= (x < 0) << i;
+    }
     args.rval().setInt32(result);
     return true;
 }
 
 #define SIGN_MASK(type) \
 static bool type##SignMask(JSContext *cx, unsigned argc, Value *vp) { \
     return SignMask<type>(cx, argc, vp); \
 }
--- a/js/src/jit-test/tests/asm.js/testSIMD.js
+++ b/js/src/jit-test/tests/asm.js/testSIMD.js
@@ -183,17 +183,17 @@ assertAsmTypeFail('glob', USE_ASM + "fun
 assertAsmTypeFail('glob', USE_ASM + "function f() {var x=42.; return x.signMask;} return f");
 assertAsmTypeFail('glob', USE_ASM + FROUND + "function f() {var x=f32(42.); return x.signMask;} return f");
 
 assertEq(asmLink(asmCompile('glob', USE_ASM + I32 + 'function f() { var x=i4(1,2,3,4); return x.signMask | 0 } return f'), this)(), 0b0000);
 assertEq(asmLink(asmCompile('glob', USE_ASM + I32 + 'function f() { var x=i4(0,-1, ' + INT32_MAX + ',' + INT32_MIN  + '); return x.signMask | 0 } return f'), this)(), 0b1010);
 
 assertEq(asmLink(asmCompile('glob', USE_ASM + F32 + FROUND + 'var Infinity = glob.Infinity; function f() { var x=f4(0,0,0,0); x=f4(f32(1), f32(-13.37), f32(42), f32(-Infinity)); return x.signMask | 0 } return f'), this)(), 0b1010);
 assertEq(asmLink(asmCompile('glob', USE_ASM + F32 + FROUND + 'var Infinity = glob.Infinity; function f() { var x=f4(0,0,0,0); x=f4(f32(-1), f32(0), f32(-0.000001), f32(Infinity)); return x.signMask | 0 } return f'), this)(), 0b0101);
-assertEq(asmLink(asmCompile('glob', USE_ASM + F32 + FROUND + 'var NaN = glob.NaN; function f() { var x=f4(0,0,0,0); x=f4(f32(-1), f32(NaN), f32(3.), f32(4.)); return x.signMask | 0 } return f'), this)(), 0b0001);
+assertEq(asmLink(asmCompile('glob', USE_ASM + F32 + FROUND + 'var NaN = glob.NaN; function f() { var x=f4(0,0,0,0); x=f4(f32(-1), f32(NaN), f32(-0), f32(0)); return x.signMask | 0 } return f'), this)(), 0b0101);
 
 // 1.3.3. Variable assignments
 assertAsmTypeFail('glob', USE_ASM + I32 + I32A + "function f() {var x=i4(1,2,3,4); x=i4();} return f");
 assertAsmTypeFail('glob', USE_ASM + I32 + I32A + "function f() {var x=i4(1,2,3,4); x=i4(1);} return f");
 assertAsmTypeFail('glob', USE_ASM + I32 + I32A + "function f() {var x=i4(1,2,3,4); x=i4(1, 2);} return f");
 assertAsmTypeFail('glob', USE_ASM + I32 + I32A + "function f() {var x=i4(1,2,3,4); x=i4(1, 2, 3);} return f");
 assertAsmTypeFail('glob', USE_ASM + I32 + I32A + "function f() {var x=i4(1,2,3,4); x=i4(1.0, 2, 3, 4);} return f");
 assertAsmTypeFail('glob', USE_ASM + I32 + I32A + "function f() {var x=i4(1,2,3,4); x=i4(1, 2.0, 3, 4);} return f");
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_7/SIMD/signmask.js
@@ -0,0 +1,33 @@
+// |reftest| skip-if(!this.hasOwnProperty("SIMD"))
+
+var float32x4 = SIMD.float32x4;
+var int32x4 = SIMD.int32x4;
+
+function test_float32x4() {
+  var v, w;
+  for ([v, w] of [[float32x4(-1, 20, 30, 4), 0b0001],
+                  [float32x4(9.999, 2.1234, 30.4443, -4), 0b1000],
+                  [float32x4(0, -Infinity, +Infinity, -0), 0b1010]])
+  {
+      assertEq(v.signMask, w);
+  }
+
+  if (typeof reportCompare === "function")
+    reportCompare(true, true);
+}
+
+function test_int32x4() {
+  var v, w;
+  for ([v, w] of [[int32x4(-1, 20, 30, 4), 0b0001],
+                  [int32x4(10, 2, 30.2, -4), 0b1000],
+                  [int32x4(0, 0x80000000, 0x7fffffff, -0), 0b0010]])
+  {
+      assertEq(v.signMask, w);
+  }
+
+  if (typeof reportCompare === "function")
+    reportCompare(true, true);
+}
+
+test_float32x4();
+test_int32x4();
--- a/mfbt/IntegerTypeTraits.h
+++ b/mfbt/IntegerTypeTraits.h
@@ -74,16 +74,21 @@ struct StdintTypeForSizeAndSignedness<8,
 
 } // namespace detail
 
 template<size_t Size>
 struct UnsignedStdintTypeForSize
   : detail::StdintTypeForSizeAndSignedness<Size, false>
 {};
 
+template<size_t Size>
+struct SignedStdintTypeForSize
+  : detail::StdintTypeForSizeAndSignedness<Size, true>
+{};
+
 template<typename IntegerType>
 struct PositionOfSignBit
 {
   static_assert(IsIntegral<IntegerType>::value,
                 "PositionOfSignBit is only for integral types");
   // 8 here should be CHAR_BIT from limits.h, but the world has moved on.
   static const size_t value = 8 * sizeof(IntegerType) - 1;
 };