DFG should not speculate array even when predictions say that the base is not an...

https://bugs.webkit.org/show_bug.cgi?id=75160
<rdar://problem/10622646>
<rdar://problem/10622649>

Reviewed by Oliver Hunt.

Added the ability to call slow path when the base is known to not be an array.
Also rationalized the logic for deciding when the index is not an int, and
cleaned up the logic for deciding when to speculate typed array.

Neutral for the most part, with odd speed-ups and slow-downs. The slow-downs can
likely be mitigated by having the notion of a polymorphic array access, where we
try, but don't speculate, to access the array one way before either trying some
other ways or calling slow path.

* bytecode/PredictedType.h:
(JSC::isActionableMutableArrayPrediction):
(JSC::isActionableArrayPrediction):
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGNode.h:
(JSC::DFG::Node::shouldSpeculateInt8Array):
(JSC::DFG::Node::shouldSpeculateInt16Array):
(JSC::DFG::Node::shouldSpeculateInt32Array):
(JSC::DFG::Node::shouldSpeculateUint8Array):
(JSC::DFG::Node::shouldSpeculateUint16Array):
(JSC::DFG::Node::shouldSpeculateUint32Array):
(JSC::DFG::Node::shouldSpeculateFloat32Array):
(JSC::DFG::Node::shouldSpeculateFloat64Array):
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::byValIsPure):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetIndexedPropertyStorage):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 859eec98432e13834e0f61b63a69c72b86273f3b..31a8517150b6b97adb3dcbffa4e727342fc340ec 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,44 @@
+2011-12-22  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG should not speculate array even when predictions say that the base is not an array
+        https://bugs.webkit.org/show_bug.cgi?id=75160
+        <rdar://problem/10622646>
+        <rdar://problem/10622649>
+
+        Reviewed by Oliver Hunt.
+        
+        Added the ability to call slow path when the base is known to not be an array.
+        Also rationalized the logic for deciding when the index is not an int, and
+        cleaned up the logic for deciding when to speculate typed array.
+        
+        Neutral for the most part, with odd speed-ups and slow-downs. The slow-downs can
+        likely be mitigated by having the notion of a polymorphic array access, where we
+        try, but don't speculate, to access the array one way before either trying some
+        other ways or calling slow path.
+
+        * bytecode/PredictedType.h:
+        (JSC::isActionableMutableArrayPrediction):
+        (JSC::isActionableArrayPrediction):
+        * dfg/DFGAbstractState.cpp:
+        (JSC::DFG::AbstractState::execute):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::shouldSpeculateInt8Array):
+        (JSC::DFG::Node::shouldSpeculateInt16Array):
+        (JSC::DFG::Node::shouldSpeculateInt32Array):
+        (JSC::DFG::Node::shouldSpeculateUint8Array):
+        (JSC::DFG::Node::shouldSpeculateUint16Array):
+        (JSC::DFG::Node::shouldSpeculateUint32Array):
+        (JSC::DFG::Node::shouldSpeculateFloat32Array):
+        (JSC::DFG::Node::shouldSpeculateFloat64Array):
+        * dfg/DFGPropagator.cpp:
+        (JSC::DFG::Propagator::byValIsPure):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetIndexedPropertyStorage):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+
 2011-12-22  Gavin Barraclough  <barraclough@apple.com>
 
         Unreviewed - fix stylebot issues from last patch.

diff --git a/Source/JavaScriptCore/bytecode/PredictedType.h b/Source/JavaScriptCore/bytecode/PredictedType.h
index 94da64edac93908805369549c5911a1b3ec5e1db..5835fdb746cefda7cfc3ad4ef69377c39096597c 100644 (file)
--- a/Source/JavaScriptCore/bytecode/PredictedType.h
+++ b/Source/JavaScriptCore/bytecode/PredictedType.h
@@ -151,6 +151,28 @@ inline bool isFloat64ArrayPrediction(PredictedType value)
     return value == PredictFloat64Array;
 }
 
