Bug 1142668: Fix int32x4 to float32x4 conversions in the JIT; r=sunfish

--HG--
extra : rebase_source : 977a3f5335d0044e9fb15b04a16baa58a0f1855a

js/src/asmjs/AsmJSValidate.cpp

@@ -2525,6 +2525,7 @@ class FunctionCompiler
                                            options, alloc_,
                                            graph_, info_, optimizationInfo,
                                            &m().onOutOfBoundsLabel(),
+                                           /* conversionErrorLabel = */ nullptr,
                                            m().usesSignalHandlersForOOB());
 
         if (!newBlock(/* pred = */ nullptr, &curBlock_, fn_))

js/src/jit-test/tests/SIMD/convert.js

@@ -1,6 +1,6 @@
 load(libdir + 'simd.js');
 
-setJitCompilerOption("ion.warmup.trigger", 50);
+setJitCompilerOption("ion.warmup.trigger", 30);
 
 var cast = (function() {
     var i32 = new Int32Array(1);
@@ -19,6 +19,7 @@ var cast = (function() {
 })();
 
 function f() {
+    // No bailout here.
     var f4 = SIMD.float32x4(1, 2, 3, 4);
     var i4 = SIMD.int32x4(1, 2, 3, 4);
     var BitOrZero = (x) => x | 0;
@@ -30,5 +31,40 @@ function f() {
     }
 }
 
+function uglyDuckling(val) {
+    // We bail out when i == 149 because the conversion will return
+    // 0x80000000 and the input actually wasn't in bounds.
+    print('entering uglyDuckling');
+    val = Math.fround(val);
+    for (var i = 0; i < 150; i++) {
+        var caught = false;
+        try {
+            var v = SIMD.float32x4(i < 149 ? 0 : val, 0, 0, 0)
+            SIMD.int32x4.fromFloat32x4(v);
+        } catch(e) {
+            assertEq(e instanceof RangeError, true);
+            assertEq(i, 149);
+            caught = true;
+        }
+        assertEq(i < 149 || caught, true);
+    }
+}
+
+function dontBail() {
+    print('entering dontbail');
+    // On x86, the conversion will return 0x80000000, which will imply that we
+    // check the input values. However, we shouldn't bail out in this case.
+    for (var i = 0; i < 150; i++) {
+        var v = SIMD.float32x4(i < 149 ? 0 : -Math.pow(2, 31), 0, 0, 0)
+        SIMD.int32x4.fromFloat32x4(v);
+    }
+}
+
 f();
 
+dontBail();
+dontBail();
+
+uglyDuckling(Math.pow(2, 31));
+uglyDuckling(NaN);
+uglyDuckling(-Math.pow(2, 32));

js/src/jit/IonTypes.h

@@ -545,7 +545,19 @@ static inline bool
 IsSimdType(MIRType type)
 {
     return type == MIRType_Int32x4 || type == MIRType_Float32x4;
-};
+}
+
+static inline bool
+IsFloatingPointSimdType(MIRType type)
+{
+    return type == MIRType_Float32x4;
+}
+
+static inline bool
+IsIntegerSimdType(MIRType type)
+{
+    return type == MIRType_Int32x4;
+}
 
 static inline bool
 IsMagicType(MIRType type)

js/src/jit/LIR-Common.h

@@ -3762,12 +3762,19 @@ class LInt32x4ToFloat32x4 : public LInstructionHelper<1, 1, 0>
     }
 };
 
