Have all MemPasses preserve the def-use manager.

Originally the passes that extended from MemPass were those that are
of the def-use manager.  I am assuming they would be able to preserve
it because of that.

Added a check to verify consistency of the IRContext. The IRContext
relies on the pass to tell it if something is invalidated.
It is possible that the pass lied.  To help identify those situations,
we will check if the valid analyses are correct after each pass.

This will be enabled by default for the debug build, and disabled in the
production build.  It can be disabled in the debug build by adding
"-DSPIRV_CHECK_CONTEXT=OFF" to the cmake command.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d53f01b8931341afb576589d14561c6fee9669a0..4ce9615323cab9db25fc62cdb9aa571113d2757a 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -173,6 +173,13 @@ if ("${SPIRV_SKIP_EXECUTABLES}")
   set(SPIRV_SKIP_TESTS ON)
 endif()
 
+# Defaults to ON.  The checks can be time consuming.
+# Turn off if they take too long.
+option(SPIRV_CHECK_CONTEXT "In a debug build, check if the IR context is in a valid state." ON)
+if (${SPIRV_CHECK_CONTEXT})
+  add_definitions(-DSPIRV_CHECK_CONTEXT)
+endif()
+
 add_subdirectory(external)
 
 add_subdirectory(source)

diff --git a/source/opt/aggressive_dead_code_elim_pass.h b/source/opt/aggressive_dead_code_elim_pass.h
index b19e3755c4fa2ba58f793a72152189a5fb7ea266..9d2ad0885d8434649028fc07211911d77699bc8c 100644 (file)
--- a/source/opt/aggressive_dead_code_elim_pass.h
+++ b/source/opt/aggressive_dead_code_elim_pass.h
@@ -44,6 +44,10 @@ class AggressiveDCEPass : public MemPass {
   const char* name() const override { return "eliminate-dead-code-aggressive"; }
   Status Process(ir::IRContext* c) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+
  private:
   // Return true if |varId| is variable of |storageClass|.
   bool IsVarOfStorage(uint32_t varId, uint32_t storageClass);

diff --git a/source/opt/cfg_cleanup_pass.h b/source/opt/cfg_cleanup_pass.h
index 450882fa732316ee6523e786033d717b808ca351..116e11d1b69cd3294fd26b909460782b8818e6e7 100644 (file)
--- a/source/opt/cfg_cleanup_pass.h
+++ b/source/opt/cfg_cleanup_pass.h
@@ -28,6 +28,10 @@ class CFGCleanupPass : public MemPass {
   const char* name() const override { return "cfg-cleanup"; }
   Status Process(ir::IRContext* c) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+
  private:
   // Initialize the pass.
   void Initialize(ir::IRContext* c);

diff --git a/source/opt/dead_branch_elim_pass.h b/source/opt/dead_branch_elim_pass.h
index e7b648e82b38da75ecfcf9d88aae6d560b7375eb..cdf2e96685f87ff47eedb6093fa76b835b4ef7d3 100644 (file)
--- a/source/opt/dead_branch_elim_pass.h
+++ b/source/opt/dead_branch_elim_pass.h
@@ -44,6 +44,10 @@ class DeadBranchElimPass : public MemPass {
   const char* name() const override { return "eliminate-dead-branches"; }
   Status Process(ir::IRContext* context) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+
  private:
   // If |condId| is boolean constant, return conditional value in |condVal| and
   // return true, otherwise return false.

diff --git a/source/opt/dead_variable_elimination.h b/source/opt/dead_variable_elimination.h
index 8b6a27faaba382bd45cb2af33248fe78e28c554f..f016e78b2d42668aaea67aec3aa2e1948271d9c2 100644 (file)
--- a/source/opt/dead_variable_elimination.h
+++ b/source/opt/dead_variable_elimination.h
@@ -29,6 +29,10 @@ class DeadVariableElimination : public MemPass {
   const char* name() const override { return "dead-variable-elimination"; }
   Status Process(ir::IRContext* c) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+
  private:
   // Deletes the OpVariable instruction who result id is |result_id|.
   void DeleteVariable(uint32_t result_id);

diff --git a/source/opt/def_use_manager.cpp b/source/opt/def_use_manager.cpp
index d3c050cab4771eac275b307268fe97a13d870afa..3be6980cc8980617106c0b94dc3032b56c1fd8aa 100644 (file)
--- a/source/opt/def_use_manager.cpp
+++ b/source/opt/def_use_manager.cpp
@@ -151,6 +151,30 @@ void DefUseManager::EraseUseRecordsOfOperandIds(const ir::Instruction* inst) {
   }
 }
 
+bool operator==(const DefUseManager& lhs, const DefUseManager& rhs) {
+  if (lhs.id_to_def_ != rhs.id_to_def_) {
+    return false;
+  }
+
+  for (auto use : lhs.id_to_uses_) {
+    auto rhs_iter = rhs.id_to_uses_.find(use.first);
+    if (rhs_iter == rhs.id_to_uses_.end()) {
+      return false;
+    }
+    use.second.sort();
+    UseList rhs_uselist = rhs_iter->second;
+    rhs_uselist.sort();
+    if (use.second != rhs_uselist) {
+      return false;
+    }
+  }
+
+  if (lhs.inst_to_used_ids_ != lhs.inst_to_used_ids_) {
+    return false;
+  }
+  return true;
+}
+
 }  // namespace analysis
 }  // namespace opt
 }  // namespace spvtools

