- Revision
- 92593
- Author
- [email protected]
- Date
- 2011-08-08 05:50:59 -0700 (Mon, 08 Aug 2011)
Log Message
DFG JIT does not track speculation decisions for global variables
https://bugs.webkit.org/show_bug.cgi?id=65825
Reviewed by Gavin Barraclough.
Added the capability to track predictions for global variables, and
ensured that code can abstract over the source of prediction (local
versus global variable) wherever it is appropriate to do so. Also
cleaned up the code in SpeculativeJIT that decides how to speculate
based on recorded predictions (for example instead of using isInteger,
which makes sense for local predictions where the GetLocal would
return an integer value, we now tend to use shouldSpeculateInteger,
which checks if the value is either already an integer or should be
speculated to be an integer).
This is an 0.8% win on SunSpider, almost entirely thanks to a 25%
win on controlflow-recursive. It's also a 4.8% win on v8-crypto.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::predictArray):
(JSC::DFG::ByteCodeParser::predictInt32):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::predictGlobalVar):
(JSC::DFG::Graph::predict):
(JSC::DFG::Graph::getGlobalVarPrediction):
(JSC::DFG::Graph::getPrediction):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::shouldSpeculateInteger):
(JSC::DFG::SpeculativeJIT::shouldSpeculateDouble):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (92592 => 92593)
--- trunk/Source/_javascript_Core/ChangeLog 2011-08-08 12:44:33 UTC (rev 92592)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-08-08 12:50:59 UTC (rev 92593)
@@ -1,3 +1,40 @@
+2011-08-08 Filip Pizlo <[email protected]>
+
+ DFG JIT does not track speculation decisions for global variables
+ https://bugs.webkit.org/show_bug.cgi?id=65825
+
+ Reviewed by Gavin Barraclough.
+
+ Added the capability to track predictions for global variables, and
+ ensured that code can abstract over the source of prediction (local
+ versus global variable) wherever it is appropriate to do so. Also
+ cleaned up the code in SpeculativeJIT that decides how to speculate
+ based on recorded predictions (for example instead of using isInteger,
+ which makes sense for local predictions where the GetLocal would
+ return an integer value, we now tend to use shouldSpeculateInteger,
+ which checks if the value is either already an integer or should be
+ speculated to be an integer).
+
+ This is an 0.8% win on SunSpider, almost entirely thanks to a 25%
+ win on controlflow-recursive. It's also a 4.8% win on v8-crypto.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::predictArray):
+ (JSC::DFG::ByteCodeParser::predictInt32):
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::dump):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::predictGlobalVar):
+ (JSC::DFG::Graph::predict):
+ (JSC::DFG::Graph::getGlobalVarPrediction):
+ (JSC::DFG::Graph::getPrediction):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::SpeculativeJIT::shouldSpeculateInteger):
+ (JSC::DFG::SpeculativeJIT::shouldSpeculateDouble):
+
2011-08-07 Martin Robinson <[email protected]>
Distribution fix for GTK+.
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (92592 => 92593)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2011-08-08 12:44:33 UTC (rev 92592)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2011-08-08 12:50:59 UTC (rev 92593)
@@ -430,10 +430,7 @@
void predictArray(NodeIndex nodeIndex)
{
- Node* nodePtr = &m_graph[nodeIndex];
-
- if (nodePtr->op == GetLocal)
- m_graph.predict(nodePtr->local(), PredictArray);
+ m_graph.predict(m_graph[nodeIndex], PredictArray);
}
void predictInt32(NodeIndex nodeIndex)
@@ -445,9 +442,8 @@
if (nodePtr->op == ValueToInt32)
nodePtr = &m_graph[nodePtr->child1()];
-
- if (nodePtr->op == GetLocal)
- m_graph.predict(nodePtr->local(), PredictInt32);
+
+ m_graph.predict(*nodePtr, PredictInt32);
}
JSGlobalData* m_globalData;
@@ -701,13 +697,13 @@
NodeIndex op2 = get(currentInstruction[3].u.operand);
// If both operands can statically be determined to the numbers, then this is an arithmetic add.
// Otherwise, we must assume this may be performing a concatenation to a string.
- if (m_graph[op1].hasNumericResult() && m_graph[op2].hasNumericResult()) {
- if (isSmallInt32Constant(op1) || isSmallInt32Constant(op2)) {
- predictInt32(op1);
- predictInt32(op2);
- }
+ if (isSmallInt32Constant(op1) || isSmallInt32Constant(op2)) {
+ predictInt32(op1);
+ predictInt32(op2);
+ }
+ if (m_graph[op1].hasNumericResult() && m_graph[op2].hasNumericResult())
set(currentInstruction[1].u.operand, addToGraph(ArithAdd, toNumber(op1), toNumber(op2)));
- } else
+ else
set(currentInstruction[1].u.operand, addToGraph(ValueAdd, op1, op2));
NEXT_OPCODE(op_add);
}
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (92592 => 92593)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2011-08-08 12:44:33 UTC (rev 92592)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2011-08-08 12:50:59 UTC (rev 92593)
@@ -136,6 +136,8 @@
if (node.hasLocal())
printf(" predicting %s", predictionToString(getPrediction(node.local())));
+ if (node.hasVarNumber())
+ printf(" predicting %s", predictionToString(getGlobalVarPrediction(node.varNumber())));
printf("\n");
}
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (92592 => 92593)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2011-08-08 12:44:33 UTC (rev 92592)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2011-08-08 12:50:59 UTC (rev 92593)
@@ -30,6 +30,7 @@
#include <RegisterFile.h>
#include <dfg/DFGNode.h>
+#include <wtf/HashMap.h>
#include <wtf/Vector.h>
#include <wtf/StdLibExtras.h>
@@ -194,8 +195,32 @@
m_argumentPredictions[argument].m_value |= prediction;
} else if ((unsigned)operand < m_variablePredictions.size())
m_variablePredictions[operand].m_value |= prediction;
-
}
+
+ void predictGlobalVar(unsigned varNumber, PredictedType prediction)
+ {
+ HashMap<unsigned, PredictionSlot>::iterator iter = m_globalVarPredictions.find(varNumber + 1);
+ if (iter == m_globalVarPredictions.end()) {
+ PredictionSlot predictionSlot;
+ predictionSlot.m_value |= prediction;
+ m_globalVarPredictions.add(varNumber + 1, predictionSlot);
+ } else
+ iter->second.m_value |= prediction;
+ }
+
+ void predict(Node& node, PredictedType prediction)
+ {
+ switch (node.op) {
+ case GetLocal:
+ predict(node.local(), prediction);
+ break;
+ case GetGlobalVar:
+ predictGlobalVar(node.varNumber(), prediction);
+ break;
+ default:
+ break;
+ }
+ }
PredictedType getPrediction(int operand)
{
@@ -207,7 +232,35 @@
return m_variablePredictions[operand].m_value;
return PredictNone;
}
+
+ PredictedType getGlobalVarPrediction(unsigned varNumber)
+ {
+ HashMap<unsigned, PredictionSlot>::iterator iter = m_globalVarPredictions.find(varNumber + 1);
+ if (iter == m_globalVarPredictions.end())
+ return PredictNone;
+ return iter->second.m_value;
+ }
+
+ PredictedType getPrediction(Node& node)
+ {
+ Node* nodePtr = &node;
+
+ if (nodePtr->op == ValueToNumber)
+ nodePtr = &(*this)[nodePtr->child1()];
+ if (nodePtr->op == ValueToInt32)
+ nodePtr = &(*this)[nodePtr->child1()];
+
+ switch (nodePtr->op) {
+ case GetLocal:
+ return getPrediction(nodePtr->local());
+ case GetGlobalVar:
+ return getGlobalVarPrediction(nodePtr->varNumber());
+ default:
+ return PredictNone;
+ }
+ }
+
#ifndef NDEBUG
static const char *opName(NodeType);
#endif
@@ -223,6 +276,7 @@
Vector<PredictionSlot, 16> m_argumentPredictions;
Vector<PredictionSlot, 16> m_variablePredictions;
+ HashMap<unsigned, PredictionSlot> m_globalVarPredictions;
};
} } // namespace JSC::DFG
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (92592 => 92593)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-08-08 12:44:33 UTC (rev 92592)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-08-08 12:50:59 UTC (rev 92593)
@@ -654,7 +654,7 @@
}
case ValueToNumber: {
- if (isInteger(node.child1())) {
+ if (shouldSpeculateInteger(node.child1())) {
SpeculateIntegerOperand op1(this, node.child1());
GPRTemporary result(this, op1);
m_jit.move(op1.gpr(), result.gpr());
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (92592 => 92593)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-08-08 12:44:33 UTC (rev 92592)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2011-08-08 12:50:59 UTC (rev 92593)
@@ -155,8 +155,19 @@
return (info.registerFormat() | DataFormatJS) == DataFormatJSInteger
|| (info.spillFormat() | DataFormatJS) == DataFormatJSInteger;
}
+
+ bool shouldSpeculateInteger(NodeIndex nodeIndex)
+ {
+ if (isInteger(nodeIndex))
+ return true;
+
+ if (isInt32Prediction(m_jit.graph().getPrediction(m_jit.graph()[nodeIndex])))
+ return true;
+
+ return false;
+ }
- bool isRegisterDataFormatDouble(NodeIndex nodeIndex)
+ bool shouldSpeculateDouble(NodeIndex nodeIndex)
{
Node& node = m_jit.graph()[nodeIndex];
VirtualRegister virtualRegister = node.virtualRegister();
@@ -166,7 +177,7 @@
|| (info.spillFormat() | DataFormatJS) == DataFormatJSDouble)
return true;
- if (node.op == GetLocal && isDoublePrediction(m_jit.graph().getPrediction(node.local())))
+ if (isDoublePrediction(m_jit.graph().getPrediction(node)))
return true;
return false;
@@ -174,7 +185,7 @@
bool shouldSpeculateInteger(NodeIndex op1, NodeIndex op2)
{
- return !(isRegisterDataFormatDouble(op1) || isRegisterDataFormatDouble(op2)) && (isInteger(op1) || isInteger(op2));
+ return !(shouldSpeculateDouble(op1) || shouldSpeculateDouble(op2)) && (shouldSpeculateInteger(op1) || shouldSpeculateInteger(op2));
}
bool compare(Node&, MacroAssembler::RelationalCondition, MacroAssembler::DoubleCondition, Z_DFGOperation_EJJ);
