Bug 1492733: Remove jitSupportsFloatingPoint checks from Baseline/IC code r=tcampbell,jandem
authorIain Ireland <iireland@mozilla.com>
Wed, 17 Oct 2018 14:00:16 +0000
changeset 490788 f092493f896a931f1eba48bb341797bd8885b249
parent 490787 e58c42a7b469627f21c75c25d39364fb7a4dc0a4
child 490789 ae1d789f73409413b98a6f7e7d21d8974e8abd98
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerstcampbell, jandem
bugs1492733
milestone64.0a1
Bug 1492733: Remove jitSupportsFloatingPoint checks from Baseline/IC code r=tcampbell,jandem Differential Revision: https://phabricator.services.mozilla.com/D8784
js/src/jit/BaselineCacheIRCompiler.cpp
js/src/jit/BaselineIC.cpp
js/src/jit/CacheIR.cpp
js/src/jit/CacheIRCompiler.cpp
js/src/jit/shared/CodeGenerator-shared.cpp
--- a/js/src/jit/BaselineCacheIRCompiler.cpp
+++ b/js/src/jit/BaselineCacheIRCompiler.cpp
@@ -1325,26 +1325,21 @@ BaselineCacheIRCompiler::emitStoreDenseE
 
     // Fail if we need to clone copy on write elements or to throw due
     // to a frozen element.
     masm.branchTest32(Assembler::NonZero, elementsFlags,
                       Imm32(ObjectElements::COPY_ON_WRITE |
                             ObjectElements::FROZEN),
                       failure->label());
 
-    // We need to convert int32 values being stored into doubles. Note that
-    // double arrays are only created by IonMonkey, so if we have no FP support
-    // Ion is disabled and there should be no double arrays.
-    if (cx_->runtime()->jitSupportsFloatingPoint) {
-        // It's fine to convert the value in place in Baseline. We can't do
-        // this in Ion.
-        masm.convertInt32ValueToDouble(val);
-    } else {
-        masm.assumeUnreachable("There shouldn't be double arrays when there is no FP support.");
-    }
+    // We need to convert int32 values being stored into doubles. Note
+    // that double arrays are only created by IonMonkey. It's fine to
+    // convert the value in place in Baseline. We can't do this in
+    // Ion.
+    masm.convertInt32ValueToDouble(val);
 
     masm.bind(&noSpecialHandling);
 
     // Call the type update IC. After this everything must be infallible as we
     // don't save all registers here.
     LiveGeneralRegisterSet saveRegs;
     saveRegs.add(obj);
     saveRegs.add(index);
@@ -1453,26 +1448,21 @@ BaselineCacheIRCompiler::emitStoreDenseE
     }
 
     // Check if we have to convert a double element.
     Label noConversion;
     masm.branchTest32(Assembler::Zero, elementsFlags,
                       Imm32(ObjectElements::CONVERT_DOUBLE_ELEMENTS),
                       &noConversion);
 
-    // We need to convert int32 values being stored into doubles. Note that
-    // double arrays are only created by IonMonkey, so if we have no FP support
-    // Ion is disabled and there should be no double arrays.
-    if (cx_->runtime()->jitSupportsFloatingPoint) {
-        // It's fine to convert the value in place in Baseline. We can't do
-        // this in Ion.
-        masm.convertInt32ValueToDouble(val);
-    } else {
-        masm.assumeUnreachable("There shouldn't be double arrays when there is no FP support.");
-    }
+    // We need to convert int32 values being stored into doubles. Note
+    // that double arrays are only created by IonMonkey. It's fine to
+    // convert the value in place in Baseline. We can't do this in
+    // Ion.
+    masm.convertInt32ValueToDouble(val);
 
     masm.bind(&noConversion);
 
     // Call the type update IC. After this everything must be infallible as we
     // don't save all registers here.
     LiveGeneralRegisterSet saveRegs;
     saveRegs.add(obj);
     saveRegs.add(index);
@@ -1586,26 +1576,21 @@ BaselineCacheIRCompiler::emitArrayPush()
     masm.bind(&capacityOk);
 
     // Check if we have to convert a double element.
     Label noConversion;
     masm.branchTest32(Assembler::Zero, elementsFlags,
                       Imm32(ObjectElements::CONVERT_DOUBLE_ELEMENTS),
                       &noConversion);
 
