Changeset 197655 in webkit

Timestamp:

Mar 6, 2016, 7:21:08 PM (11 years ago)

Author:

benjamin@webkit.org

Message:

[JSC] Improve DFG's Int32 ArithMul if one operand is a constant
​https://bugs.webkit.org/show_bug.cgi?id=155066

Reviewed by Filip Pizlo.

When multiplying an integer by a constant, DFG was doing quite
a bit worse than baseline JIT.
We were loading the constant into a register, doing the multiply,
the checking the result and both operands for negative zero.

This patch changes:
-Use the multiply-by-immediate form on x86.
-Do as few checks as possible to detect negative-zero.

In most cases, this reduce the negative-zero checks
to zero or one TEST+JUMP.

• assembler/MacroAssembler.h:

(JSC::MacroAssembler::mul32):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileArithMul):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssembler.h (modified) (2 diffs)
• dfg/DFGSpeculativeJIT.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-03-06  Benjamin Poulain  <benjamin@webkit.org>
+
+        [JSC] Improve DFG's Int32 ArithMul if one operand is a constant
+        https://bugs.webkit.org/show_bug.cgi?id=155066
+
+        Reviewed by Filip Pizlo.
+
+        When multiplying an integer by a constant, DFG was doing quite
+        a bit worse than baseline JIT.
+        We were loading the constant into a register, doing the multiply,
+        the checking the result and both operands for negative zero.
+
+        This patch changes:
+        -Use the multiply-by-immediate form on x86.
+        -Do as few checks as possible to detect negative-zero.
+
+        In most cases, this reduce the negative-zero checks
+        to zero or one TEST+JUMP.
+
+        * assembler/MacroAssembler.h:
+        (JSC::MacroAssembler::mul32):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileArithMul):
+
 2016-03-06  Benjamin Poulain  <benjamin@webkit.org>
 

trunk/Source/JavaScriptCore/assembler/MacroAssembler.h

@@ -116 +116 @@
     using MacroAssemblerBase::move;
     using MacroAssemblerBase::add32;
+    using MacroAssemblerBase::mul32;
     using MacroAssemblerBase::and32;
     using MacroAssemblerBase::branchAdd32;
@@ -1489 +1490 @@
     }
 
+    void mul32(Imm32 imm, RegisterID src, RegisterID dest)
+    {
+        if (shouldBlind(imm)) {
+            if (src != dest || haveScratchRegisterForBlinding()) {
+                if (src == dest) {
+                    move(src, scratchRegisterForBlinding());
+                    src = scratchRegisterForBlinding();
+                }
+                loadXorBlindedConstant(xorBlindConstant(imm), dest);
+                mul32(src, dest);
+                return;
+            }
+            // 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();
+        }
+        mul32(imm.asTrustedImm32(), src, dest);
+    }
+
     void and32(Imm32 imm, RegisterID dest)
     {

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -3672 +3672 @@
     switch (node->binaryUseKind()) {
     case Int32Use: {
+        if (node->child2()->isInt32Constant()) {
+            SpeculateInt32Operand op1(this, node->child1());
+            GPRTemporary result(this);
+
+            int32_t imm = node->child2()->asInt32();
+            GPRReg op1GPR = op1.gpr();
+            GPRReg resultGPR = result.gpr();
+
+            if (!shouldCheckOverflow(node->arithMode()))
+                m_jit.mul32(Imm32(imm), op1GPR, resultGPR);
+            else {
+                speculationCheck(Overflow, JSValueRegs(), 0,
+                    m_jit.branchMul32(MacroAssembler::Overflow, op1GPR, Imm32(imm), resultGPR));
+            }
+
+            // The only way to create negative zero with a constant is:
+            // -negative-op1 * 0.
+            // -zero-op1 * negative constant.
+            if (shouldCheckNegativeZero(node->arithMode())) {
+                if (!imm)
+                    speculationCheck(NegativeZero, JSValueRegs(), 0, m_jit.branchTest32(MacroAssembler::Signed, op1GPR));
+                else if (imm < 0) {
+                    if (shouldCheckOverflow(node->arithMode()))
+                        speculationCheck(NegativeZero, JSValueRegs(), 0, m_jit.branchTest32(MacroAssembler::Zero, resultGPR));
+                    else
+                        speculationCheck(NegativeZero, JSValueRegs(), 0, m_jit.branchTest32(MacroAssembler::Zero, op1GPR));
+                }
+            }
+
+            int32Result(resultGPR, node);
+            return;
+        }
         SpeculateInt32Operand op1(this, node->child1());
         SpeculateInt32Operand op2(this, node->child2());
@@ -3694 +3726 @@
         if (shouldCheckNegativeZero(node->arithMode())) {
             MacroAssembler::Jump resultNonZero = m_jit.branchTest32(MacroAssembler::NonZero, result.gpr());
-            speculationCheck(NegativeZero, JSValueRegs(), 0, m_jit.branch32(MacroAssembler::LessThan, reg1, TrustedImm32(0)));
-            speculationCheck(NegativeZero, JSValueRegs(), 0, m_jit.branch32(MacroAssembler::LessThan, reg2, TrustedImm32(0)));
+            speculationCheck(NegativeZero, JSValueRegs(), 0, m_jit.branchTest32(MacroAssembler::Signed, reg1));
+            speculationCheck(NegativeZero, JSValueRegs(), 0, m_jit.branchTest32(MacroAssembler::Signed, reg2));
             resultNonZero.link(&m_jit);
         }
@@ -3702 +3734 @@
         return;
     }
-    
-#if USE(JSVALUE64)   
+
+#if USE(JSVALUE64)
     case Int52RepUse: {
         ASSERT(shouldCheckOverflow(node->arithMode()));