Modified: trunk/Source/_javascript_Core/ChangeLog (144972 => 144973)
--- trunk/Source/_javascript_Core/ChangeLog 2013-03-06 21:21:48 UTC (rev 144972)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-03-06 21:23:39 UTC (rev 144973)
@@ -1,3 +1,30 @@
+2013-03-06 Filip Pizlo <[email protected]>
+
+ DFG should not run full CSE after the optimization fixpoint, since it really just wants store elimination
+ https://bugs.webkit.org/show_bug.cgi?id=111536
+
+ Reviewed by Oliver Hunt and Mark Hahnenberg.
+
+ The fixpoint will do aggressive load elimination and pure CSE. There's no need to do it after the fixpoint.
+ On the other hand, the fixpoint does not profit from doing store elimination (except for SetLocal/Flush).
+ Previously we had CSE do both, and had it avoid doing some store elimination during the fixpoint by querying
+ the fixpoint state. This changes CSE to be templated on mode - either NormalCSE or StoreElimination - so
+ that we explicitly put it into one of those modes depending on where we call it from. The goal is to reduce
+ time spent doing load elimination after the fixpoint, since that is just wasted cycles.
+
+ * dfg/DFGCSEPhase.cpp:
+ (JSC::DFG::CSEPhase::CSEPhase):
+ (JSC::DFG::CSEPhase::run):
+ (JSC::DFG::CSEPhase::performNodeCSE):
+ (JSC::DFG::CSEPhase::performBlockCSE):
+ (JSC::DFG::performCSE):
+ (DFG):
+ (JSC::DFG::performStoreElimination):
+ * dfg/DFGCSEPhase.h:
+ (DFG):
+ * dfg/DFGDriver.cpp:
+ (JSC::DFG::compile):
+
2013-03-06 Andreas Kling <[email protected]>
Pack Structure members better.
Modified: trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (144972 => 144973)
--- trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-03-06 21:21:48 UTC (rev 144972)
+++ trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-03-06 21:23:39 UTC (rev 144973)
@@ -35,15 +35,21 @@
namespace JSC { namespace DFG {
+enum CSEMode { NormalCSE, StoreElimination };
+
+template<CSEMode cseMode>
class CSEPhase : public Phase {
public:
CSEPhase(Graph& graph)
- : Phase(graph, "common subexpression elimination")
+ : Phase(graph, cseMode == NormalCSE ? "common subexpression elimination" : "store elimination")
{
}
bool run()
{
+ ASSERT((cseMode == NormalCSE) == (m_graph.m_fixpointState == FixpointNotConverged));
+ ASSERT(m_graph.m_fixpointState != BeforeFixpoint);
+
m_changed = false;
for (unsigned block = 0; block < m_graph.m_blocks.size(); ++block)
@@ -1011,7 +1017,8 @@
void performNodeCSE(Node* node)
{
- m_graph.performSubstitution(node);
+ if (cseMode == NormalCSE)
+ m_graph.performSubstitution(node);
if (node->op() == SetLocal)
node->child1()->mergeFlags(NodeRelevantToOSR);
@@ -1032,6 +1039,8 @@
switch (node->op()) {
case Identity:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(node->child1().node());
break;
@@ -1069,19 +1078,27 @@
case TypeOf:
case CompareEqConstant:
case ValueToInt32:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(pureCSE(node));
break;
case Int32ToDouble:
case ForwardInt32ToDouble:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(int32ToDoubleCSE(node));
break;
case GetCallee:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(getCalleeLoadElimination(node->codeOrigin.inlineCallFrame));
break;
case GetLocal: {
+ if (cseMode == StoreElimination)
+ break;
VariableAccessData* variableAccessData = node->variableAccessData();
if (!variableAccessData->isCaptured())
break;
@@ -1108,6 +1125,8 @@
}
case GetLocalUnlinked: {
+ if (cseMode == StoreElimination)
+ break;
Node* relevantLocalOpIgnored;
setReplacement(getLocalLoadElimination(node->unlinkedLocal(), relevantLocalOpIgnored, true));
break;
@@ -1122,7 +1141,7 @@
ASSERT(replacement->variableAccessData() == variableAccessData);
// FIXME: We should be able to remove SetLocals that can exit; we just need
// to replace them with appropriate type checks.
- if (m_graph.m_fixpointState == FixpointNotConverged) {
+ if (cseMode == NormalCSE) {
// Need to be conservative at this time; if the SetLocal has any chance of performing
// any speculations then we cannot do anything.
if (variableAccessData->isCaptured()) {
@@ -1159,21 +1178,29 @@
}
case JSConstant:
+ if (cseMode == StoreElimination)
+ break;
// This is strange, but necessary. Some phases will convert nodes to constants,
// which may result in duplicated constants. We use CSE to clean this up.
setReplacement(constantCSE(node));
break;
case WeakJSConstant:
+ if (cseMode == StoreElimination)
+ break;
// FIXME: have CSE for weak constants against strong constants and vice-versa.
setReplacement(weakConstantCSE(node));
break;
case GetArrayLength:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(getArrayLengthElimination(node->child1().node()));
break;
case GetMyScope:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(getMyScopeLoadElimination(node->codeOrigin.inlineCallFrame));
break;
@@ -1185,6 +1212,8 @@
case CompareGreater:
case CompareGreaterEq:
case CompareEq: {
+ if (cseMode == StoreElimination)
+ break;
if (m_graph.isPredictedNumerical(node)) {
Node* replacement = pureCSE(node);
if (replacement && m_graph.isPredictedNumerical(replacement))
@@ -1196,39 +1225,49 @@
// Finally handle heap accesses. These are not quite pure, but we can still
// optimize them provided that some subtle conditions are met.
case GetGlobalVar:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(globalVarLoadElimination(node->registerPointer()));
break;
case GetScopedVar: {
+ if (cseMode == StoreElimination)
+ break;
setReplacement(scopedVarLoadElimination(node->child1().node(), node->varNumber()));
break;
}
case GlobalVarWatchpoint:
+ if (cseMode == StoreElimination)
+ break;
if (globalVarWatchpointElimination(node->registerPointer()))
eliminate();
break;
case PutGlobalVar:
case PutGlobalVarCheck:
- if (m_graph.m_fixpointState == FixpointNotConverged)
+ if (cseMode == NormalCSE)
break;
eliminate(globalVarStoreElimination(node->registerPointer()));
break;
case PutScopedVar: {
- if (m_graph.m_fixpointState == FixpointNotConverged)
+ if (cseMode == NormalCSE)
break;
eliminate(scopedVarStoreElimination(node->child1().node(), node->child2().node(), node->varNumber()));
break;
}
case GetByVal:
+ if (cseMode == StoreElimination)
+ break;
if (m_graph.byValIsPure(node))
setReplacement(getByValLoadElimination(node->child1().node(), node->child2().node()));
break;
case PutByVal: {
+ if (cseMode == StoreElimination)
+ break;
Edge child1 = m_graph.varArgChild(node, 0);
Edge child2 = m_graph.varArgChild(node, 1);
if (node->arrayMode().canCSEStorage()) {
@@ -1242,52 +1281,68 @@
case CheckStructure:
case ForwardCheckStructure:
+ if (cseMode == StoreElimination)
+ break;
if (checkStructureElimination(node->structureSet(), node->child1().node()))
eliminate();
break;
case StructureTransitionWatchpoint:
case ForwardStructureTransitionWatchpoint:
+ if (cseMode == StoreElimination)
+ break;
if (structureTransitionWatchpointElimination(node->structure(), node->child1().node()))
eliminate();
break;
case PutStructure:
- if (m_graph.m_fixpointState == FixpointNotConverged)
+ if (cseMode == NormalCSE)
break;
eliminate(putStructureStoreElimination(node->child1().node()), PhantomPutStructure);
break;
case CheckFunction:
+ if (cseMode == StoreElimination)
+ break;
if (checkFunctionElimination(node->function(), node->child1().node()))
eliminate();
break;
case CheckExecutable:
+ if (cseMode == StoreElimination)
+ break;
if (checkExecutableElimination(node->executable(), node->child1().node()))
eliminate();
break;
case CheckArray:
+ if (cseMode == StoreElimination)
+ break;
if (checkArrayElimination(node->child1().node(), node->arrayMode()))
eliminate();
break;
case GetIndexedPropertyStorage: {
+ if (cseMode == StoreElimination)
+ break;
setReplacement(getIndexedPropertyStorageLoadElimination(node->child1().node(), node->arrayMode()));
break;
}
case GetButterfly:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(getPropertyStorageLoadElimination(node->child1().node()));
break;
case GetByOffset:
+ if (cseMode == StoreElimination)
+ break;
setReplacement(getByOffsetLoadElimination(m_graph.m_storageAccessData[node->storageAccessDataIndex()].identifierNumber, node->child1().node()));
break;
case PutByOffset:
- if (m_graph.m_fixpointState == FixpointNotConverged)
+ if (cseMode == NormalCSE)
break;
eliminate(putByOffsetStoreElimination(m_graph.m_storageAccessData[node->storageAccessDataIndex()].identifierNumber, node->child1().node()));
break;
@@ -1350,6 +1405,12 @@
m_currentNode = block->at(m_indexInBlock);
performNodeCSE(m_currentNode);
}
+
+ if (!ASSERT_DISABLED && cseMode == StoreElimination) {
+ // Nobody should have replacements set.
+ for (unsigned i = 0; i < block->size(); ++i)
+ ASSERT(!block->at(i)->replacement);
+ }
}
BasicBlock* m_currentBlock;
@@ -1362,9 +1423,15 @@
bool performCSE(Graph& graph)
{
SamplingRegion samplingRegion("DFG CSE Phase");
- return runPhase<CSEPhase>(graph);
+ return runPhase<CSEPhase<NormalCSE> >(graph);
}
+bool performStoreElimination(Graph& graph)
+{
+ SamplingRegion samplingRegion("DFG Store Elimination Phase");
+ return runPhase<CSEPhase<StoreElimination> >(graph);
+}
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
Modified: trunk/Source/_javascript_Core/dfg/DFGCSEPhase.h (144972 => 144973)
--- trunk/Source/_javascript_Core/dfg/DFGCSEPhase.h 2013-03-06 21:21:48 UTC (rev 144972)
+++ trunk/Source/_javascript_Core/dfg/DFGCSEPhase.h 2013-03-06 21:23:39 UTC (rev 144973)
@@ -40,9 +40,11 @@
// it is rather profitable. It has fairly accurate heap modeling and will match
// a wide range of subexpression similarities. It's known to produce big wins
// on a few benchmarks, and is relatively cheap to run.
-
bool performCSE(Graph&);
+// Perform just block-local store elimination.
+bool performStoreElimination(Graph&);
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
Modified: trunk/Source/_javascript_Core/dfg/DFGDriver.cpp (144972 => 144973)
--- trunk/Source/_javascript_Core/dfg/DFGDriver.cpp 2013-03-06 21:21:48 UTC (rev 144972)
+++ trunk/Source/_javascript_Core/dfg/DFGDriver.cpp 2013-03-06 21:23:39 UTC (rev 144973)
@@ -152,8 +152,8 @@
dataLogF("DFG optimization fixpoint converged in %u iterations.\n", cnt);
dfg.m_fixpointState = FixpointConverged;
- performCSE(dfg);
- performCPSRethreading(dfg); // This should usually be a no-op since CSE rarely dethreads the graph.
+ performStoreElimination(dfg);
+ performCPSRethreading(dfg); // This should usually be a no-op since store elimination rarely dethreads the graph.
performDCE(dfg);
performVirtualRegisterAllocation(dfg);