-    // We need to convert int32 values being stored into doubles. Note that
-    // double arrays are only created by IonMonkey, so if we have no FP support
-    // Ion is disabled and there should be no double arrays.
-    if (cx_->runtime()->jitSupportsFloatingPoint) {
-        // It's fine to convert the value in place in Baseline. We can't do
-        // this in Ion.
-        masm.convertInt32ValueToDouble(val);
-    } else {
-        masm.assumeUnreachable("There shouldn't be double arrays when there is no FP support.");
-    }
+    // We need to convert int32 values being stored into doubles. Note
+    // that double arrays are only created by IonMonkey. It's fine to
+    // convert the value in place in Baseline. We can't do this in
+    // Ion.
+    masm.convertInt32ValueToDouble(val);
 
     masm.bind(&noConversion);
 
     // Call the type update IC. After this everything must be infallible as we
     // don't save all registers here.
     LiveGeneralRegisterSet saveRegs;
     saveRegs.add(obj);
     saveRegs.add(val);
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -2183,45 +2183,37 @@ StoreToTypedArray(JSContext* cx, MacroAs
         Label clamped;
         masm.bind(&clamped);
         masm.storeToTypedIntArray(type, scratch, dest);
         masm.jump(&done);
 
         // If the value is a double, clamp to uint8 and jump back.
         // Else, jump to failure.
         masm.bind(&notInt32);
-        if (cx->runtime()->jitSupportsFloatingPoint) {
-            masm.branchTestDouble(Assembler::NotEqual, value, failure);
-            masm.unboxDouble(value, FloatReg0);
-            masm.clampDoubleToUint8(FloatReg0, scratch);
-            masm.jump(&clamped);
-        } else {
-            masm.jump(failure);
-        }
+        masm.branchTestDouble(Assembler::NotEqual, value, failure);
+        masm.unboxDouble(value, FloatReg0);
+        masm.clampDoubleToUint8(FloatReg0, scratch);
+        masm.jump(&clamped);
     } else {
         Label notInt32;
         masm.branchTestInt32(Assembler::NotEqual, value, &notInt32);
         masm.unboxInt32(value, scratch);
 
         Label isInt32;
         masm.bind(&isInt32);
         masm.storeToTypedIntArray(type, scratch, dest);
         masm.jump(&done);
 
         // If the value is a double, truncate and jump back.
         // Else, jump to failure.
         masm.bind(&notInt32);
-        if (cx->runtime()->jitSupportsFloatingPoint) {
-            masm.branchTestDouble(Assembler::NotEqual, value, failure);
-            masm.unboxDouble(value, FloatReg0);
-            masm.branchTruncateDoubleMaybeModUint32(FloatReg0, scratch, failure);
-            masm.jump(&isInt32);
-        } else {
-            masm.jump(failure);
-        }
+        masm.branchTestDouble(Assembler::NotEqual, value, failure);
+        masm.unboxDouble(value, FloatReg0);
+        masm.branchTruncateDoubleMaybeModUint32(FloatReg0, scratch, failure);
+        masm.jump(&isInt32);
     }
 
     masm.bind(&done);
 }
 
 template void
 StoreToTypedArray(JSContext* cx, MacroAssembler& masm, Scalar::Type type,
                   const ValueOperand& value, const Address& dest, Register scratch,
@@ -5149,29 +5141,16 @@ ICCall_ScriptedFunCall::Compiler::genera
     // Enter type monitor IC to type-check result.
     EmitEnterTypeMonitorIC(masm);
 
     masm.bind(&failure);
     EmitStubGuardFailure(masm);
     return true;
 }
 
