Improve SSA dominator computation memory usage

Author: mikedn

src/jit/block.h

@@ -1202,9 +1202,15 @@ struct JitPtrKeyFuncs<BasicBlock> : public JitKeyFuncsDefEquals<const BasicBlock
 // A set of blocks.
 typedef JitHashTable<BasicBlock*, JitPtrKeyFuncs<BasicBlock>, bool> BlkSet;
 
+// A vector of blocks.
+typedef jitstd::vector<BasicBlock*> BlkVector;
+
 // A map of block -> set of blocks, can be used as sparse block trees.
 typedef JitHashTable<BasicBlock*, JitPtrKeyFuncs<BasicBlock>, BlkSet*> BlkToBlkSetMap;
 
+// A map of block -> vector of blocks, can be used as sparse block trees.
+typedef JitHashTable<BasicBlock*, JitPtrKeyFuncs<BasicBlock>, BlkVector> BlkToBlkVectorMap;
+
 // Map from Block to Block.  Used for a variety of purposes.
 typedef JitHashTable<BasicBlock*, JitPtrKeyFuncs<BasicBlock>, BasicBlock*> BlockToBlockMap;
 

src/jit/compiler.h

@@ -25,6 +25,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 #include "jit.h"
 #include "opcode.h"
 #include "varset.h"
+#include "jitstd.h"
+#include "jithashtable.h"
 #include "gentree.h"
 #include "lir.h"
 #include "block.h"
@@ -33,10 +35,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 #include "instr.h"
 #include "regalloc.h"
 #include "sm.h"
-#include "jithashtable.h"
 #include "cycletimer.h"
 #include "blockset.h"
-#include "jitstd.h"
 #include "arraystack.h"
 #include "hashbv.h"
 #include "fp.h"

src/jit/copyprop.cpp

@@ -412,7 +412,7 @@ void Compiler::optVnCopyProp()
     CompAllocator allocator(this, CMK_CopyProp);
 
     // Compute the domTree to use.
-    BlkToBlkSetMap* domTree = new (&allocator) BlkToBlkSetMap(&allocator);
+    BlkToBlkVectorMap* domTree = new (&allocator) BlkToBlkVectorMap(&allocator);
     domTree->Reallocate(fgBBcount * 3 / 2); // Prime the allocation
     SsaBuilder::ComputeDominators(this, domTree);
 
@@ -455,12 +455,12 @@ void Compiler::optVnCopyProp()
         optBlockCopyProp(block, &curSsaName);
 
         // Add dom children to work on.
-        BlkSet* pBlkSet;
-        if (domTree->Lookup(block, &pBlkSet))
+        BlkVector* domChildren = domTree->LookupPointer(block);
+        if (domChildren != nullptr)
         {
-            for (BlkSet::KeyIterator child = pBlkSet->Begin(); !child.Equal(pBlkSet->End()); ++child)
+            for (BasicBlock* child : *domChildren)
             {
-                worklist->push_back(BlockWork(child.Get()));
+                worklist->push_back(BlockWork(child));
             }
         }
     }

src/jit/gentree.h

@@ -24,6 +24,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 #include "ssaconfig.h" // For "SsaConfig::RESERVED_SSA_NUM"
 #include "reglist.h"
 #include "valuenumtype.h"
+#include "jitstd.h"
 #include "jithashtable.h"
 #include "nodeinfo.h"
 #include "simd.h"

src/jit/jithashtable.h

@@ -264,13 +264,47 @@ class JitHashTable
         }
         else
         {
-            Node* pNewNode = new (m_alloc) Node(k, v, m_table[index]);
+            Node* pNewNode = new (m_alloc) Node(m_table[index], k, v);
             m_table[index] = pNewNode;
             m_tableCount++;
             return false;
         }
     }
 
