[webkit-changes] [91807] trunk/Source/JavaScriptCore

Tue, 26 Jul 2011 18:28:02 -0700

Title: [91807] trunk/Source/_javascript_Core
Revision
91807
Author
[email protected]
Date
2011-07-26 18:27:43 -0700 (Tue, 26 Jul 2011)

Log Message

DFG speculative JIT never emits inline double comparisons, even when it
would be obvious more efficient to do so.
https://bugs.webkit.org/show_bug.cgi?id=65212

Patch by Filip Pizlo <[email protected]> on 2011-07-26
Reviewed by Gavin Barraclough.

This handles the obvious case of inlining double comparisons: it only addresses
the speculative JIT, and only for fused compare/branch sequences.  But it does
handle the case where both operands are double (and there is no slow path),
or where one operand is double and the other is unknown type (in which case it
attempts to unbox the double, otherwise taking slow path).  This is an 0.8%
speed-up on SunSpider.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::convertToDouble):
(JSC::DFG::SpeculativeJIT::compilePeepHoleDoubleBranch):
(JSC::DFG::SpeculativeJIT::compare):
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::isRegisterDataFormatDouble):
(JSC::DFG::SpeculativeJIT::shouldSpeculateInteger):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (91806 => 91807)


--- trunk/Source/_javascript_Core/ChangeLog	2011-07-27 01:12:50 UTC (rev 91806)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-07-27 01:27:43 UTC (rev 91807)
@@ -1,5 +1,29 @@
 2011-07-26  Filip Pizlo  <[email protected]>
 
+        DFG speculative JIT never emits inline double comparisons, even when it
+        would be obvious more efficient to do so.
+        https://bugs.webkit.org/show_bug.cgi?id=65212
+
+        Reviewed by Gavin Barraclough.
+        
+        This handles the obvious case of inlining double comparisons: it only addresses
+        the speculative JIT, and only for fused compare/branch sequences.  But it does
+        handle the case where both operands are double (and there is no slow path),
+        or where one operand is double and the other is unknown type (in which case it
+        attempts to unbox the double, otherwise taking slow path).  This is an 0.8%
+        speed-up on SunSpider.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::convertToDouble):
+        (JSC::DFG::SpeculativeJIT::compilePeepHoleDoubleBranch):
+        (JSC::DFG::SpeculativeJIT::compare):
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::isRegisterDataFormatDouble):
+        (JSC::DFG::SpeculativeJIT::shouldSpeculateInteger):
+
+2011-07-26  Filip Pizlo  <[email protected]>
+
         https://bugs.webkit.org/show_bug.cgi?id=64969
         DFG JIT generates inefficient code for speculation failures.

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (91806 => 91807)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2011-07-27 01:12:50 UTC (rev 91806)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2011-07-27 01:27:43 UTC (rev 91807)
@@ -383,8 +383,107 @@
         addBranch(m_jit.jump(), notTaken);
 }
 
