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 d53f01b..4ce9615 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -173,6 +173,13 @@
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 b19e375..9d2ad08 100644
--- a/source/opt/aggressive_dead_code_elim_pass.h
+++ b/source/opt/aggressive_dead_code_elim_pass.h
@@ -44,6 +44,10 @@
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 450882f..116e11d 100644
--- a/source/opt/cfg_cleanup_pass.h
+++ b/source/opt/cfg_cleanup_pass.h
@@ -28,6 +28,10 @@
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 e7b648e..cdf2e96 100644
--- a/source/opt/dead_branch_elim_pass.h
+++ b/source/opt/dead_branch_elim_pass.h
@@ -44,6 +44,10 @@
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 8b6a27f..f016e78 100644
--- a/source/opt/dead_variable_elimination.h
+++ b/source/opt/dead_variable_elimination.h
@@ -29,6 +29,10 @@
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 d3c050c..3be6980 100644
--- a/source/opt/def_use_manager.cpp
+++ b/source/opt/def_use_manager.cpp
@@ -151,6 +151,30 @@
}
}
+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 1b9f41e..74d4528 100644
--- a/source/opt/def_use_manager.h
+++ b/source/opt/def_use_manager.h
@@ -37,6 +37,22 @@
// 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 @@
// 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 @@
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 dae2fca..adb41bb 100644
--- a/source/opt/eliminate_dead_functions_pass.h
+++ b/source/opt/eliminate_dead_functions_pass.h
@@ -29,6 +29,10 @@
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 122aff0..a482402 100644
--- a/source/opt/ir_context.cpp
+++ b/source/opt/ir_context.cpp
@@ -64,7 +64,7 @@
}
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 @@
// 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 7945a56..907ff96 100644
--- a/source/opt/ir_context.h
+++ b/source/opt/ir_context.h
@@ -188,6 +188,19 @@
// 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 152e71c..b8862a8 100644
--- a/source/opt/local_access_chain_convert_pass.h
+++ b/source/opt/local_access_chain_convert_pass.h
@@ -39,6 +39,10 @@
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 5ceb378..4edf116 100644
--- a/source/opt/local_single_block_elim_pass.h
+++ b/source/opt/local_single_block_elim_pass.h
@@ -39,6 +39,10 @@
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 1d0f1ff..1ecc867 100644
--- a/source/opt/local_single_store_elim_pass.h
+++ b/source/opt/local_single_store_elim_pass.h
@@ -40,6 +40,10 @@
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 ae293be..12b107d 100644
--- a/source/opt/local_ssa_elim_pass.h
+++ b/source/opt/local_ssa_elim_pass.h
@@ -44,6 +44,10 @@
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 3affc48..812caf0 100644
--- a/source/opt/mem_pass.cpp
+++ b/source/opt/mem_pass.cpp
@@ -755,7 +755,9 @@
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 96e2ab1..6eb381a 100644
--- a/source/opt/pass.cpp
+++ b/source/opt/pass.cpp
@@ -107,6 +107,7 @@
if (status == Status::SuccessWithChange) {
ctx->InvalidateAnalysesExceptFor(GetPreservedAnalyses());
}
+ assert(ctx->IsConsistent());
return status;
}
