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: Identifier: 205510@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237136 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Oct 15, 2018 · 981565a · 981565a
1 parent 4b9076c
commit 981565a
Show file tree

Hide file tree

Showing 14 changed files with 167 additions and 17 deletions.
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
@@ -1,3 +1,14 @@
+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>
 
         JSArray::shiftCountWithArrayStorage is wrong when an array has holes

diff --git a/JSTests/stress/double-to-int32-NaN.js b/JSTests/stress/double-to-int32-NaN.js
@@ -0,0 +1,22 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+
+function foo(view) {
+    let x = view.getFloat64(0);
+    return [x, x | 0];
+}
+noInline(foo);
+
+let buffer = new ArrayBuffer(8);
+let view = new DataView(buffer);
+for (let i = 0; i < 1000000; ++i) {
+    for (let i = 0; i < 8; ++i) {
+        view.setInt8(i, Math.random() * 255);
+    }
+
+    let [a, b] = foo(view);
+    if (isNaN(a))
+        assert(b === 0);
+}
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,35 @@
+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>
 
         JSArray::shiftCountWithArrayStorage is wrong when an array has holes

diff --git a/Source/JavaScriptCore/assembler/ARM64Assembler.h b/Source/JavaScriptCore/assembler/ARM64Assembler.h
@@ -2533,6 +2533,11 @@ class ARM64Assembler {
         insn(floatingPointIntegerConversions(DATASIZE_OF(srcsize), DATASIZE_OF(dstsize), FPIntConvOp_UCVTF, rn, vd));
     }
 
+    ALWAYS_INLINE void fjcvtzs(RegisterID rd, FPRegisterID dn)
+    {
+        insn(fjcvtzsInsn(dn, rd));
+    }
+
     // Admin methods:
 
     AssemblerLabel labelIgnoringWatchpoints()
@@ -3744,7 +3749,12 @@ class ARM64Assembler {
     {
         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
     // before 64-bit multiply-accumulate instructions.

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.cpp b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.cpp
@@ -533,9 +533,9 @@ void MacroAssembler::probe(Probe::Function function, void* arg)
 
 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
         // kernel passes CPU feature flags in AT_HWCAP auxiliary vector which is passed
@@ -551,10 +551,12 @@ void MacroAssemblerARM64::collectCPUFeatures()
 #endif
 
         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
-    });
 }
 
 MacroAssemblerARM64::CPUIDCheckState MacroAssemblerARM64::s_jscvtCheckState = CPUIDCheckState::NotChecked;

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
@@ -3758,6 +3758,23 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {
     {
         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)
     // This will use scratch registers if the offset is not legal.

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -2344,11 +2344,16 @@ void SpeculativeJIT::compileValueToInt32(Node* node)
         SpeculateDoubleOperand op1(this, node->child1());
         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;
     }
@@ -2395,10 +2400,16 @@ void SpeculativeJIT::compileValueToInt32(Node* node)
 
             // 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());
 

diff --git a/Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.cpp b/Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.cpp
@@ -818,7 +818,7 @@ const char* const A64DOpcodeFloatingPointIntegerConversions::s_opNames[32] = {
     "fcvtns", "fcvtnu", "scvtf", "ucvtf", "fcvtas", "fcvtau", "fmov", "fmov",
     "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
 };
 
 const char* A64DOpcodeFloatingPointIntegerConversions::format()

diff --git a/Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.h b/Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.h
@@ -115,7 +115,7 @@ class A64DOpcode {
 
     void appendInstructionName(const char* instructionName)
     {
-        bufferPrintf("   %-7.7s", instructionName);
+        bufferPrintf("   %-8.8s", instructionName);
     }
 
     void appendRegisterName(unsigned registerNumber, bool is64Bit = true);

diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -14427,6 +14427,18 @@ class LowerDFGToB3 {
 
     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);
 

diff --git a/Source/JavaScriptCore/jit/JITRightShiftGenerator.cpp b/Source/JavaScriptCore/jit/JITRightShiftGenerator.cpp
@@ -70,7 +70,14 @@ void JITRightShiftGenerator::generateFastPath(CCallHelpers& jit)
             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 (shiftAmount) {
                 if (m_shiftType == SignedShift)
@@ -122,7 +129,14 @@ void JITRightShiftGenerator::generateFastPath(CCallHelpers& jit)
 
             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)
                 jit.rshift32(m_right.payloadGPR(), m_scratchGPR);

diff --git a/Source/JavaScriptCore/runtime/MathCommon.h b/Source/JavaScriptCore/runtime/MathCommon.h
@@ -139,7 +139,13 @@ ALWAYS_INLINE int32_t toInt32Internal(double number)
 
 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
 }
 
 // This implements ToUInt32, defined in ECMA-262 9.6.

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
@@ -1,3 +1,12 @@
+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>
 
         Use pragma once in WTF

diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
@@ -1042,6 +1042,10 @@
 #endif
 #endif
 
+#if CPU(ARM64E) && OS(DARWIN)
+#define HAVE_FJCVTZS_INSTRUCTION 1
+#endif
+
 #if PLATFORM(IOS_FAMILY)
 #if !PLATFORM(WATCHOS) && !PLATFORM(APPLETV) && !PLATFORM(IOSMAC)
 #define USE_QUICK_LOOK 1