+JITCompiler::Jump SpeculativeJIT::convertToDouble(GPRReg value, FPRReg result, GPRReg tmp)
+{
+    JITCompiler::Jump isInteger = m_jit.branchPtr(MacroAssembler::AboveOrEqual, value, GPRInfo::tagTypeNumberRegister);
+    
+    JITCompiler::Jump notNumber = m_jit.branchTestPtr(MacroAssembler::Zero, value, GPRInfo::tagTypeNumberRegister);
+    
+    m_jit.move(value, tmp);
+    m_jit.addPtr(GPRInfo::tagTypeNumberRegister, tmp);
+    m_jit.movePtrToDouble(tmp, result);
+    
+    JITCompiler::Jump done = m_jit.jump();
+    
+    isInteger.link(&m_jit);
+    
+    m_jit.convertInt32ToDouble(value, result);
+    
+    done.link(&m_jit);
+
+    return notNumber;
+}
+
+void SpeculativeJIT::compilePeepHoleDoubleBranch(Node& node, NodeIndex branchNodeIndex, JITCompiler::DoubleCondition condition, Z_DFGOperation_EJJ operation)
+{
+    Node& branchNode = m_jit.graph()[branchNodeIndex];
+    BlockIndex taken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.takenBytecodeOffset());
+    BlockIndex notTaken = m_jit.graph().blockIndexForBytecodeOffset(branchNode.notTakenBytecodeOffset());
+    
+    bool op1Numeric = isKnownNumeric(node.child1());
+    bool op2Numeric = isKnownNumeric(node.child2());
+    
+    if (op1Numeric && op2Numeric) {
+        SpeculateDoubleOperand op1(this, node.child1());
+        SpeculateDoubleOperand op2(this, node.child2());
+        
+        addBranch(m_jit.branchDouble(condition, op1.fpr(), op2.fpr()), taken);
+    } else if (op1Numeric) {
+        SpeculateDoubleOperand op1(this, node.child1());
+        JSValueOperand op2(this, node.child2());
+        
+        FPRTemporary fprTmp(this);
+        GPRTemporary gprTmp(this);
+        
+        FPRReg op1FPR = op1.fpr();
+        GPRReg op2GPR = op2.gpr();
+        FPRReg op2FPR = fprTmp.fpr();
+        GPRReg gpr = gprTmp.gpr();
+        
+        JITCompiler::Jump slowPath = convertToDouble(op2GPR, op2FPR, gpr);
+        
+        addBranch(m_jit.branchDouble(condition, op1FPR, op2FPR), taken);
+        addBranch(m_jit.jump(), notTaken);
+        
+        slowPath.link(&m_jit);
+        
+        boxDouble(op1FPR, gpr);
+        
+        silentSpillAllRegisters(gpr);
+        setupStubArguments(gpr, op2GPR);
+        m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+        appendCallWithExceptionCheck(operation);
+        m_jit.move(GPRInfo::returnValueGPR, gpr);
+        silentFillAllRegisters(gpr);
+        
+        addBranch(m_jit.branchTest8(JITCompiler::NonZero, gpr), taken);
+    } else {
+        JSValueOperand op1(this, node.child1());
+        SpeculateDoubleOperand op2(this, node.child2());
+        
+        FPRTemporary fprTmp(this);
+        GPRTemporary gprTmp(this);
+        
+        FPRReg op2FPR = op2.fpr();
+        GPRReg op1GPR = op1.gpr();
+        FPRReg op1FPR = fprTmp.fpr();
+        GPRReg gpr = gprTmp.gpr();
+        
+        JITCompiler::Jump slowPath = convertToDouble(op1GPR, op1FPR, gpr);
+        
+        addBranch(m_jit.branchDouble(condition, op1FPR, op2FPR), taken);
+        addBranch(m_jit.jump(), notTaken);
+        
+        slowPath.link(&m_jit);
+        
+        boxDouble(op2FPR, gpr);
+        
+        silentSpillAllRegisters(gpr);
+        setupStubArguments(op1GPR, gpr);
+        m_jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+        appendCallWithExceptionCheck(operation);
+        m_jit.move(GPRInfo::returnValueGPR, gpr);
+        silentFillAllRegisters(gpr);
+        
+        addBranch(m_jit.branchTest8(JITCompiler::NonZero, gpr), taken);
+    }
+    
+    if (notTaken != (m_block + 1))
+        addBranch(m_jit.jump(), notTaken);
+}
+
 // Returns true if the compare is fused with a subsequent branch.
