Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (96561 => 96562)
--- trunk/Source/_javascript_Core/ChangeLog 2011-10-04 00:29:13 UTC (rev 96561)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-10-04 01:05:38 UTC (rev 96562)
@@ -1,3 +1,41 @@
+2011-10-03 Filip Pizlo <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, rolling out r96526.
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (96561 => 96562)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2011-10-04 00:29:13 UTC (rev 96561)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2011-10-04 01:05:38 UTC (rev 96562)
@@ -204,8 +204,8 @@
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);
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (96561 => 96562)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2011-10-04 00:29:13 UTC (rev 96561)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2011-10-04 01:05:38 UTC (rev 96562)
@@ -186,33 +186,6 @@
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 @@
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)
{
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h (96561 => 96562)
--- trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h 2011-10-04 00:29:13 UTC (rev 96561)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h 2011-10-04 01:05:38 UTC (rev 96562)
@@ -283,7 +283,7 @@
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)
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (96561 => 96562)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2011-10-04 00:29:13 UTC (rev 96561)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2011-10-04 01:05:38 UTC (rev 96562)
@@ -428,6 +428,7 @@
, codeOrigin(codeOrigin)
, m_virtualRegister(InvalidVirtualRegister)
, m_refCount(0)
+ , m_prediction(PredictNone)
{
ASSERT(!(op & NodeHasVarArgs));
ASSERT(!hasArithNodeFlags());
@@ -443,6 +444,7 @@
, 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 @@
, 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 @@
, 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 @@
return m_opInfo2;
}
- bool hasPrediction()
+ bool hasHeapPrediction()
{
switch (op) {
case GetById:
@@ -774,15 +778,15 @@
}
}
- 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 @@
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 @@
// 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
Modified: trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp (96561 => 96562)
--- trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2011-10-04 00:29:13 UTC (rev 96561)
+++ trunk/Source/_javascript_Core/dfg/DFGPropagator.cpp 2011-10-04 01:05:38 UTC (rev 96562)
@@ -42,18 +42,11 @@
, 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 @@
{
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 @@
}
case SetLocal: {
- changed |= node.variableAccessData()->predict(m_predictions[node.child1()]);
+ changed |= node.variableAccessData()->predict(m_graph[node.child1()].prediction());
break;
}
@@ -342,8 +337,8 @@
}
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 @@
}
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 @@
}
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 @@
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 @@
}
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 @@
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 @@
}
case GetByOffset: {
- if (node.getPrediction())
- changed |= mergePrediction(node.getPrediction());
+ if (node.getHeapPrediction())
+ changed |= mergePrediction(node.getHeapPrediction());
break;
}
@@ -466,17 +461,17 @@
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 @@
}
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 @@
case ResolveBase:
case ResolveBaseStrictPut:
case ResolveGlobal: {
- PredictedType prediction = node.getPrediction();
+ PredictedType prediction = node.getHeapPrediction();
if (prediction)
changed |= mergePrediction(prediction);
break;
@@ -539,7 +534,7 @@
}
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 @@
}
#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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
// 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 @@
NodeIndex m_start;
NodeIndex m_compileIndex;
- Vector<PredictedType, 16> m_predictions;
-
#if ENABLE(DFG_DEBUG_PROPAGATION_VERBOSE)
unsigned m_count;
#endif
