Add the decoration manager to the IRContext.
authorSteven Perron <stevenperron@google.com>
Mon, 13 Nov 2017 20:31:43 +0000 (15:31 -0500)
committerSteven Perron <stevenperron@google.com>
Wed, 15 Nov 2017 17:48:03 +0000 (12:48 -0500)
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.

23 files changed:
source/link/linker.cpp
source/opcode.cpp
source/opcode.h
source/opt/common_uniform_elim_pass.cpp
source/opt/common_uniform_elim_pass.h
source/opt/dead_branch_elim_pass.cpp
source/opt/dead_variable_elimination.cpp
source/opt/decoration_manager.cpp
source/opt/decoration_manager.h
source/opt/eliminate_dead_functions_pass.cpp
source/opt/inline_pass.cpp
source/opt/inline_pass.h
source/opt/instruction.cpp
source/opt/instruction.h
source/opt/ir_context.cpp
source/opt/ir_context.h
source/opt/local_access_chain_convert_pass.cpp
source/opt/mem_pass.cpp
source/opt/mem_pass.h
source/opt/pass.h
source/opt/remove_duplicates_pass.cpp
source/opt/remove_duplicates_pass.h
test/opt/ir_context_test.cpp

index 35cba98..59ea36c 100644 (file)
@@ -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
index 1d229e0..650e635 100644 (file)
@@ -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;
+}
index 4e06efd..aec67f7 100644 (file)
@@ -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_
index 5c3d872..d68ed71 100644 (file)
@@ -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();
index 0e62660..6f1e65f 100644 (file)
@@ -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_;
 
index 7d543f9..e3bf25f 100644 (file)
@@ -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);
     }
index 70d2eb5..723dfa4 100644 (file)
@@ -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
index 77d5945..aa926db 100644 (file)
 
 #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
index 10e7b71..a87a8f9 100644 (file)
@@ -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
index 8eae3e9..8c7d02f 100644 (file)
@@ -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);
index f37d4d0..f52277b 100644 (file)
@@ -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);
index 578deb6..7ecf772 100644 (file)
@@ -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_;
 
index 78906c7..f26fb1d 100644 (file)
@@ -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
index 33f3c33..ff0acdb 100644 (file)
@@ -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
 
index d7c9280..b89f519 100644 (file)
@@ -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
index 464749e..23f59ed 100644 (file)
 #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));
 }
 
index 41c9288..9663e88 100644 (file)
@@ -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);
   }
 }
index e017997..72e4f73 100644 (file)
@@ -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();
index 2d6e2c7..c02e179 100644 (file)
@@ -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.
index e11d6e4..73305e1 100644 (file)
@@ -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(); }
 
index 6bb6212..198655b 100644 (file)
@@ -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;
       }
index d61079e..72c1924 100644 (file)
@@ -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;
 };
 
index 8d3d4d3..f418022 100644 (file)
@@ -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