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

Wed, 29 Jun 2011 16:50:22 -0700

Title: [90065] trunk/Source/_javascript_Core
Revision
90065
Author
[email protected]
Date
2011-06-29 16:50:12 -0700 (Wed, 29 Jun 2011)

Log Message

https://bugs.webkit.org/show_bug.cgi?id=63669
DFG JIT - fix spectral-norm regression

Reviewed by Geoff Garen.

The problem is a mis-speculation leading to us falling off the speculative path.
Make the speculation logic slightly smarter, don't predict int if one of the
operands is already loaded as a double (we use this logic already for compares).

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::shouldSpeculateInteger):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (90064 => 90065)


--- trunk/Source/_javascript_Core/ChangeLog	2011-06-29 23:45:52 UTC (rev 90064)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-06-29 23:50:12 UTC (rev 90065)
@@ -1,3 +1,19 @@
+2011-06-29  Gavin Barraclough  <[email protected]>
+
+        Reviewed by Geoff Garen.
+
+        https://bugs.webkit.org/show_bug.cgi?id=63669
+        DFG JIT - fix spectral-norm regression
+
+        The problem is a mis-speculation leading to us falling off the speculative path.
+        Make the speculation logic slightly smarter, don't predict int if one of the
+        operands is already loaded as a double (we use this logic already for compares).
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::shouldSpeculateInteger):
+
 2011-06-29  Filip Pizlo  <[email protected]>
 
         Reviewed by Gavin Barraclough.

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (90064 => 90065)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2011-06-29 23:45:52 UTC (rev 90064)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2011-06-29 23:50:12 UTC (rev 90065)
@@ -561,7 +561,7 @@
 
     case ValueAdd:
     case ArithAdd: {
-        if (isInteger(node.child1) || isInteger(node.child2)) {
+        if (shouldSpeculateInteger(node.child1, node.child2)) {
             if (isInt32Constant(node.child1)) {
                 int32_t imm1 = valueOfInt32Constant(node.child1);
                 SpeculateIntegerOperand op2(this, node.child2);
@@ -617,7 +617,7 @@
     }
 
     case ArithSub: {
-        if (isInteger(node.child1) || isInteger(node.child2)) {
+        if (shouldSpeculateInteger(node.child1, node.child2)) {
             if (isInt32Constant(node.child2)) {
                 SpeculateIntegerOperand op1(this, node.child1);
                 int32_t imm2 = valueOfInt32Constant(node.child2);
@@ -638,6 +638,7 @@
             integerResult(result.gpr(), m_compileIndex);
             break;
         }
+
         SpeculateDoubleOperand op1(this, node.child1);
         SpeculateDoubleOperand op2(this, node.child2);
         FPRTemporary result(this, op1);
@@ -651,7 +652,7 @@
     }
 
     case ArithMul: {
-        if (isInteger(node.child1) && isInteger(node.child2)) {
+        if (shouldSpeculateInteger(node.child1, node.child2)) {
             SpeculateIntegerOperand op1(this, node.child1);
             SpeculateIntegerOperand op2(this, node.child2);
             GPRTemporary result(this);
@@ -668,6 +669,7 @@
             integerResult(result.gpr(), m_compileIndex);
             break;
         }
+
         SpeculateDoubleOperand op1(this, node.child1);
         SpeculateDoubleOperand op2(this, node.child2);
         FPRTemporary result(this, op1, op2);
@@ -744,7 +746,7 @@
             // so can be no intervening nodes to also reference the compare. 
             ASSERT(node.adjustedRefCount() == 1);
 
-            if (compareIsInteger(node.child1, node.child2))
+            if (shouldSpeculateInteger(node.child1, node.child2))
                 compilePeepHoleIntegerBranch(node, branchNodeIndex, JITCompiler::LessThan);
             else
                 compilePeepHoleCall(node, branchNodeIndex, operationCompareLess);
@@ -776,7 +778,7 @@
             // so can be no intervening nodes to also reference the compare. 
             ASSERT(node.adjustedRefCount() == 1);
 
-            if (compareIsInteger(node.child1, node.child2))
+            if (shouldSpeculateInteger(node.child1, node.child2))
                 compilePeepHoleIntegerBranch(node, branchNodeIndex, JITCompiler::LessThanOrEqual);
             else
                 compilePeepHoleCall(node, branchNodeIndex, operationCompareLessEq);
@@ -808,7 +810,7 @@
             // so can be no intervening nodes to also reference the compare. 
             ASSERT(node.adjustedRefCount() == 1);
 
-            if (compareIsInteger(node.child1, node.child2))
+            if (shouldSpeculateInteger(node.child1, node.child2))
                 compilePeepHoleIntegerBranch(node, branchNodeIndex, JITCompiler::Equal);
             else
                 compilePeepHoleCall(node, branchNodeIndex, operationCompareEq);

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (90064 => 90065)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2011-06-29 23:45:52 UTC (rev 90064)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2011-06-29 23:50:12 UTC (rev 90065)
@@ -179,7 +179,7 @@
         return (info.registerFormat() | DataFormatJS) == DataFormatJSDouble;
     }
 
-    bool compareIsInteger(NodeIndex op1, NodeIndex op2)
+    bool shouldSpeculateInteger(NodeIndex op1, NodeIndex op2)
     {
         return !(isDataFormatDouble(op1) || isDataFormatDouble(op2)) && (isInteger(op1) || isInteger(op2));
     }

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

Reply via email to