diff --git a/source/opt/def_use_manager.h b/source/opt/def_use_manager.h
index 1b9f41eab0810c3065e7057f4ba0633617be8cd1..74d45288f1d04c75471f6f12d942d47acd2907cf 100644 (file)
--- a/source/opt/def_use_manager.h
+++ b/source/opt/def_use_manager.h
@@ -37,6 +37,22 @@ struct Use {
                            // the index of result type id.
 };
 
+inline bool operator==(const Use& lhs, const Use& rhs) {
+  return lhs.inst == rhs.inst && lhs.operand_index == rhs.operand_index;
+}
+
+inline bool operator!=(const Use& lhs, const Use& rhs) {
+  return !(lhs == rhs);
+}
+
+inline bool operator<(const Use& lhs, const Use& rhs) {
+  if (lhs.inst < rhs.inst)
+    return true;
+  if (lhs.inst > rhs.inst)
+    return false;
+  return lhs.operand_index < rhs.operand_index;
+}
+
 using UseList = std::list<Use>;
 
 // A class for analyzing and managing defs and uses in an ir::Module.
@@ -95,6 +111,11 @@ class DefUseManager {
   // Erases the records that a given instruction uses its operand ids.
   void EraseUseRecordsOfOperandIds(const ir::Instruction* inst);
 
+  friend  bool operator==(const DefUseManager&, const DefUseManager&);
+  friend  bool operator!=(const DefUseManager& lhs, const DefUseManager& rhs) {
+    return !(lhs == rhs);
+  }
+
  private:
   using InstToUsedIdsMap =
       std::unordered_map<const ir::Instruction*, std::vector<uint32_t>>;
@@ -107,6 +128,7 @@ class DefUseManager {
   IdToUsesMap id_to_uses_;  // Mapping from ids to their uses
   // Mapping from instructions to the ids used in the instruction.
   InstToUsedIdsMap inst_to_used_ids_;
+
 };
 
 }  // namespace analysis

diff --git a/source/opt/eliminate_dead_functions_pass.h b/source/opt/eliminate_dead_functions_pass.h
index dae2fca9bec0282dccfdb258df4098d92999900a..adb41bb392bf53b98b33b3abb3bc851d72227dad 100644 (file)
--- a/source/opt/eliminate_dead_functions_pass.h
+++ b/source/opt/eliminate_dead_functions_pass.h
@@ -29,6 +29,10 @@ class EliminateDeadFunctionsPass : public MemPass {
   const char* name() const override { return "eliminate-dead-functions"; }
   Status Process(ir::IRContext* c) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+
  private:
   void EliminateFunction(ir::Function* func);
 };

