Improve SSA DF computation memory usage

DF(b) can be stored in a vector instead of a hashtable (set). Nothing needs a O(1) membership test, the duplicates that may be generated during DF construction can be easily avoided by observing that:
* each block in the graph is processed exactly once
* during processing the block is added to the relevant DF vectors, at the end of each vector. If the same DF vector is encountered multiple times then checking if the block is already a member is trivial.

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/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,50 @@ 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 supplied 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* n = m_table[index];
+        while ((n != nullptr) && !KeyFuncs::Equals(k, n->m_key))
+        {
+            n = n->m_next;
+        }
+
+        if (n == nullptr)
+        {
+            n = new (m_alloc) Node(m_table[index], k, jitstd::forward<Args>(args)...);
+
+            m_table[index] = n;
+            m_tableCount++;
+        }
+
+        return &n->m_val;
+    }
+
     //------------------------------------------------------------------------
     // Remove: Remove the specified key and its associated value.
     //
@@ -592,7 +629,7 @@ class JitHashTable
         // Assumptions:
         //    This must not be the "end" iterator.
         //
-        const Value& GetValue() const
+        Value& GetValue() const
         {
             assert(m_node != nullptr);
 
@@ -721,7 +758,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

@@ -513,7 +513,7 @@ void SsaBuilder::DisplayDominators(BlkToBlkSetMap* domTree)
 // dominance frontiers by a closure operation.
 BlkToBlkSetMap* SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock** postOrder, int count)
 {
-    BlkToBlkSetMap* frontier = new (&m_allocator) BlkToBlkSetMap(&m_allocator);
+    BlkToBlkVectorMap mapDF(&m_allocator);
 
     DBG_SSA_JITDUMP("Computing IDF: First computing DF.\n");
 
@@ -534,8 +534,8 @@ BlkToBlkSetMap* SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock** postOr
 
         flowList* blockPreds = m_pCompiler->BlockPredsWithEH(block);
 
-        // If block has more 0/1 predecessor, skip.
-        if (blockPreds == nullptr || blockPreds->flNext == nullptr)
+        // If block has 0/1 predecessor, skip.
+        if ((blockPreds == nullptr) || (blockPreds->flNext == nullptr))
         {
             DBG_SSA_JITDUMP("   Has %d preds; skipping.\n", blockPreds == nullptr ? 0 : 1);
             continue;
@@ -544,7 +544,7 @@ BlkToBlkSetMap* SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock** postOr
         // Otherwise, there are > 1 preds.  Each is a candidate B2 in the definition --
         // *unless* it dominates "block"/B3.
 
-        for (flowList* pred = blockPreds; pred; pred = pred->flNext)
+        for (flowList* pred = blockPreds; pred != nullptr; pred = pred->flNext)
         {
             DBG_SSA_JITDUMP("   Considering predecessor BB%02u.\n", pred->flBlock->bbNum);
 
@@ -560,13 +560,13 @@ BlkToBlkSetMap* SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock** postOr
                  b1             = b1->bbIDom)
             {
                 DBG_SSA_JITDUMP("      Adding BB%02u to dom frontier of pred dom BB%02u.\n", block->bbNum, b1->bbNum);
-                BlkSet* pBlkSet;
-                if (!frontier->Lookup(b1, &pBlkSet))
+
+                BlkVector& b1DF = *mapDF.Emplace(b1, &m_allocator);
+                // It's possible to encounter the same DF multiple times, ensure that we don't add duplicates.
+                if (b1DF.empty() || (b1DF.back() != block))
                 {
-                    pBlkSet = new (&m_allocator) BlkSet(&m_allocator);
-                    frontier->Set(b1, pBlkSet);
+                    b1DF.push_back(block);
                 }
-                pBlkSet->Set(block, true);
             }
         }
     }
@@ -577,20 +577,20 @@ BlkToBlkSetMap* SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock** postOr
         printf("\nComputed DF:\n");
         for (int i = 0; i < count; ++i)
         {
-            BasicBlock* block = postOrder[i];
-            printf("Block BB%02u := {", block->bbNum);
+            BasicBlock* b = postOrder[i];
+            printf("Block BB%02u := {", b->bbNum);
 
-            bool    first = true;
-            BlkSet* blkDf;
-            if (frontier->Lookup(block, &blkDf))
+            bool       first = true;
+            BlkVector* bDF   = mapDF.LookupPointer(b);
+            if (bDF != nullptr)
             {
-                for (BlkSet::KeyIterator blkDfIter = blkDf->Begin(); !blkDfIter.Equal(blkDf->End()); blkDfIter++)
+                for (BasicBlock* f : *bDF)
                 {
                     if (!first)
                     {
                         printf(",");
                     }
-                    printf("BB%02u", blkDfIter.Get()->bbNum);
+                    printf("BB%02u", f->bbNum);
                     first = false;
                 }
             }
@@ -602,8 +602,7 @@ BlkToBlkSetMap* SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock** postOr
     // Now do the closure operation to make the dominance frontier into an IDF.
     // There's probably a better way to do this...
     BlkToBlkSetMap* idf = new (&m_allocator) BlkToBlkSetMap(&m_allocator);
-    for (BlkToBlkSetMap::KeyIterator kiFrontBlks = frontier->Begin(); !kiFrontBlks.Equal(frontier->End());
-         kiFrontBlks++)
+    for (BlkToBlkVectorMap::KeyIterator kiFrontBlks = mapDF.Begin(); !kiFrontBlks.Equal(mapDF.End()); kiFrontBlks++)
     {
         // Create IDF(b)
         BlkSet* blkIdf = new (&m_allocator) BlkSet(&m_allocator);
@@ -623,16 +622,16 @@ BlkToBlkSetMap* SsaBuilder::ComputeIteratedDominanceFrontier(BasicBlock** postOr
             delta->Remove(curBlk);
 
             // Get DF(x).
-            BlkSet* blkDf;
-            if (frontier->Lookup(curBlk, &blkDf))
+            BlkVector* blkDf = mapDF.LookupPointer(curBlk);
+            if (blkDf != nullptr)
             {
                 // Add DF(x) to IDF(b) and update "delta" i.e., new additions to IDF(b).
-                for (BlkSet::KeyIterator ki = blkDf->Begin(); !ki.Equal(blkDf->End()); ki++)
+                for (BasicBlock* f : *blkDf)
                 {
-                    if (!blkIdf->Lookup(ki.Get()))
+                    if (!blkIdf->Lookup(f))
                     {
-                        delta->Set(ki.Get(), true);
-                        blkIdf->Set(ki.Get(), true);
+                        delta->Set(f, true);
+                        blkIdf->Set(f, true);
                     }
                 }
             }