diff options
author | Steven Perron <stevenperron@google.com> | 2017-11-13 15:31:43 -0500 |
---|---|---|
committer | Steven Perron <stevenperron@google.com> | 2017-11-15 12:48:03 -0500 |
commit | eb4653a67f69b0b558ee20d45d57aba7a4b83b87 (patch) | |
tree | af16fd219cd8d50e1fca198eee3940f5733fb4f9 | |
parent | a92d69b43d4203218362b78980164d50e21ac3f5 (diff) | |
download | SPIRV-Tools-eb4653a67f69b0b558ee20d45d57aba7a4b83b87.tar.gz SPIRV-Tools-eb4653a67f69b0b558ee20d45d57aba7a4b83b87.tar.bz2 SPIRV-Tools-eb4653a67f69b0b558ee20d45d57aba7a4b83b87.zip |
Add the decoration manager to the IRContext.
To make the decoration manger available everywhere, and to reduce the
number of times it needs to be build, I add one the IRContext.
As the same time, I move code that modifies decoration instruction into
the IRContext from mempass and the decoration manager. This will make
it easier to keep everything up to date.
This should take care of issue #928.
-rw-r--r-- | source/link/linker.cpp | 15 | ||||
-rw-r--r-- | source/opcode.cpp | 14 | ||||
-rw-r--r-- | source/opcode.h | 3 | ||||
-rw-r--r-- | source/opt/common_uniform_elim_pass.cpp | 30 | ||||
-rw-r--r-- | source/opt/common_uniform_elim_pass.h | 9 | ||||
-rw-r--r-- | source/opt/dead_branch_elim_pass.cpp | 3 | ||||
-rw-r--r-- | source/opt/dead_variable_elimination.cpp | 7 | ||||
-rw-r--r-- | source/opt/decoration_manager.cpp | 160 | ||||
-rw-r--r-- | source/opt/decoration_manager.h | 16 | ||||
-rw-r--r-- | source/opt/eliminate_dead_functions_pass.cpp | 2 | ||||
-rw-r--r-- | source/opt/inline_pass.cpp | 9 | ||||
-rw-r--r-- | source/opt/inline_pass.h | 3 | ||||
-rw-r--r-- | source/opt/instruction.cpp | 1 | ||||
-rw-r--r-- | source/opt/instruction.h | 8 | ||||
-rw-r--r-- | source/opt/ir_context.cpp | 50 | ||||
-rw-r--r-- | source/opt/ir_context.h | 29 | ||||
-rw-r--r-- | source/opt/local_access_chain_convert_pass.cpp | 2 | ||||
-rw-r--r-- | source/opt/mem_pass.cpp | 41 | ||||
-rw-r--r-- | source/opt/mem_pass.h | 17 | ||||
-rw-r--r-- | source/opt/pass.h | 4 | ||||
-rw-r--r-- | source/opt/remove_duplicates_pass.cpp | 9 | ||||
-rw-r--r-- | source/opt/remove_duplicates_pass.h | 3 | ||||
-rw-r--r-- | test/opt/ir_context_test.cpp | 48 |
23 files changed, 280 insertions, 203 deletions
diff --git a/source/link/linker.cpp b/source/link/linker.cpp index 35cba981..59ea36ce 100644 --- a/source/link/linker.cpp +++ b/source/link/linker.cpp @@ -227,16 +227,15 @@ spv_result_t Linker::Link(const uint32_t* const* binaries, // Phase 4: Find the import/export pairs LinkageTable linkings_to_do; - DecorationManager decoration_manager(linked_context.module()); - res = GetImportExportPairs(consumer, linked_context, - *linked_context.get_def_use_mgr(), - decoration_manager, &linkings_to_do); + res = GetImportExportPairs( + consumer, linked_context, *linked_context.get_def_use_mgr(), + *linked_context.get_decoration_mgr(), &linkings_to_do); if (res != SPV_SUCCESS) return res; // Phase 5: Ensure the import and export have the same types and decorations. res = CheckImportExportCompatibility(consumer, linkings_to_do, *linked_context.get_def_use_mgr(), - decoration_manager); + *linked_context.get_decoration_mgr()); if (res != SPV_SUCCESS) return res; // Phase 6: Remove duplicates @@ -248,9 +247,9 @@ spv_result_t Linker::Link(const uint32_t* const* binaries, // Phase 7: Remove linkage specific instructions, such as import/export // attributes, linkage capability, etc. if applicable - res = RemoveLinkageSpecificInstructions(consumer, !options.GetCreateLibrary(), - linkings_to_do, &decoration_manager, - &linked_context); + res = RemoveLinkageSpecificInstructions( + consumer, !options.GetCreateLibrary(), linkings_to_do, + linked_context.get_decoration_mgr(), &linked_context); if (res != SPV_SUCCESS) return res; // Phase 8: Rematch import variables/functions to export variables/functions diff --git a/source/opcode.cpp b/source/opcode.cpp index 1d229e08..650e635e 100644 --- a/source/opcode.cpp +++ b/source/opcode.cpp @@ -316,3 +316,17 @@ int32_t spvOpcodeGeneratesType(SpvOp op) { } return 0; } + +bool spvOpcodeIsDecoration(const SpvOp opcode) { + switch(opcode) { + case SpvOpDecorate: + case SpvOpDecorateId: + case SpvOpMemberDecorate: + case SpvOpGroupDecorate: + case SpvOpGroupMemberDecorate: + return true; + default: + break; + } + return false; +} diff --git a/source/opcode.h b/source/opcode.h index 4e06efdf..aec67f73 100644 --- a/source/opcode.h +++ b/source/opcode.h @@ -87,4 +87,7 @@ bool spvOpcodeReturnsLogicalVariablePointer(const SpvOp opcode); // non-zero otherwise. int32_t spvOpcodeGeneratesType(SpvOp opcode); +// Returns true if the opcode add a decoration to an id. +bool spvOpcodeIsDecoration(const SpvOp opcode); + #endif // LIBSPIRV_OPCODE_H_ diff --git a/source/opt/common_uniform_elim_pass.cpp b/source/opt/common_uniform_elim_pass.cpp index 5c3d8725..d68ed71a 100644 --- a/source/opt/common_uniform_elim_pass.cpp +++ b/source/opt/common_uniform_elim_pass.cpp @@ -99,7 +99,7 @@ ir::Instruction* CommonUniformElimPass::GetPtr(ir::Instruction* ip, bool CommonUniformElimPass::IsVolatileStruct(uint32_t type_id) { assert(get_def_use_mgr()->GetDef(type_id)->opcode() == SpvOpTypeStruct); bool has_volatile_deco = false; - dec_mgr_->ForEachDecoration(type_id, SpvDecorationVolatile, + get_decoration_mgr()->ForEachDecoration(type_id, SpvDecorationVolatile, [&has_volatile_deco](const ir::Instruction&) { has_volatile_deco = true; }); @@ -195,33 +195,10 @@ bool CommonUniformElimPass::HasOnlyNamesAndDecorates(uint32_t id) const { return true; } -void CommonUniformElimPass::KillNamesAndDecorates(uint32_t id) { - // TODO(greg-lunarg): Remove id from any OpGroupDecorate and - // kill if no other operands. - analysis::UseList* uses = get_def_use_mgr()->GetUses(id); - if (uses == nullptr) return; - std::list<ir::Instruction*> killList; - for (auto u : *uses) { - const SpvOp op = u.inst->opcode(); - if (op != SpvOpName && !IsNonTypeDecorate(op)) continue; - killList.push_back(u.inst); - } - for (auto kip : killList) context()->KillInst(kip); -} - -void CommonUniformElimPass::KillNamesAndDecorates(ir::Instruction* inst) { - // TODO(greg-lunarg): Remove inst from any OpGroupDecorate and - // kill if not other operands. - const uint32_t rId = inst->result_id(); - if (rId == 0) return; - KillNamesAndDecorates(rId); -} - void CommonUniformElimPass::DeleteIfUseless(ir::Instruction* inst) { const uint32_t resId = inst->result_id(); assert(resId != 0); if (HasOnlyNamesAndDecorates(resId)) { - KillNamesAndDecorates(resId); context()->KillInst(inst); } } @@ -230,7 +207,7 @@ void CommonUniformElimPass::ReplaceAndDeleteLoad(ir::Instruction* loadInst, uint32_t replId, ir::Instruction* ptrInst) { const uint32_t loadId = loadInst->result_id(); - KillNamesAndDecorates(loadId); + context()->KillNamesAndDecorates(loadId); (void)context()->ReplaceAllUsesWith(loadId, replId); // remove load instruction context()->KillInst(loadInst); @@ -490,7 +467,7 @@ bool CommonUniformElimPass::CommonExtractElimination(ir::Function* func) { ii = ii.InsertBefore(std::move(newExtract)); for (auto instItr : idxItr.second) { uint32_t resId = instItr->result_id(); - KillNamesAndDecorates(resId); + context()->KillNamesAndDecorates(resId); (void)context()->ReplaceAllUsesWith(resId, replId); context()->KillInst(instItr); } @@ -516,7 +493,6 @@ void CommonUniformElimPass::Initialize(ir::IRContext* c) { // Clear collections. comp2idx2inst_.clear(); - dec_mgr_.reset(new analysis::DecorationManager(get_module())); // Initialize extension whitelist InitExtensions(); diff --git a/source/opt/common_uniform_elim_pass.h b/source/opt/common_uniform_elim_pass.h index 0e626600..6f1e65fc 100644 --- a/source/opt/common_uniform_elim_pass.h +++ b/source/opt/common_uniform_elim_pass.h @@ -86,12 +86,6 @@ class CommonUniformElimPass : public Pass { // Return true if all uses of |id| are only name or decorate ops. bool HasOnlyNamesAndDecorates(uint32_t id) const; - // Kill all name and decorate ops using |inst| - void KillNamesAndDecorates(ir::Instruction* inst); - - // Kill all name and decorate ops using |id| - void KillNamesAndDecorates(uint32_t id); - // Delete inst if it has no uses. Assumes inst has a resultId. void DeleteIfUseless(ir::Instruction* inst); @@ -178,9 +172,6 @@ class CommonUniformElimPass : public Pass { void Initialize(ir::IRContext* c); Pass::Status ProcessImpl(); - // Decorations for the module we are processing - std::unique_ptr<analysis::DecorationManager> dec_mgr_; - // Map from uniform variable id to its common load id std::unordered_map<uint32_t, uint32_t> uniform2load_id_; diff --git a/source/opt/dead_branch_elim_pass.cpp b/source/opt/dead_branch_elim_pass.cpp index 7d543f98..e3bf25fd 100644 --- a/source/opt/dead_branch_elim_pass.cpp +++ b/source/opt/dead_branch_elim_pass.cpp @@ -18,6 +18,7 @@ #include "cfa.h" #include "iterator.h" +#include "ir_context.h" namespace spvtools { namespace opt { @@ -307,7 +308,7 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) { ++pii; } const uint32_t phiId = pii->result_id(); - KillNamesAndDecorates(phiId); + context()->KillNamesAndDecorates(phiId); (void)context()->ReplaceAllUsesWith(phiId, replId); context()->KillInst(&*pii); } diff --git a/source/opt/dead_variable_elimination.cpp b/source/opt/dead_variable_elimination.cpp index 70d2eb5b..723dfa44 100644 --- a/source/opt/dead_variable_elimination.cpp +++ b/source/opt/dead_variable_elimination.cpp @@ -15,6 +15,7 @@ #include "dead_variable_elimination.h" #include "reflect.h" +#include "ir_context.h" namespace spvtools { namespace opt { @@ -29,9 +30,6 @@ Pass::Status DeadVariableElimination::Process(ir::IRContext* c) { // value kMustKeep as the reference count. InitializeProcessing(c); - // Decoration manager to help organize decorations. - analysis::DecorationManager decoration_manager(context()->module()); - std::vector<uint32_t> ids_to_remove; // Get the reference count for all of the global OpVariable instructions. @@ -45,7 +43,7 @@ Pass::Status DeadVariableElimination::Process(ir::IRContext* c) { // Check the linkage. If it is exported, it could be reference somewhere // else, so we must keep the variable around. - decoration_manager.ForEachDecoration( + get_decoration_mgr()->ForEachDecoration( result_id, SpvDecorationLinkageAttributes, [&count](const ir::Instruction& linkage_instruction) { uint32_t last_operand = linkage_instruction.NumOperands() - 1; @@ -109,7 +107,6 @@ void DeadVariableElimination::DeleteVariable(uint32_t result_id) { } } } - this->KillNamesAndDecorates(result_id); context()->KillDef(result_id); } } // namespace opt diff --git a/source/opt/decoration_manager.cpp b/source/opt/decoration_manager.cpp index 77d5945d..aa926dbf 100644 --- a/source/opt/decoration_manager.cpp +++ b/source/opt/decoration_manager.cpp @@ -14,49 +14,18 @@ #include "decoration_manager.h" -#include <stack> +#include <algorithm> #include <iostream> +#include <stack> namespace spvtools { namespace opt { namespace analysis { -void DecorationManager::RemoveDecorationsFrom(uint32_t id, bool keep_linkage) { +void DecorationManager::RemoveDecorationsFrom(uint32_t id) { auto const ids_iter = id_to_decoration_insts_.find(id); if (ids_iter == id_to_decoration_insts_.end()) return; - - for (ir::Instruction* inst : ids_iter->second) { - switch (inst->opcode()) { - case SpvOpDecorate: - case SpvOpDecorateId: - case SpvOpMemberDecorate: - if (!(keep_linkage && - inst->GetSingleWordInOperand(1u) == - SpvDecorationLinkageAttributes)) - inst->ToNop(); - break; - case SpvOpGroupDecorate: - for (uint32_t i = 1u; i < inst->NumInOperands(); ++i) { - if (inst->GetSingleWordInOperand(i) == inst->result_id()) { - // TODO(pierremoreau): This could be optimised by copying the last - // operand over this one, or using a compacting - // filtering algorithm over all other IDs - inst->RemoveInOperand(i); - } - } - break; - case SpvOpGroupMemberDecorate: - for (uint32_t i = 1u; i < inst->NumInOperands(); i += 2u) { - if (inst->GetSingleWordInOperand(i) == inst->result_id()) { - // TODO(pierremoreau): Same optimisation opportunity as above. - inst->RemoveInOperand(i); - } - } - break; - default: - break; - } - } + id_to_decoration_insts_.erase(ids_iter); } std::vector<ir::Instruction*> DecorationManager::GetDecorationsFor( @@ -144,41 +113,44 @@ void DecorationManager::AnalyzeDecorations() { // For each group and instruction, collect all their decoration instructions. for (ir::Instruction& inst : module_->annotations()) { - switch (inst.opcode()) { - case SpvOpDecorate: - case SpvOpDecorateId: - case SpvOpMemberDecorate: { - auto const target_id = inst.GetSingleWordInOperand(0u); + AddDecoration(&inst); + } +} +void DecorationManager::AddDecoration(ir::Instruction* inst) { + switch (inst->opcode()) { + case SpvOpDecorate: + case SpvOpDecorateId: + case SpvOpMemberDecorate: { + auto const target_id = inst->GetSingleWordInOperand(0u); + auto const group_iter = group_to_decoration_insts_.find(target_id); + if (group_iter != group_to_decoration_insts_.end()) + group_iter->second.push_back(inst); + else + id_to_decoration_insts_[target_id].push_back(inst); + break; + } + case SpvOpGroupDecorate: + for (uint32_t i = 1u; i < inst->NumInOperands(); ++i) { + auto const target_id = inst->GetSingleWordInOperand(i); auto const group_iter = group_to_decoration_insts_.find(target_id); if (group_iter != group_to_decoration_insts_.end()) - group_iter->second.push_back(&inst); + group_iter->second.push_back(inst); else - id_to_decoration_insts_[target_id].push_back(&inst); - break; + id_to_decoration_insts_[target_id].push_back(inst); } - case SpvOpGroupDecorate: - for (uint32_t i = 1u; i < inst.NumInOperands(); ++i) { - auto const target_id = inst.GetSingleWordInOperand(i); - auto const group_iter = group_to_decoration_insts_.find(target_id); - if (group_iter != group_to_decoration_insts_.end()) - group_iter->second.push_back(&inst); - else - id_to_decoration_insts_[target_id].push_back(&inst); - } - break; - case SpvOpGroupMemberDecorate: - for (uint32_t i = 1u; i < inst.NumInOperands(); i += 2u) { - auto const target_id = inst.GetSingleWordInOperand(i); - auto const group_iter = group_to_decoration_insts_.find(target_id); - if (group_iter != group_to_decoration_insts_.end()) - group_iter->second.push_back(&inst); - else - id_to_decoration_insts_[target_id].push_back(&inst); - } - break; - default: - break; - } + break; + case SpvOpGroupMemberDecorate: + for (uint32_t i = 1u; i < inst->NumInOperands(); i += 2u) { + auto const target_id = inst->GetSingleWordInOperand(i); + auto const group_iter = group_to_decoration_insts_.find(target_id); + if (group_iter != group_to_decoration_insts_.end()) + group_iter->second.push_back(inst); + else + id_to_decoration_insts_[target_id].push_back(inst); + } + break; + default: + break; } } @@ -231,9 +203,9 @@ std::vector<T> DecorationManager::InternalGetDecorationsFor( return decorations; } -void DecorationManager::ForEachDecoration(uint32_t id, - uint32_t decoration, - std::function<void(const ir::Instruction&)> f) { +void DecorationManager::ForEachDecoration( + uint32_t id, uint32_t decoration, + std::function<void(const ir::Instruction&)> f) { for (const ir::Instruction* inst : GetDecorationsFor(id, true)) { switch (inst->opcode()) { case SpvOpMemberDecorate: @@ -253,8 +225,8 @@ void DecorationManager::ForEachDecoration(uint32_t id, } } -void DecorationManager::CloneDecorations(uint32_t from, uint32_t to, - std::function<void(ir::Instruction&, bool)> f) { +void DecorationManager::CloneDecorations( + uint32_t from, uint32_t to, std::function<void(ir::Instruction&, bool)> f) { assert(f && "Missing function parameter f"); auto const decoration_list = id_to_decoration_insts_.find(from); if (decoration_list == id_to_decoration_insts_.end()) return; @@ -263,7 +235,8 @@ void DecorationManager::CloneDecorations(uint32_t from, uint32_t to, case SpvOpGroupDecorate: f(*inst, false); // add |to| to list of decorated id's - inst->AddOperand(ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID, {to})); + inst->AddOperand( + ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID, {to})); id_to_decoration_insts_[to].push_back(inst); f(*inst, true); break; @@ -273,8 +246,9 @@ void DecorationManager::CloneDecorations(uint32_t from, uint32_t to, const uint32_t num_operands = inst->NumOperands(); for (uint32_t i = 1; i < num_operands; i += 2) { ir::Operand op = inst->GetOperand(i); - if (op.words[0] == from) { // add new pair of operands: (to, literal) - inst->AddOperand(ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID, {to})); + if (op.words[0] == from) { // add new pair of operands: (to, literal) + inst->AddOperand( + ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID, {to})); op = inst->GetOperand(i + 1); inst->AddOperand(std::move(op)); } @@ -300,6 +274,44 @@ void DecorationManager::CloneDecorations(uint32_t from, uint32_t to, } } +void DecorationManager::RemoveDecoration(ir::Instruction* inst) { + switch (inst->opcode()) { + case SpvOpDecorate: + case SpvOpDecorateId: + case SpvOpMemberDecorate: { + auto const target_id = inst->GetSingleWordInOperand(0u); + RemoveInstructionFromTarget(inst, target_id); + } break; + case SpvOpGroupDecorate: + for (uint32_t i = 1u; i < inst->NumInOperands(); ++i) { + auto const target_id = inst->GetSingleWordInOperand(i); + RemoveInstructionFromTarget(inst, target_id); + } + break; + case SpvOpGroupMemberDecorate: + for (uint32_t i = 1u; i < inst->NumInOperands(); i += 2u) { + auto const target_id = inst->GetSingleWordInOperand(i); + RemoveInstructionFromTarget(inst, target_id); + } + break; + default: + break; + } +} + +void DecorationManager::RemoveInstructionFromTarget(ir::Instruction* inst, + const uint32_t target_id) { + auto const group_iter = group_to_decoration_insts_.find(target_id); + if (group_iter != group_to_decoration_insts_.end()) { + remove(group_iter->second.begin(), group_iter->second.end(), inst); + } else { + auto target_list_iter = id_to_decoration_insts_.find(target_id); + if (target_list_iter != id_to_decoration_insts_.end()) { + remove(target_list_iter->second.begin(), target_list_iter->second.end(), + inst); + } + } +} } // namespace analysis } // namespace opt } // namespace spvtools diff --git a/source/opt/decoration_manager.h b/source/opt/decoration_manager.h index 10e7b719..a87a8f9d 100644 --- a/source/opt/decoration_manager.h +++ b/source/opt/decoration_manager.h @@ -35,9 +35,12 @@ class DecorationManager { } DecorationManager() = delete; - // Removes all decorations from |id|, which should not be a group ID, except - // for linkage decorations if |keep_linkage| is set. - void RemoveDecorationsFrom(uint32_t id, bool keep_linkage); + // Removes all decorations from |id|, which should not be a group ID. + void RemoveDecorationsFrom(uint32_t id); + + // Removes all decorations from the result id of |inst|. + void RemoveDecoration(ir::Instruction* inst); + // Returns a vector of all decorations affecting |id|. If a group is applied // to |id|, the decorations of that group are returned rather than the group // decoration instruction. If |include_linkage| is not set, linkage @@ -70,7 +73,14 @@ class DecorationManager { // with |true| afterwards. void CloneDecorations(uint32_t from, uint32_t to, std::function<void(ir::Instruction&, bool)> f); + // Informs the decoration manager of a new decoration that it needs to track. + void AddDecoration(ir::Instruction* inst); + private: + // Removes the instruction from the set of decorations targeting |target_id|. + void RemoveInstructionFromTarget(ir::Instruction* inst, + const uint32_t target_id); + using IdToDecorationInstsMap = std::unordered_map<uint32_t, std::vector<ir::Instruction*>>; // Analyzes the defs and uses in the given |module| and populates data diff --git a/source/opt/eliminate_dead_functions_pass.cpp b/source/opt/eliminate_dead_functions_pass.cpp index 8eae3e9a..8c7d02f4 100644 --- a/source/opt/eliminate_dead_functions_pass.cpp +++ b/source/opt/eliminate_dead_functions_pass.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include "eliminate_dead_functions_pass.h" +#include "ir_context.h" #include <unordered_set> @@ -51,7 +52,6 @@ void EliminateDeadFunctionsPass::EliminateFunction(ir::Function* func) { // Remove all of the instruction in the function body func->ForEachInst( [this](ir::Instruction* inst) { - KillNamesAndDecorates(inst); context()->KillInst(inst); }, true); diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp index f37d4d00..f52277bd 100644 --- a/source/opt/inline_pass.cpp +++ b/source/opt/inline_pass.cpp @@ -145,7 +145,7 @@ void InlinePass::CloneAndMapLocals( while (callee_var_itr->opcode() == SpvOp::SpvOpVariable) { std::unique_ptr<ir::Instruction> var_inst(callee_var_itr->Clone()); uint32_t newId = TakeNextId(); - dec_mgr_->CloneDecorations(callee_var_itr->result_id(), newId, update_def_use_mgr_); + get_decoration_mgr()->CloneDecorations(callee_var_itr->result_id(), newId, update_def_use_mgr_); var_inst->SetResultId(newId); (*callee2caller)[callee_var_itr->result_id()] = newId; new_vars->push_back(std::move(var_inst)); @@ -174,7 +174,7 @@ uint32_t InlinePass::CreateReturnVar( {SpvStorageClassFunction}}})); new_vars->push_back(std::move(var_inst)); } - dec_mgr_->CloneDecorations(calleeFn->result_id(), returnVarId, update_def_use_mgr_); + get_decoration_mgr()->CloneDecorations(calleeFn->result_id(), returnVarId, update_def_use_mgr_); return returnVarId; } @@ -199,7 +199,7 @@ void InlinePass::CloneSameBlockOps( CloneSameBlockOps(&sb_inst, postCallSB, preCallSB, block_ptr); const uint32_t rid = sb_inst->result_id(); const uint32_t nid = this->TakeNextId(); - dec_mgr_->CloneDecorations(rid, nid, update_def_use_mgr_); + get_decoration_mgr()->CloneDecorations(rid, nid, update_def_use_mgr_); sb_inst->SetResultId(nid); (*postCallSB)[rid] = nid; *iid = nid; @@ -479,7 +479,7 @@ void InlinePass::GenInlineCode( callee2caller[rid] = nid; } cp_inst->SetResultId(nid); - dec_mgr_->CloneDecorations(rid, nid, update_def_use_mgr_); + get_decoration_mgr()->CloneDecorations(rid, nid, update_def_use_mgr_); } new_blk_ptr->AddInstruction(std::move(cp_inst)); } break; @@ -646,7 +646,6 @@ bool InlinePass::IsInlinableFunction(ir::Function* func) { void InlinePass::InitializeInline(ir::IRContext* c) { InitializeProcessing(c); - dec_mgr_.reset(new analysis::DecorationManager(c->module())); update_def_use_mgr_ = [this] (ir::Instruction& inst, bool has_changed) { if (has_changed) get_def_use_mgr()->AnalyzeInstDefUse(&inst); diff --git a/source/opt/inline_pass.h b/source/opt/inline_pass.h index 578deb61..7ecf772f 100644 --- a/source/opt/inline_pass.h +++ b/source/opt/inline_pass.h @@ -165,9 +165,6 @@ class InlinePass : public Pass { // Update the DefUseManager when cloning decorations. std::function<void(ir::Instruction&, bool)> update_def_use_mgr_; - // Decorations for the module we are processing TODO: move this to ir_context as well - std::unique_ptr<analysis::DecorationManager> dec_mgr_; - // Map from function's result id to function. std::unordered_map<uint32_t, ir::Function*> id2function_; diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 78906c72..f26fb1d3 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -109,6 +109,5 @@ void Instruction::ReplaceOperands(const std::vector<Operand>& new_operands) { operands_.insert(operands_.begin(), new_operands.begin(), new_operands.end()); operands_.shrink_to_fit(); } - } // namespace ir } // namespace spvtools diff --git a/source/opt/instruction.h b/source/opt/instruction.h index 33f3c33f..ff0acdb0 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -19,6 +19,7 @@ #include <functional> #include <utility> #include <vector> +#include <opcode.h> #include "operand.h" #include "util/ilist_node.h" @@ -237,6 +238,9 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> { // this instruction. void ReplaceOperands(const std::vector<Operand>& new_operands); + // Returns true if the instruction annotates an id with a decoration. + inline bool IsDecoration(); + private: // Returns the total count of result type id and result id. uint32_t TypeResultIdCount() const { @@ -399,6 +403,10 @@ inline bool Instruction::HasLabels() const { return false; } +bool Instruction::IsDecoration() { + return spvOpcodeIsDecoration(opcode()); +} + } // namespace ir } // namespace spvtools diff --git a/source/opt/ir_context.cpp b/source/opt/ir_context.cpp index d7c92800..b89f5193 100644 --- a/source/opt/ir_context.cpp +++ b/source/opt/ir_context.cpp @@ -14,6 +14,7 @@ #include "ir_context.h" #include "log.h" +#include "mem_pass.h" namespace spvtools { namespace ir { @@ -25,6 +26,9 @@ void IRContext::BuildInvalidAnalyses(IRContext::Analysis set) { if (set & kAnalysisInstrToBlockMapping) { BuildInstrToBlockMapping(); } + if (set & kAnalysisDecorations) { + BuildDecorationManager(); + } } void IRContext::InvalidateAnalysesExceptFor( @@ -40,6 +44,9 @@ void IRContext::InvalidateAnalyses(IRContext::Analysis analyses_to_invalidate) { if (analyses_to_invalidate & kAnalysisInstrToBlockMapping) { instr_to_block_.clear(); } + if (analyses_to_invalidate & kAnalysisDecorations) { + decoration_mgr_.reset(nullptr); + } valid_analyses_ = Analysis(valid_analyses_ & ~analyses_to_invalidate); } @@ -48,12 +55,22 @@ void IRContext::KillInst(ir::Instruction* inst) { return; } + KillNamesAndDecorates(inst); + if (AreAnalysesValid(kAnalysisDefUse)) { get_def_use_mgr()->ClearInst(inst); } if (AreAnalysesValid(kAnalysisInstrToBlockMapping)) { instr_to_block_.erase(inst); } + if (AreAnalysesValid(kAnalysisDecorations)) { + if (inst->result_id() != 0) { + decoration_mgr_->RemoveDecorationsFrom(inst->result_id()); + } + if (inst->IsDecoration()) { + decoration_mgr_->RemoveDecoration(inst); + } + } inst->ToNop(); } @@ -125,12 +142,45 @@ void spvtools::ir::IRContext::ForgetUses(Instruction* inst) { if (AreAnalysesValid(kAnalysisDefUse)) { get_def_use_mgr()->EraseUseRecordsOfOperandIds(inst); } + if (AreAnalysesValid(kAnalysisDecorations)) { + if (inst->IsDecoration()) { + get_decoration_mgr()->RemoveDecoration(inst); + } + } } void IRContext::AnalyzeUses(Instruction* inst) { if (AreAnalysesValid(kAnalysisDefUse)) { get_def_use_mgr()->AnalyzeInstUse(inst); } + if (AreAnalysesValid(kAnalysisDecorations)) { + if (inst->IsDecoration()) { + get_decoration_mgr()->AddDecoration(inst); + } + } +} + +void IRContext::KillNamesAndDecorates(uint32_t id) { + std::vector<ir::Instruction*> decorations = + get_decoration_mgr()->GetDecorationsFor(id, true); + + for (Instruction* inst : decorations) { + KillInst(inst); + } + + for (auto& di : debugs2()) { + if (di.opcode() == SpvOpMemberName || di.opcode() == SpvOpName) { + if (di.GetSingleWordInOperand(0) == id) { + KillInst(&di); + } + } + } +} + +void IRContext::KillNamesAndDecorates(Instruction* inst) { + const uint32_t rId = inst->result_id(); + if (rId == 0) return; + KillNamesAndDecorates(rId); } } // namespace ir diff --git a/source/opt/ir_context.h b/source/opt/ir_context.h index 464749ec..23f59ed9 100644 --- a/source/opt/ir_context.h +++ b/source/opt/ir_context.h @@ -15,9 +15,11 @@ #ifndef SPIRV_TOOLS_IR_CONTEXT_H #define SPIRV_TOOLS_IR_CONTEXT_H +#include "decoration_manager.h" #include "def_use_manager.h" #include "module.h" +#include <algorithm> #include <iostream> namespace spvtools { @@ -42,7 +44,8 @@ class IRContext { kAnalysisBegin = 1 << 0, kAnalysisDefUse = kAnalysisBegin, kAnalysisInstrToBlockMapping = 1 << 1, - kAnalysisEnd = 1 << 2 + kAnalysisDecorations = 1 << 2, + kAnalysisEnd = 1 << 3 }; friend inline Analysis operator|(Analysis lhs, Analysis rhs); @@ -170,6 +173,15 @@ class IRContext { return (entry != instr_to_block_.end()) ? entry->second : nullptr; } + // Returns a pointer the decoration manager. If the decoration manger is + // invalid, it is rebuilt first. + opt::analysis::DecorationManager* get_decoration_mgr() { + if (!AreAnalysesValid(kAnalysisDecorations)) { + BuildDecorationManager(); + } + return decoration_mgr_.get(); + }; + // Sets the message consumer to the given |consumer|. |consumer| which will be // invoked every time there is a message to be communicated to the outside. void SetMessageConsumer(spvtools::MessageConsumer c) { @@ -221,6 +233,12 @@ class IRContext { // will be updated accordingly. void AnalyzeUses(Instruction* inst); + // Kill all name and decorate ops targeting |id|. + void KillNamesAndDecorates(uint32_t id); + + // Kill all name and decorate ops targeting the result id of |inst|. + void KillNamesAndDecorates(ir::Instruction* inst); + private: // Builds the def-use manager from scratch, even if it was already valid. void BuildDefUseManager() { @@ -241,9 +259,15 @@ class IRContext { valid_analyses_ = valid_analyses_ | kAnalysisInstrToBlockMapping; } + void BuildDecorationManager() { + decoration_mgr_.reset(new opt::analysis::DecorationManager(module())); + valid_analyses_ = valid_analyses_ | kAnalysisDecorations; + } + std::unique_ptr<Module> module_; spvtools::MessageConsumer consumer_; std::unique_ptr<opt::analysis::DefUseManager> def_use_mgr_; + std::unique_ptr<opt::analysis::DecorationManager> decoration_mgr_; // A map from instructions the the basic block they belong to. This mapping is // built on-demand when get_instr_block() is called. @@ -428,6 +452,9 @@ void IRContext::AddDebug3Inst(std::unique_ptr<Instruction>&& d) { } void IRContext::AddAnnotationInst(std::unique_ptr<Instruction>&& a) { + if (AreAnalysesValid(kAnalysisDecorations)) { + get_decoration_mgr()->AddDecoration(a.get()); + } module()->AddAnnotationInst(std::move(a)); } diff --git a/source/opt/local_access_chain_convert_pass.cpp b/source/opt/local_access_chain_convert_pass.cpp index 41c92885..9663e880 100644 --- a/source/opt/local_access_chain_convert_pass.cpp +++ b/source/opt/local_access_chain_convert_pass.cpp @@ -17,6 +17,7 @@ #include "local_access_chain_convert_pass.h" #include "iterator.h" +#include "ir_context.h" namespace spvtools { namespace opt { @@ -34,7 +35,6 @@ void LocalAccessChainConvertPass::DeleteIfUseless(ir::Instruction* inst) { const uint32_t resId = inst->result_id(); assert(resId != 0); if (HasOnlyNamesAndDecorates(resId)) { - KillNamesAndDecorates(resId); context()->KillInst(inst); } } diff --git a/source/opt/mem_pass.cpp b/source/opt/mem_pass.cpp index e0179979..72e4f73f 100644 --- a/source/opt/mem_pass.cpp +++ b/source/opt/mem_pass.cpp @@ -19,6 +19,7 @@ #include "basic_block.h" #include "cfa.h" #include "iterator.h" +#include "ir_context.h" namespace spvtools { namespace opt { @@ -116,21 +117,9 @@ ir::Instruction* MemPass::GetPtr(ir::Instruction* ip, uint32_t* varId) { return GetPtr(ptrId, varId); } -void MemPass::FindNamedOrDecoratedIds() { - named_or_decorated_ids_.clear(); - for (auto& di : get_module()->debugs2()) - if (di.opcode() == SpvOpName) - named_or_decorated_ids_.insert(di.GetSingleWordInOperand(0)); - for (auto& ai : get_module()->annotations()) - if (ai.opcode() == SpvOpDecorate || ai.opcode() == SpvOpDecorateId) - named_or_decorated_ids_.insert(ai.GetSingleWordInOperand(0)); -} - bool MemPass::HasOnlyNamesAndDecorates(uint32_t id) const { analysis::UseList* uses = get_def_use_mgr()->GetUses(id); if (uses == nullptr) return true; - if (named_or_decorated_ids_.find(id) == named_or_decorated_ids_.end()) - return false; for (auto u : *uses) { const SpvOp op = u.inst->opcode(); if (op != SpvOpName && !IsNonTypeDecorate(op)) return false; @@ -138,29 +127,8 @@ bool MemPass::HasOnlyNamesAndDecorates(uint32_t id) const { return true; } -void MemPass::KillNamesAndDecorates(uint32_t id) { - // TODO(greg-lunarg): Remove id from any OpGroupDecorate and - // kill if no other operands. - if (named_or_decorated_ids_.find(id) == named_or_decorated_ids_.end()) return; - analysis::UseList* uses = get_def_use_mgr()->GetUses(id); - if (uses == nullptr) return; - std::list<ir::Instruction*> killList; - for (auto u : *uses) { - const SpvOp op = u.inst->opcode(); - if (op == SpvOpName || IsNonTypeDecorate(op)) killList.push_back(u.inst); - } - for (auto kip : killList) context()->KillInst(kip); -} - -void MemPass::KillNamesAndDecorates(ir::Instruction* inst) { - const uint32_t rId = inst->result_id(); - if (rId == 0) return; - KillNamesAndDecorates(rId); -} - void MemPass::KillAllInsts(ir::BasicBlock* bp) { bp->ForEachInst([this](ir::Instruction* ip) { - KillNamesAndDecorates(ip); context()->KillInst(ip); }); } @@ -229,7 +197,6 @@ void MemPass::DCEInst(ir::Instruction* inst) { uint32_t varId = 0; // Remember variable if dead load if (di->opcode() == SpvOpLoad) (void)GetPtr(di, &varId); - KillNamesAndDecorates(di); context()->KillInst(di); // For all operands with no remaining uses, add their instruction // to the dead instruction queue. @@ -245,7 +212,7 @@ void MemPass::DCEInst(ir::Instruction* inst) { void MemPass::ReplaceAndDeleteLoad(ir::Instruction* loadInst, uint32_t replId) { const uint32_t loadId = loadInst->result_id(); - KillNamesAndDecorates(loadId); + context()->KillNamesAndDecorates(loadId); (void)context()->ReplaceAllUsesWith(loadId, replId); DCEInst(loadInst); } @@ -633,7 +600,7 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) { // and delete load. Kill any names or decorates using id before // replacing to prevent incorrect replacement in those instructions. const uint32_t loadId = ii->result_id(); - KillNamesAndDecorates(loadId); + context()->KillNamesAndDecorates(loadId); (void)context()->ReplaceAllUsesWith(loadId, replId); context()->KillInst(&*ii); } break; @@ -770,14 +737,12 @@ void MemPass::RemoveBlock(ir::Function::iterator* bi) { // instruction is needed to identify the block, which is needed by the // removal of phi operands. if (inst != rm_block.GetLabelInst()) { - KillNamesAndDecorates(inst); context()->KillInst(inst); } }); // Remove the label instruction last. auto label = rm_block.GetLabelInst(); - KillNamesAndDecorates(label); context()->KillInst(label); *bi = bi->Erase(); diff --git a/source/opt/mem_pass.h b/source/opt/mem_pass.h index 2d6e2c75..c02e1793 100644 --- a/source/opt/mem_pass.h +++ b/source/opt/mem_pass.h @@ -41,12 +41,6 @@ class MemPass : public Pass { virtual ~MemPass() = default; protected: - // Initialize basic data structures for the pass. - void InitializeProcessing(ir::IRContext* c) { - Pass::InitializeProcessing(c); - FindNamedOrDecoratedIds(); - } - // Returns true if |typeInst| is a scalar type // or a vector or matrix bool IsBaseTargetType(const ir::Instruction* typeInst) const; @@ -74,18 +68,9 @@ class MemPass : public Pass { // Return true if all uses of |id| are only name or decorate ops. bool HasOnlyNamesAndDecorates(uint32_t id) const; - // Kill all name and decorate ops using |inst| - void KillNamesAndDecorates(ir::Instruction* inst); - - // Kill all name and decorate ops using |id| - void KillNamesAndDecorates(uint32_t id); - // Kill all instructions in block |bp|. void KillAllInsts(ir::BasicBlock* bp); - // Collect all named or decorated ids in module - void FindNamedOrDecoratedIds(); - // Return true if any instruction loads from |varId| bool HasLoads(uint32_t varId) const; @@ -207,8 +192,6 @@ class MemPass : public Pass { // |reachable_blocks|. void RemovePhiOperands(ir::Instruction* phi, std::unordered_set<ir::BasicBlock*> reachable_blocks); - // named or decorated ids - std::unordered_set<uint32_t> named_or_decorated_ids_; // Map from block's label id to a map of a variable to its value at the // end of the block. diff --git a/source/opt/pass.h b/source/opt/pass.h index e11d6e43..73305e13 100644 --- a/source/opt/pass.h +++ b/source/opt/pass.h @@ -79,6 +79,10 @@ class Pass { return context()->get_def_use_mgr(); } + analysis::DecorationManager* get_decoration_mgr() const { + return context()->get_decoration_mgr(); + } + // Returns a pointer to the current module for this pass. ir::Module* get_module() const { return context_->module(); } diff --git a/source/opt/remove_duplicates_pass.cpp b/source/opt/remove_duplicates_pass.cpp index 6bb62120..198655b0 100644 --- a/source/opt/remove_duplicates_pass.cpp +++ b/source/opt/remove_duplicates_pass.cpp @@ -36,11 +36,9 @@ using opt::analysis::DefUseManager; using opt::analysis::DecorationManager; Pass::Status RemoveDuplicatesPass::Process(ir::IRContext* irContext) { - DecorationManager decManager(irContext->module()); - bool modified = RemoveDuplicateCapabilities(irContext); modified |= RemoveDuplicatesExtInstImports(irContext); - modified |= RemoveDuplicateTypes(irContext, decManager); + modified |= RemoveDuplicateTypes(irContext); modified |= RemoveDuplicateDecorations(irContext); return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; @@ -92,8 +90,7 @@ bool RemoveDuplicatesPass::RemoveDuplicatesExtInstImports( return modified; } -bool RemoveDuplicatesPass::RemoveDuplicateTypes( - ir::IRContext* irContext, DecorationManager& decManager) const { +bool RemoveDuplicatesPass::RemoveDuplicateTypes(ir::IRContext* irContext) const { bool modified = false; std::vector<Instruction> visitedTypes; @@ -110,7 +107,7 @@ bool RemoveDuplicatesPass::RemoveDuplicateTypes( // Is the current type equal to one of the types we have aready visited? SpvId idToKeep = 0u; for (auto j : visitedTypes) { - if (AreTypesEqual(*i, j, *irContext->get_def_use_mgr(), decManager)) { + if (AreTypesEqual(*i, j, *irContext->get_def_use_mgr(), *irContext->get_decoration_mgr())) { idToKeep = j.result_id(); break; } diff --git a/source/opt/remove_duplicates_pass.h b/source/opt/remove_duplicates_pass.h index d61079e9..72c19249 100644 --- a/source/opt/remove_duplicates_pass.h +++ b/source/opt/remove_duplicates_pass.h @@ -43,8 +43,7 @@ class RemoveDuplicatesPass : public Pass { private: bool RemoveDuplicateCapabilities(ir::IRContext* irContext) const; bool RemoveDuplicatesExtInstImports(ir::IRContext* irContext) const; - bool RemoveDuplicateTypes(ir::IRContext* irContext, - analysis::DecorationManager& decManager) const; + bool RemoveDuplicateTypes(ir::IRContext* irContext) const; bool RemoveDuplicateDecorations(ir::IRContext* irContext) const; }; diff --git a/test/opt/ir_context_test.cpp b/test/opt/ir_context_test.cpp index 8d3d4d3a..f4180222 100644 --- a/test/opt/ir_context_test.cpp +++ b/test/opt/ir_context_test.cpp @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include <external/googletest/googlemock/include/gmock/gmock-matchers.h> #include <gtest/gtest.h> #include <algorithm> @@ -25,6 +26,7 @@ namespace { using namespace spvtools; using ir::IRContext; using Analysis = IRContext::Analysis; +using ::testing::Each; class DummyPassPreservesNothing : public opt::Pass { public: @@ -135,7 +137,7 @@ TEST_F(IRContextTest, AllPreservedAfterPassWithChange) { } } -TEST_F(IRContextTest, AllPreserveFirstOnlyAfterPassWithChange) { +TEST_F(IRContextTest, PreserveFirstOnlyAfterPassWithChange) { std::unique_ptr<ir::Module> module = MakeUnique<ir::Module>(); IRContext context(std::move(module), spvtools::MessageConsumer()); @@ -153,4 +155,48 @@ TEST_F(IRContextTest, AllPreserveFirstOnlyAfterPassWithChange) { EXPECT_FALSE(context.AreAnalysesValid(i)); } } + +TEST_F(IRContextTest, KillMemberName) { + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %2 "main" + OpExecutionMode %2 OriginUpperLeft + OpSource GLSL 430 + OpName %3 "stuff" + OpMemberName %3 0 "refZ" + OpMemberDecorate %3 0 Offset 0 + OpDecorate %3 Block + %4 = OpTypeFloat 32 + %3 = OpTypeStruct %4 + %5 = OpTypeVoid + %6 = OpTypeFunction %5 + %2 = OpFunction %5 None %6 + %7 = OpLabel + OpReturn + OpFunctionEnd +)"; + + std::unique_ptr<ir::Module> module = + BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text); + ir::IRContext context(std::move(module), spvtools::MessageConsumer()); + + // Build the decoration manager. + context.get_decoration_mgr(); + + // Delete the OpTypeStruct. Should delete the OpName, OpMemberName, and + // OpMemberDecorate associated with it. + context.KillDef(3); + + // Make sure all of the name are removed. + for (auto& inst : context.debugs2()) { + EXPECT_EQ(inst.opcode(), SpvOpNop); + } + + // Make sure all of the decorations are removed. + for (auto& inst : context.annotations()) { + EXPECT_EQ(inst.opcode(), SpvOpNop); + } +} } // anonymous namespace |