Changeset 237136 in webkit

Timestamp:

Oct 15, 2018 12:47:09 PM (6 years ago)

Author:

sbarati@apple.com

Message:

Emit fjcvtzs on ARM64E on Darwin
​https://bugs.webkit.org/show_bug.cgi?id=184023

Reviewed by Yusuke Suzuki and Filip Pizlo.

JSTests:

• stress/double-to-int32-NaN.js: Added.

(assert):
(foo):

Source/JavaScriptCore:

ARMv8.3 introduced the fjcvtzs instruction which does double->int32
conversion using the semantics defined by JavaScript:
​http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0801g/hko1477562192868.html
This patch teaches JSC to use that instruction when possible.

• assembler/ARM64Assembler.h:

(JSC::ARM64Assembler::fjcvtzs):
(JSC::ARM64Assembler::fjcvtzsInsn):

• assembler/MacroAssemblerARM64.cpp:

(JSC::MacroAssemblerARM64::collectCPUFeatures):

• assembler/MacroAssemblerARM64.h:

(JSC::MacroAssemblerARM64::supportsDoubleToInt32ConversionUsingJavaScriptSemantics):
(JSC::MacroAssemblerARM64::convertDoubleToInt32UsingJavaScriptSemantics):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileValueToInt32):

• disassembler/ARM64/A64DOpcode.cpp:
• disassembler/ARM64/A64DOpcode.h:

(JSC::ARM64Disassembler::A64DOpcode::appendInstructionName):

• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::doubleToInt32):

• jit/JITRightShiftGenerator.cpp:

(JSC::JITRightShiftGenerator::generateFastPath):

• runtime/MathCommon.h:

(JSC::toInt32):

Source/WTF:

• wtf/Platform.h:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/double-to-int32-NaN.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/assembler/ARM64Assembler.h (modified) (2 diffs)
• Source/JavaScriptCore/assembler/MacroAssemblerARM64.cpp (modified) (2 diffs)
• Source/JavaScriptCore/assembler/MacroAssemblerARM64.h (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (modified) (2 diffs)
• Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.cpp (modified) (1 diff)
• Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.h (modified) (1 diff)
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (modified) (1 diff)
• Source/JavaScriptCore/jit/JITRightShiftGenerator.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/MathCommon.h (modified) (1 diff)
• Source/WTF/ChangeLog (modified) (1 diff)
• Source/WTF/wtf/Platform.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2018-10-15  Saam barati  <sbarati@apple.com>
+
+        Emit fjcvtzs on ARM64E on Darwin
+        https://bugs.webkit.org/show_bug.cgi?id=184023
+
+        Reviewed by Yusuke Suzuki and Filip Pizlo.
+
+        * stress/double-to-int32-NaN.js: Added.
+        (assert):
+        (foo):
+
 2018-10-15  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-10-15  Saam barati  <sbarati@apple.com>
+
+        Emit fjcvtzs on ARM64E on Darwin
+        https://bugs.webkit.org/show_bug.cgi?id=184023
+
+        Reviewed by Yusuke Suzuki and Filip Pizlo.
+
+        ARMv8.3 introduced the fjcvtzs instruction which does double->int32
+        conversion using the semantics defined by JavaScript:
+        http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0801g/hko1477562192868.html
+        This patch teaches JSC to use that instruction when possible.
+
+        * assembler/ARM64Assembler.h:
+        (JSC::ARM64Assembler::fjcvtzs):
+        (JSC::ARM64Assembler::fjcvtzsInsn):
+        * assembler/MacroAssemblerARM64.cpp:
+        (JSC::MacroAssemblerARM64::collectCPUFeatures):
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::supportsDoubleToInt32ConversionUsingJavaScriptSemantics):
+        (JSC::MacroAssemblerARM64::convertDoubleToInt32UsingJavaScriptSemantics):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileValueToInt32):
+        * disassembler/ARM64/A64DOpcode.cpp:
+        * disassembler/ARM64/A64DOpcode.h:
+        (JSC::ARM64Disassembler::A64DOpcode::appendInstructionName):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::doubleToInt32):
+        * jit/JITRightShiftGenerator.cpp:
+        (JSC::JITRightShiftGenerator::generateFastPath):
+        * runtime/MathCommon.h:
+        (JSC::toInt32):
+
 2018-10-15  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/assembler/ARM64Assembler.h

@@ -2534 +2534 @@
     }
 
