Attachment 339295 Details for Bug 185192 – Patch

[patch] Patch

bug-185192-20180502191831.patch (text/plain), 19.10 KB, created by Yusuke Suzuki on 2018-05-02 03:18:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-02 03:18:32 PDT

Size: 19.10 KB

patch

obsolete

>Subversion Revision: 231226
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4a1162741a4ea0721993ac2a4fcb300b9910b63e..76d66159a2cd101cf39b2518c798e8605edaf3ec 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,49 @@
>+2018-05-02  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [JSC] Add compareDouble and compareFloat for ARM64, X86, and X86_64
>+        https://bugs.webkit.org/show_bug.cgi?id=185192
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Now Object.is starts using compareDouble. So we would like to have
>+        efficient implementation for compareDouble and compareFloat for
>+        major architectures, ARM64, X86, and X86_64.
>+
>+        This patch adds compareDouble and compareFloat implementations for
>+        these architectures. And generic implementation is moved to each
>+        architecture's MacroAssembler implementation.
>+
>+        We also add tests for them in testmasm. To implement this test
>+        easily, we also add loadFloat(TrustedImmPtr, FPRegisterID) for the
>+        major architectures.
>+
>+        * assembler/MacroAssembler.h:
>+        (JSC::MacroAssembler::compareDouble): Deleted.
>+        (JSC::MacroAssembler::compareFloat): Deleted.
>+        * assembler/MacroAssemblerARM.h:
>+        (JSC::MacroAssemblerARM::compareDouble):
>+        * assembler/MacroAssemblerARM64.h:
>+        (JSC::MacroAssemblerARM64::compareDouble):
>+        (JSC::MacroAssemblerARM64::compareFloat):
>+        (JSC::MacroAssemblerARM64::loadFloat):
>+        (JSC::MacroAssemblerARM64::floatingPointCompare):
>+        * assembler/MacroAssemblerARMv7.h:
>+        (JSC::MacroAssemblerARMv7::compareDouble):
>+        * assembler/MacroAssemblerMIPS.h:
>+        (JSC::MacroAssemblerMIPS::compareDouble):
>+        * assembler/MacroAssemblerX86Common.h:
>+        (JSC::MacroAssemblerX86Common::loadFloat):
>+        (JSC::MacroAssemblerX86Common::compareDouble):
>+        (JSC::MacroAssemblerX86Common::compareFloat):
>+        (JSC::MacroAssemblerX86Common::floatingPointCompare):
>+        * assembler/X86Assembler.h:
>+        (JSC::X86Assembler::movss_mr):
>+        (JSC::X86Assembler::movss_rm):
>+        * assembler/testmasm.cpp:
>+        (JSC::floatOperands):
>+        (JSC::testCompareFloat):
>+        (JSC::run):
>+
> 2018-05-01  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [JSC] Add SameValue DFG node
>diff --git a/Source/JavaScriptCore/assembler/MacroAssembler.h b/Source/JavaScriptCore/assembler/MacroAssembler.h
>index 8679261fb8eb88898c4d06ee1cd789c19a874cfe..e23e81595d08657079e05f3e0fe00e497de5edca 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssembler.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssembler.h
>@@ -1462,26 +1462,6 @@ class MacroAssembler : public MacroAssemblerBase {
>     // We should implement this the right way eventually, but for now, it's fine because it arises so
>     // infrequently.
> 
>-#if !CPU(X86) && !CPU(X86_64)
>-    void compareDouble(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>-    {
>-        move(TrustedImm32(0), dest);
>-        Jump falseCase = branchDouble(invert(cond), left, right);
>-        move(TrustedImm32(1), dest);
>-        falseCase.link(this);
>-    }
>-#endif
>-
>-#if ENABLE(B3_JIT)
>-    void compareFloat(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>-    {
>-        move(TrustedImm32(0), dest);
>-        Jump falseCase = branchFloat(invert(cond), left, right);
>-        move(TrustedImm32(1), dest);
>-        falseCase.link(this);
>-    }
>-#endif
>-
>     void lea32(Address address, RegisterID dest)
>     {
>         add32(TrustedImm32(address.offset), address.base, dest);
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM.h
>index e144ac4ec72e227d0700570c89d93fc8aecd585e..36d5cdf1222f1c07c11ce02429b59a8d07c8f5c8 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerARM.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARM.h
>@@ -1432,6 +1432,14 @@ class MacroAssemblerARM : public AbstractMacroAssembler<Assembler> {
>         return Jump(m_assembler.jmp(static_cast<ARMAssembler::Condition>(cond & ~DoubleConditionMask)));
>     }
> 
>+    void compareDouble(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>+    {
>+        move(TrustedImm32(0), dest);
>+        Jump falseCase = branchDouble(invert(cond), left, right);
>+        move(TrustedImm32(1), dest);
>+        falseCase.link(this);
>+    }
>+
>     // 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
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
>index bbf38dc9b102ba0dae8ff74016506d8e3bc31208..9c72c7ff7db91618db372f0d54c15354d4fea3cd 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
>@@ -1697,6 +1697,20 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {
>         return jumpAfterFloatingPointCompare(cond);
>     }
> 
>+    void compareDouble(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>+    {
>+        floatingPointCompare(cond, left, right, dest, [this] (FPRegisterID arg1, FPRegisterID arg2) {
>+            m_assembler.fcmp<64>(arg1, arg2);
>+        });
>+    }
>+
>+    void compareFloat(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>+    {
>+        floatingPointCompare(cond, left, right, dest, [this] (FPRegisterID arg1, FPRegisterID arg2) {
>+            m_assembler.fcmp<32>(arg1, arg2);
>+        });
>+    }
>+
>     Jump branchDoubleNonZero(FPRegisterID reg, FPRegisterID)
>     {
>         m_assembler.fcmp_0<64>(reg);
>@@ -1849,6 +1863,12 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {
>         m_assembler.ldr<32>(dest, address.base, memoryTempRegister);
>     }
> 
>+    void loadFloat(TrustedImmPtr address, FPRegisterID dest)
>+    {
>+        moveToCachedReg(address, cachedMemoryTempRegister());
>+        m_assembler.ldr<32>(dest, memoryTempRegister, ARM64Registers::zr);
>+    }
>+
>     void moveDouble(FPRegisterID src, FPRegisterID dest)
>     {
>         m_assembler.fmov<64>(dest, src);
>@@ -4453,6 +4473,31 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {
>         return makeBranch(cond);
>     }
> 
>+    template<typename Function>
>+    void floatingPointCompare(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest, Function compare)
>+    {
>+        if (cond == DoubleNotEqual) {
>+            // ConditionNE sets 1 if NotEqual *or* unordered - force the unordered cases not to set 1.
>+            move(TrustedImm32(0), dest);
>+            compare(left, right);
>+            Jump unordered = makeBranch(Assembler::ConditionVS);
>+            m_assembler.cset<32>(dest, Assembler::ConditionNE);
>+            unordered.link(this);
>+            return;
>+        }
>+        if (cond == DoubleEqualOrUnordered) {
>+            // ConditionEQ sets 1 only if Equal - force the unordered cases to set 1 too.
>+            move(TrustedImm32(1), dest);
>+            compare(left, right);
>+            Jump unordered = makeBranch(Assembler::ConditionVS);
>+            m_assembler.cset<32>(dest, Assembler::ConditionEQ);
>+            unordered.link(this);
>+            return;
>+        }
>+        compare(left, right);
>+        m_assembler.cset<32>(dest, ARM64Condition(cond));
>+    }
>+
>     friend class LinkBuffer;
> 
>     template<PtrTag tag>
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h b/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
>index cd104590c499f1d9342322e4239c572aaeaf6e4c..86dd95abba759733cd0ab64d9981ece3a08c88ff 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
>@@ -1186,6 +1186,14 @@ class MacroAssemblerARMv7 : public AbstractMacroAssembler<Assembler> {
>         return makeBranch(cond);
>     }
> 
>+    void compareDouble(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>+    {
>+        move(TrustedImm32(0), dest);
>+        Jump falseCase = branchDouble(invert(cond), left, right);
>+        move(TrustedImm32(1), dest);
>+        falseCase.link(this);
>+    }
>+
>     enum BranchTruncateType { BranchIfTruncateFailed, BranchIfTruncateSuccessful };
>     Jump branchTruncateDoubleToInt32(FPRegisterID src, RegisterID dest, BranchTruncateType branchType = BranchIfTruncateFailed)
>     {
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h b/Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h
>index 524e550c289c831011fc207d754309c420b0b7bb..0042260be776f4d90b9b1c5697282a79605b852c 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h
>@@ -3284,6 +3284,14 @@ class MacroAssemblerMIPS : public AbstractMacroAssembler<Assembler> {
>         return Jump();
>     }
> 
>+    void compareDouble(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>+    {
>+        move(TrustedImm32(0), dest);
>+        Jump falseCase = branchDouble(invert(cond), left, right);
>+        move(TrustedImm32(1), dest);
>+        falseCase.link(this);
>+    }
>+
>     // Truncates 'src' to an integer, and places the resulting 'dest'.
>     // If the result is not representable as a 32 bit value, branch.
>     enum BranchTruncateType { BranchIfTruncateFailed, BranchIfTruncateSuccessful };
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h b/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
>index 1f43f407ef1d7d3cf388d237463d5777ec26cfb1..e5178f68d1679d598415723480c942d6298f7d7f 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
>@@ -1455,6 +1455,17 @@ class MacroAssemblerX86Common : public AbstractMacroAssembler<Assembler> {
>         m_assembler.movsd_mr(address.offset, address.base, address.index, address.scale, dest);
>     }
> 
>+    void loadFloat(TrustedImmPtr address, FPRegisterID dest)
>+    {
>+#if CPU(X86)
>+        ASSERT(isSSE2Present());
>+        m_assembler.movss_mr(address.asPtr(), dest);
>+#else
>+        move(address, scratchRegister());
>+        loadFloat(scratchRegister(), dest);
>+#endif
>+    }
>+
>     void loadFloat(ImplicitAddress address, FPRegisterID dest)
>     {
>         ASSERT(isSSE2Present());
>@@ -2016,44 +2027,18 @@ class MacroAssemblerX86Common : public AbstractMacroAssembler<Assembler> {
> 
>     void compareDouble(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>     {
>-        if (cond & DoubleConditionBitSpecial) {
>-            ASSERT(!(cond & DoubleConditionBitInvert));
>-            if (cond == DoubleEqual) {
>-                if (left == right) {
>-                    m_assembler.ucomisd_rr(right, left);
>-                    set32(X86Assembler::ConditionNP, dest);
>-                    return;
>-                }
>-
>-                move(TrustedImm32(0), dest);
>-                m_assembler.ucomisd_rr(right, left);
>-                Jump isUnordered = m_assembler.jp();
>-                set32(X86Assembler::ConditionE, dest);
>-                isUnordered.link(this);
>-                return;
>-            }
>-            if (cond == DoubleNotEqualOrUnordered) {
>-                if (left == right) {
>-                    m_assembler.ucomisd_rr(right, left);
>-                    set32(X86Assembler::ConditionP, dest);
>-                    return;
>-                }
>-
>-                move(TrustedImm32(1), dest);
>-                m_assembler.ucomisd_rr(right, left);
>-                Jump isUnordered = m_assembler.jp();
>-                set32(X86Assembler::ConditionNE, dest);
>-                isUnordered.link(this);
>-                return;
>-            }
>-            return;
>-        }
>+        ASSERT(isSSE2Present());
>+        floatingPointCompare(cond, left, right, dest, [this] (FPRegisterID arg1, FPRegisterID arg2) {
>+            m_assembler.ucomisd_rr(arg1, arg2);
>+        });
>+    }
> 
>-        if (cond & DoubleConditionBitInvert)
>-            m_assembler.ucomisd_rr(left, right);
>-        else
>-            m_assembler.ucomisd_rr(right, left);
>-        set32(static_cast<X86Assembler::Condition>(cond & ~DoubleConditionBits), dest);
>+    void compareFloat(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest)
>+    {
>+        ASSERT(isSSE2Present());
>+        floatingPointCompare(cond, left, right, dest, [this] (FPRegisterID arg1, FPRegisterID arg2) {
>+            m_assembler.ucomiss_rr(arg1, arg2);
>+        });
>     }
> 
>     // Truncates 'src' to an integer, and places the resulting 'dest'.
>@@ -4141,6 +4126,49 @@ class MacroAssemblerX86Common : public AbstractMacroAssembler<Assembler> {
>         skipNonZeroCase.link(this);
>     }
> 
>+    template<typename Function>
>+    void floatingPointCompare(DoubleCondition cond, FPRegisterID left, FPRegisterID right, RegisterID dest, Function compare)
>+    {
>+        if (cond & DoubleConditionBitSpecial) {
>+            ASSERT(!(cond & DoubleConditionBitInvert));
>+            if (cond == DoubleEqual) {
>+                if (left == right) {
>+                    compare(right, left);
>+                    set32(X86Assembler::ConditionNP, dest);
>+                    return;
>+                }
>+
>+                move(TrustedImm32(0), dest);
>+                compare(right, left);
>+                Jump isUnordered = m_assembler.jp();
>+                set32(X86Assembler::ConditionE, dest);
>+                isUnordered.link(this);
>+                return;
>+            }
>+            if (cond == DoubleNotEqualOrUnordered) {
>+                if (left == right) {
>+                    compare(right, left);
>+                    set32(X86Assembler::ConditionP, dest);
>+                    return;
>+                }
>+
>+                move(TrustedImm32(1), dest);
>+                compare(right, left);
>+                Jump isUnordered = m_assembler.jp();
>+                set32(X86Assembler::ConditionNE, dest);
>+                isUnordered.link(this);
>+                return;
>+            }
>+            return;
>+        }
>+
>+        if (cond & DoubleConditionBitInvert)
>+            compare(left, right);
>+        else
>+            compare(right, left);
>+        set32(static_cast<X86Assembler::Condition>(cond & ~DoubleConditionBits), dest);
>+    }
>+
>     Jump jumpAfterFloatingPointCompare(DoubleCondition cond, FPRegisterID left, FPRegisterID right)
>     {
>         if (cond == DoubleEqual) {
>diff --git a/Source/JavaScriptCore/assembler/X86Assembler.h b/Source/JavaScriptCore/assembler/X86Assembler.h
>index 51fb3334e9320feb36b31010c2cb814245eb3638..1ea7a75ed8ff11f044d9fca524eae891786c7868 100644
>--- a/Source/JavaScriptCore/assembler/X86Assembler.h
>+++ b/Source/JavaScriptCore/assembler/X86Assembler.h
>@@ -3161,6 +3161,16 @@ class X86Assembler {
>         m_formatter.prefix(PRE_SSE_F2);
>         m_formatter.twoByteOpAddr(OP2_MOVSD_WsdVsd, (RegisterID)src, bitwise_cast<uint32_t>(address));
>     }
>+    void movss_mr(const void* address, XMMRegisterID dst)
>+    {
>+        m_formatter.prefix(PRE_SSE_F3);
>+        m_formatter.twoByteOpAddr(OP2_MOVSD_VsdWsd, (RegisterID)dst, bitwise_cast<uint32_t>(address));
>+    }
>+    void movss_rm(XMMRegisterID src, const void* address)
>+    {
>+        m_formatter.prefix(PRE_SSE_F3);
>+        m_formatter.twoByteOpAddr(OP2_MOVSD_WsdVsd, (RegisterID)src, bitwise_cast<uint32_t>(address));
>+    }
> #endif
> 
>     void mulsd_rr(XMMRegisterID src, XMMRegisterID dst)
>diff --git a/Source/JavaScriptCore/assembler/testmasm.cpp b/Source/JavaScriptCore/assembler/testmasm.cpp
>index e4fe153baff233b393895e5d708eb1221106d84c..7fef2dbe0e2b0c9ba5ccb801719aa8b48b6f9ff1 100644
>--- a/Source/JavaScriptCore/assembler/testmasm.cpp
>+++ b/Source/JavaScriptCore/assembler/testmasm.cpp
>@@ -246,6 +246,25 @@ static Vector<double> doubleOperands()
> }
> 
> 
>+static Vector<float> floatOperands()
>+{
>+    return Vector<float> {
>+        0,
>+        -0,
>+        1,
>+        -1,
>+        42,
>+        -42,
>+        std::numeric_limits<float>::max(),
>+        std::numeric_limits<float>::min(),
>+        std::numeric_limits<float>::lowest(),
>+        std::numeric_limits<float>::quiet_NaN(),
>+        std::numeric_limits<float>::infinity(),
>+        -std::numeric_limits<float>::infinity(),
>+    };
>+}
>+
>+
> void testCompareDouble(MacroAssembler::DoubleCondition condition)
> {
>     double arg1 = 0;
>@@ -287,6 +306,48 @@ void testCompareDouble(MacroAssembler::DoubleCondition condition)
>     }
> }
> 
>+#if CPU(X86) || CPU(X86_64) || CPU(ARM64)
>+void testCompareFloat(MacroAssembler::DoubleCondition condition)
>+{
>+    float arg1 = 0;
>+    float arg2 = 0;
>+
>+    auto compareFloat = compile([&, condition] (CCallHelpers& jit) {
>+        jit.emitFunctionPrologue();
>+
>+        jit.loadFloat(CCallHelpers::TrustedImmPtr(&arg1), FPRInfo::fpRegT0);
>+        jit.loadFloat(CCallHelpers::TrustedImmPtr(&arg2), FPRInfo::fpRegT1);
>+        jit.move(CCallHelpers::TrustedImm32(-1), GPRInfo::returnValueGPR);
>+        jit.compareFloat(condition, FPRInfo::fpRegT0, FPRInfo::fpRegT1, GPRInfo::returnValueGPR);
>+
>+        jit.emitFunctionEpilogue();
>+        jit.ret();
>+    });
>+
>+    auto compareFloatGeneric = compile([&, condition] (CCallHelpers& jit) {
>+        jit.emitFunctionPrologue();
>+
>+        jit.loadFloat(CCallHelpers::TrustedImmPtr(&arg1), FPRInfo::fpRegT0);
>+        jit.loadFloat(CCallHelpers::TrustedImmPtr(&arg2), FPRInfo::fpRegT1);
>+        jit.move(CCallHelpers::TrustedImm32(1), GPRInfo::returnValueGPR);
>+        auto jump = jit.branchFloat(condition, FPRInfo::fpRegT0, FPRInfo::fpRegT1);
>+        jit.move(CCallHelpers::TrustedImm32(0), GPRInfo::returnValueGPR);
>+        jump.link(&jit);
>+
>+        jit.emitFunctionEpilogue();
>+        jit.ret();
>+    });
>+
>+    auto operands = floatOperands();
>+    for (auto a : operands) {
>+        for (auto b : operands) {
>+            arg1 = a;
>+            arg2 = b;
>+            CHECK_EQ(invoke<int>(compareFloat), invoke<int>(compareFloatGeneric));
>+        }
>+    }
>+}
>+#endif
> 
> #if ENABLE(MASM_PROBE)
> void testProbeReadsArgumentRegisters()
>@@ -861,6 +922,21 @@ void run(const char* filter)
>     RUN(testCompareDouble(MacroAssembler::DoubleLessThanOrUnordered));
>     RUN(testCompareDouble(MacroAssembler::DoubleLessThanOrEqualOrUnordered));
> 
>+#if CPU(X86) || CPU(X86_64) || CPU(ARM64)
>+    RUN(testCompareFloat(MacroAssembler::DoubleEqual));
>+    RUN(testCompareFloat(MacroAssembler::DoubleNotEqual));
>+    RUN(testCompareFloat(MacroAssembler::DoubleGreaterThan));
>+    RUN(testCompareFloat(MacroAssembler::DoubleGreaterThanOrEqual));
>+    RUN(testCompareFloat(MacroAssembler::DoubleLessThan));
>+    RUN(testCompareFloat(MacroAssembler::DoubleLessThanOrEqual));
>+    RUN(testCompareFloat(MacroAssembler::DoubleEqualOrUnordered));
>+    RUN(testCompareFloat(MacroAssembler::DoubleNotEqualOrUnordered));
>+    RUN(testCompareFloat(MacroAssembler::DoubleGreaterThanOrUnordered));
>+    RUN(testCompareFloat(MacroAssembler::DoubleGreaterThanOrEqualOrUnordered));
>+    RUN(testCompareFloat(MacroAssembler::DoubleLessThanOrUnordered));
>+    RUN(testCompareFloat(MacroAssembler::DoubleLessThanOrEqualOrUnordered));
>+#endif
>+
> #if ENABLE(MASM_PROBE)
>     RUN(testProbeReadsArgumentRegisters());
>     RUN(testProbeWritesArgumentRegisters());

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185192: 339294 | 339295