diff --git a/source/opt/ir_context.cpp b/source/opt/ir_context.cpp
index 122aff0394e2f0c0eaf33a0b964e2194766164cc..a4824026a77b00602c4158e3fa0bb1d823eb0455 100644 (file)
--- a/source/opt/ir_context.cpp
+++ b/source/opt/ir_context.cpp
@@ -64,7 +64,7 @@ bool IRContext::ReplaceAllUsesWith(uint32_t before, uint32_t after) {
   }
 
   for (opt::analysis::Use& use : uses_to_update) {
-    get_def_use_mgr()->EraseUseRecordsOfOperandIds(use.inst);
+    ForgetUses(use.inst);
     const uint32_t type_result_id_count =
         (use.inst->result_id() != 0) + (use.inst->type_id() != 0);
 
@@ -88,9 +88,35 @@ bool IRContext::ReplaceAllUsesWith(uint32_t before, uint32_t after) {
       // Make the modification in the instruction.
       use.inst->SetInOperand(in_operand_pos, {after});
     }
-    get_def_use_mgr()->AnalyzeInstUse(use.inst);
+    AnalyzeUses(use.inst);
   }
   return true;
 }
+
+bool IRContext::IsConsistent() {
+#ifndef SPIRV_CHECK_CONTEXT
+  return true;
+#endif
+
+  if (AreAnalysesValid(kAnalysisDefUse)) {
+    opt::analysis::DefUseManager new_def_use(module());
+    if (*get_def_use_mgr() != new_def_use) {
+      return false;
+    }
+  }
+  return true;
+}
+
+void spvtools::ir::IRContext::ForgetUses(Instruction* inst) {
+  if (AreAnalysesValid(kAnalysisDefUse)) {
+    get_def_use_mgr()->EraseUseRecordsOfOperandIds(inst);
+  }
+}
+
+void IRContext::AnalyzeUses(Instruction* inst) {
+  if (AreAnalysesValid(kAnalysisDefUse)) {
+    get_def_use_mgr()->AnalyzeInstUse(inst);
+  }
+}
 }  // namespace ir
 }  // namespace spvtools

