Diff
Modified: branches/dfgFourthTier/Source/_javascript_Core/ChangeLog (152827 => 152828)
--- branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-07-18 02:13:49 UTC (rev 152827)
+++ branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-07-18 04:38:54 UTC (rev 152828)
@@ -1,5 +1,39 @@
2013-07-16 Filip Pizlo <[email protected]>
+ fourthTier: Rationalize Node::replacement
+ https://bugs.webkit.org/show_bug.cgi?id=118774
+
+ Reviewed by Oliver Hunt.
+
+ - Clearing of replacements is now done in Graph::clearReplacements().
+
+ - New nodes now have replacement set to 0.
+
+ - Node::replacement is now part of a 'misc' union. I'll be putting at least
+ one other field into that union as part of LICM work (see
+ https://bugs.webkit.org/show_bug.cgi?id=118749).
+
+ * dfg/DFGCPSRethreadingPhase.cpp:
+ (JSC::DFG::CPSRethreadingPhase::run):
+ (JSC::DFG::CPSRethreadingPhase::freeUnnecessaryNodes):
+ (JSC::DFG::CPSRethreadingPhase::canonicalizeGetLocalFor):
+ * dfg/DFGCSEPhase.cpp:
+ (JSC::DFG::CSEPhase::run):
+ (JSC::DFG::CSEPhase::setReplacement):
+ (JSC::DFG::CSEPhase::performBlockCSE):
+ * dfg/DFGGraph.cpp:
+ (DFG):
+ (JSC::DFG::Graph::clearReplacements):
+ * dfg/DFGGraph.h:
+ (JSC::DFG::Graph::performSubstitutionForEdge):
+ (Graph):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::Node):
+ * dfg/DFGSSAConversionPhase.cpp:
+ (JSC::DFG::SSAConversionPhase::run):
+
+2013-07-16 Filip Pizlo <[email protected]>
+
fourthTier: NaturalLoops should be able to quickly answer questions like "what loops own this basic block"
https://bugs.webkit.org/show_bug.cgi?id=118750
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGCPSRethreadingPhase.cpp (152827 => 152828)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGCPSRethreadingPhase.cpp 2013-07-18 02:13:49 UTC (rev 152827)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGCPSRethreadingPhase.cpp 2013-07-18 04:38:54 UTC (rev 152828)
@@ -49,6 +49,7 @@
clearIsLoadedFrom();
freeUnnecessaryNodes();
+ m_graph.clearReplacements();
canonicalizeLocalsInBlocks();
propagatePhis<LocalOperand>();
propagatePhis<ArgumentOperand>();
@@ -102,7 +103,6 @@
default:
break;
}
- node->replacement = 0; // Reset the replacement since the next phase will use it.
block->at(toIndex++) = node;
}
block->resize(toIndex);
@@ -198,12 +198,12 @@
if (otherNode->op() == GetLocal) {
// Replace all references to this GetLocal with otherNode.
- node->replacement = otherNode;
+ node->misc.replacement = otherNode;
return;
}
ASSERT(otherNode->op() == SetLocal);
- node->replacement = otherNode->child1().node();
+ node->misc.replacement = otherNode->child1().node();
return;
}
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGCSEPhase.cpp (152827 => 152828)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-07-18 02:13:49 UTC (rev 152827)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGCSEPhase.cpp 2013-07-18 04:38:54 UTC (rev 152828)
@@ -52,6 +52,8 @@
m_changed = false;
+ m_graph.clearReplacements();
+
for (unsigned blockIndex = 0; blockIndex < m_graph.numBlocks(); ++blockIndex)
performBlockCSE(m_graph.block(blockIndex));
@@ -1003,7 +1005,7 @@
eliminateIrrelevantPhantomChildren(m_currentNode);
// At this point we will eliminate all references to this node.
- m_currentNode->replacement = replacement;
+ m_currentNode->misc.replacement = replacement;
m_changed = true;
@@ -1393,12 +1395,10 @@
for (unsigned i = 0; i < LastNodeType; ++i)
m_lastSeen[i] = UINT_MAX;
- // All Phis need to already be marked as relevant to OSR, and have their
- // replacements cleared, so we don't get confused while doing substitutions on
- // GetLocal's.
- for (unsigned i = 0; i < block->phis.size(); ++i) {
- ASSERT(block->phis[i]->flags() & NodeRelevantToOSR);
- block->phis[i]->replacement = 0;
+ // All Phis need to already be marked as relevant to OSR.
+ if (!ASSERT_DISABLED) {
+ for (unsigned i = 0; i < block->phis.size(); ++i)
+ ASSERT(block->phis[i]->flags() & NodeRelevantToOSR);
}
// Make all of my SetLocal and GetLocal nodes relevant to OSR, and do some other
@@ -1406,8 +1406,6 @@
for (unsigned i = 0; i < block->size(); ++i) {
Node* node = block->at(i);
- node->replacement = 0;
-
switch (node->op()) {
case SetLocal:
case GetLocal: // FIXME: The GetLocal case is only necessary until we do https://bugs.webkit.org/show_bug.cgi?id=106707.
@@ -1427,7 +1425,7 @@
if (!ASSERT_DISABLED && cseMode == StoreElimination) {
// Nobody should have replacements set.
for (unsigned i = 0; i < block->size(); ++i)
- ASSERT(!block->at(i)->replacement);
+ ASSERT(!block->at(i)->misc.replacement);
}
}
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.cpp (152827 => 152828)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.cpp 2013-07-18 02:13:49 UTC (rev 152827)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.cpp 2013-07-18 04:38:54 UTC (rev 152828)
@@ -533,6 +533,19 @@
addForDepthFirstSort(result, worklist, seen, block->successor(i));
}
}
+
+void Graph::clearReplacements()
+{
+ for (BlockIndex blockIndex = numBlocks(); blockIndex--;) {
+ BasicBlock* block = m_blocks[blockIndex].get();
+ if (!block)
+ continue;
+ for (unsigned phiIndex = block->phis.size(); phiIndex--;)
+ block->phis[phiIndex]->misc.replacement = 0;
+ for (unsigned nodeIndex = block->size(); nodeIndex--;)
+ block->at(nodeIndex)->misc.replacement = 0;
+ }
+}
} } // namespace JSC::DFG
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.h (152827 => 152828)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.h 2013-07-18 02:13:49 UTC (rev 152827)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.h 2013-07-18 04:38:54 UTC (rev 152828)
@@ -119,7 +119,7 @@
return;
// Check if there is any replacement.
- Node* replacement = child->replacement;
+ Node* replacement = child->misc.replacement;
if (!replacement)
return;
@@ -127,7 +127,7 @@
// There is definitely a replacement. Assert that the replacement does not
// have a replacement.
- ASSERT(!child->replacement);
+ ASSERT(!child->misc.replacement);
}
#define DFG_DEFINE_ADD_NODE(templatePre, templatePost, typeParams, valueParamsComma, valueParams, valueArgs) \
@@ -646,6 +646,8 @@
void invalidateCFG();
+ void clearReplacements();
+
void getBlocksInDepthFirstOrder(Vector<BasicBlock*>& result);
Profiler::Compilation* compilation() { return m_plan.compilation.get(); }
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGNode.h (152827 => 152828)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGNode.h 2013-07-18 02:13:49 UTC (rev 152827)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGNode.h 2013-07-18 04:38:54 UTC (rev 152828)
@@ -167,6 +167,7 @@
, m_refCount(1)
, m_prediction(SpecNone)
{
+ misc.replacement = 0;
setOpAndDefaultFlags(op);
}
@@ -178,6 +179,7 @@
, m_refCount(1)
, m_prediction(SpecNone)
{
+ misc.replacement = 0;
setOpAndDefaultFlags(op);
ASSERT(!(m_flags & NodeHasVarArgs));
}
@@ -191,6 +193,7 @@
, m_prediction(SpecNone)
, m_opInfo(imm.m_value)
{
+ misc.replacement = 0;
setOpAndDefaultFlags(op);
ASSERT(!(m_flags & NodeHasVarArgs));
}
@@ -205,6 +208,7 @@
, m_opInfo(imm1.m_value)
, m_opInfo2(imm2.m_value)
{
+ misc.replacement = 0;
setOpAndDefaultFlags(op);
ASSERT(!(m_flags & NodeHasVarArgs));
}
@@ -219,6 +223,7 @@
, m_opInfo(imm1.m_value)
, m_opInfo2(imm2.m_value)
{
+ misc.replacement = 0;
setOpAndDefaultFlags(op);
ASSERT(m_flags & NodeHasVarArgs);
}
@@ -1391,7 +1396,9 @@
public:
// Fields used by various analyses.
AbstractValue value;
- Node* replacement;
+ union {
+ Node* replacement;
+ } misc;
};
inline bool nodeComparator(Node* a, Node* b)
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp (152827 => 152828)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp 2013-07-18 02:13:49 UTC (rev 152827)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSSAConversionPhase.cpp 2013-07-18 04:38:54 UTC (rev 152828)
@@ -194,15 +194,7 @@
// do a replacement for (3).
// Clear all replacements, since other phases may have used them.
- for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
- BasicBlock* block = m_graph.block(blockIndex);
- if (!block)
- continue;
- for (unsigned phiIndex = block->phis.size(); phiIndex--;)
- block->phis[phiIndex]->replacement = 0;
- for (unsigned nodeIndex = block->size(); nodeIndex--;)
- block->at(nodeIndex)->replacement = 0;
- }
+ m_graph.clearReplacements();
// For all of the old CPS Phis, figure out what they correspond to in SSA.
for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
@@ -217,7 +209,7 @@
", and its replacement in ", *block, ", ",
block->variablesAtHead.operand(phi->local()), "\n");
}
- phi->replacement = block->variablesAtHead.operand(phi->local());
+ phi->misc.replacement = block->variablesAtHead.operand(phi->local());
}
}
@@ -232,8 +224,8 @@
Node* node = block->variablesAtHead[i];
if (!node)
continue;
- while (node->replacement)
- node = node->replacement;
+ while (node->misc.replacement)
+ node = node->misc.replacement;
block->variablesAtHead[i] = node;
}
}
@@ -275,11 +267,11 @@
continue;
for (unsigned phiIndex = block->phis.size(); phiIndex--;) {
- block->phis[phiIndex]->replacement =
+ block->phis[phiIndex]->misc.replacement =
block->variablesAtHead.operand(block->phis[phiIndex]->local());
}
for (unsigned nodeIndex = block->size(); nodeIndex--;)
- ASSERT(!block->at(nodeIndex)->replacement);
+ ASSERT(!block->at(nodeIndex)->misc.replacement);
for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
Node* node = block->at(nodeIndex);
@@ -293,7 +285,7 @@
node->mergeFlags(NodeMustGenerate);
else
node->setOpAndDefaultFlags(MovHint);
- node->replacement = node->child1().node(); // Only for Upsilons.
+ node->misc.replacement = node->child1().node(); // Only for Upsilons.
break;
}
@@ -307,7 +299,7 @@
if (variable->isCaptured())
break;
node->convertToPhantom();
- node->replacement = block->variablesAtHead.operand(variable->local());
+ node->misc.replacement = block->variablesAtHead.operand(variable->local());
break;
}
@@ -315,7 +307,7 @@
node->children.reset();
// This is only for Upsilons. An Upsilon will only refer to a Flush if
// there were no SetLocals or GetLocals in the block.
- node->replacement = block->variablesAtHead.operand(node->local());
+ node->misc.replacement = block->variablesAtHead.operand(node->local());
break;
}
@@ -327,7 +319,7 @@
node->convertToPhantom();
// This is only for Upsilons. An Upsilon will only refer to a
// PhantomLocal if there were no SetLocals or GetLocals in the block.
- node->replacement = block->variablesAtHead.operand(variable->local());
+ node->misc.replacement = block->variablesAtHead.operand(variable->local());
break;
}