+    ALWAYS_INLINE void fjcvtzs(RegisterID rd, FPRegisterID dn)
+    {
+        insn(fjcvtzsInsn(dn, rd));
+    }
+
     // Admin methods:
 
@@ -3745 +3750 @@
         return 0x08007c00 | size << 30 | result << 16 | fence << 15 | dst << 5 | src;
     }
-    
+
+    static int fjcvtzsInsn(FPRegisterID dn, RegisterID rd)
+    {
+        return 0x1e7e0000 | (dn << 5) | rd;
+    }
+
     // Workaround for Cortex-A53 erratum (835769). Emit an extra nop if the
     // last instruction in the buffer is a load, store or prefetch. Needed

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.cpp

@@ -534 +534 @@
 void MacroAssemblerARM64::collectCPUFeatures()
 {
+#if OS(LINUX)
     static std::once_flag onceKey;
     std::call_once(onceKey, [] {
-#if OS(LINUX)
         // A register for describing ARM64 CPU features are only accessible in kernel mode.
         // Thus, some kernel support is necessary to collect CPU features. In Linux, the
@@ -552 +552 @@
 
         s_jscvtCheckState = (hwcaps & HWCAP_JSCVT) ? CPUIDCheckState::Set : CPUIDCheckState::Clear;
+    });
+#elif HAVE(FJCVTZS_INSTRUCTION)
+    s_jscvtCheckState = CPUIDCheckState::Set;
 #else
-        s_jscvtCheckState = CPUIDCheckState::Clear;
+    s_jscvtCheckState = CPUIDCheckState::Clear;
 #endif
-    });
 }
 

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -3759 +3759 @@
         m_assembler.eor<64>(dest, src, src);
     }
+
+    ALWAYS_INLINE static bool supportsDoubleToInt32ConversionUsingJavaScriptSemantics()
+    {
+#if HAVE(FJCVTZS_INSTRUCTION)
+        return true;
+#else
+        if (s_jscvtCheckState == CPUIDCheckState::NotChecked)
+            collectCPUFeatures();
+
+        return s_jscvtCheckState == CPUIDCheckState::Set;
+#endif
+    }
+
+    void convertDoubleToInt32UsingJavaScriptSemantics(FPRegisterID src, RegisterID dest)
+    {
+        m_assembler.fjcvtzs(dest, src); // This zero extends.
+    }
     
 #if ENABLE(FAST_TLS_JIT)

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -2345 +2345 @@
         FPRReg fpr = op1.fpr();
         GPRReg gpr = result.gpr();
-        JITCompiler::Jump notTruncatedToInteger = m_jit.branchTruncateDoubleToInt32(fpr, gpr, JITCompiler::BranchIfTruncateFailed);
-        
-        addSlowPathGenerator(slowPathCall(notTruncatedToInteger, this,
-            hasSensibleDoubleToInt() ? operationToInt32SensibleSlow : operationToInt32, NeedToSpill, ExceptionCheckRequirement::CheckNotNeeded, gpr, fpr));
-        
+#if CPU(ARM64)
+        if (MacroAssemblerARM64::supportsDoubleToInt32ConversionUsingJavaScriptSemantics())
+            m_jit.convertDoubleToInt32UsingJavaScriptSemantics(fpr, gpr);
+        else
+#endif
+        {
+            JITCompiler::Jump notTruncatedToInteger = m_jit.branchTruncateDoubleToInt32(fpr, gpr, JITCompiler::BranchIfTruncateFailed);
+            addSlowPathGenerator(slowPathCall(notTruncatedToInteger, this,
+                hasSensibleDoubleToInt() ? operationToInt32SensibleSlow : operationToInt32, NeedToSpill, ExceptionCheckRequirement::CheckNotNeeded, gpr, fpr));
+        }
         int32Result(gpr, node);
         return;
@@ -2396 +2401 @@
             // First, if we get here we have a double encoded as a JSValue
             unboxDouble(gpr, resultGpr, fpr);
-
-            silentSpillAllRegisters(resultGpr);
-            callOperation(operationToInt32, resultGpr, fpr);
-            silentFillAllRegisters();
+#if CPU(ARM64)
+            if (MacroAssemblerARM64::supportsDoubleToInt32ConversionUsingJavaScriptSemantics())
+                m_jit.convertDoubleToInt32UsingJavaScriptSemantics(fpr, resultGpr);
+            else
+#endif
+            {
+                silentSpillAllRegisters(resultGpr);
+                callOperation(operationToInt32, resultGpr, fpr);
+                silentFillAllRegisters();
+            }
 
             converted.append(m_jit.jump());