+inline bool isActionableMutableArrayPrediction(PredictedType value)
+{
+    return isArrayPrediction(value)
+        || isByteArrayPrediction(value)
+        || isInt8ArrayPrediction(value)
+        || isInt16ArrayPrediction(value)
+        || isInt32ArrayPrediction(value)
+        || isUint8ArrayPrediction(value)
+        || isUint16ArrayPrediction(value)
+        || isUint32ArrayPrediction(value)
+#if CPU(X86) || CPU(X86_64)
+        || isFloat32ArrayPrediction(value)
+#endif
+        || isFloat64ArrayPrediction(value);
+}
+
+inline bool isActionableArrayPrediction(PredictedType value)
+{
+    return isStringPrediction(value)
+        || isActionableMutableArrayPrediction(value);
+}
+
 inline bool isArrayOrOtherPrediction(PredictedType value)
 {
     return !!(value & (PredictArray | PredictOther)) && !(value & ~(PredictArray | PredictOther));

diff --git a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
index d2e7c083ecbd6df4985be466432e64660e50d93e..40ad857cf723da3caef533c3a27a43af18fa04c3 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
+++ b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
@@ -402,8 +402,11 @@ bool AbstractState::execute(NodeIndex nodeIndex)
         break;
             
     case GetByVal: {
-        PredictedType indexPrediction = m_graph[node.child2()].prediction();
-        if (!(indexPrediction & PredictInt32) && indexPrediction) {
+        if (!node.prediction() || !m_graph[node.child1()].prediction() || !m_graph[node.child2()].prediction()) {
+            m_isValid = false;
+            break;
+        }
+        if (!isActionableArrayPrediction(m_graph[node.child1()].prediction()) || !m_graph[node.child2()].shouldSpeculateInteger()) {
             clobberStructures(nodeIndex);
             forNode(nodeIndex).makeTop();
             break;
@@ -469,6 +472,7 @@ bool AbstractState::execute(NodeIndex nodeIndex)
             forNode(nodeIndex).set(PredictDouble);
             break;
         }
+        ASSERT(m_graph[node.child1()].shouldSpeculateArray());
         forNode(node.child1()).filter(PredictArray);
         forNode(node.child2()).filter(PredictInt32);
         forNode(nodeIndex).makeTop();
@@ -477,8 +481,12 @@ bool AbstractState::execute(NodeIndex nodeIndex)
             
     case PutByVal:
     case PutByValAlias: {
-        PredictedType indexPrediction = m_graph[node.child2()].prediction();
-        if (!(indexPrediction & PredictInt32) && indexPrediction) {
+        if (!m_graph[node.child1()].prediction() || !m_graph[node.child2()].prediction()) {
+            m_isValid = false;
+            break;
+        }
+        if (!m_graph[node.child2()].shouldSpeculateInteger() || !isActionableMutableArrayPrediction(m_graph[node.child1()].prediction())) {
+            ASSERT(node.op == PutByVal);
             clobberStructures(nodeIndex);
             forNode(nodeIndex).makeTop();
             break;
@@ -538,7 +546,7 @@ bool AbstractState::execute(NodeIndex nodeIndex)
             forNode(node.child3()).filter(PredictNumber);
             break;
         }
-            
+        ASSERT(m_graph[node.child1()].shouldSpeculateArray());
         forNode(node.child1()).filter(PredictArray);
         forNode(node.child2()).filter(PredictInt32);
         break;

diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
index 0e834d2388bab7620cb3dcf1ebb0f23254e4e8a2..30232140c5c84984aae23950ea1316708ef4b07a 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGNode.h
+++ b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -931,38 +931,38 @@ struct Node {
     
     bool shouldSpeculateInt8Array()
     {
-        return prediction() == PredictInt8Array;
+        return isInt8ArrayPrediction(prediction());
     }
     
     bool shouldSpeculateInt16Array()
     {
-        return prediction() == PredictInt16Array;
+        return isInt16ArrayPrediction(prediction());
     }
     
     bool shouldSpeculateInt32Array()
     {
-        return prediction() == PredictInt32Array;
+        return isInt32ArrayPrediction(prediction());
     }
     
     bool shouldSpeculateUint8Array()
     {
-        return prediction() == PredictUint8Array;
+        return isUint8ArrayPrediction(prediction());
     }
     
     bool shouldSpeculateUint16Array()
     {
-        return prediction() == PredictUint16Array;
+        return isUint16ArrayPrediction(prediction());
     }
     
     bool shouldSpeculateUint32Array()
     {
-        return prediction() == PredictUint32Array;
+        return isUint32ArrayPrediction(prediction());
     }
     
     bool shouldSpeculateFloat32Array()
     {
 #if CPU(X86) || CPU(X86_64)
-        return !!(prediction() & PredictFloat32Array);
+        return isFloat32ArrayPrediction(prediction());
 #else
         return false;
 #endif
@@ -970,7 +970,7 @@ struct Node {
     
     bool shouldSpeculateFloat64Array()
     {
-        return prediction() == PredictFloat64Array;
+        return isFloat64ArrayPrediction(prediction());
     }
     
     bool shouldSpeculateArrayOrOther()

diff --git a/Source/JavaScriptCore/dfg/DFGPropagator.cpp b/Source/JavaScriptCore/dfg/DFGPropagator.cpp
index 1804c95e0e27079ba48f87b4afa2bbbb635c475c..392fad38bde15835b5ef45ef0c3049089e934b76 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGPropagator.cpp
+++ b/Source/JavaScriptCore/dfg/DFGPropagator.cpp
@@ -1087,8 +1087,10 @@ private:
     
     bool byValIsPure(Node& node)
     {
-        PredictedType prediction = m_graph[node.child2()].prediction();
-        return (prediction & PredictInt32) || !prediction;
+        return m_graph[node.child2()].shouldSpeculateInteger()
+            && ((node.op == PutByVal || node.op == PutByValAlias)
+                ? isActionableMutableArrayPrediction(m_graph[node.child1()].prediction())
+                : isActionableArrayPrediction(m_graph[node.child1()].prediction()));
     }
     
     bool clobbersWorld(NodeIndex nodeIndex)

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 975938066fb6b0fdc3105f6981646f1499c5d3dc..99da1b080c8e06ad77a755e588be132c0681168a 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -2376,6 +2376,11 @@ bool SpeculativeJIT::compileStrictEq(Node& node)
 
 void SpeculativeJIT::compileGetIndexedPropertyStorage(Node& node)
 {
+    if (!node.prediction() || !at(node.child1()).prediction() || !at(node.child2()).prediction()) {
+        terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
+        return;
+    }
+        
     SpeculateCellOperand base(this, node.child1());
     GPRReg baseReg = base.gpr();
     

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
index e75d278613558b3beeaed24e2f1fca87b27c65c1..37b2ea075f213ceff5446c186548b4053d4e2de7 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
@@ -2205,8 +2205,12 @@ void SpeculativeJIT::compile(Node& node)
     }
 
     case GetByVal: {
-        PredictedType basePrediction = at(node.child2()).prediction();
-        if (!(basePrediction & PredictInt32) && basePrediction) {
+        if (!node.prediction() || !at(node.child1()).prediction() || !at(node.child2()).prediction()) {
+            terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
+            break;
+        }
+        
+        if (!at(node.child2()).shouldSpeculateInteger() || !isActionableArrayPrediction(at(node.child1()).prediction())) {
             SpeculateCellOperand base(this, node.child1()); // Save a register, speculate cell. We'll probably be right.
             JSValueOperand property(this, node.child2());
             GPRReg baseGPR = base.gpr();
@@ -2291,6 +2295,8 @@ void SpeculativeJIT::compile(Node& node)
                 return;
             break;            
         }
+        
+        ASSERT(at(node.child1()).shouldSpeculateArray());
 
         SpeculateStrictInt32Operand property(this, node.child2());
         StorageOperand storage(this, node.child3());
@@ -2325,8 +2331,12 @@ void SpeculativeJIT::compile(Node& node)
     }
 
     case PutByVal: {
-        PredictedType basePrediction = at(node.child2()).prediction();
-        if (!(basePrediction & PredictInt32) && basePrediction) {
+        if (!at(node.child1()).prediction() || !at(node.child2()).prediction()) {
+            terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
+            break;
+        }
+        
+        if (!at(node.child2()).shouldSpeculateInteger() || !isActionableMutableArrayPrediction(at(node.child1()).prediction())) {
             SpeculateCellOperand base(this, node.child1()); // Save a register, speculate cell. We'll probably be right.
             JSValueOperand property(this, node.child2());
             JSValueOperand value(this, node.child3());
@@ -2409,6 +2419,8 @@ void SpeculativeJIT::compile(Node& node)
                 return;
             break;            
         }
+        
+        ASSERT(at(node.child1()).shouldSpeculateArray());
 
         JSValueOperand value(this, node.child3());
         GPRTemporary scratch(this);
@@ -2472,6 +2484,14 @@ void SpeculativeJIT::compile(Node& node)
     }
 
     case PutByValAlias: {
+        if (!at(node.child1()).prediction() || !at(node.child2()).prediction()) {
+            terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
+            break;
+        }
+        
+        ASSERT(isActionableMutableArrayPrediction(at(node.child1()).prediction()));
+        ASSERT(at(node.child2()).shouldSpeculateInteger());
+
         SpeculateCellOperand base(this, node.child1());
         SpeculateStrictInt32Operand property(this, node.child2());
 
@@ -2536,6 +2556,8 @@ void SpeculativeJIT::compile(Node& node)
             break;            
         }
 
+        ASSERT(at(node.child1()).shouldSpeculateArray());
+
         JSValueOperand value(this, node.child3());
         GPRTemporary scratch(this, base);
         

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
index 293b40860d38bdaeb2d061da6ec505fae75c6eb2..9172fc13a847b9e2fe213ea5bf5b7a900d685eea 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -2286,8 +2286,12 @@ void SpeculativeJIT::compile(Node& node)
     }
 
     case GetByVal: {
-        PredictedType basePrediction = at(node.child2()).prediction();
-        if (!(basePrediction & PredictInt32) && basePrediction) {
+        if (!node.prediction() || !at(node.child1()).prediction() || !at(node.child2()).prediction()) {
+            terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
+            break;
+        }
+        
+        if (!at(node.child2()).shouldSpeculateInteger() || !isActionableArrayPrediction(at(node.child1()).prediction())) {
             JSValueOperand base(this, node.child1());
             JSValueOperand property(this, node.child2());
             GPRReg baseGPR = base.gpr();
@@ -2370,6 +2374,8 @@ void SpeculativeJIT::compile(Node& node)
                 return;
             break;            
         }
+        
+        ASSERT(at(node.child1()).shouldSpeculateArray());
 
         SpeculateCellOperand base(this, node.child1());
         SpeculateStrictInt32Operand property(this, node.child2());
@@ -2398,8 +2404,12 @@ void SpeculativeJIT::compile(Node& node)
     }
 
     case PutByVal: {
-        PredictedType basePrediction = at(node.child2()).prediction();
-        if (!(basePrediction & PredictInt32) && basePrediction) {
+        if (!at(node.child1()).prediction() || !at(node.child2()).prediction()) {
+            terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
+            break;
+        }
+        
+        if (!at(node.child2()).shouldSpeculateInteger() || !isActionableMutableArrayPrediction(at(node.child1()).prediction())) {
             JSValueOperand arg1(this, node.child1());
             JSValueOperand arg2(this, node.child2());
             JSValueOperand arg3(this, node.child3());
@@ -2476,6 +2486,8 @@ void SpeculativeJIT::compile(Node& node)
                 return;
             break;            
         }
+            
+        ASSERT(at(node.child1()).shouldSpeculateArray());
 
         JSValueOperand value(this, node.child3());
         GPRTemporary scratch(this);
@@ -2537,8 +2549,13 @@ void SpeculativeJIT::compile(Node& node)
     }
 
     case PutByValAlias: {
-        PredictedType basePrediction = at(node.child2()).prediction();
-        ASSERT_UNUSED(basePrediction, (basePrediction & PredictInt32) || !basePrediction);
+        if (!at(node.child1()).prediction() || !at(node.child2()).prediction()) {
+            terminateSpeculativeExecution(Uncountable, JSValueRegs(), NoNode);
+            break;
+        }
+        
+        ASSERT(isActionableMutableArrayPrediction(at(node.child1()).prediction()));
+        ASSERT(at(node.child2()).shouldSpeculateInteger());
 
         SpeculateCellOperand base(this, node.child1());
         SpeculateStrictInt32Operand property(this, node.child2());
@@ -2602,6 +2619,8 @@ void SpeculativeJIT::compile(Node& node)
                 return;
             break;            
         }
+        
+        ASSERT(at(node.child1()).shouldSpeculateArray());
 
         JSValueOperand value(this, node.child3());
         GPRTemporary scratch(this);