+    //------------------------------------------------------------------------
+    // Emplace: Associates the specified key with a value constructed in-place
+    // using the supplies args if the key is not already present.
+    //
+    // Arguments:
+    //    k - the key
+    //    args - the args used to construct the value
+    //
+    // Return Value:
+    //    A pointer to the existing or newly constructed value.
+    //
+    template <class... Args>
+    Value* Emplace(Key k, Args&&... args)
+    {
+        CheckGrowth();
+
+        assert(m_tableSizeInfo.prime != 0);
+
+        unsigned index = GetIndexForKey(k);
+
+        Node* pN = m_table[index];
+        while ((pN != nullptr) && !KeyFuncs::Equals(k, pN->m_key))
+        {
+            pN = pN->m_next;
+        }
+        if (pN == nullptr)
+        {
+            pN             = new (m_alloc) Node(m_table[index], k, jitstd::forward<Args>(args)...);
+            m_table[index] = pN;
+            m_tableCount++;
+        }
+        return &pN->m_val;
+    }
+
     //------------------------------------------------------------------------
     // Remove: Remove the specified key and its associated value.
     //
@@ -592,7 +626,7 @@ class JitHashTable
         // Assumptions:
         //    This must not be the "end" iterator.
         //
-        const Value& GetValue() const
+        Value& GetValue() const
         {
             assert(m_node != nullptr);
 
@@ -721,7 +755,8 @@ class JitHashTable
         Key   m_key;
         Value m_val;
 
-        Node(Key k, Value v, Node* next) : m_next(next), m_key(k), m_val(v)
+        template <class... Args>
+        Node(Node* next, Key k, Args&&... args) : m_next(next), m_key(k), m_val(jitstd::forward<Args>(args)...)
         {
         }
 

src/jit/ssabuilder.cpp

@@ -359,22 +359,22 @@ void SsaBuilder::ComputeImmediateDom(BasicBlock** postOrder, int count)
  * @remarks This would help us answer queries such as "a dom b?" in constant time.
  *          For example, if a dominated b, then Pre[a] < Pre[b] but Post[a] > Post[b]
  */
-void SsaBuilder::DomTreeWalk(BasicBlock* curBlock, BlkToBlkSetMap* domTree, int* preIndex, int* postIndex)
+void SsaBuilder::DomTreeWalk(BasicBlock* curBlock, BlkToBlkVectorMap* domTree, int* preIndex, int* postIndex)
 {
     JITDUMP("[SsaBuilder::DomTreeWalk] block %s:\n", curBlock->dspToString());
 
     // Store the order number at the block number in the pre order list.
     m_pDomPreOrder[curBlock->bbNum] = *preIndex;
     ++(*preIndex);
 
-    BlkSet* pBlkSet;
-    if (domTree->Lookup(curBlock, &pBlkSet))
+    BlkVector* domChildren = domTree->LookupPointer(curBlock);
+    if (domChildren != nullptr)
     {
-        for (BlkSet::KeyIterator ki = pBlkSet->Begin(); !ki.Equal(pBlkSet->End()); ++ki)
+        for (BasicBlock* child : *domChildren)
         {
-            if (curBlock != ki.Get())
+            if (curBlock != child)
             {
-                DomTreeWalk(ki.Get(), domTree, preIndex, postIndex);
+                DomTreeWalk(child, domTree, preIndex, postIndex);
             }
         }
     }
@@ -393,7 +393,7 @@ void SsaBuilder::DomTreeWalk(BasicBlock* curBlock, BlkToBlkSetMap* domTree, int*
  *
  */
 /* static */
-void SsaBuilder::ConstructDomTreeForBlock(Compiler* pCompiler, BasicBlock* block, BlkToBlkSetMap* domTree)
+void SsaBuilder::ConstructDomTreeForBlock(Compiler* pCompiler, BasicBlock* block, BlkToBlkVectorMap* domTree)
 {
     BasicBlock* bbIDom = block->bbIDom;
 
@@ -404,16 +404,11 @@ void SsaBuilder::ConstructDomTreeForBlock(Compiler* pCompiler, BasicBlock* block
     }
 
     // If the bbIDom map key doesn't exist, create one.
-    BlkSet* pBlkSet;
-    if (!domTree->Lookup(bbIDom, &pBlkSet))
-    {
-        pBlkSet = new (domTree->GetAllocator()) BlkSet(domTree->GetAllocator());
-        domTree->Set(bbIDom, pBlkSet);
-    }
+    BlkVector* domChildren = domTree->Emplace(bbIDom, domTree->GetAllocator());
 
     DBG_SSA_JITDUMP("Inserting BB%02u as dom child of BB%02u.\n", block->bbNum, bbIDom->bbNum);
     // Insert the block into the block's set.
-    pBlkSet->Set(block, true);
+    domChildren->push_back(block);
 }
 
 /**
@@ -426,7 +421,7 @@ void SsaBuilder::ConstructDomTreeForBlock(Compiler* pCompiler, BasicBlock* block
  *
  */
 /* static */
-void SsaBuilder::ComputeDominators(Compiler* pCompiler, BlkToBlkSetMap* domTree)
+void SsaBuilder::ComputeDominators(Compiler* pCompiler, BlkToBlkVectorMap* domTree)
 {
     JITDUMP("*************** In SsaBuilder::ComputeDominators(Compiler*, ...)\n");
 
@@ -450,7 +445,7 @@ void SsaBuilder::ComputeDominators(Compiler* pCompiler, BlkToBlkSetMap* domTree)
  * @param domTree A map of (block -> set of blocks) tree representation that is empty.
  *
  */
-void SsaBuilder::ComputeDominators(BasicBlock** postOrder, int count, BlkToBlkSetMap* domTree)
+void SsaBuilder::ComputeDominators(BasicBlock** postOrder, int count, BlkToBlkVectorMap* domTree)
 {
     JITDUMP("*************** In SsaBuilder::ComputeDominators(BasicBlock** postOrder, int count, ...)\n");
 
@@ -486,21 +481,22 @@ void SsaBuilder::ComputeDominators(BasicBlock** postOrder, int count, BlkToBlkSe
  * @param domTree A map of (block -> set of blocks) tree representation.
  */
 /* static */
-void SsaBuilder::DisplayDominators(BlkToBlkSetMap* domTree)
+void SsaBuilder::DisplayDominators(BlkToBlkVectorMap* domTree)
 {
     printf("After computing dominator tree: \n");
-    for (BlkToBlkSetMap::KeyIterator nodes = domTree->Begin(); !nodes.Equal(domTree->End()); ++nodes)
+    for (BlkToBlkVectorMap::KeyIterator nodes = domTree->Begin(); !nodes.Equal(domTree->End()); ++nodes)
     {
         printf("BB%02u := {", nodes.Get()->bbNum);
+        bool first = true;
 
-        BlkSet* pBlkSet = nodes.GetValue();
-        for (BlkSet::KeyIterator ki = pBlkSet->Begin(); !ki.Equal(pBlkSet->End()); ++ki)
+        for (BasicBlock* child : nodes.GetValue())
         {
-            if (!ki.Equal(pBlkSet->Begin()))
+            if (!first)
             {
                 printf(",");
             }
-            printf("BB%02u", ki.Get()->bbNum);
+            first = false;
+            printf("BB%02u", child->bbNum);
         }
         printf("}\n");
     }
@@ -1657,7 +1653,7 @@ void SsaBuilder::BlockPopStacks(BasicBlock* block, SsaRenameState* pRenameState)
  *      and Destruction of Static Single Assignment Form."
  */
 
-void SsaBuilder::RenameVariables(BlkToBlkSetMap* domTree, SsaRenameState* pRenameState)
+void SsaBuilder::RenameVariables(BlkToBlkVectorMap* domTree, SsaRenameState* pRenameState)
 {
     JITDUMP("*************** In SsaBuilder::RenameVariables()\n");
 
@@ -1750,13 +1746,13 @@ void SsaBuilder::RenameVariables(BlkToBlkSetMap* domTree, SsaRenameState* pRenam
             AssignPhiNodeRhsVariables(block, pRenameState);
 
             // Recurse with the block's DOM children.
-            BlkSet* pBlkSet;
-            if (domTree->Lookup(block, &pBlkSet))
+            BlkVector* domChildren = domTree->LookupPointer(block);
+            if (domChildren != nullptr)
             {
-                for (BlkSet::KeyIterator child = pBlkSet->Begin(); !child.Equal(pBlkSet->End()); ++child)
+                for (BasicBlock* child : *domChildren)
                 {
-                    DBG_SSA_JITDUMP("[SsaBuilder::RenameVariables](pushing dom child BB%02u)\n", child.Get()->bbNum);
-                    blocksToDo->push_back(BlockWork(child.Get()));
+                    DBG_SSA_JITDUMP("[SsaBuilder::RenameVariables](pushing dom child BB%02u)\n", child->bbNum);
+                    blocksToDo->push_back(BlockWork(child));
                 }
             }
         }
@@ -1867,7 +1863,7 @@ void SsaBuilder::Build()
     ComputeImmediateDom(postOrder, count);
 
     // Compute the dominator tree.
-    BlkToBlkSetMap* domTree = new (&m_allocator) BlkToBlkSetMap(&m_allocator);
+    BlkToBlkVectorMap* domTree = new (&m_allocator) BlkToBlkVectorMap(&m_allocator);
     ComputeDominators(postOrder, count, domTree);
     EndPhase(PHASE_BUILD_SSA_DOMS);
 

src/jit/ssabuilder.h

@@ -62,7 +62,7 @@ class SsaBuilder
     // Using IDom of each basic block, compute the whole domTree. If a block "b" has IDom "i",
     // then, block "b" is dominated by "i". The mapping then is i -> { ..., b, ... }, in
     // other words, "domTree" is a tree represented by nodes mapped to their children.
-    static void ComputeDominators(Compiler* pCompiler, BlkToBlkSetMap* domTree);
+    static void ComputeDominators(Compiler* pCompiler, BlkToBlkVectorMap* domTree);
 
 private:
     // Ensures that the basic block graph has a root for the dominator graph, by ensuring
@@ -88,21 +88,21 @@ class SsaBuilder
     // as children.) Requires "preIndex" and "postIndex" to be initialized to 0 at entry into recursion.
     // Computes arrays "m_pDomPreOrder" and "m_pDomPostOrder" of block indices such that the blocks of a
     // "domTree" are in pre and postorder respectively.
-    void DomTreeWalk(BasicBlock* curBlock, BlkToBlkSetMap* domTree, int* preIndex, int* postIndex);
+    void DomTreeWalk(BasicBlock* curBlock, BlkToBlkVectorMap* domTree, int* preIndex, int* postIndex);
 #endif
 
-    // Requires all blocks to have computed "bbIDom." Requires "domTree" to be a preallocated BlkToBlkSetMap.
+    // Requires all blocks to have computed "bbIDom." Requires "domTree" to be a preallocated BlkToBlkVectorMap.
     // Helper to compute "domTree" from the pre-computed bbIDom of the basic blocks.
-    static void ConstructDomTreeForBlock(Compiler* pCompiler, BasicBlock* block, BlkToBlkSetMap* domTree);
+    static void ConstructDomTreeForBlock(Compiler* pCompiler, BasicBlock* block, BlkToBlkVectorMap* domTree);
 
     // Requires "postOrder" to hold the blocks of the flowgraph in topologically sorted order. Requires
     // count to be the valid entries in the "postOrder" array. Computes "domTree" as a adjacency list
     // like object, i.e., a set of blocks with a set of blocks as children defining the DOM relation.
-    void ComputeDominators(BasicBlock** postOrder, int count, BlkToBlkSetMap* domTree);
+    void ComputeDominators(BasicBlock** postOrder, int count, BlkToBlkVectorMap* domTree);
 
 #ifdef DEBUG
     // Display the dominator tree.
-    static void DisplayDominators(BlkToBlkSetMap* domTree);
+    static void DisplayDominators(BlkToBlkVectorMap* domTree);
 #endif // DEBUG
 
     // Requires "postOrder" to hold the blocks of the flowgraph in topologically sorted order. Requires
@@ -123,7 +123,7 @@ class SsaBuilder
     // Requires "domTree" to be the dominator tree relation defined by a DOM b.
     // Requires "pRenameState" to have counts and stacks at their initial state.
     // Assigns gtSsaNames to all variables.
-    void RenameVariables(BlkToBlkSetMap* domTree, SsaRenameState* pRenameState);
+    void RenameVariables(BlkToBlkVectorMap* domTree, SsaRenameState* pRenameState);
 
     // Requires "block" to be any basic block participating in variable renaming, and has at least a
     // definition that pushed a ssa number into the rename stack for a variable. Requires "pRenameState"