-static bool
-DoubleValueToInt32ForSwitch(Value* v)
-{
-    double d = v->toDouble();
-    int32_t truncated = int32_t(d);
-    if (d != double(truncated)) {
-        return false;
-    }
-
-    v->setInt32(truncated);
-    return true;
-}
-
 bool
 ICTableSwitch::Compiler::generateStubCode(MacroAssembler& masm)
 {
     Label isInt32, notInt32, outOfRange;
     Register scratch = R1.scratchReg();
 
     masm.branchTestInt32(Assembler::NotEqual, R0, &notInt32);
 
@@ -5188,37 +5167,20 @@ ICTableSwitch::Compiler::generateStubCod
     masm.loadPtr(BaseIndex(scratch, key, ScalePointer), scratch);
 
     EmitChangeICReturnAddress(masm, scratch);
     EmitReturnFromIC(masm);
 
     masm.bind(&notInt32);
 
     masm.branchTestDouble(Assembler::NotEqual, R0, &outOfRange);
-    if (cx->runtime()->jitSupportsFloatingPoint) {
-        masm.unboxDouble(R0, FloatReg0);
-
-        // N.B. -0 === 0, so convert -0 to a 0 int32.
-        masm.convertDoubleToInt32(FloatReg0, key, &outOfRange, /* negativeZeroCheck = */ false);
-    } else {
-        // Pass pointer to double value.
-        masm.pushValue(R0);
-        masm.moveStackPtrTo(R0.scratchReg());
-
-        masm.setupUnalignedABICall(scratch);
-        masm.passABIArg(R0.scratchReg());
-        masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, DoubleValueToInt32ForSwitch));
-
-        // If the function returns |true|, the value has been converted to
-        // int32.
-        masm.movePtr(ReturnReg, scratch);
-        masm.popValue(R0);
-        masm.branchIfFalseBool(scratch, &outOfRange);
-        masm.unboxInt32(R0, key);
-    }
+    masm.unboxDouble(R0, FloatReg0);
+
+    // N.B. -0 === 0, so convert -0 to a 0 int32.
+    masm.convertDoubleToInt32(FloatReg0, key, &outOfRange, /* negativeZeroCheck = */ false);
     masm.jump(&isInt32);
 
     masm.bind(&outOfRange);
 
     masm.loadPtr(Address(ICStubReg, offsetof(ICTableSwitch, defaultTarget_)), scratch);
 
     EmitChangeICReturnAddress(masm, scratch);
     EmitReturnFromIC(masm);
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -1691,20 +1691,16 @@ GetPropIRGenerator::tryAttachUnboxed(Han
         return false;
     }
 
     const UnboxedLayout::Property* property = obj->as<UnboxedPlainObject>().layout().lookup(id);
     if (!property) {
         return false;
     }
 
