[JSC] Use (x + x) instead of (x * 2) when possible
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 16 Aug 2015 08:13:50 +0000 (08:13 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 16 Aug 2015 08:13:50 +0000 (08:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148051

Patch by Benjamin Poulain <bpoulain@apple.com> on 2015-08-16
Reviewed by Michael Saboff.

When multiplying a number by 2, JSC was loading a constant "2"
in register and multiplying it with the first number:

    mov $0x4000000000000000, %rcx
    movd %rcx, %xmm0
    mulsd %xmm0, %xmm1

This is a problem for a few reasons.
1) "movd %rcx, %xmm0" only set half of XMM0. This instruction
   has to wait for any preceding instruction on XMM0 to finish
   before executing.
2) The load and transform itself is large and unecessary.

To fix that, I added a StrengthReductionPhase to transform
multiplications by 2 into a addition.

Unfortunately, that turned the code into:
    movsd %xmm0 %xmm1
    mulsd %xmm1 %xmm0

The reason is GenerationInfo::canReuse() was not accounting
for nodes using other nodes multiple times.

After fixing that too, we now have the multiplications by 2
done as:
    addsd %xmm0 %xmm0

* dfg/DFGGenerationInfo.h:
(JSC::DFG::GenerationInfo::useCount):
(JSC::DFG::GenerationInfo::canReuse): Deleted.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::FPRTemporary::FPRTemporary):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::canReuse):
(JSC::DFG::GPRTemporary::GPRTemporary):
* dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@188519 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGGenerationInfo.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp

index 0395a08..fe93c60 100644 (file)
@@ -1,3 +1,48 @@
+2015-08-16  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Use (x + x) instead of (x * 2) when possible
+        https://bugs.webkit.org/show_bug.cgi?id=148051
+
+        Reviewed by Michael Saboff.
+
+        When multiplying a number by 2, JSC was loading a constant "2"
+        in register and multiplying it with the first number:
+
+            mov $0x4000000000000000, %rcx
+            movd %rcx, %xmm0
+            mulsd %xmm0, %xmm1
+
+        This is a problem for a few reasons.
+        1) "movd %rcx, %xmm0" only set half of XMM0. This instruction
+           has to wait for any preceding instruction on XMM0 to finish
+           before executing.
+        2) The load and transform itself is large and unecessary.
+
+        To fix that, I added a StrengthReductionPhase to transform
+        multiplications by 2 into a addition.
+
+        Unfortunately, that turned the code into:
+            movsd %xmm0 %xmm1
+            mulsd %xmm1 %xmm0
+
+        The reason is GenerationInfo::canReuse() was not accounting
+        for nodes using other nodes multiple times.
+
+        After fixing that too, we now have the multiplications by 2
+        done as:
+            addsd %xmm0 %xmm0
+
+        * dfg/DFGGenerationInfo.h:
+        (JSC::DFG::GenerationInfo::useCount):
+        (JSC::DFG::GenerationInfo::canReuse): Deleted.
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::FPRTemporary::FPRTemporary):
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::canReuse):
+        (JSC::DFG::GPRTemporary::GPRTemporary):
+        * dfg/DFGStrengthReductionPhase.cpp:
+        (JSC::DFG::StrengthReductionPhase::handleNode):
+
 2015-08-14  Basile Clement  <basile_clement@apple.com>
 
         Occasional failure in v8-v6/v8-raytrace.js.ftl-eager
index b30f350..6390cda 100644 (file)
@@ -189,10 +189,10 @@ public:
     // Used to check the operands of operations to see if they are on
     // their last use; in some cases it may be safe to reuse the same
     // machine register for the result of the operation.
-    bool canReuse()
+    uint32_t useCount()
     {
         ASSERT(m_useCount);
-        return m_useCount == 1;
+        return m_useCount;
     }
 
     // Get the format of the value in machine registers (or 'none').
index 484bf8d..f5791db 100644 (file)
@@ -1186,6 +1186,8 @@ FPRTemporary::FPRTemporary(SpeculativeJIT* jit, SpeculateDoubleOperand& op1, Spe
         m_fpr = m_jit->reuse(op1.fpr());
     else if (m_jit->canReuse(op2.node()))
         m_fpr = m_jit->reuse(op2.fpr());
+    else if (m_jit->canReuse(op1.node(), op2.node()) && op1.fpr() == op2.fpr())
+        m_fpr = m_jit->reuse(op1.fpr());
     else
         m_fpr = m_jit->fprAllocate();
 }
index 545ff2a..ca07992 100644 (file)
@@ -162,7 +162,11 @@ public:
     // and its machine registers may be reused.
     bool canReuse(Node* node)
     {
-        return generationInfo(node).canReuse();
+        return generationInfo(node).useCount() == 1;
+    }
+    bool canReuse(Node* nodeA, Node* nodeB)
+    {
+        return nodeA == nodeB && generationInfo(nodeA).useCount() == 2;
     }
     bool canReuse(Edge nodeUse)
     {
@@ -2717,6 +2721,8 @@ public:
             m_gpr = m_jit->reuse(op1.gpr());
         else if (m_jit->canReuse(op2.node()))
             m_gpr = m_jit->reuse(op2.gpr());
+        else if (m_jit->canReuse(op1.node(), op2.node()) && op1.gpr() == op2.gpr())
+            m_gpr = m_jit->reuse(op1.gpr());
         else
             m_gpr = m_jit->allocate();
     }
index 03515d5..2130b60 100644 (file)
@@ -114,10 +114,36 @@ private:
             }
             break;
             
-        case ArithMul:
+        case ArithMul: {
             handleCommutativity();
+            Edge& child2 = m_node->child2();
+            if (child2->isNumberConstant() && child2->asNumber() == 2) {
+                switch (m_node->binaryUseKind()) {
+                case DoubleRepUse:
+                    // It is always valuable to get rid of a double multiplication by 2.
+                    // We won't have half-register dependencies issues on x86 and we won't have to load the constants.
+                    m_node->setOp(ArithAdd);
+                    child2.setNode(m_node->child1().node());
+                    m_changed = true;
+                    break;
+#if USE(JSVALUE64)
+                case Int52RepUse:
+#endif
+                case Int32Use:
+                    // For integers, we can only convert compatible modes.
+                    // ArithAdd does handle do negative zero check for example.
+                    if (m_node->arithMode() == Arith::CheckOverflow || m_node->arithMode() == Arith::Unchecked) {
+                        m_node->setOp(ArithAdd);
+                        child2.setNode(m_node->child1().node());
+                        m_changed = true;
+                    }
+                    break;
+                default:
+                    break;
+                }
+            }
             break;
-            
+        }
         case ArithSub:
             if (m_node->child2()->isInt32Constant()
                 && m_node->isBinaryUseKind(Int32Use)) {