diff --git a/source/opt/ir_context.h b/source/opt/ir_context.h
index 7945a56167676d594703ba74a5e61b95bacb74f9..907ff964ffa8a8fd48a057a8fef06a0b2880c4a6 100644 (file)
--- a/source/opt/ir_context.h
+++ b/source/opt/ir_context.h
@@ -188,6 +188,19 @@ class IRContext {
   // false.
   bool ReplaceAllUsesWith(uint32_t before, uint32_t after);
 
+  // Returns true if all of the analyses that are suppose to be valid are
+  // actually valid.
+  bool IsConsistent();
+
+  // Informs the IRContext that the uses of |inst| are going to change, and that
+  // is should forget everything it know about the current uses.  Any valid
+  // analyses will be updated accordingly.
+  void ForgetUses(Instruction* inst);
+
+  // The IRContext will look at the uses of |inst| and update any valid analyses
+  // will be updated accordingly.
+  void AnalyzeUses(Instruction* inst);
+
  private:
   std::unique_ptr<Module> module_;
   spvtools::MessageConsumer consumer_;

diff --git a/source/opt/local_access_chain_convert_pass.h b/source/opt/local_access_chain_convert_pass.h
index 152e71c7ee77519a0beee6e93e99b898cc2e79b7..b8862a88dcac30620dba3f614a5abc6609154951 100644 (file)
--- a/source/opt/local_access_chain_convert_pass.h
+++ b/source/opt/local_access_chain_convert_pass.h
@@ -39,6 +39,10 @@ class LocalAccessChainConvertPass : public MemPass {
   const char* name() const override { return "convert-local-access-chains"; }
   Status Process(ir::IRContext* c) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+
   using ProcessFunction = std::function<bool(ir::Function*)>;
 
  private:

diff --git a/source/opt/local_single_block_elim_pass.h b/source/opt/local_single_block_elim_pass.h
index 5ceb378478d4bc8f758c3b6df69989deca649d91..4edf116ce8a9518f7e1c702e421a6e498b622d09 100644 (file)
--- a/source/opt/local_single_block_elim_pass.h
+++ b/source/opt/local_single_block_elim_pass.h
@@ -39,6 +39,10 @@ class LocalSingleBlockLoadStoreElimPass : public MemPass {
   const char* name() const override { return "eliminate-local-single-block"; }
   Status Process(ir::IRContext* c) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+  
  private:
   // Return true if all uses of |varId| are only through supported reference
   // operations ie. loads and store. Also cache in supported_ref_ptrs_.

diff --git a/source/opt/local_single_store_elim_pass.h b/source/opt/local_single_store_elim_pass.h
index 1d0f1ff9dc7da9319183150dc41a908b6b4bf147..1ecc86775259947a15190f2db1d3807f88b6ae75 100644 (file)
--- a/source/opt/local_single_store_elim_pass.h
+++ b/source/opt/local_single_store_elim_pass.h
@@ -40,6 +40,10 @@ class LocalSingleStoreElimPass : public MemPass {
   LocalSingleStoreElimPass();
   const char* name() const override { return "eliminate-local-single-store"; }
   Status Process(ir::IRContext* irContext) override;
+  
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
 
  private:
   // Return true if all refs through |ptrId| are only loads or stores and

diff --git a/source/opt/local_ssa_elim_pass.h b/source/opt/local_ssa_elim_pass.h
index ae293be00ca7d2283e0c886a5ccc608bc02d40a0..12b107d8935497d126409c61e23f786bc24b797e 100644 (file)
--- a/source/opt/local_ssa_elim_pass.h
+++ b/source/opt/local_ssa_elim_pass.h
@@ -44,6 +44,10 @@ class LocalMultiStoreElimPass : public MemPass {
   const char* name() const override { return "eliminate-local-multi-store"; }
   Status Process(ir::IRContext* c) override;
 
+  ir::IRContext::Analysis GetPreservedAnalyses() override {
+    return ir::IRContext::kAnalysisDefUse;
+  }
+
  private:
   // Initialize extensions whitelist
   void InitExtensions();

diff --git a/source/opt/mem_pass.cpp b/source/opt/mem_pass.cpp
index 3affc48486cbc7c9c66b72aa6c9a67105750df87..812caf03274983753c886f7b4371182dc7a97066 100644 (file)
--- a/source/opt/mem_pass.cpp
+++ b/source/opt/mem_pass.cpp
@@ -755,7 +755,9 @@ void MemPass::RemovePhiOperands(
     i += 2;
   }
 
+  context()->ForgetUses(phi);
   phi->ReplaceOperands(keep_operands);
+  context()->AnalyzeUses(phi);
 }
 
 void MemPass::RemoveBlock(ir::Function::iterator* bi) {

diff --git a/source/opt/pass.cpp b/source/opt/pass.cpp
index 96e2ab1822003b706ebd6385c8fb34d33bbfaa62..6eb381a6c87abbeba18249372c5c404c4459a1cb 100644 (file)
--- a/source/opt/pass.cpp
+++ b/source/opt/pass.cpp
@@ -107,6 +107,7 @@ Pass::Status Pass::Run(ir::IRContext* ctx) {
   if (status == Status::SuccessWithChange) {
     ctx->InvalidateAnalysesExceptFor(GetPreservedAnalyses());
   }
+  assert(ctx->IsConsistent());
   return status;
 }