-class LFloat32x4ToInt32x4 : public LInstructionHelper<1, 1, 0>
+class LFloat32x4ToInt32x4 : public LInstructionHelper<1, 1, 1>
 {
   public:
     LIR_HEADER(Float32x4ToInt32x4);
-    explicit LFloat32x4ToInt32x4(const LAllocation& input) {
+    explicit LFloat32x4ToInt32x4(const LAllocation& input, const LDefinition& temp) {
         setOperand(0, input);
+        setTemp(0, temp);
+    }
+    const LDefinition* temp() {
+        return getTemp(0);
+    }
+    const MSimdConvert* mir() const {
+        return mir_->toSimdConvert();
     }
 };
 

js/src/jit/Lowering.cpp

@@ -3890,11 +3890,13 @@ LIRGenerator::visitSimdConvert(MSimdConvert* ins)
 {
     MOZ_ASSERT(IsSimdType(ins->type()));
     MDefinition* input = ins->input();
-    LUse use = useRegisterAtStart(input);
-
+    LUse use = useRegister(input);
     if (ins->type() == MIRType_Int32x4) {
         MOZ_ASSERT(input->type() == MIRType_Float32x4);
-        define(new(alloc()) LFloat32x4ToInt32x4(use), ins);
+        LFloat32x4ToInt32x4* lir = new(alloc()) LFloat32x4ToInt32x4(use, temp());
+        if (!gen->conversionErrorLabel())
+            assignSnapshot(lir, Bailout_BoundsCheck);
+        define(lir, ins);
     } else if (ins->type() == MIRType_Float32x4) {
         MOZ_ASSERT(input->type() == MIRType_Int32x4);
         define(new(alloc()) LInt32x4ToFloat32x4(use), ins);

js/src/jit/MIR.h

@@ -1531,9 +1531,14 @@ class MSimdConvert
       : MUnaryInstruction(obj)
     {
         MOZ_ASSERT(IsSimdType(toType));
-        setMovable();
         setResultType(toType);
         specialization_ = fromType; // expects fromType as input
+
+        setMovable();
+        if (IsFloatingPointSimdType(fromType) && IsIntegerSimdType(toType)) {
+            // Does the extra range check => do not remove
+            setGuard();
+        }
     }
 
   public:

js/src/jit/MIRGenerator.h

@@ -38,7 +38,9 @@ class MIRGenerator
     MIRGenerator(CompileCompartment* compartment, const JitCompileOptions& options,
                  TempAllocator* alloc, MIRGraph* graph,
                  CompileInfo* info, const OptimizationInfo* optimizationInfo,
-                 Label* outOfBoundsLabel = nullptr, bool usesSignalHandlersForAsmJSOOB = false);
+                 Label* outOfBoundsLabel = nullptr,
+                 Label* conversionErrorLabel = nullptr,
+                 bool usesSignalHandlersForAsmJSOOB = false);
 
     TempAllocator& alloc() {
         return *alloc_;
@@ -202,6 +204,10 @@ class MIRGenerator
     void addAbortedPreliminaryGroup(ObjectGroup* group);
 
     Label* outOfBoundsLabel_;
+    // Label where we should jump in asm.js mode, in the case where we have an
+    // invalid conversion or a loss of precision (when converting from a
+    // floating point SIMD type into an integer SIMD type).
+    Label* conversionErrorLabel_;
 #if defined(ASMJS_MAY_USE_SIGNAL_HANDLERS_FOR_OOB)
     bool usesSignalHandlersForAsmJSOOB_;
 #endif
@@ -229,7 +235,12 @@ class MIRGenerator
         return nurseryObjects_;
     }
 
+    Label* conversionErrorLabel() const {
+        MOZ_ASSERT((conversionErrorLabel_ != nullptr) == compilingAsmJS());
+        return conversionErrorLabel_;
+    }
     Label* outOfBoundsLabel() const {
+        MOZ_ASSERT(compilingAsmJS());
         return outOfBoundsLabel_;
     }
     bool needsAsmJSBoundsCheckBranch(const MAsmJSHeapAccess* access) const;

js/src/jit/MIRGraph.cpp

@@ -20,7 +20,9 @@ using mozilla::Swap;
 MIRGenerator::MIRGenerator(CompileCompartment* compartment, const JitCompileOptions& options,
                            TempAllocator* alloc, MIRGraph* graph, CompileInfo* info,
                            const OptimizationInfo* optimizationInfo,
-                           Label* outOfBoundsLabel, bool usesSignalHandlersForAsmJSOOB)
+                           Label* outOfBoundsLabel,
+                           Label* conversionErrorLabel,
+                           bool usesSignalHandlersForAsmJSOOB)
   : compartment(compartment),
     info_(info),
     optimizationInfo_(optimizationInfo),
@@ -42,6 +44,7 @@ MIRGenerator::MIRGenerator(CompileCompartment* compartment, const JitCompileOpti
     instrumentedProfilingIsCached_(false),
     nurseryObjects_(*alloc),
     outOfBoundsLabel_(outOfBoundsLabel),
+    conversionErrorLabel_(conversionErrorLabel),
 #if defined(ASMJS_MAY_USE_SIGNAL_HANDLERS_FOR_OOB)
     usesSignalHandlersForAsmJSOOB_(usesSignalHandlersForAsmJSOOB),
 #endif

js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp

@@ -2175,7 +2175,58 @@ CodeGeneratorX86Shared::visitFloat32x4ToInt32x4(LFloat32x4ToInt32x4* ins)
 {
     FloatRegister in = ToFloatRegister(ins->input());
     FloatRegister out = ToFloatRegister(ins->output());
+    Register temp = ToRegister(ins->temp());
+
     masm.convertFloat32x4ToInt32x4(in, out);
+
+    OutOfLineSimdFloatToIntCheck *ool = new(alloc()) OutOfLineSimdFloatToIntCheck(temp, in, ins);
+    addOutOfLineCode(ool, ins->mir());
+
+    static const SimdConstant InvalidResult = SimdConstant::SplatX4(int32_t(-2147483648));
+
+    masm.loadConstantInt32x4(InvalidResult, ScratchSimdReg);
+    masm.packedEqualInt32x4(Operand(out), ScratchSimdReg);
+    // TODO (bug 1156228): If we have SSE4.1, we can use PTEST here instead of
+    // the two following instructions.
+    masm.vmovmskps(ScratchSimdReg, temp);
+    masm.cmp32(temp, Imm32(0));
+    masm.j(Assembler::NotEqual, ool->entry());
+
+    masm.bind(ool->rejoin());
+}
+
+void
+CodeGeneratorX86Shared::visitOutOfLineSimdFloatToIntCheck(OutOfLineSimdFloatToIntCheck *ool)
+{
+    static const SimdConstant Int32MaxX4 = SimdConstant::SplatX4(2147483647.f);
+    static const SimdConstant Int32MinX4 = SimdConstant::SplatX4(-2147483648.f);
+
+    Label bail;
+    Label* onConversionError = gen->conversionErrorLabel();
+    if (!onConversionError)
+        onConversionError = &bail;
+
+    FloatRegister input = ool->input();
+    Register temp = ool->temp();
+
+    masm.loadConstantFloat32x4(Int32MinX4, ScratchSimdReg);
+    masm.vcmpleps(Operand(input), ScratchSimdReg, ScratchSimdReg);
+    masm.vmovmskps(ScratchSimdReg, temp);
+    masm.cmp32(temp, Imm32(15));
+    masm.j(Assembler::NotEqual, onConversionError);
+
+    masm.loadConstantFloat32x4(Int32MaxX4, ScratchSimdReg);
+    masm.vcmpleps(Operand(input), ScratchSimdReg, ScratchSimdReg);
+    masm.vmovmskps(ScratchSimdReg, temp);
+    masm.cmp32(temp, Imm32(0));
+    masm.j(Assembler::NotEqual, onConversionError);
+
+    masm.jump(ool->rejoin());
+
+    if (bail.used()) {
+        masm.bind(&bail);
+        bailout(ool->ins()->snapshot());
+    }
 }
 
 void

js/src/jit/x86-shared/CodeGenerator-x86-shared.h

@@ -69,6 +69,28 @@ class CodeGeneratorX86Shared : public CodeGeneratorShared
         }
     };
 
