From 1eec0ed4b950ff2cb23256fadb2dc96cc961747f Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 15 Aug 2016 13:41:47 -0400 Subject: [PATCH] Make analyses RAII-like and turn disable copy/move constructors. --- source/opt/def_use_manager.cpp | 10 +++++----- source/opt/def_use_manager.h | 18 +++++++++++------- source/opt/passes.cpp | 3 +-- source/opt/type_manager.h | 6 +++++- test/opt/test_def_use.cpp | 12 ++++-------- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/source/opt/def_use_manager.cpp b/source/opt/def_use_manager.cpp index 50ac197..d0bf79c 100644 --- a/source/opt/def_use_manager.cpp +++ b/source/opt/def_use_manager.cpp @@ -36,11 +36,6 @@ namespace spvtools { namespace opt { namespace analysis { -void DefUseManager::AnalyzeDefUse(ir::Module* module) { - module->ForEachInst(std::bind(&DefUseManager::AnalyzeInstDefUse, this, - std::placeholders::_1)); -} - void DefUseManager::AnalyzeInstDefUse(ir::Instruction* inst) { const uint32_t def_id = inst->result_id(); // Clear the records of def_id first if it has been recorded before. @@ -101,6 +96,11 @@ bool DefUseManager::ReplaceAllUsesWith(uint32_t before, uint32_t after) { return true; } +void DefUseManager::AnalyzeDefUse(ir::Module* module) { + module->ForEachInst(std::bind(&DefUseManager::AnalyzeInstDefUse, this, + std::placeholders::_1)); +} + void DefUseManager::ClearDef(uint32_t def_id) { if (id_to_def_.count(def_id) == 0) return; diff --git a/source/opt/def_use_manager.h b/source/opt/def_use_manager.h index 9c631c0..99d073f 100644 --- a/source/opt/def_use_manager.h +++ b/source/opt/def_use_manager.h @@ -39,8 +39,8 @@ namespace spvtools { namespace opt { namespace analysis { -// Class for representing a use of id. Note that result: -// * Type id is a use. +// Class for representing a use of id. Note that: +// * Result type id is a use. // * Ids referenced in OpSectionMerge & OpLoopMerge are considered as use. // * Ids referenced in OpPhi's in operands are considered as use. struct Use { @@ -57,11 +57,11 @@ class DefUseManager { using IdToDefMap = std::unordered_map; using IdToUsesMap = std::unordered_map; - // Analyzes the defs and uses in the given |module| and populates data - // structures in this class. - // TODO(antiagainst): This method should not modify the given module. Create - // const overload for ForEachInst(). - void AnalyzeDefUse(ir::Module* module); + inline explicit DefUseManager(ir::Module* module) { AnalyzeDefUse(module); } + DefUseManager(const DefUseManager&) = delete; + DefUseManager(DefUseManager&&) = delete; + DefUseManager& operator=(const DefUseManager&) = delete; + DefUseManager& operator=(DefUseManager&&) = delete; // Analyzes the defs and uses in the given |inst|. void AnalyzeInstDefUse(ir::Instruction* inst); @@ -91,6 +91,10 @@ class DefUseManager { bool ReplaceAllUsesWith(uint32_t before, uint32_t after); private: + // Analyzes the defs and uses in the given |module| and populates data + // structures in this class. + void AnalyzeDefUse(ir::Module* module); + // Clear the internal def-use record of a defined id if the given |def_id| is // recorded by this manager. This method will erase both the uses of |def_id| // and the |def_id|-generating instruction's use information kept in this diff --git a/source/opt/passes.cpp b/source/opt/passes.cpp index edc5976..13df8c9 100644 --- a/source/opt/passes.cpp +++ b/source/opt/passes.cpp @@ -80,8 +80,7 @@ bool FreezeSpecConstantValuePass::Process(ir::Module* module) { } bool EliminateDeadConstantPass::Process(ir::Module* module) { - analysis::DefUseManager def_use; - def_use.AnalyzeDefUse(module); + analysis::DefUseManager def_use(module); std::unordered_set working_list; // Traverse all the instructions to get the initial set of dead constants as // working list and count number of real uses for constants. Uses in diff --git a/source/opt/type_manager.h b/source/opt/type_manager.h index c6690cc..530f483 100644 --- a/source/opt/type_manager.h +++ b/source/opt/type_manager.h @@ -45,7 +45,7 @@ class TypeManager { using TypeToIdMap = std::unordered_map; using ForwardPointerVector = std::vector>; - TypeManager(const spvtools::ir::Module& module) { AnalyzeTypes(module); } + inline explicit TypeManager(const spvtools::ir::Module& module); TypeManager(const TypeManager&) = delete; TypeManager(TypeManager&&) = delete; TypeManager& operator=(const TypeManager&) = delete; @@ -84,6 +84,10 @@ class TypeManager { std::unordered_set unresolved_forward_pointers_; }; +inline TypeManager::TypeManager(const spvtools::ir::Module& module) { + AnalyzeTypes(module); +} + } // namespace analysis } // namespace opt } // namespace spvtools diff --git a/test/opt/test_def_use.cpp b/test/opt/test_def_use.cpp index 9bcc114..0a27506 100644 --- a/test/opt/test_def_use.cpp +++ b/test/opt/test_def_use.cpp @@ -143,8 +143,7 @@ TEST_P(ParseDefUseTest, Case) { ASSERT_NE(nullptr, module); // Analyze def and use. - opt::analysis::DefUseManager manager; - manager.AnalyzeDefUse(module.get()); + opt::analysis::DefUseManager manager(module.get()); CheckDef(tc.du, manager.id_to_defs()); CheckUse(tc.du, manager.id_to_uses()); @@ -525,8 +524,7 @@ TEST_P(ReplaceUseTest, Case) { ASSERT_NE(nullptr, module); // Analyze def and use. - opt::analysis::DefUseManager manager; - manager.AnalyzeDefUse(module.get()); + opt::analysis::DefUseManager manager(module.get()); // Do the substitution. for (const auto& candiate : tc.candidates) { @@ -828,8 +826,7 @@ TEST_P(KillDefTest, Case) { ASSERT_NE(nullptr, module); // Analyze def and use. - opt::analysis::DefUseManager manager; - manager.AnalyzeDefUse(module.get()); + opt::analysis::DefUseManager manager(module.get()); // Do the substitution. for (const auto id : tc.ids_to_kill) manager.KillDef(id); @@ -1079,8 +1076,7 @@ TEST(DefUseTest, OpSwitch) { ASSERT_NE(nullptr, module); // Analyze def and use. - opt::analysis::DefUseManager manager; - manager.AnalyzeDefUse(module.get()); + opt::analysis::DefUseManager manager(module.get()); // Do a bunch replacements. manager.ReplaceAllUsesWith(9, 900); // to unused id -- 2.7.4