DFG BasicBlock should group the Phi nodes together and separate them

from the other nodes
https://bugs.webkit.org/show_bug.cgi?id=80361

Reviewed by Filip Pizlo.

This would make it more efficient to remove the redundant Phi nodes or
insert new Phi nodes for SSA, besides providing a cleaner BasicBlock structure.
This is performance neutral on SunSpider, V8 and Kraken.

* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::clobberStructures):
(JSC::DFG::AbstractState::dump):
* dfg/DFGBasicBlock.h:
(JSC::DFG::BasicBlock::BasicBlock):
(BasicBlock):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::addToGraph):
(JSC::DFG::ByteCodeParser::insertPhiNode):
* dfg/DFGCFAPhase.cpp:
(JSC::DFG::CFAPhase::performBlockCFA):
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::pureCSE):
(JSC::DFG::CSEPhase::impureCSE):
(JSC::DFG::CSEPhase::globalVarLoadElimination):
(JSC::DFG::CSEPhase::getByValLoadElimination):
(JSC::DFG::CSEPhase::checkFunctionElimination):
(JSC::DFG::CSEPhase::checkStructureLoadElimination):
(JSC::DFG::CSEPhase::getByOffsetLoadElimination):
(JSC::DFG::CSEPhase::getPropertyStorageLoadElimination):
(JSC::DFG::CSEPhase::getIndexedPropertyStorageLoadElimination):
(JSC::DFG::CSEPhase::getScopeChainLoadElimination):
(JSC::DFG::CSEPhase::performBlockCSE):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compile):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@110008 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 19443b6..ed0c417 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,43 @@
+2012-03-06  Yuqiang Xian  <yuqiang.xian@intel.com>
+
+        DFG BasicBlock should group the Phi nodes together and separate them
+        from the other nodes
+        https://bugs.webkit.org/show_bug.cgi?id=80361
+
+        Reviewed by Filip Pizlo.
+
+        This would make it more efficient to remove the redundant Phi nodes or
+        insert new Phi nodes for SSA, besides providing a cleaner BasicBlock structure.
+        This is performance neutral on SunSpider, V8 and Kraken.
+
+        * dfg/DFGAbstractState.cpp:
+        (JSC::DFG::AbstractState::clobberStructures):
+        (JSC::DFG::AbstractState::dump):
+        * dfg/DFGBasicBlock.h:
+        (JSC::DFG::BasicBlock::BasicBlock):
+        (BasicBlock):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::addToGraph):
+        (JSC::DFG::ByteCodeParser::insertPhiNode):
+        * dfg/DFGCFAPhase.cpp:
+        (JSC::DFG::CFAPhase::performBlockCFA):
+        * dfg/DFGCSEPhase.cpp:
+        (JSC::DFG::CSEPhase::pureCSE):
+        (JSC::DFG::CSEPhase::impureCSE):
+        (JSC::DFG::CSEPhase::globalVarLoadElimination):
+        (JSC::DFG::CSEPhase::getByValLoadElimination):
+        (JSC::DFG::CSEPhase::checkFunctionElimination):
+        (JSC::DFG::CSEPhase::checkStructureLoadElimination):
+        (JSC::DFG::CSEPhase::getByOffsetLoadElimination):
+        (JSC::DFG::CSEPhase::getPropertyStorageLoadElimination):
+        (JSC::DFG::CSEPhase::getIndexedPropertyStorageLoadElimination):
+        (JSC::DFG::CSEPhase::getScopeChainLoadElimination):
+        (JSC::DFG::CSEPhase::performBlockCSE):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::dump):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+
 2012-03-06  Mark Hahnenberg  <mhahnenberg@apple.com>
 
         GCActivityCallback timer should vary with the length of the previous GC

diff --git a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
index 95a4c0f..afe0ec7 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
+++ b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
@@ -984,7 +984,7 @@ inline void AbstractState::clobberStructures(unsigned indexInBlock)
     PROFILE(FLAG_FOR_STRUCTURE_CLOBBERING);
     if (!m_haveStructures)
         return;
-    for (size_t i = indexInBlock + 1; i-- > m_block->startExcludingPhis;)
+    for (size_t i = indexInBlock + 1; i--;)
         forNode(m_block->at(i)).clobberStructures();
     for (size_t i = 0; i < m_variables.numberOfArguments(); ++i)
         m_variables.argument(i).clobberStructures();
