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 35cba98189e3a22f57e8d34b79aa02d3c797b18a..59ea36ce06a1625a6d018b2a03910d913a2e0424 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 1d229e082ec52d55dabaa25c0cd16a1ba428d1b7..650e635ed867fa45b4beda463c1eefa5a99dee4e 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 4e06efdf2bafdef343fdfb4dd263877f0094183d..aec67f73cac1e57546b3378cc2cbdce1009519e7 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 5c3d8725e97e3708c539af060c09a8c74dc2e395..d68ed71a6c27235e482df83422f3820b734f28c4 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 0e6266006a62892d189bad35ce2d2d4c1fb6d4eb..6f1e65fc9006beb5e6ac593dc92f2eb7c3cb530c 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 7d543f981e55be2f92d1a2b46880638c0011b627..e3bf25fd6d90ee8cc5ab7c800b23b558d55b89a6 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 70d2eb5b45c88d1d89d0f2902518cac485f97cb2..723dfa44544106b87f9ccbed904c3e27081f0e76 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 77d5945d9f3613cdc8cc1638f93406a720bd7f95..aa926dbf7f74920513e68cd8a52d8f7becb48f70 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 10e7b7190dc81dc565ddd2622279e412337416e2..a87a8f9d69558526572c3b782f700c8d0b281ccc 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 8eae3e9a6123b4fcc9413ef2c1c2013924d62822..8c7d02f4a43041f51f59a46693a29eb87501cdcf 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 f37d4d001a7956634a81ea1285543dba5071673a..f52277bd236985c96717185ba390846ba11113a3 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 578deb616d4167fdf0a5e4fbc8f8f292a1b84b45..7ecf772ff803bc3db98966134a063e9b76a0d854 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 78906c725fdbd66e3fdb1be7fecafafc76b5cbe3..f26fb1d3834de1a816660791d0e7517e7df93c5e 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 33f3c33f6dff1bac9dfbf4bb9791e66db349067b..ff0acdb0ebe6a8eef0880d1db556a524ea3cf542 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 d7c928008a360611b9d41243fad01e792ef90401..b89f51934f8fe179f27ec96cba762cf0de2a6ab7 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 464749ec296544768a61193b3e0f08aa94d9b597..23f59ed99b38126a8e40276cbbb3e9ed0a780d81 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 41c92885bcd0c848d2c6daffe10a7b37d88dadc5..9663e880f59d1b1b3de43633438997cffe62b566 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 e0179979b2a4bb60651e1bcc1be8e58873a7d863..72e4f73fce856fecb81a5a128012cbfda7bbeeb3 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 2d6e2c755e75d472bc798a0ca03ca49cf29b9f6b..c02e1793b91021d8152761f98d48a17463d62568 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 e11d6e43088d130583e05338ab177ab8036749df..73305e13598488df3aad532fbc4ca6a49f6cfab0 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 6bb621202b72d127dd05b6019e34bcb5f12f2242..198655b02e5b9f03b11b4fb5a0b72ac962b2a44d 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 d61079e90db1fba66291d1ba27c7904103904f61..72c192498d1175589fb158138e631699e3164a62 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 8d3d4d3aee4890a5f1f0fa467617af1def220f85..f41802221055687673eacfc0fc06d36da686e75c 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