[JSC][MIPS] Use fcsr to check the validity of the result of trunc.w.d
authorguijemont@igalia.com <guijemont@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Nov 2017 09:02:10 +0000 (09:02 +0000)
committerguijemont@igalia.com <guijemont@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Nov 2017 09:02:10 +0000 (09:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179446

Reviewed by Žan Doberšek.

The trunc.w.d mips instruction should give a 0x7fffffff result when
the source value is Infinity, NaN, or rounds to an integer outside the
range -2^31 to 2^31 -1. This is what branchTruncateDoubleToInt32() and
branchTruncateDoubleToUInt32() have been relying on. It turns out that
this assumption is not true on some CPUs, including on the ci20 on
which we run the testbot (we get 0x80000000 instead). We should the
invalid operation cause bit instead to check whether the source value
could be properly truncated. This requires the addition of the cfc1
instruction, as well as the special registers that can be used with it
(control registers of CP1).

* assembler/MIPSAssembler.h:
(JSC::MIPSAssembler::firstSPRegister):
(JSC::MIPSAssembler::lastSPRegister):
(JSC::MIPSAssembler::numberOfSPRegisters):
(JSC::MIPSAssembler::sprName):
Added control registers of CP1.
(JSC::MIPSAssembler::cfc1):
Added.
* assembler/MacroAssemblerMIPS.h:
(JSC::MacroAssemblerMIPS::branchOnTruncateResult):
(JSC::MacroAssemblerMIPS::branchTruncateDoubleToInt32):
(JSC::MacroAssemblerMIPS::branchTruncateDoubleToUint32):
Use fcsr to check if the value could be properly truncated.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224623 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/MIPSAssembler.h
Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h

index 55e753a..1a4443d 100644 (file)
@@ -1,3 +1,35 @@
+2017-11-09  Guillaume Emont  <guijemont@igalia.com>
+
+        [JSC][MIPS] Use fcsr to check the validity of the result of trunc.w.d
+        https://bugs.webkit.org/show_bug.cgi?id=179446
+
+        Reviewed by Žan Doberšek.
+
+        The trunc.w.d mips instruction should give a 0x7fffffff result when
+        the source value is Infinity, NaN, or rounds to an integer outside the
+        range -2^31 to 2^31 -1. This is what branchTruncateDoubleToInt32() and
+        branchTruncateDoubleToUInt32() have been relying on. It turns out that
+        this assumption is not true on some CPUs, including on the ci20 on
+        which we run the testbot (we get 0x80000000 instead). We should the
+        invalid operation cause bit instead to check whether the source value
+        could be properly truncated. This requires the addition of the cfc1
+        instruction, as well as the special registers that can be used with it
+        (control registers of CP1).
+
+        * assembler/MIPSAssembler.h:
+        (JSC::MIPSAssembler::firstSPRegister):
+        (JSC::MIPSAssembler::lastSPRegister):
+        (JSC::MIPSAssembler::numberOfSPRegisters):
+        (JSC::MIPSAssembler::sprName):
+        Added control registers of CP1.
+        (JSC::MIPSAssembler::cfc1):
+        Added.
+        * assembler/MacroAssemblerMIPS.h:
+        (JSC::MacroAssemblerMIPS::branchOnTruncateResult):
+        (JSC::MacroAssemblerMIPS::branchTruncateDoubleToInt32):
+        (JSC::MacroAssemblerMIPS::branchTruncateDoubleToUint32):
+        Use fcsr to check if the value could be properly truncated.
+
 2017-11-08  Jeremy Jones  <jeremyj@apple.com>
 
         HTMLMediaElement should not use element fullscreen on iOS
index aefbe0c..f86daf3 100644 (file)
@@ -108,10 +108,12 @@ typedef enum {
     ra = r31
 } RegisterID;
 
-// Currently, we don't have support for any special purpose registers.
 typedef enum {
-    firstInvalidSPR,
-    lastInvalidSPR = -1,
+    fir = 0,
+    fccr = 25,
+    fexr = 26,
+    fenr = 28,
+    fcsr = 31
 } SPRegisterID;
 
 typedef enum {
@@ -162,9 +164,9 @@ public:
     static constexpr RegisterID lastRegister() { return MIPSRegisters::r31; }
     static constexpr unsigned numberOfRegisters() { return lastRegister() - firstRegister() + 1; }
 
-    static constexpr SPRegisterID firstSPRegister() { return MIPSRegisters::firstInvalidSPR; }
-    static constexpr SPRegisterID lastSPRegister() { return MIPSRegisters::lastInvalidSPR; }
-    static constexpr unsigned numberOfSPRegisters() { return 0; }
+    static constexpr SPRegisterID firstSPRegister() { return MIPSRegisters::fir; }
+    static constexpr SPRegisterID lastSPRegister() { return MIPSRegisters::fcsr; }
+    static constexpr unsigned numberOfSPRegisters() { return 5; }
 
     static constexpr FPRegisterID firstFPRegister() { return MIPSRegisters::f0; }
     static constexpr FPRegisterID lastFPRegister() { return MIPSRegisters::f31; }
@@ -184,8 +186,20 @@ public:
 
     static const char* sprName(SPRegisterID id)
     {
-        // Currently, we don't have support for any special purpose registers.
-        RELEASE_ASSERT_NOT_REACHED();
+        switch (id) {
+        case MIPSRegisters::fir:
+            return "fir";
+        case MIPSRegisters::fccr:
+            return "fccr";
+        case MIPSRegisters::fexr:
+            return "fexr";
+        case MIPSRegisters::fenr:
+            return "fenr";
+        case MIPSRegisters::fcsr:
+            return "fcsr";
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+        }
     }
 
     static const char* fprName(FPRegisterID id)
@@ -224,6 +238,11 @@ public:
         OP_SH_FT = 16
     };
 
+    // FCSR Bits
+    enum {
+        FP_CAUSE_INVALID_OPERATION = 1 << 16
+    };
+
     void emitInst(MIPSWord op)
     {
         void* oldBase = m_buffer.data();
@@ -717,6 +736,12 @@ public:
         copDelayNop();
     }
 
+    void cfc1(RegisterID rt, SPRegisterID fs)
+    {
+        emitInst(0x44400000 | (rt << OP_SH_RT) | (fs << OP_SH_FS));
+        copDelayNop();
+    }
+
     // General helpers
 
     AssemblerLabel labelIgnoringWatchpoints()
index 23d27ed..c99ad8a 100644 (file)
@@ -2952,21 +2952,29 @@ public:
 
     // Truncates 'src' to an integer, and places the resulting 'dest'.
     // If the result is not representable as a 32 bit value, branch.
-    // May also branch for some values that are representable in 32 bits
-    // (specifically, in this case, INT_MAX 0x7fffffff).
     enum BranchTruncateType { BranchIfTruncateFailed, BranchIfTruncateSuccessful };
+
+    Jump branchOnTruncateResult(BranchTruncateType branchType)
+    {
+        m_assembler.cfc1(dataTempRegister, MIPSRegisters::fcsr);
+        and32(TrustedImm32(MIPSAssembler::FP_CAUSE_INVALID_OPERATION), dataTempRegister);
+        return branch32(branchType == BranchIfTruncateFailed ? NotEqual : Equal, dataTempRegister, MIPSRegisters::zero);
+    }
+
     Jump branchTruncateDoubleToInt32(FPRegisterID src, RegisterID dest, BranchTruncateType branchType = BranchIfTruncateFailed)
     {
         m_assembler.truncwd(fpTempRegister, src);
+        Jump truncateResult = branchOnTruncateResult(branchType);
         m_assembler.mfc1(dest, fpTempRegister);
-        return branch32(branchType == BranchIfTruncateFailed ? Equal : NotEqual, dest, TrustedImm32(0x7fffffff));
+        return truncateResult;
     }
 
     Jump branchTruncateDoubleToUint32(FPRegisterID src, RegisterID dest, BranchTruncateType branchType = BranchIfTruncateFailed)
     {
         m_assembler.truncwd(fpTempRegister, src);
+        Jump truncateResult = branchOnTruncateResult(branchType);
         m_assembler.mfc1(dest, fpTempRegister);
-        return branch32(branchType == BranchIfTruncateFailed ? Equal : NotEqual, dest, TrustedImm32(0x7fffffff));
+        return truncateResult;
     }
 
     // Result is undefined if the value is outside of the integer range.