trunk/Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.cpp

@@ -819 +819 @@
     "fcvtps", "fcvtpu", 0, 0, 0, 0, "fmov", "fmov",
     "fcvtms", "fcvtmu", 0, 0, 0, 0, 0, 0,
-    "fcvtzs", "fcvtzu", 0, 0, 0, 0, 0, 0
+    "fcvtzs", "fcvtzu", 0, 0, 0, 0, "fjcvtzs", 0
 };
 

trunk/Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.h

@@ -116 +116 @@
     void appendInstructionName(const char* instructionName)
     {
-        bufferPrintf("   %-7.7s", instructionName);
+        bufferPrintf("   %-8.8s", instructionName);
     }
 

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -14428 +14428 @@
     LValue doubleToInt32(LValue doubleValue)
     {
+#if CPU(ARM64)
+        if (MacroAssemblerARM64::supportsDoubleToInt32ConversionUsingJavaScriptSemantics()) {
+            PatchpointValue* patchpoint = m_out.patchpoint(Int32);
+            patchpoint->append(ConstrainedValue(doubleValue, B3::ValueRep::SomeRegister));
+            patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+                jit.convertDoubleToInt32UsingJavaScriptSemantics(params[1].fpr(), params[0].gpr());
+            });
+            patchpoint->effects = Effects::none();
+            return patchpoint;
+        }
+#endif
+
         if (hasSensibleDoubleToInt())
             return sensibleDoubleToInt32(doubleValue);

trunk/Source/JavaScriptCore/jit/JITRightShiftGenerator.cpp

@@ -71 +71 @@
 
             jit.unboxDoubleNonDestructive(m_left, m_leftFPR, m_scratchGPR, m_scratchFPR);
-            m_slowPathJumpList.append(jit.branchTruncateDoubleToInt32(m_leftFPR, m_scratchGPR));
+#if CPU(ARM64)
+            if (MacroAssemblerARM64::supportsDoubleToInt32ConversionUsingJavaScriptSemantics())
+                jit.convertDoubleToInt32UsingJavaScriptSemantics(m_leftFPR, m_scratchGPR);
+            else
+#endif
+            {
+                m_slowPathJumpList.append(jit.branchTruncateDoubleToInt32(m_leftFPR, m_scratchGPR));
+            }
 
             if (shiftAmount) {
@@ -123 +130 @@
             m_slowPathJumpList.append(jit.branchIfNotNumber(m_left, m_scratchGPR));
             jit.unboxDoubleNonDestructive(m_left, m_leftFPR, m_scratchGPR, m_scratchFPR);
-            m_slowPathJumpList.append(jit.branchTruncateDoubleToInt32(m_leftFPR, m_scratchGPR));
+#if CPU(ARM64)
+            if (MacroAssemblerARM64::supportsDoubleToInt32ConversionUsingJavaScriptSemantics())
+                jit.convertDoubleToInt32UsingJavaScriptSemantics(m_leftFPR, m_scratchGPR);
+            else
+#endif
+            {
+                m_slowPathJumpList.append(jit.branchTruncateDoubleToInt32(m_leftFPR, m_scratchGPR));
+            }
 
             if (m_shiftType == SignedShift)

trunk/Source/JavaScriptCore/runtime/MathCommon.h

@@ -140 +140 @@
 ALWAYS_INLINE int32_t toInt32(double number)
 {
+#if HAVE(FJCVTZS_INSTRUCTION)
+    int32_t result = 0;
+    __asm__ ("fjcvtzs %w0, %d1" : "=r" (result) : "w" (number) : "cc");
+    return result;
+#else
     return toInt32Internal<ToInt32Mode::Generic>(number);
+#endif
 }
 

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2018-10-15  Saam barati  <sbarati@apple.com>
+
+        Emit fjcvtzs on ARM64E on Darwin
+        https://bugs.webkit.org/show_bug.cgi?id=184023
+
+        Reviewed by Yusuke Suzuki and Filip Pizlo.
+
+        * wtf/Platform.h:
+
 2018-10-15  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WTF/wtf/Platform.h

@@ -1043 +1043 @@
 #endif
 
+#if CPU(ARM64E) && OS(DARWIN)
+#define HAVE_FJCVTZS_INSTRUCTION 1
+#endif
+
 #if PLATFORM(IOS_FAMILY)
 #if !PLATFORM(WATCHOS) && !PLATFORM(APPLETV) && !PLATFORM(IOSMAC)