Changeset 90065 in webkit

Timestamp:

Jun 29, 2011 4:50:12 PM (13 years ago)

Author:

barraclough@apple.com

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGSpeculativeJIT.cpp (modified) (8 diffs)
• dfg/DFGSpeculativeJIT.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-06-29  Gavin Barraclough  <barraclough@apple.com>
+
+        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  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -562 +562 @@
     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);
@@ -618 +618 @@
 
     case ArithSub: {
-        if (isInteger(node.child1) || isInteger(node.child2)) {
+        if (shouldSpeculateInteger(node.child1, node.child2)) {
             if (isInt32Constant(node.child2)) {
                 SpeculateIntegerOperand op1(this, node.child1);
@@ -639 +639 @@
             break;
         }
+
         SpeculateDoubleOperand op1(this, node.child1);
         SpeculateDoubleOperand op2(this, node.child2);
@@ -652 +653 @@
 
     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);
@@ -669 +670 @@
             break;
         }
+
         SpeculateDoubleOperand op1(this, node.child1);
         SpeculateDoubleOperand op2(this, node.child2);
@@ -745 +747 @@
             ASSERT(node.adjustedRefCount() == 1);
 
-            if (compareIsInteger(node.child1, node.child2))
+            if (shouldSpeculateInteger(node.child1, node.child2))
                 compilePeepHoleIntegerBranch(node, branchNodeIndex, JITCompiler::LessThan);
             else
@@ -777 +779 @@
             ASSERT(node.adjustedRefCount() == 1);
 
-            if (compareIsInteger(node.child1, node.child2))
+            if (shouldSpeculateInteger(node.child1, node.child2))
                 compilePeepHoleIntegerBranch(node, branchNodeIndex, JITCompiler::LessThanOrEqual);
             else
@@ -809 +811 @@
             ASSERT(node.adjustedRefCount() == 1);
 
-            if (compareIsInteger(node.child1, node.child2))
+            if (shouldSpeculateInteger(node.child1, node.child2))
                 compilePeepHoleIntegerBranch(node, branchNodeIndex, JITCompiler::Equal);
             else

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h

@@ -180 +180 @@
     }
 
-    bool compareIsInteger(NodeIndex op1, NodeIndex op2)
+    bool shouldSpeculateInteger(NodeIndex op1, NodeIndex op2)
     {
         return !(isDataFormatDouble(op1) || isDataFormatDouble(op2)) && (isInteger(op1) || isInteger(op2));