+    // Additional bounds check for vector Float to Int conversion, when the
+    // undefined pattern is seen. Might imply a bailout.
+    class OutOfLineSimdFloatToIntCheck : public OutOfLineCodeBase<CodeGeneratorX86Shared>
+    {
+        Register temp_;
+        FloatRegister input_;
+        LInstruction* ins_;
+
+      public:
+        OutOfLineSimdFloatToIntCheck(Register temp, FloatRegister input, LInstruction *ins)
+          : temp_(temp), input_(input), ins_(ins)
+        {}
+
+        Register temp() const { return temp_; }
+        FloatRegister input() const { return input_; }
+        LInstruction* ins() const { return ins_; }
+
+        void accept(CodeGeneratorX86Shared* codegen) {
+            codegen->visitOutOfLineSimdFloatToIntCheck(this);
+        }
+    };
+
     // Functions for emitting bounds-checking code with branches.
     MOZ_WARN_UNUSED_RESULT
     uint32_t emitAsmJSBoundsCheckBranch(const MAsmJSHeapAccess* mir, const MInstruction* ins,
@@ -282,6 +304,7 @@ class CodeGeneratorX86Shared : public CodeGeneratorShared
     void visitModOverflowCheck(ModOverflowCheck* ool);
     void visitReturnZero(ReturnZero* ool);
     void visitOutOfLineTableSwitch(OutOfLineTableSwitch* ool);
+    void visitOutOfLineSimdFloatToIntCheck(OutOfLineSimdFloatToIntCheck* ool);
     void generateInvalidateEpilogue();
 };
 

js/src/jit/x86-shared/MacroAssembler-x86-shared.h

@@ -934,11 +934,10 @@ class MacroAssemblerX86Shared : public Assembler
     }
 
     void convertFloat32x4ToInt32x4(FloatRegister src, FloatRegister dest) {
-        // TODO: Note that if the conversion failed (because the converted
+        // Note that if the conversion failed (because the converted
         // result is larger than the maximum signed int32, or less than the
         // least signed int32, or NaN), this will return the undefined integer
-        // value (0x8000000). Spec should define what to do in such cases. See
-        // also bug 1068020.
+        // value (0x8000000).
         vcvttps2dq(src, dest);
     }
     void convertInt32x4ToFloat32x4(FloatRegister src, FloatRegister dest) {