@@ -1143,7 +1143,7 @@ inline bool AbstractState::mergeVariableBetweenBlocks(AbstractValue& destination
 void AbstractState::dump(FILE* out)
 {
     bool first = true;
-    for (size_t i = m_block->startExcludingPhis; i < m_block->size(); ++i) {
+    for (size_t i = 0; i < m_block->size(); ++i) {
         NodeIndex index = m_block->at(i);
         AbstractValue& value = m_nodes[index];
         if (value.isClear())

diff --git a/Source/JavaScriptCore/dfg/DFGBasicBlock.h b/Source/JavaScriptCore/dfg/DFGBasicBlock.h
index 27c0255..1c890b4 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGBasicBlock.h
+++ b/Source/JavaScriptCore/dfg/DFGBasicBlock.h
@@ -48,7 +48,6 @@ struct BasicBlock : Vector<NodeIndex, 8> {
         , isLinked(false)
 #endif
         , isReachable(false)
-        , startExcludingPhis(0)
         , variablesAtHead(numArguments, numLocals)
         , variablesAtTail(numArguments, numLocals)
         , valuesAtHead(numArguments, numLocals)
@@ -75,8 +74,8 @@ struct BasicBlock : Vector<NodeIndex, 8> {
     bool isLinked;
 #endif
     bool isReachable;
-    unsigned startExcludingPhis;
     
+    Vector<NodeIndex> phis;
     PredecessorList m_predecessors;
     
     Operands<NodeIndex, NodeIndexTraits> variablesAtHead;

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
index 28eb063..699c619 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -569,6 +569,7 @@ private:
     {
         NodeIndex resultIndex = (NodeIndex)m_graph.size();
         m_graph.append(Node(op, currentCodeOrigin(), child1, child2, child3));
+        ASSERT(op != Phi);
         m_currentBlock->append(resultIndex);
 
         if (op & NodeMustGenerate)
@@ -579,7 +580,10 @@ private:
     {
         NodeIndex resultIndex = (NodeIndex)m_graph.size();
         m_graph.append(Node(op, currentCodeOrigin(), info, child1, child2, child3));
-        m_currentBlock->append(resultIndex);
+        if (op == Phi)
+            m_currentBlock->phis.append(resultIndex);
+        else
+            m_currentBlock->append(resultIndex);
 
         if (op & NodeMustGenerate)
             m_graph.ref(resultIndex);
@@ -589,6 +593,7 @@ private:
     {
         NodeIndex resultIndex = (NodeIndex)m_graph.size();
         m_graph.append(Node(op, currentCodeOrigin(), info1, info2, child1, child2, child3));
+        ASSERT(op != Phi);
         m_currentBlock->append(resultIndex);
 
         if (op & NodeMustGenerate)
@@ -600,6 +605,7 @@ private:
     {
         NodeIndex resultIndex = (NodeIndex)m_graph.size();
         m_graph.append(Node(Node::VarArg, op, currentCodeOrigin(), info1, info2, m_graph.m_varArgChildren.size() - m_numPassedVarArgs, m_numPassedVarArgs));
+        ASSERT(op != Phi);
         m_currentBlock->append(resultIndex);
         
         m_numPassedVarArgs = 0;
@@ -613,8 +619,7 @@ private:
     {
         NodeIndex resultIndex = (NodeIndex)m_graph.size();
         m_graph.append(Node(Phi, currentCodeOrigin(), info));
-        block->prepend(resultIndex);
-        ++block->startExcludingPhis;
+        block->phis.append(resultIndex);
 
         return resultIndex;
     }

diff --git a/Source/JavaScriptCore/dfg/DFGCFAPhase.cpp b/Source/JavaScriptCore/dfg/DFGCFAPhase.cpp
index 34d2d07..0bb6759 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGCFAPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGCFAPhase.cpp
@@ -82,7 +82,7 @@ private:
         dumpOperands(block->valuesAtHead, WTF::dataFile());
         dataLog("\n");
 #endif
-        for (unsigned i = block->startExcludingPhis; i < block->size(); ++i) {
+        for (unsigned i = 0; i < block->size(); ++i) {
             NodeIndex nodeIndex = block->at(i);
             if (!m_graph[nodeIndex].shouldGenerate())
                 continue;

diff --git a/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp b/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
index 08d454f..4a91efb 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
@@ -88,7 +88,7 @@ private:
         NodeIndex child2 = canonicalize(node.child2());
         NodeIndex child3 = canonicalize(node.child3());
         
-        for (unsigned i = endIndexForPureCSE(); i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = endIndexForPureCSE(); i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1 || index == child2 || index == child3)
                 break;
@@ -175,7 +175,7 @@ private:
         NodeIndex child2 = canonicalize(node.child2());
         NodeIndex child3 = canonicalize(node.child3());
         
-        for (unsigned i = m_indexInBlock; i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = m_indexInBlock; i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1 || index == child2 || index == child3)
                 break;
@@ -207,7 +207,7 @@ private:
     
     NodeIndex globalVarLoadElimination(unsigned varNumber, JSGlobalObject* globalObject)
     {
-        for (unsigned i = m_indexInBlock; i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = m_indexInBlock; i--;) {
             NodeIndex index = m_currentBlock->at(i);
             Node& node = m_graph[index];
             switch (node.op) {
@@ -230,7 +230,7 @@ private:
     
     NodeIndex getByValLoadElimination(NodeIndex child1, NodeIndex child2)
     {
-        for (unsigned i = m_indexInBlock; i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = m_indexInBlock; i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1 || index == canonicalize(child2)) 
                 break;
@@ -274,7 +274,7 @@ private:
 
     bool checkFunctionElimination(JSFunction* function, NodeIndex child1)
     {
-        for (unsigned i = endIndexForPureCSE(); i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = endIndexForPureCSE(); i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1) 
                 break;
@@ -288,7 +288,7 @@ private:
 
     bool checkStructureLoadElimination(const StructureSet& structureSet, NodeIndex child1)
     {
-        for (unsigned i = m_indexInBlock; i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = m_indexInBlock; i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1) 
                 break;
@@ -334,7 +334,7 @@ private:
     
     NodeIndex getByOffsetLoadElimination(unsigned identifierNumber, NodeIndex child1)
     {
-        for (unsigned i = m_indexInBlock; i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = m_indexInBlock; i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1) 
                 break;
@@ -380,7 +380,7 @@ private:
     
     NodeIndex getPropertyStorageLoadElimination(NodeIndex child1)
     {
-        for (unsigned i = m_indexInBlock; i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = m_indexInBlock; i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1) 
                 break;
@@ -419,7 +419,7 @@ private:
 
     NodeIndex getIndexedPropertyStorageLoadElimination(NodeIndex child1, bool hasIntegerIndexPrediction)
     {
-        for (unsigned i = m_indexInBlock; i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = m_indexInBlock; i--;) {
             NodeIndex index = m_currentBlock->at(i);
             if (index == child1) 
                 break;
@@ -460,7 +460,7 @@ private:
     
     NodeIndex getScopeChainLoadElimination(unsigned depth)
     {
-        for (unsigned i = endIndexForPureCSE(); i-- > m_currentBlock->startExcludingPhis;) {
+        for (unsigned i = endIndexForPureCSE(); i--;) {
             NodeIndex index = m_currentBlock->at(i);
             Node& node = m_graph[index];
             if (node.op == GetScopeChain
@@ -681,7 +681,7 @@ private:
         for (unsigned i = 0; i < LastNodeId; ++i)
             m_lastSeen[i] = UINT_MAX;
 
-        for (m_indexInBlock = block.startExcludingPhis; m_indexInBlock < block.size(); ++m_indexInBlock) {
+        for (m_indexInBlock = 0; m_indexInBlock < block.size(); ++m_indexInBlock) {
             m_compileIndex = block[m_indexInBlock];
             performNodeCSE(m_graph[m_compileIndex]);
         }

diff --git a/Source/JavaScriptCore/dfg/DFGGraph.cpp b/Source/JavaScriptCore/dfg/DFGGraph.cpp
index d228a87..322c5a4 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGGraph.cpp
+++ b/Source/JavaScriptCore/dfg/DFGGraph.cpp
@@ -265,6 +265,9 @@ void Graph::dump()
     for (size_t b = 0; b < m_blocks.size(); ++b) {
         BasicBlock* block = m_blocks[b].get();
         dataLog("Block #%u (bc#%u): %s%s\n", (int)b, block->bytecodeBegin, block->isReachable ? "" : " (skipped)", block->isOSRTarget ? " (OSR target)" : "");
+        dataLog("  Phi Nodes:\n");
+        for (size_t i = 0; i < block->phis.size(); ++i)
+            dump(block->phis[i]);
         dataLog("  vars before: ");
         if (block->cfaHasVisited)
             dumpOperands(block->valuesAtHead, WTF::dataFile());

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 4a4acae..a3d61fb 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -961,7 +961,7 @@ void SpeculativeJIT::compile(BasicBlock& block)
         verificationSucceeded.link(&m_jit);
     }
 
-    for (m_indexInBlock = block.startExcludingPhis; m_indexInBlock < block.size(); ++m_indexInBlock) {
+    for (m_indexInBlock = 0; m_indexInBlock < block.size(); ++m_indexInBlock) {
         m_compileIndex = block[m_indexInBlock];
         Node& node = at(m_compileIndex);
         m_codeOriginForOSR = node.codeOrigin;