Attachment 339706 Details for Bug 185371 – Patch

[patch] Patch

bug-185371-20180507153403.patch (text/plain), 10.07 KB, created by Yusuke Suzuki on 2018-05-06 23:34:04 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-06 23:34:04 PDT

Size: 10.07 KB

patch

obsolete

>Subversion Revision: 231404
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index fb2176fa3b51d9f8651b978d2fc78ae7bba88687..da593710b63cfb81d077fbee99a35e69ff3f3bce 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2018-05-06  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [DFG][MIPS] Simplify DFG code by increasing MIPS temporary registers
>+        https://bugs.webkit.org/show_bug.cgi?id=185371
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Since MIPS GPRInfo claims it has only 7 registers, some of DFG code exhausts registers.
>+        As a result, we need to maintain separated code for MIPS. This increases DFG maintenance burden,
>+        but actually MIPS have much more registers.
>+
>+        This patch adds $a0 - $a3 to temporary registers. This is OK since our temporary registers can be overlapped with
>+        argument registers (see ARM, X86 implementations). These registers are caller-save ones, so we do not need to
>+        have extra mechanism.
>+
>+        Then, we remove several unnecessary MIPS code in our JIT infrastructure.
>+
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::handleIntrinsicCall):
>+        * dfg/DFGFixupPhase.cpp:
>+        (JSC::DFG::FixupPhase::fixupNode):
>+        * dfg/DFGSpeculativeJIT32_64.cpp:
>+        (JSC::DFG::SpeculativeJIT::compile):
>+        * jit/CCallHelpers.h:
>+        * jit/GPRInfo.h:
>+        (JSC::GPRInfo::toRegister):
>+        (JSC::GPRInfo::toIndex):
>+        * offlineasm/mips.rb:
>+
> 2018-05-06  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [JSC] Remove "using namespace std;" from JSC, bmalloc, WTF
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index 84a784ce6b371c5bd8706ebecbf0457262834790..35e35227b2da2a3f462e3e345374c738e984d9c1 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -2200,7 +2200,7 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
>         
>     case ArrayPushIntrinsic: {
> #if USE(JSVALUE32_64)
>-        if (isX86() || isMIPS()) {
>+        if (isX86()) {
>             if (argumentCountIncludingThis > 2)
>                 return false;
>         }
>@@ -2235,7 +2235,7 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
> 
>     case ArraySliceIntrinsic: {
> #if USE(JSVALUE32_64)
>-        if (isX86() || isMIPS()) {
>+        if (isX86()) {
>             // There aren't enough registers for this to be done easily.
>             return false;
>         }
>diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>index 0d6bfef982f28df18d402453050dee07d8f8f2c0..26ad695986c1de1944b85eda63da0bbe93cbbb1c 100644
>--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>@@ -1477,7 +1477,7 @@ class FixupPhase : public Phase {
> 
>         case HasOwnProperty: {
>             fixEdge<ObjectUse>(node->child1());
>-#if (CPU(X86) || CPU(MIPS)) && USE(JSVALUE32_64)
>+#if CPU(X86) && USE(JSVALUE32_64)
>             // We don't have enough registers to do anything interesting on x86 and mips.
>             fixEdge<UntypedUse>(node->child2());
> #else
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>index 962bd20ed839ea5623e1fda0cb21b389ab1f2674..71f856d2fe0c3ab5d3b506a6a0a59e1888c8b03d 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>@@ -2701,48 +2701,6 @@ void SpeculativeJIT::compile(Node* node)
>         m_jit.poke(valuePayload, index++);
>         m_jit.poke(valueTag, index++);
> 
>-        flushRegisters();
>-        appendCall(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValWithThisStrict : operationPutByValWithThis);
>-        m_jit.exceptionCheck();
>-#elif CPU(MIPS)
>-        // We don't have enough registers on MIPS either but the ABI is a little different.
>-        unsigned index = 4;
>-        m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
>-        {
>-            JSValueOperand base(this, m_jit.graph().varArgChild(node, 0));
>-            GPRReg baseTag = base.tagGPR();
>-            GPRReg basePayload = base.payloadGPR();
>-
>-            JSValueOperand thisValue(this, m_jit.graph().varArgChild(node, 1));
>-            GPRReg thisValueTag = thisValue.tagGPR();
>-            GPRReg thisValuePayload = thisValue.payloadGPR();
>-
>-            JSValueOperand property(this, m_jit.graph().varArgChild(node, 2));
>-            GPRReg propertyTag = property.tagGPR();
>-            GPRReg propertyPayload = property.payloadGPR();
>-
>-            // for operationPutByValWithThis[Strict](), base is a 64 bits
>-            // argument, so it should be double word aligned on the stack.
>-            // This requirement still applies when it's in argument registers
>-            // instead of on the stack.
>-            m_jit.move(basePayload, GPRInfo::argumentGPR2);
>-            m_jit.move(baseTag, GPRInfo::argumentGPR3);
>-
>-            m_jit.poke(thisValuePayload, index++);
>-            m_jit.poke(thisValueTag, index++);
>-
>-            m_jit.poke(propertyPayload, index++);
>-            m_jit.poke(propertyTag, index++);
>-
>-            flushRegisters();
>-        }
>-
>-        JSValueOperand value(this, m_jit.graph().varArgChild(node, 3));
>-        GPRReg valueTag = value.tagGPR();
>-        GPRReg valuePayload = value.payloadGPR();
>-        m_jit.poke(valuePayload, index++);
>-        m_jit.poke(valueTag, index++);
>-
>         flushRegisters();
>         appendCall(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValWithThisStrict : operationPutByValWithThis);
>         m_jit.exceptionCheck();
>@@ -3804,7 +3762,7 @@ void SpeculativeJIT::compile(Node* node)
>         break;
> 
>     case HasOwnProperty: {
>-#if CPU(X86) || CPU(MIPS)
>+#if CPU(X86)
>         ASSERT(node->child2().useKind() == UntypedUse);
>         SpeculateCellOperand object(this, node->child1());
>         JSValueOperand key(this, node->child2());
>diff --git a/Source/JavaScriptCore/jit/CCallHelpers.h b/Source/JavaScriptCore/jit/CCallHelpers.h
>index ce7cb2f73f0bd4c288f8b27331ff91fd4117deb1..31a34415c92eeb1b904a2e7f3bf607fab971d074 100644
>--- a/Source/JavaScriptCore/jit/CCallHelpers.h
>+++ b/Source/JavaScriptCore/jit/CCallHelpers.h
>@@ -41,14 +41,6 @@ namespace JSC {
> #define POKE_ARGUMENT_OFFSET 0
> #endif
> 
>-// EncodedJSValue in JSVALUE32_64 is a 64-bit integer. When being compiled in ARM EABI, it must be aligned even-numbered register (r0, r2 or [sp]).
>-// To avoid assemblies from using wrong registers, let's occupy r1 or r3 with a dummy argument when necessary.
>-#if (COMPILER_SUPPORTS(EABI) && CPU(ARM)) || CPU(MIPS)
>-#define EABI_32BIT_DUMMY_ARG      CCallHelpers::TrustedImm32(0),
>-#else
>-#define EABI_32BIT_DUMMY_ARG
>-#endif
>-
> class ExecState;
> class Structure;
> namespace DFG {
>diff --git a/Source/JavaScriptCore/jit/GPRInfo.h b/Source/JavaScriptCore/jit/GPRInfo.h
>index 7e21054cf7a9487041de2a515b0f24cbee52b157..8a086ddb50d0495cbfd293e4f7b7d56a9466ed18 100644
>--- a/Source/JavaScriptCore/jit/GPRInfo.h
>+++ b/Source/JavaScriptCore/jit/GPRInfo.h
>@@ -729,7 +729,7 @@ class GPRInfo {
> class GPRInfo {
> public:
>     typedef GPRReg RegisterType;
>-    static const unsigned numberOfRegisters = 7;
>+    static const unsigned numberOfRegisters = 11;
>     static const unsigned numberOfArgumentRegisters = NUMBER_OF_ARGUMENT_REGISTERS;
> 
>     // regT0 must be v0 for returning a 32-bit value.
>@@ -743,6 +743,10 @@ class GPRInfo {
>     static const GPRReg regT4 = MIPSRegisters::t4;
>     static const GPRReg regT5 = MIPSRegisters::t5;
>     static const GPRReg regT6 = MIPSRegisters::t6;
>+    static const GPRReg regT7 = MIPSRegisters::a0;
>+    static const GPRReg regT8 = MIPSRegisters::a1;
>+    static const GPRReg regT9 = MIPSRegisters::a2;
>+    static const GPRReg regT10 = MIPSRegisters::a3;
>     // These registers match the baseline JIT.
>     static const GPRReg callFrameRegister = MIPSRegisters::fp;
>     // These constants provide the names for the general purpose argument & return value registers.
>@@ -758,7 +762,7 @@ class GPRInfo {
>     static GPRReg toRegister(unsigned index)
>     {
>         ASSERT(index < numberOfRegisters);
>-        static const GPRReg registerForIndex[numberOfRegisters] = { regT0, regT1, regT2, regT3, regT4, regT5, regT6 };
>+        static const GPRReg registerForIndex[numberOfRegisters] = { regT0, regT1, regT2, regT3, regT4, regT5, regT6, regT7, regT8, regT9, regT10 };
>         return registerForIndex[index];
>     }
> 
>@@ -774,7 +778,7 @@ class GPRInfo {
>         ASSERT(reg != InvalidGPRReg);
>         ASSERT(reg < 32);
>         static const unsigned indexForRegister[32] = {
>-            InvalidIndex, InvalidIndex, 0, 1, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex,
>+            InvalidIndex, InvalidIndex, 0, 1, 7, 8, 9, 10,
>             InvalidIndex, InvalidIndex, 2, 3, 4, 5, 6, InvalidIndex,
>             InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex,
>             InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex
>diff --git a/Source/JavaScriptCore/offlineasm/mips.rb b/Source/JavaScriptCore/offlineasm/mips.rb
>index 1cd368853d65571dc776cfcc79d058fc0104e741..8b71f287f0ac2ccf17091efe99afe1c954d563e2 100644
>--- a/Source/JavaScriptCore/offlineasm/mips.rb
>+++ b/Source/JavaScriptCore/offlineasm/mips.rb
>@@ -26,10 +26,10 @@
> 
> # GPR conventions, to match the baseline JIT
> #
>-# $a0 => a0
>-# $a1 => a1
>-# $a2 => a2
>-# $a3 => a3
>+# $a0 => a0, t7
>+# $a1 => a1, t8
>+# $a2 => a2, t9
>+# $a3 => a3, t10
> # $v0 => t0, r0
> # $v1 => t1, r1
> # $t0 =>            (scratch)
>@@ -113,13 +113,13 @@ def mipsMoveImmediate(value, register)
> class RegisterID
>     def mipsOperand
>         case name
>-        when "a0"
>+        when "a0", "t7"
>             "$a0"
>-        when "a1"
>+        when "a1", "t8"
>             "$a1"
>-        when "a2"
>+        when "a2", "t9"
>             "$a2"
>-        when "a3"
>+        when "a3", "t10"
>             "$a3"
>         when "t0", "r0"
>             "$v0"

Flags:

mark.lam: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 185371: 339706 | 339712