Bug 1497955: Fix wasm saturating conversions codegen; r=sunfish
authorBenjamin Bouvier <benj@benj.me>
Wed, 10 Oct 2018 19:56:23 +0200
changeset 499280 ee4f12cc7136126f1d4adec65432a76123f11212
parent 499279 d9951c225adde0f0c0daac61bf61c8fb1c9d2780
child 499281 7a0840d602524d3b4552a867092267bb3fd5e79e
child 499307 402b42c38c70da770675e64c3f5bc005a98d3b84
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssunfish
bugs1497955
milestone64.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 1497955: Fix wasm saturating conversions codegen; r=sunfish - the saturating conversion to u64 callout was incorrect for an input precisely equal to -1, in which case it would return -1 instead of 0. - the fixup paths in the OOL for saturating conversions on x86 were incorrect when the input is -0, in which case it was improperly considered equal to 0 and thus resulted in the maximum value. - on x86, the OOL paths for saturated conversions are quite different from the ones for non-saturated ones; code generation needs to take this into account.
js/src/jit/x64/MacroAssembler-x64-inl.h
js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
js/src/jit/x86/MacroAssembler-x86.cpp
js/src/wasm/WasmBuiltins.cpp
--- a/js/src/jit/x64/MacroAssembler-x64-inl.h
+++ b/js/src/jit/x64/MacroAssembler-x64-inl.h
@@ -911,17 +911,18 @@ MacroAssembler::truncateFloat32ToUInt64(
                                         FloatRegister floatTemp)
 {
     Label done;
 
     loadFloat32(src, floatTemp);
 
     truncateFloat32ToInt64(src, dest, temp);
 
-    // For unsigned conversion the case of [INT64, UINT64] needs to get handle seperately.
+    // For unsigned conversion the case of [INT64, UINT64] needs to get handled
+    // separately.
     loadPtr(dest, temp);
     branchPtr(Assembler::Condition::NotSigned, temp, Imm32(0), &done);
 
     // Move the value inside INT64 range.
     storeFloat32(floatTemp, dest);
     loadConstantFloat32(double(int64_t(0x8000000000000000)), floatTemp);
     vaddss(Operand(dest), floatTemp, floatTemp);
     storeFloat32(floatTemp, dest);
--- a/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
+++ b/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp
@@ -858,35 +858,36 @@ MacroAssembler::oolWasmTruncateCheckF64T
                                              TruncFlags flags, wasm::BytecodeOffset off,
                                              Label* rejoin)
 {
     bool isUnsigned = flags & TRUNC_UNSIGNED;
     bool isSaturating = flags & TRUNC_SATURATING;
 
     if (isSaturating) {
         if (isUnsigned) {
-            // Negative overflow and NaN both are converted to 0, and the only other case
-            // is positive overflow which is converted to UINT32_MAX.
+            // Negative overflow and NaN both are converted to 0, and the only
+            // other case is positive overflow which is converted to
+            // UINT32_MAX.
             Label nonNegative;
             loadConstantDouble(0.0, ScratchDoubleReg);
             branchDouble(Assembler::DoubleGreaterThanOrEqual, input, ScratchDoubleReg, &nonNegative);
             move32(Imm32(0), output);
             jump(rejoin);
+
             bind(&nonNegative);
-
             move32(Imm32(UINT32_MAX), output);
         } else {
-            // Negative overflow is already saturated to INT32_MIN, so we only have
-            // to handle NaN and positive overflow here.
+            // Negative overflow is already saturated to INT32_MIN, so we only
+            // have to handle NaN and positive overflow here.
             Label notNaN;
             branchDouble(Assembler::DoubleOrdered, input, input, &notNaN);
             move32(Imm32(0), output);
             jump(rejoin);
+
             bind(&notNaN);
-
             loadConstantDouble(0.0, ScratchDoubleReg);
             branchDouble(Assembler::DoubleLessThan, input, ScratchDoubleReg, rejoin);
             sub32(Imm32(1), output);
         }
         jump(rejoin);
         return;
     }
 
@@ -917,35 +918,36 @@ MacroAssembler::oolWasmTruncateCheckF32T
                                              TruncFlags flags, wasm::BytecodeOffset off,
                                              Label* rejoin)
 {
     bool isUnsigned = flags & TRUNC_UNSIGNED;
     bool isSaturating = flags & TRUNC_SATURATING;
 
     if (isSaturating) {
         if (isUnsigned) {
-            // Negative overflow and NaN both are converted to 0, and the only other case
-            // is positive overflow which is converted to UINT32_MAX.
+            // Negative overflow and NaN both are converted to 0, and the only
+            // other case is positive overflow which is converted to
+            // UINT32_MAX.
             Label nonNegative;
             loadConstantFloat32(0.0f, ScratchDoubleReg);
             branchFloat(Assembler::DoubleGreaterThanOrEqual, input, ScratchDoubleReg, &nonNegative);
             move32(Imm32(0), output);
             jump(rejoin);
+
             bind(&nonNegative);
-
             move32(Imm32(UINT32_MAX), output);
         } else {
-            // Negative overflow is already saturated to INT32_MIN, so we only have
-            // to handle NaN and positive overflow here.
+            // Negative overflow is already saturated to INT32_MIN, so we only
+            // have to handle NaN and positive overflow here.
             Label notNaN;
             branchFloat(Assembler::DoubleOrdered, input, input, &notNaN);
             move32(Imm32(0), output);
             jump(rejoin);
+
             bind(&notNaN);
-
             loadConstantFloat32(0.0f, ScratchFloat32Reg);
             branchFloat(Assembler::DoubleLessThan, input, ScratchFloat32Reg, rejoin);
             sub32(Imm32(1), output);
         }
         jump(rejoin);
         return;
     }
 
@@ -974,35 +976,36 @@ MacroAssembler::oolWasmTruncateCheckF64T
                                              TruncFlags flags, wasm::BytecodeOffset off,
                                              Label* rejoin)
 {
     bool isUnsigned = flags & TRUNC_UNSIGNED;
     bool isSaturating = flags & TRUNC_SATURATING;
 
     if (isSaturating) {
         if (isUnsigned) {
-            // Negative overflow and NaN both are converted to 0, and the only other case
-            // is positive overflow which is converted to UINT64_MAX.
-            Label nonNegative;
+            // Negative overflow and NaN both are converted to 0, and the only
+            // other case is positive overflow which is converted to
+            // UINT64_MAX.
+            Label positive;
             loadConstantDouble(0.0, ScratchDoubleReg);
-            branchDouble(Assembler::DoubleGreaterThanOrEqual, input, ScratchDoubleReg, &nonNegative);
+            branchDouble(Assembler::DoubleGreaterThan, input, ScratchDoubleReg, &positive);
             move64(Imm64(0), output);
             jump(rejoin);
-            bind(&nonNegative);
 
+            bind(&positive);
             move64(Imm64(UINT64_MAX), output);
         } else {
-            // Negative overflow is already saturated to INT64_MIN, so we only have
-            // to handle NaN and positive overflow here.
+            // Negative overflow is already saturated to INT64_MIN, so we only
+            // have to handle NaN and positive overflow here.
             Label notNaN;
             branchDouble(Assembler::DoubleOrdered, input, input, &notNaN);
             move64(Imm64(0), output);
             jump(rejoin);
+
             bind(&notNaN);
-
             loadConstantDouble(0.0, ScratchDoubleReg);
             branchDouble(Assembler::DoubleLessThan, input, ScratchDoubleReg, rejoin);
             sub64(Imm64(1), output);
         }
         jump(rejoin);
         return;
     }
 
@@ -1034,35 +1037,36 @@ MacroAssembler::oolWasmTruncateCheckF32T
                                              TruncFlags flags, wasm::BytecodeOffset off,
                                              Label* rejoin)
 {
     bool isUnsigned = flags & TRUNC_UNSIGNED;
     bool isSaturating = flags & TRUNC_SATURATING;
 
     if (isSaturating) {
         if (isUnsigned) {
-            // Negative overflow and NaN both are converted to 0, and the only other case
-            // is positive overflow which is converted to UINT64_MAX.
-            Label nonNegative;
-            loadConstantFloat32(0.0f, ScratchDoubleReg);
-            branchFloat(Assembler::DoubleGreaterThanOrEqual, input, ScratchDoubleReg, &nonNegative);
+            // Negative overflow and NaN both are converted to 0, and the only
+            // other case is positive overflow which is converted to
+            // UINT64_MAX.
+            Label positive;
+            loadConstantFloat32(0.0f, ScratchFloat32Reg);
+            branchFloat(Assembler::DoubleGreaterThan, input, ScratchFloat32Reg, &positive);
             move64(Imm64(0), output);
             jump(rejoin);
-            bind(&nonNegative);
 
+            bind(&positive);
             move64(Imm64(UINT64_MAX), output);
         } else {
-            // Negative overflow is already saturated to INT64_MIN, so we only have
-            // to handle NaN and positive overflow here.
+            // Negative overflow is already saturated to INT64_MIN, so we only
+            // have to handle NaN and positive overflow here.
             Label notNaN;
             branchFloat(Assembler::DoubleOrdered, input, input, &notNaN);
             move64(Imm64(0), output);
             jump(rejoin);
+
             bind(&notNaN);
-
             loadConstantFloat32(0.0f, ScratchFloat32Reg);
             branchFloat(Assembler::DoubleLessThan, input, ScratchFloat32Reg, rejoin);
             sub64(Imm64(1), output);
         }
         jump(rejoin);
         return;
     }
 
--- a/js/src/jit/x86/MacroAssembler-x86.cpp
+++ b/js/src/jit/x86/MacroAssembler-x86.cpp
@@ -1037,71 +1037,90 @@ MacroAssembler::wasmTruncateFloat32ToInt
 void
 MacroAssembler::wasmTruncateDoubleToUInt64(FloatRegister input, Register64 output,
                                            bool isSaturating, Label* oolEntry,
                                            Label* oolRejoin, FloatRegister tempReg)
 {
     Label fail, convert;
     Register temp = output.high;
 
-    // Make sure input fits in (u)int64.
+    // Make sure input fits in uint64.
     reserveStack(2 * sizeof(int32_t));
     storeDouble(input, Operand(esp, 0));
     branchDoubleNotInUInt64Range(Address(esp, 0), temp, &fail);
+    size_t stackBeforeBranch = framePushed();
     jump(&convert);
 
-    // Handle failure in ool.
     bind(&fail);
     freeStack(2 * sizeof(int32_t));
     jump(oolEntry);
-    bind(oolRejoin);
-    reserveStack(2 * sizeof(int32_t));
-    storeDouble(input, Operand(esp, 0));
+    if (isSaturating) {
+        // The OOL path computes the right values.
+        setFramePushed(stackBeforeBranch);
+    } else {
+        // The OOL path just checks the input values.
+        bind(oolRejoin);
+        reserveStack(2 * sizeof(int32_t));
+        storeDouble(input, Operand(esp, 0));
+    }
 
-    // Convert the double/float to int64.
+    // Convert the double/float to uint64.
     bind(&convert);
     truncateDoubleToUInt64(Address(esp, 0), Address(esp, 0), temp, tempReg);
 
     // Load value into int64 register.
     load64(Address(esp, 0), output);
     freeStack(2 * sizeof(int32_t));
+
+    if (isSaturating) {
+        bind(oolRejoin);
+    }
 }
 
 void
 MacroAssembler::wasmTruncateFloat32ToUInt64(FloatRegister input, Register64 output,
                                             bool isSaturating, Label* oolEntry,
                                             Label* oolRejoin, FloatRegister tempReg)
 {
     Label fail, convert;
     Register temp = output.high;
 
-    // Make sure input fits in (u)int64.
+    // Make sure input fits in uint64.
     reserveStack(2 * sizeof(int32_t));
     storeFloat32(input, Operand(esp, 0));
     branchFloat32NotInUInt64Range(Address(esp, 0), temp, &fail);
+    size_t stackBeforeBranch = framePushed();
     jump(&convert);
 
-    // Handle failure in ool.
     bind(&fail);
     freeStack(2 * sizeof(int32_t));
     jump(oolEntry);
-    bind(oolRejoin);
-    reserveStack(2 * sizeof(int32_t));
-    storeFloat32(input, Operand(esp, 0));
+    if (isSaturating) {
+        // The OOL path computes the right values.
+        setFramePushed(stackBeforeBranch);
+    } else {
+        // The OOL path just checks the input values.
+        bind(oolRejoin);
+        reserveStack(2 * sizeof(int32_t));
+        storeFloat32(input, Operand(esp, 0));
+    }
 
-    // Convert the double/float to int64.
+    // Convert the float to uint64.
     bind(&convert);
     truncateFloat32ToUInt64(Address(esp, 0), Address(esp, 0), temp, tempReg);
 
     // Load value into int64 register.
     load64(Address(esp, 0), output);
     freeStack(2 * sizeof(int32_t));
+
+    if (isSaturating) {
+        bind(oolRejoin);
+    }
 }
 
-
 // ========================================================================
 // Convert floating point.
 
 // vpunpckldq requires 16-byte boundary for memory operand.
 // See convertUInt64ToDouble for the details.
 MOZ_ALIGNED_DECL(static const uint64_t, 16) TO_DOUBLE[4] = {
     0x4530000043300000LL,
     0x0LL,
--- a/js/src/wasm/WasmBuiltins.cpp
+++ b/js/src/wasm/WasmBuiltins.cpp
@@ -457,17 +457,17 @@ SaturatingTruncateDoubleToInt64(double i
 static uint64_t
 SaturatingTruncateDoubleToUint64(double input)
 {
     // Handle positive overflow.
     if (input >= -double(INT64_MIN) * 2.0) {
         return UINT64_MAX;
     }
     // Handle in-range values.
-    if (input >= -1.0) {
+    if (input > -1.0) {
         return uint64_t(input);
     }
     // Handle NaN and negative overflow.
     return 0;
 }
 
 static double
 Int64ToDouble(int32_t x_hi, uint32_t x_lo)