Changeset 224623 in webkit

Timestamp:

Nov 9, 2017 1:02:10 AM (6 years ago)

Author:

guijemont@igalia.com

Message:

[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.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MIPSAssembler.h (modified) (5 diffs)
• assembler/MacroAssemblerMIPS.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/assembler/MIPSAssembler.h

@@ -109 +109 @@
 } 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;
 
@@ -163 +165 @@
     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; }
@@ -185 +187 @@
     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();
+        }
     }
 
@@ -225 +239 @@
     };
 
+    // FCSR Bits
+    enum {
+        FP_CAUSE_INVALID_OPERATION = 1 << 16
+    };
+
     void emitInst(MIPSWord op)
     {
@@ -715 +734 @@
     {
         emitInst(0x46200035 | (fs << OP_SH_FS) | (ft << OP_SH_FT));
+        copDelayNop();
+    }
+
+    void cfc1(RegisterID rt, SPRegisterID fs)
+    {
+        emitInst(0x44400000 | (rt << OP_SH_RT) | (fs << OP_SH_FS));
         copDelayNop();
     }

trunk/Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h

@@ -2953 +2953 @@
     // 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;
     }
 
@@ -2966 +2973 @@
     {
         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;
     }