-    if (!cx_->runtime()->jitSupportsFloatingPoint) {
-        return false;
-    }
-
     maybeEmitIdGuard(id);
     writer.guardGroupForLayout(objId, obj->group());
     writer.loadUnboxedPropertyResult(objId, property->type,
                                      UnboxedPlainObject::offsetOfData() + property->offset);
     if (property->type == JSVAL_TYPE_OBJECT) {
         writer.typeMonitorResult();
     } else {
         writer.returnFromIC();
@@ -1758,17 +1754,17 @@ GetTypedThingLayout(const Class* clasp)
 
 bool
 GetPropIRGenerator::tryAttachTypedObject(HandleObject obj, ObjOperandId objId, HandleId id)
 {
     if (!obj->is<TypedObject>()) {
         return false;
     }
 
-    if (!cx_->runtime()->jitSupportsFloatingPoint || cx_->zone()->detachedTypedObjects) {
+    if (cx_->zone()->detachedTypedObjects) {
         return false;
     }
 
     TypedObject* typedObj = &obj->as<TypedObject>();
     if (!typedObj->typeDescr().is<StructTypeDescr>()) {
         return false;
     }
 
@@ -2240,37 +2236,24 @@ PrimitiveArrayTypedObjectType(JSObject* 
 static Scalar::Type
 TypedThingElementType(JSObject* obj)
 {
     return obj->is<TypedArrayObject>()
            ? obj->as<TypedArrayObject>().type()
            : PrimitiveArrayTypedObjectType(obj);
 }
 
-static bool
-TypedThingRequiresFloatingPoint(JSObject* obj)
-{
-    Scalar::Type type = TypedThingElementType(obj);
-    return type == Scalar::Uint32 ||
-           type == Scalar::Float32 ||
-           type == Scalar::Float64;
-}
-
 bool
 GetPropIRGenerator::tryAttachTypedElement(HandleObject obj, ObjOperandId objId,
                                           uint32_t index, Int32OperandId indexId)
 {
     if (!obj->is<TypedArrayObject>() && !IsPrimitiveArrayTypedObject(obj)) {
         return false;
     }
 
-    if (!cx_->runtime()->jitSupportsFloatingPoint && TypedThingRequiresFloatingPoint(obj)) {
-        return false;
-    }
-
     // Ensure the index is in-bounds so the element type gets monitored.
     if (obj->is<TypedArrayObject>() && index >= obj->as<TypedArrayObject>().length()) {
         return false;
     }
 
     // Don't attach typed object stubs if the underlying storage could be
     // detached, as the stub will always bail out.
     if (IsPrimitiveArrayTypedObject(obj) && cx_->zone()->detachedTypedObjects) {
@@ -3320,19 +3303,16 @@ IRGenerator::maybeGuardInt32Index(const 
         int32_t indexSigned;
         if (index.isInt32()) {
             indexSigned = index.toInt32();
         } else {
             // We allow negative zero here.
             if (!mozilla::NumberEqualsInt32(index.toDouble(), &indexSigned)) {
                 return false;
             }
-            if (!cx_->runtime()->jitSupportsFloatingPoint) {
-                return false;
-            }
         }
 
         if (indexSigned < 0) {
             return false;
         }
 
         *int32Index = uint32_t(indexSigned);
         *int32IndexId = writer.guardIsInt32Index(indexId);
@@ -3608,20 +3588,16 @@ EmitGuardUnboxedPropertyType(CacheIRWrit
 bool
 SetPropIRGenerator::tryAttachUnboxedProperty(HandleObject obj, ObjOperandId objId, HandleId id,
                                              ValOperandId rhsId)
 {
     if (!obj->is<UnboxedPlainObject>()) {
         return false;
     }
 
-    if (!cx_->runtime()->jitSupportsFloatingPoint) {
-        return false;
-    }
-
     const UnboxedLayout::Property* property = obj->as<UnboxedPlainObject>().layout().lookup(id);
     if (!property) {
         return false;
     }
 
     maybeEmitIdGuard(id);
     writer.guardGroupForLayout(objId, obj->group());
     EmitGuardUnboxedPropertyType(writer, property->type, rhsId);
@@ -3640,17 +3616,17 @@ SetPropIRGenerator::tryAttachUnboxedProp
 bool
 SetPropIRGenerator::tryAttachTypedObjectProperty(HandleObject obj, ObjOperandId objId, HandleId id,
                                                  ValOperandId rhsId)
 {
     if (!obj->is<TypedObject>()) {
         return false;
     }
 
-    if (!cx_->runtime()->jitSupportsFloatingPoint || cx_->zone()->detachedTypedObjects) {
+    if (cx_->zone()->detachedTypedObjects) {
         return false;
     }
 
     if (!obj->as<TypedObject>().typeDescr().is<StructTypeDescr>()) {
         return false;
     }
 
     StructTypeDescr* structDescr = &obj->as<TypedObject>().typeDescr().as<StructTypeDescr>();
@@ -4076,20 +4052,16 @@ SetPropIRGenerator::tryAttachSetTypedEle
     if (!obj->is<TypedArrayObject>() && !IsPrimitiveArrayTypedObject(obj)) {
         return false;
     }
 
     if (!rhsVal_.isNumber()) {
         return false;
     }
 
-    if (!cx_->runtime()->jitSupportsFloatingPoint && TypedThingRequiresFloatingPoint(obj)) {
-        return false;
-    }
-
     bool handleOutOfBounds = false;
     if (obj->is<TypedArrayObject>()) {
         handleOutOfBounds = (index >= obj->as<TypedArrayObject>().length());
     } else {
         // Typed objects throw on out of bounds accesses. Don't attach
         // a stub in this case.
         if (index >= obj->as<TypedObject>().length()) {
             return false;
@@ -5282,20 +5254,16 @@ CompareIRGenerator::tryAttachInt32(ValOp
 
     trackAttached(lhsVal_.isBoolean() ? "Boolean" : "Int32");
     return true;
 }
 
 bool
 CompareIRGenerator::tryAttachNumber(ValOperandId lhsId, ValOperandId rhsId)
 {
-    if (!cx_->runtime()->jitSupportsFloatingPoint) {
-        return false;
-    }
-
     if (!lhsVal_.isNumber() || !rhsVal_.isNumber()) {
         return false;
     }
 
     writer.guardIsNumber(lhsId);
     writer.guardIsNumber(rhsId);
     writer.compareDoubleResult(op_, lhsId, rhsId);
     writer.returnFromIC();
@@ -5613,17 +5581,17 @@ ToBoolIRGenerator::tryAttachInt32()
     writer.returnFromIC();
     trackAttached("ToBoolInt32");
     return true;
 }
 
 bool
 ToBoolIRGenerator::tryAttachDouble()
 {
-    if (!val_.isDouble() || !cx_->runtime()->jitSupportsFloatingPoint) {
+    if (!val_.isDouble()) {
         return false;
     }
 
     ValOperandId valId(writer.setInputOperandId(0));
     writer.guardType(valId, JSVAL_TYPE_DOUBLE);
     writer.loadDoubleTruthyResult(valId);
     writer.returnFromIC();
     trackAttached("ToBoolDouble");
@@ -5775,17 +5743,17 @@ UnaryArithIRGenerator::tryAttachInt32()
 
     writer.returnFromIC();
     return true;
 }
 
 bool
 UnaryArithIRGenerator::tryAttachNumber()
 {
-    if (!val_.isNumber() || !res_.isNumber() || !cx_->runtime()->jitSupportsFloatingPoint) {
+    if (!val_.isNumber() || !res_.isNumber()) {
         return false;
     }
 
     ValOperandId valId(writer.setInputOperandId(0));
     writer.guardIsNumber(valId);
     Int32OperandId truncatedId;
     switch (op_) {
       case JSOP_BITNOT:
@@ -5948,20 +5916,16 @@ BinaryArithIRGenerator::tryAttachDouble(
         return false;
     }
 
     // Check guard conditions
     if (!lhs_.isNumber() || !rhs_.isNumber()) {
         return false;
     }
 
-    if (!cx_->runtime()->jitSupportsFloatingPoint) {
-        return false;
-    }
-
     ValOperandId lhsId(writer.setInputOperandId(0));
     ValOperandId rhsId(writer.setInputOperandId(1));
 
     writer.guardIsNumber(lhsId);
     writer.guardIsNumber(rhsId);
 
     switch (op_) {
        case JSOP_ADD:
--- a/js/src/jit/CacheIRCompiler.cpp
+++ b/js/src/jit/CacheIRCompiler.cpp
@@ -1611,39 +1611,35 @@ CacheIRCompiler::emitGuardIsInt32Index()
 
     Label notInt32, done;
     masm.branchTestInt32(Assembler::NotEqual, input, &notInt32);
     masm.unboxInt32(input, output);
     masm.jump(&done);
 
     masm.bind(&notInt32);
 
-    if (cx_->runtime()->jitSupportsFloatingPoint) {
-        masm.branchTestDouble(Assembler::NotEqual, input, failure->label());
-
-        // If we're compiling a Baseline IC, FloatReg0 is always available.
-        Label failurePopReg;
-        if (mode_ != Mode::Baseline) {
-            masm.push(FloatReg0);
-        }
-
-        masm.unboxDouble(input, FloatReg0);
-        // ToPropertyKey(-0.0) is "0", so we can truncate -0.0 to 0 here.
-        masm.convertDoubleToInt32(FloatReg0, output,
-                                  (mode_ == Mode::Baseline) ? failure->label() : &failurePopReg,
-                                  false);
-        if (mode_ != Mode::Baseline) {
-            masm.pop(FloatReg0);
-            masm.jump(&done);
-
-            masm.bind(&failurePopReg);
-            masm.pop(FloatReg0);
-            masm.jump(failure->label());
-        }
-    } else {
+    masm.branchTestDouble(Assembler::NotEqual, input, failure->label());
+
+    // If we're compiling a Baseline IC, FloatReg0 is always available.
+    Label failurePopReg;
+    if (mode_ != Mode::Baseline) {
+        masm.push(FloatReg0);
+    }
+
+    masm.unboxDouble(input, FloatReg0);
+    // ToPropertyKey(-0.0) is "0", so we can truncate -0.0 to 0 here.
+    masm.convertDoubleToInt32(FloatReg0, output,
+                              (mode_ == Mode::Baseline) ? failure->label() : &failurePopReg,
+                              false);
+    if (mode_ != Mode::Baseline) {
+        masm.pop(FloatReg0);
+        masm.jump(&done);
+
+        masm.bind(&failurePopReg);
+        masm.pop(FloatReg0);
         masm.jump(failure->label());
     }
 
     masm.bind(&done);
     return true;
 }
 
 bool
--- a/js/src/jit/shared/CodeGenerator-shared.cpp
+++ b/js/src/jit/shared/CodeGenerator-shared.cpp
@@ -1392,20 +1392,16 @@ CodeGeneratorShared::resetOsiPointRegs(L
 }
 #endif
 
 // Before doing any call to Cpp, you should ensure that volatile
 // registers are evicted by the register allocator.
 void
 CodeGeneratorShared::callVM(const VMFunction& fun, LInstruction* ins, const Register* dynStack)
 {
-    // If we're calling a function with an out parameter type of double, make
-    // sure we have an FPU.
-    MOZ_ASSERT_IF(fun.outParam == Type_Double, gen->runtime->jitSupportsFloatingPoint());
-
 #ifdef DEBUG
     if (ins->mirRaw()) {
         MOZ_ASSERT(ins->mirRaw()->isInstruction());
         MInstruction* mir = ins->mirRaw()->toInstruction();
         MOZ_ASSERT_IF(mir->needsResumePoint(), mir->resumePoint());
     }
 #endif