Speed up Phi insertion.
authorDiego Novillo <dnovillo@google.com>
Sat, 17 Feb 2018 00:12:50 +0000 (19:12 -0500)
committerDiego Novillo <dnovillo@google.com>
Tue, 20 Feb 2018 17:04:06 +0000 (12:04 -0500)
On some shader code we have in our testsuite, Phi insertion is showing
massive compile time slowdowns, particularly during destruction.  The
specific shader I was looking at has about 600 variables to keep track
of and around 3200 basic blocks.  The algorithm is currently O(var x
blocks), which means maps with around 2M entries.  This was taking about
8 minutes of compile time.

This patch changes the tracking of stored variables to be more sparse.
Instead of having every basic block contain all the tracked variables in
the map, they now have only the variables actually stored in that block.

This speeds up deallocation, which brings down compile time to about
1m20s.

Note that this is not the definite fix for this.  I will re-write Phi
insertion to use a standard SSA rewriting algorithm
(https://github.com/KhronosGroup/SPIRV-Tools/issues/893).

This contributes to
https://github.com/KhronosGroup/SPIRV-Tools/issues/1328.

source/opt/mem_pass.cpp
source/opt/mem_pass.h

index d6c9f36..373d299 100644 (file)
@@ -18,6 +18,7 @@
 
 #include "basic_block.h"
 #include "cfa.h"
+#include "dominator_analysis.h"
 #include "ir_context.h"
 #include "iterator.h"
 
@@ -251,8 +252,9 @@ void MemPass::InitSSARewrite(ir::Function* func) {
   visitedBlocks_.clear();
   type2undefs_.clear();
   supported_ref_vars_.clear();
-  label2ssa_map_.clear();
+  block_defs_map_.clear();
   phis_to_patch_.clear();
+  dominator_ = context()->GetDominatorAnalysis(func, *cfg());
 
   // Collect target (and non-) variable sets. Remove variables with
   // non-load/store refs from target variable set
@@ -285,14 +287,6 @@ bool MemPass::IsLiveAfter(uint32_t var_id, uint32_t label) const {
   return true;
 }
 
-void MemPass::SSABlockInitSinglePred(ir::BasicBlock* block_ptr) {
-  // Copy map entry from single predecessor
-  const uint32_t label = block_ptr->id();
-  const uint32_t predLabel = cfg()->preds(label).front();
-  assert(visitedBlocks_.find(predLabel) != visitedBlocks_.end());
-  label2ssa_map_[label] = label2ssa_map_[predLabel];
-}
-
 uint32_t MemPass::Type2Undef(uint32_t type_id) {
   const auto uitr = type2undefs_.find(type_id);
   if (uitr != type2undefs_.end()) return uitr->second;
@@ -305,6 +299,35 @@ uint32_t MemPass::Type2Undef(uint32_t type_id) {
   return undefId;
 }
 
+void MemPass::CollectLiveVars(uint32_t block_label,
+                              std::map<uint32_t, uint32_t>* live_vars) {
+  // Walk up the dominator chain starting at |block_label| looking for variables
+  // defined at each block in the chain.  Since we are only interested for the
+  // most recent value for each live variable, we only add a <variable, value>
+  // pair to |live_vars| if this is the first time we find the variable in the
+  // chain.
+  for (ir::BasicBlock* block = cfg()->block(block_label); block != nullptr;
+       block = dominator_->ImmediateDominator(block)) {
+    for (const auto& var_val : block_defs_map_[block->id()]) {
+      auto live_vars_it = live_vars->find(var_val.first);
+      if (live_vars_it == live_vars->end()) live_vars->insert(var_val);
+    }
+  }
+}
+
+uint32_t MemPass::GetCurrentValue(uint32_t var_id, uint32_t block_label) {
+  // Walk up the dominator chain starting at |block_label| looking for the
+  // current value of variable |var_id|.  The first block we find containing a
+  // definition for |var_id| is the one we are interested in.
+  for (ir::BasicBlock* block = cfg()->block(block_label); block != nullptr;
+       block = dominator_->ImmediateDominator(block)) {
+    const auto& block_defs = block_defs_map_[block->id()];
+    const auto& var_val_it = block_defs.find(var_id);
+    if (var_val_it != block_defs.end()) return var_val_it->second;
+  }
+  return 0;
+}
+
 void MemPass::SSABlockInitLoopHeader(
     std::list<ir::BasicBlock*>::iterator block_itr) {
   const uint32_t label = (*block_itr)->id();
@@ -332,11 +355,9 @@ void MemPass::SSABlockInitLoopHeader(
   // platforms.
   std::map<uint32_t, uint32_t> liveVars;
   for (uint32_t predLabel : cfg()->preds(label)) {
-    for (auto var_val : label2ssa_map_[predLabel]) {
-      uint32_t varId = var_val.first;
-      liveVars[varId] = var_val.second;
-    }
+    CollectLiveVars(predLabel, &liveVars);
   }
+
   // Add all stored variables in loop. Set their default value id to zero.
   for (auto bi = block_itr; (*bi)->id() != mergeLabel; ++bi) {
     ir::BasicBlock* bp = *bi;
@@ -367,13 +388,13 @@ void MemPass::SSABlockInitLoopHeader(
       for (uint32_t predLabel : cfg()->preds(label)) {
         // Skip back edge predecessor.
         if (predLabel == backLabel) continue;
-        const auto var_val_itr = label2ssa_map_[predLabel].find(varId);
+        uint32_t current_value = GetCurrentValue(varId, predLabel);
         // Missing (undef) values always cause difference with (defined) value
-        if (var_val_itr == label2ssa_map_[predLabel].end()) {
+        if (current_value == 0) {
           needsPhi = true;
           break;
         }
-        if (var_val_itr->second != val0Id) {
+        if (current_value != val0Id) {
           needsPhi = true;
           break;
         }
@@ -384,7 +405,7 @@ void MemPass::SSABlockInitLoopHeader(
 
     // If val is the same for all predecessors, enter it in map
     if (!needsPhi) {
-      label2ssa_map_[label].insert(var_val);
+      block_defs_map_[label].insert(var_val);
       continue;
     }
 
@@ -400,11 +421,11 @@ void MemPass::SSABlockInitLoopHeader(
       if (predLabel == backLabel) {
         valId = varId;
       } else {
-        const auto var_val_itr = label2ssa_map_[predLabel].find(varId);
-        if (var_val_itr == label2ssa_map_[predLabel].end())
+        uint32_t current_value = GetCurrentValue(varId, predLabel);
+        if (current_value == 0)
           valId = Type2Undef(typeId);
         else
-          valId = var_val_itr->second;
+          valId = current_value;
       }
       phi_in_operands.push_back(
           {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {valId}});
@@ -422,7 +443,7 @@ void MemPass::SSABlockInitLoopHeader(
     context()->set_instr_block(&*newPhi, *block_itr);
     insertItr = insertItr.InsertBefore(std::move(newPhi));
     ++insertItr;
-    label2ssa_map_[label].insert({varId, phiId});
+    block_defs_map_[label].insert({varId, phiId});
   }
 }
 
@@ -434,11 +455,9 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
   std::map<uint32_t, uint32_t> liveVars;
   for (uint32_t predLabel : cfg()->preds(label)) {
     assert(visitedBlocks_.find(predLabel) != visitedBlocks_.end());
-    for (auto var_val : label2ssa_map_[predLabel]) {
-      const uint32_t varId = var_val.first;
-      liveVars[varId] = var_val.second;
-    }
+    CollectLiveVars(predLabel, &liveVars);
   }
+
   // For each live variable, look for a difference in values across
   // predecessors that would require a phi and insert one.
   auto insertItr = block_ptr->begin();
@@ -448,33 +467,33 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
     const uint32_t val0Id = var_val.second;
     bool differs = false;
     for (uint32_t predLabel : cfg()->preds(label)) {
-      const auto var_val_itr = label2ssa_map_[predLabel].find(varId);
+      uint32_t current_value = GetCurrentValue(varId, predLabel);
       // Missing values cause a difference because we'll need to create an
       // undef for that predecessor.
-      if (var_val_itr == label2ssa_map_[predLabel].end()) {
+      if (current_value == 0) {
         differs = true;
         break;
       }
-      if (var_val_itr->second != val0Id) {
+      if (current_value != val0Id) {
         differs = true;
         break;
       }
     }
     // If val is the same for all predecessors, enter it in map
     if (!differs) {
-      label2ssa_map_[label].insert(var_val);
+      block_defs_map_[label].insert(var_val);
       continue;
     }
+
     // Val differs across predecessors. Add phi op to block and add its result
     // id to the map.
     std::vector<ir::Operand> phi_in_operands;
     const uint32_t typeId = GetPointeeTypeId(get_def_use_mgr()->GetDef(varId));
     for (uint32_t predLabel : cfg()->preds(label)) {
-      const auto var_val_itr = label2ssa_map_[predLabel].find(varId);
+      uint32_t current_value = GetCurrentValue(varId, predLabel);
       // If variable not defined on this path, use undef
-      const uint32_t valId = (var_val_itr != label2ssa_map_[predLabel].end())
-                                 ? var_val_itr->second
-                                 : Type2Undef(typeId);
+      const uint32_t valId =
+          (current_value > 0) ? current_value : Type2Undef(typeId);
       phi_in_operands.push_back(
           {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {valId}});
       phi_in_operands.push_back(
@@ -487,16 +506,14 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
     context()->set_instr_block(&*newPhi, block_ptr);
     insertItr = insertItr.InsertBefore(std::move(newPhi));
     ++insertItr;
-    label2ssa_map_[label].insert({varId, phiId});
+    block_defs_map_[label].insert({varId, phiId});
   }
 }
 
 void MemPass::SSABlockInit(std::list<ir::BasicBlock*>::iterator block_itr) {
   const size_t numPreds = cfg()->preds((*block_itr)->id()).size();
   if (numPreds == 0) return;
-  if (numPreds == 1)
-    SSABlockInitSinglePred(*block_itr);
-  else if ((*block_itr)->IsLoopHeader())
+  if ((*block_itr)->IsLoopHeader())
     SSABlockInitLoopHeader(block_itr);
   else
     SSABlockInitMultiPred(*block_itr);
@@ -546,13 +563,14 @@ void MemPass::PatchPhis(uint32_t header_id, uint32_t back_id) {
       ++cnt;
     });
     assert(idx != phiItr->NumInOperands());
+
     // Replace temporary phi operand with variable's value in backedge block
     // map. Use undef if variable not in map.
     const uint32_t varId = phiItr->GetSingleWordInOperand(idx);
-    const auto valItr = label2ssa_map_[back_id].find(varId);
+    uint32_t current_value = GetCurrentValue(varId, back_id);
     uint32_t valId =
-        (valItr != label2ssa_map_[back_id].end())
-            ? valItr->second
+        (current_value > 0)
+            ? current_value
             : Type2Undef(GetPointeeTypeId(get_def_use_mgr()->GetDef(varId)));
     phiItr->SetInOperand(idx, {valId});
     // Analyze uses now that they are complete
@@ -582,8 +600,7 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) {
       continue;
     }
 
-    // Initialize this block's label2ssa_map_ entry using predecessor maps.
-    // Then process all stores and loads of targeted variables.
+    // Process all stores and loads of targeted variables.
     SSABlockInit(bi);
     ir::BasicBlock* bp = *bi;
     const uint32_t label = bp->id();
@@ -596,7 +613,7 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) {
           (void)GetPtr(inst, &varId);
           if (!IsTargetVar(varId)) break;
           // Register new stored value for the variable
-          label2ssa_map_[label][varId] =
+          block_defs_map_[label][varId] =
               inst->GetSingleWordInOperand(kStoreValIdInIdx);
         } break;
         case SpvOpVariable: {
@@ -605,24 +622,20 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) {
           uint32_t varId = inst->result_id();
           if (!IsTargetVar(varId)) break;
           // Register new stored value for the variable
-          label2ssa_map_[label][varId] =
+          block_defs_map_[label][varId] =
               inst->GetSingleWordInOperand(kVariableInitIdInIdx);
         } break;
         case SpvOpLoad: {
           uint32_t varId;
           (void)GetPtr(inst, &varId);
           if (!IsTargetVar(varId)) break;
-          uint32_t replId = 0;
-          const auto ssaItr = label2ssa_map_.find(label);
-          if (ssaItr != label2ssa_map_.end()) {
-            const auto valItr = ssaItr->second.find(varId);
-            if (valItr != ssaItr->second.end()) replId = valItr->second;
-          }
-          // If variable is not defined, use undef
+          uint32_t replId = GetCurrentValue(varId, label);
+          // If the variable is not defined, use undef.
           if (replId == 0) {
             replId =
                 Type2Undef(GetPointeeTypeId(get_def_use_mgr()->GetDef(varId)));
           }
+
           // Replace load's id with the last stored value id for variable
           // and delete load. Kill any names or decorates using id before
           // replacing to prevent incorrect replacement in those instructions.
index 940c5c4..5b90508 100644 (file)
@@ -27,6 +27,7 @@
 
 #include "basic_block.h"
 #include "def_use_manager.h"
+#include "dominator_analysis.h"
 #include "module.h"
 #include "pass.h"
 
@@ -150,11 +151,7 @@ class MemPass : public Pass {
   // function |func|, specifically block predecessors and target variables.
   void InitSSARewrite(ir::Function* func);
 
-  // Initialize label2ssa_map_ entry for block |block_ptr| with single
-  // predecessor.
-  void SSABlockInitSinglePred(ir::BasicBlock* block_ptr);
-
-  // Initialize label2ssa_map_ entry for loop header block pointed to
+  // Initialize block_defs_map_ entry for loop header block pointed to
   // |block_itr| by merging entries from all predecessors. If any value
   // ids differ for any variable across predecessors, create a phi function
   // in the block and use that value id for the variable in the new map.
@@ -163,8 +160,8 @@ class MemPass : public Pass {
   // until the back edge block is visited and patch the phi value then.
   void SSABlockInitLoopHeader(std::list<ir::BasicBlock*>::iterator block_itr);
 
-  // Initialize label2ssa_map_ entry for multiple predecessor block
-  // |block_ptr| by merging label2ssa_map_ entries for all predecessors.
+  // Initialize block_defs_map_ entry for multiple predecessor block
+  // |block_ptr| by merging block_defs_map_ entries for all predecessors.
   // If any value ids differ for any variable across predecessors, create
   // a phi function in the block and use that value id for the variable in
   // the new map. Assumes all predecessors have been visited by
@@ -193,10 +190,30 @@ class MemPass : public Pass {
   void RemovePhiOperands(ir::Instruction* phi,
                          std::unordered_set<ir::BasicBlock*> reachable_blocks);
 
-  // Map from block's label id to a map of a variable to its value at the
-  // end of the block.
+  // Collects a map of all the live variables and their values along the path of
+  // dominator parents starting at |block_label|.  Each entry
+  // |live_vars[var_id]| returns the latest value of |var_id| along that
+  // dominator path.  Note that the mapping |live_vars| is never cleared,
+  // multiple calls to this function will accumulate new <var_id, value_id>
+  // mappings.  This is done to support the logic in
+  // MemPass::SSABlockInitLoopHeader.
+  void CollectLiveVars(uint32_t block_label,
+                       std::map<uint32_t, uint32_t>* live_vars);
+
+  // Returns the ID of the most current value taken by variable |var_id| on the
+  // dominator path starting at |block_id|.  This walks the dominator parents
+  // starting at |block_id| and returns the first value it finds for |var_id|.
+  // If no value for |var_id| is found along the dominator path, this returns 0.
+  uint32_t GetCurrentValue(uint32_t var_id, uint32_t block_label);
+
+  // Dominator information.
+  DominatorAnalysis* dominator_;
+
+  // Each entry |block_defs_map_[block_id]| contains a map {variable_id,
+  // value_id} associating all the variables |variable_id| stored in |block_id|
+  // to their respective value |value_id|.
   std::unordered_map<uint32_t, std::unordered_map<uint32_t, uint32_t>>
-      label2ssa_map_;
+      block_defs_map_;
 
   // Set of label ids of visited blocks
   std::unordered_set<uint32_t> visitedBlocks_;