DFG backends don't have access to per-node predictions from the propagator
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Oct 2011 01:05:38 +0000 (01:05 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Oct 2011 01:05:38 +0000 (01:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=69291

Reviewed by Oliver Hunt.

Nodes now have two notion of predictions: the heap prediction, which is
what came directly from value profiling, and the propagator's predictions,
which arise out of abstract interpretation. Every node has a propagator
prediction, but not every node has a heap prediction; and there is no
guarantee that a node that has both will keep them consistent as the
propagator may have additional information available to it.

This is performance neutral.

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
* dfg/DFGGraph.h:
* dfg/DFGJITCompiler.h:
(JSC::DFG::JITCompiler::getPrediction):
* dfg/DFGNode.h:
(JSC::DFG::Node::Node):
(JSC::DFG::Node::hasHeapPrediction):
(JSC::DFG::Node::getHeapPrediction):
(JSC::DFG::Node::predictHeap):
(JSC::DFG::Node::prediction):
(JSC::DFG::Node::predict):
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::Propagator):
(JSC::DFG::Propagator::setPrediction):
(JSC::DFG::Propagator::mergePrediction):
(JSC::DFG::Propagator::propagateNodePredictions):
(JSC::DFG::Propagator::fixupNode):
(JSC::DFG::Propagator::isPredictedNumerical):
(JSC::DFG::Propagator::logicalNotIsPure):
(JSC::DFG::Propagator::setReplacement):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGGraph.h
Source/JavaScriptCore/dfg/DFGJITCompiler.h
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGPropagator.cpp

index 05f7d1b..5855d09 100644 (file)
@@ -1,3 +1,41 @@
+2011-10-03  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG backends don't have access to per-node predictions from the propagator
+        https://bugs.webkit.org/show_bug.cgi?id=69291
+
+        Reviewed by Oliver Hunt.
+        
+        Nodes now have two notion of predictions: the heap prediction, which is
+        what came directly from value profiling, and the propagator's predictions,
+        which arise out of abstract interpretation. Every node has a propagator
+        prediction, but not every node has a heap prediction; and there is no
+        guarantee that a node that has both will keep them consistent as the
+        propagator may have additional information available to it.
+        
+        This is performance neutral.
+
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::dump):
+        * dfg/DFGGraph.h:
+        * dfg/DFGJITCompiler.h:
+        (JSC::DFG::JITCompiler::getPrediction):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::Node):
+        (JSC::DFG::Node::hasHeapPrediction):
+        (JSC::DFG::Node::getHeapPrediction):
+        (JSC::DFG::Node::predictHeap):
+        (JSC::DFG::Node::prediction):
+        (JSC::DFG::Node::predict):
+        * dfg/DFGPropagator.cpp:
+        (JSC::DFG::Propagator::Propagator):
+        (JSC::DFG::Propagator::setPrediction):
+        (JSC::DFG::Propagator::mergePrediction):
+        (JSC::DFG::Propagator::propagateNodePredictions):
+        (JSC::DFG::Propagator::fixupNode):
+        (JSC::DFG::Propagator::isPredictedNumerical):
+        (JSC::DFG::Propagator::logicalNotIsPure):
+        (JSC::DFG::Propagator::setReplacement):
+
 2011-10-03  Jer Noble  <jer.noble@apple.com>
 
         Unreviewed, rolling out r96526.
index 6ab93ab..4be2f24 100644 (file)
@@ -204,8 +204,8 @@ void Graph::dump(NodeIndex nodeIndex, CodeBlock* codeBlock)
             printf("  predicting %s", predictionToString(node.variableAccessData()->prediction()));
         else if (node.hasVarNumber())
             printf("  predicting %s", predictionToString(getGlobalVarPrediction(node.varNumber())));
