Changeset 199792 in webkit

Timestamp:

Apr 20, 2016 3:24:32 PM (8 years ago)

Author:

commit-queue@webkit.org

Message:

[JSC] Add register reuse for ArithAdd of an Int32 and constant in DFG
​https://bugs.webkit.org/show_bug.cgi?id=155164

Patch by Benjamin Poulain <​bpoulain@apple.com> on 2016-04-20
Reviewed by Mark Lam.

Every "inc" in loop was looking like this:

move rX, rY
inc rY
jo 0x230f4a200580

This patch add register Reuse to that case to remove
the extra "move".

• dfg/DFGOSRExit.h:

(JSC::DFG::SpeculationRecovery::SpeculationRecovery):
(JSC::DFG::SpeculationRecovery::immediate):

• dfg/DFGOSRExitCompiler32_64.cpp:

(JSC::DFG::OSRExitCompiler::compileExit):

• dfg/DFGOSRExitCompiler64.cpp:

(JSC::DFG::OSRExitCompiler::compileExit):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileArithAdd):

• tests/stress/arith-add-with-constant-overflow.js: Added.

(opaqueAdd):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssembler.h (modified) (1 diff)
• dfg/DFGOSRExit.h (modified) (3 diffs)
• dfg/DFGOSRExitCompiler32_64.cpp (modified) (1 diff)
• dfg/DFGOSRExitCompiler64.cpp (modified) (1 diff)
• dfg/DFGSpeculativeJIT.cpp (modified) (1 diff)
• tests/stress/arith-add-with-constant-overflow.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-04-20  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Add register reuse for ArithAdd of an Int32 and constant in DFG
+        https://bugs.webkit.org/show_bug.cgi?id=155164
+
+        Reviewed by Mark Lam.
+
+        Every "inc" in loop was looking like this:
+            move rX, rY
+            inc rY
+            jo 0x230f4a200580
+
+        This patch add register Reuse to that case to remove
+        the extra "move".
+
+        * dfg/DFGOSRExit.h:
+        (JSC::DFG::SpeculationRecovery::SpeculationRecovery):
+        (JSC::DFG::SpeculationRecovery::immediate):
+        * dfg/DFGOSRExitCompiler32_64.cpp:
+        (JSC::DFG::OSRExitCompiler::compileExit):
+        * dfg/DFGOSRExitCompiler64.cpp:
+        (JSC::DFG::OSRExitCompiler::compileExit):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileArithAdd):
+        * tests/stress/arith-add-with-constant-overflow.js: Added.
+        (opaqueAdd):
+
 2016-04-20  Saam barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/assembler/MacroAssembler.h

@@ -1695 +1695 @@
     Jump branchAdd32(ResultCondition cond, RegisterID src, Imm32 imm, RegisterID dest)
     {
-        if (src == dest)
-            ASSERT(haveScratchRegisterForBlinding());
-
         if (shouldBlind(imm)) {
-            if (src == dest) {
-                move(src, scratchRegisterForBlinding());
-                src = scratchRegisterForBlinding();
+            if (src != dest || haveScratchRegisterForBlinding()) {
+                if (src == dest) {
+                    move(src, scratchRegisterForBlinding());
+                    src = scratchRegisterForBlinding();
+                }
+                loadXorBlindedConstant(xorBlindConstant(imm), dest);
+                return branchAdd32(cond, src, dest);
             }
-            loadXorBlindedConstant(xorBlindConstant(imm), dest);
-            return branchAdd32(cond, src, dest);  
+            // If we don't have a scratch register available for use, we'll just
+            // place a random number of nops.
+            uint32_t nopCount = random() & 3;
+            while (nopCount--)
+                nop();
         }
         return branchAdd32(cond, src, imm.asTrustedImm32(), dest);            

trunk/Source/JavaScriptCore/dfg/DFGOSRExit.h

@@ -49 +49 @@
 // This enum describes the types of additional recovery that
 // may need be performed should a speculation check fail.
-enum SpeculationRecoveryType {
+enum SpeculationRecoveryType : uint8_t {
     SpeculativeAdd,
+    SpeculativeAddImmediate,
     BooleanSpeculationCheck
 };
@@ -61 +62 @@
 public:
     SpeculationRecovery(SpeculationRecoveryType type, GPRReg dest, GPRReg src)
-        : m_type(type)
+        : m_src(src)
         , m_dest(dest)
-        , m_src(src)
+        , m_type(type)
     {
+        ASSERT(m_type == SpeculativeAdd || m_type == BooleanSpeculationCheck);
+    }
+
+    SpeculationRecovery(SpeculationRecoveryType type, GPRReg dest, int32_t immediate)
+        : m_immediate(immediate)
+        , m_dest(dest)
+        , m_type(type)
+    {
+        ASSERT(m_type == SpeculativeAddImmediate);
     }
 
@@ -70 +80 @@
     GPRReg dest() { return m_dest; }
     GPRReg src() { return m_src; }
+    int32_t immediate() { return m_immediate; }
 
 private:
+    // different recovery types may required different additional information here.
+    union {
+        GPRReg m_src;
+        int32_t m_immediate;
+    };
+    GPRReg m_dest;
+
     // Indicates the type of additional recovery to be performed.
     SpeculationRecoveryType m_type;
-    // different recovery types may required different additional information here.
-    GPRReg m_dest;
-    GPRReg m_src;
 };
 

trunk/Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp

@@ -56 +56 @@
         case SpeculativeAdd:
             m_jit.sub32(recovery->src(), recovery->dest());
+            break;
+
+        case SpeculativeAddImmediate:
+            m_jit.sub32(AssemblyHelpers::Imm32(recovery->immediate()), recovery->dest());
             break;
             

trunk/Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp

@@ -60 +60 @@
         case SpeculativeAdd:
             m_jit.sub32(recovery->src(), recovery->dest());
+            m_jit.or64(GPRInfo::tagTypeNumberRegister, recovery->dest());
+            break;
+
+        case SpeculativeAddImmediate:
+            m_jit.sub32(AssemblyHelpers::Imm32(recovery->immediate()), recovery->dest());
             m_jit.or64(GPRInfo::tagTypeNumberRegister, recovery->dest());
             break;

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -3497 +3497 @@
         if (node->child2()->isInt32Constant()) {
             SpeculateInt32Operand op1(this, node->child1());
+            GPRTemporary result(this, Reuse, op1);
+
+            GPRReg gpr1 = op1.gpr();
             int32_t imm2 = node->child2()->asInt32();
+            GPRReg gprResult = result.gpr();
 
             if (!shouldCheckOverflow(node->arithMode())) {
-                GPRTemporary result(this, Reuse, op1);
-                m_jit.add32(Imm32(imm2), op1.gpr(), result.gpr());
-                int32Result(result.gpr(), node);
+                m_jit.add32(Imm32(imm2), gpr1, gprResult);
+                int32Result(gprResult, node);
                 return;
             }
 
-            GPRTemporary result(this);
-            speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branchAdd32(MacroAssembler::Overflow, op1.gpr(), Imm32(imm2), result.gpr()));
-
-            int32Result(result.gpr(), node);
+            MacroAssembler::Jump check = m_jit.branchAdd32(MacroAssembler::Overflow, gpr1, Imm32(imm2), gprResult);
+            if (gpr1 == gprResult) {
+                speculationCheck(Overflow, JSValueRegs(), 0, check,
+                    SpeculationRecovery(SpeculativeAddImmediate, gpr1, imm2));
+            } else
+                speculationCheck(Overflow, JSValueRegs(), 0, check);
+
+            int32Result(gprResult, node);
             return;
         }