Modified: trunk/Source/_javascript_Core/ChangeLog (136359 => 136360)
--- trunk/Source/_javascript_Core/ChangeLog 2012-12-03 02:30:04 UTC (rev 136359)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-12-03 03:12:08 UTC (rev 136360)
@@ -1,5 +1,33 @@
2012-12-02 Filip Pizlo <fpi...@apple.com>
+ DFG CSE should not keep alive things that aren't relevant to OSR
+ https://bugs.webkit.org/show_bug.cgi?id=103849
+
+ Reviewed by Oliver Hunt.
+
+ Most Phantom nodes are inserted by CSE, and by default have the same children as the
+ node that CSE had eliminated. This change makes CSE inspect all Phantom nodes (both
+ those it creates and those that were created by other phases) to see if they have
+ children that are redundant - i.e. children that are not interesting to OSR, which
+ is the only reason why Phantoms exist in the first place. Being relevant to OSR is
+ defined as one of: (1) you're a Phi, (2) you're a SetLocal, (3) somewhere between
+ your definition and the Phantom there was a SetLocal that referred to you.
+
+ This is a slight speed-up in a few places.
+
+ * dfg/DFGCSEPhase.cpp:
+ (JSC::DFG::CSEPhase::CSEPhase):
+ (JSC::DFG::CSEPhase::run):
+ (JSC::DFG::CSEPhase::performSubstitution):
+ (CSEPhase):
+ (JSC::DFG::CSEPhase::eliminateIrrelevantPhantomChildren):
+ (JSC::DFG::CSEPhase::setReplacement):
+ (JSC::DFG::CSEPhase::eliminate):
+ (JSC::DFG::CSEPhase::performNodeCSE):
+ (JSC::DFG::CSEPhase::performBlockCSE):
+
+2012-12-02 Filip Pizlo <fpi...@apple.com>
+
It should be possible to build and run with DFG_ENABLE(PROPAGATION_VERBOSE)
https://bugs.webkit.org/show_bug.cgi?id=103848
Modified: trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (136359 => 136360)
--- trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2012-12-03 02:30:04 UTC (rev 136359)
+++ trunk/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2012-12-03 03:12:08 UTC (rev 136360)
@@ -30,6 +30,7 @@
#include "DFGGraph.h"
#include "DFGPhase.h"
+#include <wtf/FastBitVector.h>
namespace JSC { namespace DFG {
@@ -43,13 +44,17 @@
for (unsigned i = 0; i < m_graph.size(); ++i)
m_replacements[i] = NoNode;
+
+ m_relevantToOSR.resize(m_graph.size());
}
bool run()
{
m_changed = false;
+
for (unsigned block = 0; block < m_graph.m_blocks.size(); ++block)
performBlockCSE(m_graph.m_blocks[block].get());
+
return m_changed;
}
@@ -942,9 +947,28 @@
ASSERT(m_replacements[child.index()] == NoNode);
if (addRef)
- m_graph[child].ref();
+ m_graph.ref(child);
}
+ void eliminateIrrelevantPhantomChildren(Node& node)
+ {
+ ASSERT(node.op() == Phantom);
+ for (unsigned i = 0; i < AdjacencyList::Size; ++i) {
+ Edge edge = node.children.child(i);
+ if (!edge)
+ continue;
+ if (m_relevantToOSR.get(edge.index()))
+ continue;
+
+#if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
+ dataLog(" Eliminating edge @", m_compileIndex, " -> @", edge.index());
+#endif
+ m_graph.deref(edge);
+ node.children.removeEdgeFromBag(i--);
+ m_changed = true;
+ }
+ }
+
enum PredictionHandlingMode { RequireSamePrediction, AllowPredictionMismatch };
bool setReplacement(NodeIndex replacement, PredictionHandlingMode predictionHandlingMode = RequireSamePrediction)
{
@@ -964,6 +988,7 @@
Node& node = m_graph[m_compileIndex];
node.setOpAndDefaultFlags(Phantom);
node.setRefCount(1);
+ eliminateIrrelevantPhantomChildren(node);
// At this point we will eliminate all references to this node.
m_replacements[m_compileIndex] = replacement;
@@ -983,6 +1008,7 @@
ASSERT(node.refCount() == 1);
ASSERT(node.mustGenerate());
node.setOpAndDefaultFlags(Phantom);
+ eliminateIrrelevantPhantomChildren(node);
m_changed = true;
}
@@ -996,6 +1022,8 @@
return;
ASSERT(node.mustGenerate());
node.setOpAndDefaultFlags(phantomType);
+ if (phantomType == Phantom)
+ eliminateIrrelevantPhantomChildren(node);
m_changed = true;
}
@@ -1013,6 +1041,9 @@
performSubstitution(node.children.child3(), shouldGenerate);
}
+ if (node.op() == SetLocal)
+ m_relevantToOSR.set(node.child1().index());
+
if (!shouldGenerate)
return;
@@ -1294,6 +1325,12 @@
eliminate(putByOffsetStoreElimination(m_graph.m_storageAccessData[node.storageAccessDataIndex()].identifierNumber, node.child1().index()));
break;
+ case Phantom:
+ // FIXME: we ought to remove Phantom's that have no children.
+
+ eliminateIrrelevantPhantomChildren(node);
+ break;
+
default:
// do nothing.
break;
@@ -1315,6 +1352,23 @@
m_currentBlock = block;
for (unsigned i = 0; i < LastNodeType; ++i)
m_lastSeen[i] = UINT_MAX;
+
+ // Make all of my Phi nodes relevant to OSR.
+ for (unsigned i = 0; i < block->phis.size(); ++i)
+ m_relevantToOSR.set(block->phis[i]);
+
+ // Make all of my SetLocal nodes relevant to OSR.
+ for (unsigned i = 0; i < block->size(); ++i) {
+ NodeIndex nodeIndex = block->at(i);
+ Node& node = m_graph[nodeIndex];
+ switch (node.op()) {
+ case SetLocal:
+ m_relevantToOSR.set(nodeIndex);
+ break;
+ default:
+ break;
+ }
+ }
for (m_indexInBlock = 0; m_indexInBlock < block->size(); ++m_indexInBlock) {
m_compileIndex = block->at(m_indexInBlock);
@@ -1327,6 +1381,7 @@
unsigned m_indexInBlock;
Vector<NodeIndex, 16> m_replacements;
FixedArray<unsigned, LastNodeType> m_lastSeen;
+ FastBitVector m_relevantToOSR;
bool m_changed; // Only tracks changes that have a substantive effect on other optimizations.
};