-        else if (node.hasPrediction())
-            printf("  predicting %s", predictionToString(node.getPrediction()));
+        else if (node.hasHeapPrediction())
+            printf("  predicting %s", predictionToString(node.getHeapPrediction()));
         else if (node.hasMethodCheckData()) {
             MethodCheckData& methodCheckData = m_methodCheckData[node.methodCheckDataIndex()];
             printf("  predicting function %p", methodCheckData.function);
index fe5b1cf..d170baa 100644 (file)
@@ -186,33 +186,6 @@ public:
         return m_predictions.predictGlobalVar(varNumber, prediction);
     }
     
-    bool predict(Node& node, PredictedType prediction)
-    {
-        switch (node.op) {
-        case GetLocal:
-        case SetLocal:
-        case Phi:
-        case SetArgument:
-            return node.variableAccessData()->predict(prediction);
-        case GetGlobalVar:
-            return predictGlobalVar(node.varNumber(), prediction);
-        case GetById:
-        case GetMethod:
-        case GetByVal:
-        case Call:
-        case Construct:
-        case GetByOffset:
-        case GetScopedVar:
-        case Resolve:
-        case ResolveBase:
-        case ResolveBaseStrictPut:
-        case ResolveGlobal:
-            return node.predict(prediction);
-        default:
-            return false;
-        }
-    }
-
     PredictedType getGlobalVarPrediction(unsigned varNumber)
     {
         return m_predictions.getGlobalVarPrediction(varNumber);
@@ -228,40 +201,6 @@ public:
         return predictionFromValue(node.valueOfJSConstantNode(codeBlock));
     }
     