-bool SpeculativeJIT::compare(Node& node, MacroAssembler::RelationalCondition condition, Z_DFGOperation_EJJ operation)
+bool SpeculativeJIT::compare(Node& node, MacroAssembler::RelationalCondition condition, MacroAssembler::DoubleCondition doubleCondition, Z_DFGOperation_EJJ operation)
 {
     // Fused compare & branch.
     NodeIndex branchNodeIndex = detectPeepHoleBranch();
@@ -397,6 +496,10 @@
             compilePeepHoleIntegerBranch(node, branchNodeIndex, condition);
             use(node.child1());
             use(node.child2());
+        } else if (isKnownNumeric(node.child1()) || isKnownNumeric(node.child2())) {
+            compilePeepHoleDoubleBranch(node, branchNodeIndex, doubleCondition, operation);
+            use(node.child1());
+            use(node.child2());
         } else
             nonSpeculativePeepholeBranch(node, branchNodeIndex, condition, operation);
 
@@ -750,22 +853,22 @@
     }
 
     case CompareLess:
-        if (compare(node, JITCompiler::LessThan, operationCompareLess))
+        if (compare(node, JITCompiler::LessThan, JITCompiler::DoubleLessThan, operationCompareLess))
             return;
         break;
 
     case CompareLessEq:
-        if (compare(node, JITCompiler::LessThanOrEqual, operationCompareLessEq))
+        if (compare(node, JITCompiler::LessThanOrEqual, JITCompiler::DoubleLessThanOrEqual, operationCompareLessEq))
             return;
         break;
 
     case CompareGreater:
-        if (compare(node, JITCompiler::GreaterThan, operationCompareGreater))
+        if (compare(node, JITCompiler::GreaterThan, JITCompiler::DoubleGreaterThan, operationCompareGreater))
             return;
         break;
 
     case CompareGreaterEq:
-        if (compare(node, JITCompiler::GreaterThanOrEqual, operationCompareGreaterEq))
+        if (compare(node, JITCompiler::GreaterThanOrEqual, JITCompiler::DoubleGreaterThanOrEqual, operationCompareGreaterEq))
             return;
         break;
 
@@ -780,7 +883,7 @@
                 return;
             break;
         }
-        if (compare(node, JITCompiler::Equal, operationCompareEq))
+        if (compare(node, JITCompiler::Equal, JITCompiler::DoubleEqual, operationCompareEq))
             return;
         break;

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (91806 => 91807)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2011-07-27 01:12:50 UTC (rev 91806)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2011-07-27 01:27:43 UTC (rev 91807)
@@ -156,7 +156,7 @@
             || (info.spillFormat() | DataFormatJS) == DataFormatJSInteger;
     }
 
-    bool isDataFormatDouble(NodeIndex nodeIndex)
+    bool isRegisterDataFormatDouble(NodeIndex nodeIndex)
     {
         Node& node = m_jit.graph()[nodeIndex];
         VirtualRegister virtualRegister = node.virtualRegister();
@@ -164,14 +164,17 @@
 
         return (info.registerFormat() | DataFormatJS) == DataFormatJSDouble;
     }
-
+    
     bool shouldSpeculateInteger(NodeIndex op1, NodeIndex op2)
     {
-        return !(isDataFormatDouble(op1) || isDataFormatDouble(op2)) && (isInteger(op1) || isInteger(op2));
+        return !(isRegisterDataFormatDouble(op1) || isRegisterDataFormatDouble(op2)) && (isInteger(op1) || isInteger(op2));
     }
 
-    bool compare(Node&, MacroAssembler::RelationalCondition, Z_DFGOperation_EJJ);
+    bool compare(Node&, MacroAssembler::RelationalCondition, MacroAssembler::DoubleCondition, Z_DFGOperation_EJJ);
     void compilePeepHoleIntegerBranch(Node&, NodeIndex branchNodeIndex, JITCompiler::RelationalCondition);
+    void compilePeepHoleDoubleBranch(Node&, NodeIndex branchNodeIndex, JITCompiler::DoubleCondition, Z_DFGOperation_EJJ);
+    
+    JITCompiler::Jump convertToDouble(GPRReg value, FPRReg result, GPRReg tmp);
 
     // Add a speculation check without additional recovery.
     void speculationCheck(MacroAssembler::Jump jumpToFail)

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to