-    PredictedType getPrediction(Node& node, CodeBlock* codeBlock)
-    {
-        Node* nodePtr = &node;
-        
-        if (nodePtr->op == ValueToNumber)
-            nodePtr = &(*this)[nodePtr->child1()];
-
-        if (nodePtr->op == ValueToInt32)
-            nodePtr = &(*this)[nodePtr->child1()];
-        
-        switch (nodePtr->op) {
-        case GetLocal:
-        case SetLocal:
-        case SetArgument:
-        case Phi:
-            return nodePtr->variableAccessData()->prediction();
-        case GetGlobalVar:
-            return getGlobalVarPrediction(nodePtr->varNumber());
-        case GetById:
-        case GetMethod:
-        case GetByVal:
-        case Call:
-        case Construct:
-        case GetByOffset:
-            return nodePtr->getPrediction();
-        case CheckMethod:
-            return getMethodCheckPrediction(*nodePtr);
-        case JSConstant:
-            return getJSConstantPrediction(*nodePtr, codeBlock);
-        default:
-            return PredictNone;
-        }
-    }
-    
     // Helper methods to check nodes for constants.
     bool isConstant(NodeIndex nodeIndex)
     {
index aeef256..899a24c 100644 (file)
@@ -283,7 +283,7 @@ public:
     JSFunction* valueOfFunctionConstant(NodeIndex nodeIndex) { return graph().valueOfFunctionConstant(codeBlock(), nodeIndex); }
     
     // Helper methods to get predictions
-    PredictedType getPrediction(Node& node) { return graph().getPrediction(node, codeBlock()); }
+    PredictedType getPrediction(Node& node) { return node.prediction(); }
     PredictedType getPrediction(NodeIndex nodeIndex) { return getPrediction(graph()[nodeIndex]); }
 
 #if USE(JSVALUE32_64)
index 6ae7c8d..5bafa1a 100644 (file)
@@ -428,6 +428,7 @@ struct Node {
         , codeOrigin(codeOrigin)
         , m_virtualRegister(InvalidVirtualRegister)
         , m_refCount(0)
+        , m_prediction(PredictNone)
     {
         ASSERT(!(op & NodeHasVarArgs));
         ASSERT(!hasArithNodeFlags());
@@ -443,6 +444,7 @@ struct Node {
         , m_virtualRegister(InvalidVirtualRegister)
         , m_refCount(0)
         , m_opInfo(imm.m_value)
+        , m_prediction(PredictNone)
     {
         ASSERT(!(op & NodeHasVarArgs));
         children.fixed.child1 = child1;
@@ -458,6 +460,7 @@ struct Node {
         , m_refCount(0)
         , m_opInfo(imm1.m_value)
         , m_opInfo2(safeCast<unsigned>(imm2.m_value))
+        , m_prediction(PredictNone)
     {
         ASSERT(!(op & NodeHasVarArgs));
         children.fixed.child1 = child1;
@@ -473,6 +476,7 @@ struct Node {
         , m_refCount(0)
         , m_opInfo(imm1.m_value)
         , m_opInfo2(safeCast<unsigned>(imm2.m_value))
+        , m_prediction(PredictNone)
     {
         ASSERT(op & NodeHasVarArgs);
         children.variable.firstChild = firstChild;
@@ -754,7 +758,7 @@ struct Node {
         return m_opInfo2;
     }
     
-    bool hasPrediction()
+    bool hasHeapPrediction()
     {
         switch (op) {
         case GetById:
@@ -774,15 +778,15 @@ struct Node {
         }
     }
     
-    PredictedType getPrediction()
+    PredictedType getHeapPrediction()
     {
-        ASSERT(hasPrediction());
+        ASSERT(hasHeapPrediction());
         return static_cast<PredictedType>(m_opInfo2);
     }
     
-    bool predict(PredictedType prediction)
+    bool predictHeap(PredictedType prediction)
     {
-        ASSERT(hasPrediction());
+        ASSERT(hasHeapPrediction());
         
         return mergePrediction(m_opInfo2, prediction);
     }
@@ -913,6 +917,16 @@ struct Node {
         return children.variable.numChildren;
     }
     
+    PredictedType prediction()
+    {
+        return m_prediction;
+    }
+    
+    bool predict(PredictedType prediction)
+    {
+        return mergePrediction(m_prediction, prediction);
+    }
+    
     // This enum value describes the type of the node.
     NodeType op;
     // Used to look up exception handling information (currently implemented as a bytecode index).
@@ -937,6 +951,8 @@ private:
     // big enough to store a pointer.
     uintptr_t m_opInfo;
     unsigned m_opInfo2;
+    // The prediction ascribed to this node after propagation.
+    PredictedType m_prediction;
 };
 
 } } // namespace JSC::DFG
index 13d23a0..61140f5 100644 (file)
@@ -42,18 +42,11 @@ public:
         , m_codeBlock(codeBlock)
         , m_profiledBlock(profiledBlock)
     {
-        // Predictions is a forward flow property that propagates the values seen at
-        // a particular value source to their various uses, ensuring that uses perform
-        // speculation that does not contravene the expected values.
-        m_predictions.resize(m_graph.size());
-        
         // Replacements are used to implement local common subexpression elimination.
         m_replacements.resize(m_graph.size());
         
-        for (unsigned i = 0; i < m_graph.size(); ++i) {
-            m_predictions[i] = PredictNone;
+        for (unsigned i = 0; i < m_graph.size(); ++i)
             m_replacements[i] = NoNode;
-        }
         
         for (unsigned i = 0; i < LastNodeId; ++i)
             m_lastSeen[i] = NoNode;
@@ -285,18 +278,20 @@ private:
     {
         ASSERT(m_graph[m_compileIndex].hasResult());
         
-        if (m_predictions[m_compileIndex] == prediction)
-            return false;
+        // setPrediction() is used when we know that there is no way that we can change
+        // our minds about what the prediction is going to be. There is no semantic
+        // difference between setPrediction() and mergePrediction() other than the
+        // increased checking to validate this property.
+        ASSERT(m_graph[m_compileIndex].prediction() == PredictNone || m_graph[m_compileIndex].prediction() == prediction);
         
-        m_predictions[m_compileIndex] = prediction;
-        return true;
+        return m_graph[m_compileIndex].predict(prediction);
     }
     
     bool mergePrediction(PredictedType prediction)
     {
         ASSERT(m_graph[m_compileIndex].hasResult());
         
-        return JSC::mergePrediction(m_predictions[m_compileIndex], prediction);
+        return m_graph[m_compileIndex].predict(prediction);
     }
     
     void propagateNodePredictions(Node& node)
@@ -326,7 +321,7 @@ private:
         }
             
         case SetLocal: {
-            changed |= node.variableAccessData()->predict(m_predictions[node.child1()]);
+            changed |= node.variableAccessData()->predict(m_graph[node.child1()].prediction());
             break;
         }
             
@@ -342,8 +337,8 @@ private:
         }
 
         case ArithMod: {
-            PredictedType left = m_predictions[node.child1()];
-            PredictedType right = m_predictions[node.child2()];
+            PredictedType left = m_graph[node.child1()].prediction();
+            PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right) {
                 if (isInt32Prediction(mergePredictions(left, right)) && nodeCanSpeculateInteger(node.arithNodeFlags()))
@@ -363,7 +358,7 @@ private:
         }
 
         case ValueToNumber: {
-            PredictedType prediction = m_predictions[node.child1()];
+            PredictedType prediction = m_graph[node.child1()].prediction();
             
             if (prediction) {
                 if (!(prediction & PredictDouble) && nodeCanSpeculateInteger(node.arithNodeFlags()))
@@ -376,8 +371,8 @@ private:
         }
 
         case ValueAdd: {
-            PredictedType left = m_predictions[node.child1()];
-            PredictedType right = m_predictions[node.child2()];
+            PredictedType left = m_graph[node.child1()].prediction();
+            PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right) {
                 if (isNumberPrediction(left) && isNumberPrediction(right)) {
@@ -400,8 +395,8 @@ private:
         case ArithMin:
         case ArithMax:
         case ArithDiv: {
-            PredictedType left = m_predictions[node.child1()];
-            PredictedType right = m_predictions[node.child2()];
+            PredictedType left = m_graph[node.child1()].prediction();
+            PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right) {
                 if (isInt32Prediction(mergePredictions(left, right)) && nodeCanSpeculateInteger(node.arithNodeFlags()))
@@ -418,7 +413,7 @@ private:
         }
             
         case ArithAbs: {
-            PredictedType child = m_predictions[node.child1()];
+            PredictedType child = m_graph[node.child1()].prediction();
             if (child) {
                 if (nodeCanSpeculateInteger(node.arithNodeFlags()))
                     changed |= mergePrediction(child);
@@ -443,8 +438,8 @@ private:
         case GetById:
         case GetMethod:
         case GetByVal: {
-            if (node.getPrediction())
-                changed |= mergePrediction(node.getPrediction());
+            if (node.getHeapPrediction())
+                changed |= mergePrediction(node.getHeapPrediction());
             break;
         }
             
@@ -454,8 +449,8 @@ private:
         }
             
         case GetByOffset: {
-            if (node.getPrediction())
-                changed |= mergePrediction(node.getPrediction());
+            if (node.getHeapPrediction())
+                changed |= mergePrediction(node.getHeapPrediction());
             break;
         }
             
@@ -466,17 +461,17 @@ private:
 
         case Call:
         case Construct: {
-            if (node.getPrediction())
-                changed |= mergePrediction(node.getPrediction());
+            if (node.getHeapPrediction())
+                changed |= mergePrediction(node.getHeapPrediction());
             break;
         }
             
         case ConvertThis: {
-            PredictedType prediction = m_predictions[node.child1()];
+            PredictedType prediction = m_graph[node.child1()].prediction();
             if (prediction) {
                 if (prediction & ~PredictObjectMask) {
-                    prediction &= ~PredictObjectMask;
-                    prediction |= PredictObjectUnknown;
+                    prediction &= PredictObjectMask;
+                    prediction = mergePredictions(prediction, PredictObjectUnknown);
                 }
                 changed |= mergePrediction(prediction);
             }
@@ -491,7 +486,7 @@ private:
         }
             
         case PutGlobalVar: {
-            changed |= m_graph.predictGlobalVar(node.varNumber(), m_predictions[node.child1()]);
+            changed |= m_graph.predictGlobalVar(node.varNumber(), m_graph[node.child1()].prediction());
             break;
         }
             
@@ -500,7 +495,7 @@ private:
         case ResolveBase:
         case ResolveBaseStrictPut:
         case ResolveGlobal: {
-            PredictedType prediction = node.getPrediction();
+            PredictedType prediction = node.getHeapPrediction();
             if (prediction)
                 changed |= mergePrediction(prediction);
             break;
@@ -539,7 +534,7 @@ private:
         }
             
         case ToPrimitive: {
-            PredictedType child = m_predictions[node.child1()];
+            PredictedType child = m_graph[node.child1()].prediction();
             if (child) {
                 if (isObjectPrediction(child)) {
                     // I'd love to fold this case into the case below, but I can't, because
@@ -599,7 +594,7 @@ private:
         }
 
 #if ENABLE(DFG_DEBUG_PROPAGATION_VERBOSE)
-        printf("%s ", predictionToString(m_predictions[m_compileIndex]));
+        printf("%s ", predictionToString(m_graph[m_compileIndex].prediction()));
 #endif
         
         m_changed |= changed;
@@ -652,8 +647,8 @@ private:
                 break;
             }
             
-            PredictedType left = m_predictions[node.child1()];
-            PredictedType right = m_predictions[node.child2()];
+            PredictedType left = m_graph[node.child1()].prediction();
+            PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right && isNumberPrediction(left) && isNumberPrediction(right)) {
                 if (left & PredictDouble)
@@ -677,8 +672,8 @@ private:
                 break;
             }
             
-            PredictedType left = m_predictions[node.child1()];
-            PredictedType right = m_predictions[node.child2()];
+            PredictedType left = m_graph[node.child1()].prediction();
+            PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right) {
                 if (left & PredictDouble)
@@ -695,7 +690,7 @@ private:
                 break;
             }
             
-            PredictedType prediction = m_predictions[node.child1()];
+            PredictedType prediction = m_graph[node.child1()].prediction();
             if (prediction & PredictDouble)
                 toDouble(node.child1());
             break;
@@ -707,11 +702,11 @@ private:
         }
             
         case GetById: {
-            bool isArray = isArrayPrediction(m_predictions[node.child1()]);
-            bool isString = isStringPrediction(m_predictions[node.child1()]);
+            bool isArray = isArrayPrediction(m_graph[node.child1()].prediction());
+            bool isString = isStringPrediction(m_graph[node.child1()].prediction());
             if (!isArray && !isString)
                 break;
-            if (!isInt32Prediction(m_predictions[m_compileIndex]))
+            if (!isInt32Prediction(m_graph[m_compileIndex].prediction()))
                 break;
             if (m_codeBlock->identifier(node.identifierNumber()) != m_globalData.propertyNames->length)
                 break;
@@ -860,14 +855,14 @@ private:
     
     bool isPredictedNumerical(Node& node)
     {
-        PredictedType left = m_predictions[node.child1()];
-        PredictedType right = m_predictions[node.child2()];
+        PredictedType left = m_graph[node.child1()].prediction();
+        PredictedType right = m_graph[node.child2()].prediction();
         return isNumberPrediction(left) && isNumberPrediction(right);
     }
     
     bool logicalNotIsPure(Node& node)
     {
-        PredictedType prediction = m_predictions[node.child1()];
+        PredictedType prediction = m_graph[node.child1()].prediction();
         return isBooleanPrediction(prediction) || !prediction;
     }
     
@@ -1165,7 +1160,7 @@ private:
         
         // Be safe. Don't try to perform replacements if the predictions don't
         // agree.
-        if (m_predictions[m_compileIndex] != m_predictions[replacement])
+        if (m_graph[m_compileIndex].prediction() != m_graph[replacement].prediction())
             return;
         
 #if ENABLE(DFG_DEBUG_PROPAGATION_VERBOSE)
@@ -1388,8 +1383,6 @@ private:
     NodeIndex m_start;
     NodeIndex m_compileIndex;
     
-    Vector<PredictedType, 16> m_predictions;
-
 #if ENABLE(DFG_DEBUG_PROPAGATION_VERBOSE)
     unsigned m_count;
 #endif