Fixing bugs in type manager memory management
authorAlan Baker <alanbaker@google.com>
Mon, 18 Dec 2017 22:02:19 +0000 (17:02 -0500)
committerAlan Baker <alanbaker@google.com>
Thu, 21 Dec 2017 13:59:06 +0000 (08:59 -0500)
* changed the way duplicate types are removed to stop copying
instructions
* Reworked RemoveDuplicatesPass::AreTypesSame to use type manager and
type equality
* Reworked TypeManager memory management to store a pool of unique
pointers of types
 * removed unique pointers from id map
 * fixed instances where free'd memory could be accessed

source/link/linker.cpp
source/opt/remove_duplicates_pass.cpp
source/opt/remove_duplicates_pass.h
source/opt/type_manager.cpp
source/opt/type_manager.h
test/opt/CMakeLists.txt
test/opt/type_manager_test.cpp
tools/opt/opt.cpp

index b822580..9ea9f96 100644 (file)
@@ -119,8 +119,7 @@ static spv_result_t GetImportExportPairs(
 // checked.
 static spv_result_t CheckImportExportCompatibility(
     const MessageConsumer& consumer, const LinkageTable& linkings_to_do,
-    const DefUseManager& def_use_manager,
-    const DecorationManager& decoration_manager);
+    ir::IRContext* context);
 
 // Remove linkage specific instructions, such as prototypes of imported
 // functions, declarations of imported variables, import (and export if
@@ -247,9 +246,8 @@ spv_result_t Linker::Link(const uint32_t* const* binaries,
   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(),
-                                       *linked_context.get_decoration_mgr());
+  res =
+      CheckImportExportCompatibility(consumer, linkings_to_do, &linked_context);
   if (res != SPV_SUCCESS) return res;
 
   // Phase 6: Remove duplicates
@@ -599,16 +597,17 @@ static spv_result_t GetImportExportPairs(
 
 static spv_result_t CheckImportExportCompatibility(
     const MessageConsumer& consumer, const LinkageTable& linkings_to_do,
-    const DefUseManager& def_use_manager,
-    const DecorationManager& decoration_manager) {
+    ir::IRContext* context) {
   spv_position_t position = {};
 
   // Ensure th import and export types are the same.
+  const DefUseManager& def_use_manager = *context->get_def_use_mgr();
+  const DecorationManager& decoration_manager = *context->get_decoration_mgr();
   for (const auto& linking_entry : linkings_to_do) {
     if (!RemoveDuplicatesPass::AreTypesEqual(
             *def_use_manager.GetDef(linking_entry.imported_symbol.type_id),
             *def_use_manager.GetDef(linking_entry.exported_symbol.type_id),
-            def_use_manager, decoration_manager))
+            context))
       return libspirv::DiagnosticStream(position, consumer,
                                         SPV_ERROR_INVALID_BINARY)
              << "Type mismatch between imported variable/function %"
index 24f0e37..8aa65e4 100644 (file)
@@ -25,6 +25,7 @@
 #include "decoration_manager.h"
 #include "ir_context.h"
 #include "opcode.h"
+#include "reflect.h"
 
 namespace spvtools {
 namespace opt {
@@ -104,37 +105,39 @@ bool RemoveDuplicatesPass::RemoveDuplicateTypes(
     return modified;
   }
 
-  std::vector<Instruction> visitedTypes;
-  for (auto* i = &*irContext->types_values_begin(); i;) {
+  std::vector<Instruction*> visitedTypes;
+  std::vector<Instruction*> toDelete;
+  for (auto* i = &*irContext->types_values_begin(); i; i = i->NextNode()) {
     // We only care about types.
     if (!spvOpcodeGeneratesType((i->opcode())) &&
         i->opcode() != SpvOpTypeForwardPointer) {
-      i = i->NextNode();
       continue;
     }
 
     // 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(),
-                        *irContext->get_decoration_mgr())) {
-        idToKeep = j.result_id();
+      if (AreTypesEqual(*i, *j, irContext)) {
+        idToKeep = j->result_id();
         break;
       }
     }
 
     if (idToKeep == 0u) {
       // This is a never seen before type, keep it around.
-      visitedTypes.emplace_back(*i);
-      i = i->NextNode();
+      visitedTypes.emplace_back(i);
     } else {
       // The same type has already been seen before, remove this one.
       irContext->ReplaceAllUsesWith(i->result_id(), idToKeep);
       modified = true;
-      i = irContext->KillInst(i);
+      toDelete.emplace_back(i);
     }
   }
 
+  for (auto i : toDelete) {
+    irContext->KillInst(i);
+  }
+
   return modified;
 }
 
@@ -181,102 +184,17 @@ bool RemoveDuplicatesPass::RemoveDuplicateDecorations(
 
 bool RemoveDuplicatesPass::AreTypesEqual(const Instruction& inst1,
                                          const Instruction& inst2,
-                                         const DefUseManager& defUseManager,
-                                         const DecorationManager& decManager) {
+                                         ir::IRContext* context) {
   if (inst1.opcode() != inst2.opcode()) return false;
-  if (!decManager.HaveTheSameDecorations(inst1.result_id(), inst2.result_id()))
-    return false;
-
-  switch (inst1.opcode()) {
-    case SpvOpTypeVoid:
-    case SpvOpTypeBool:
-    case SpvOpTypeSampler:
-    case SpvOpTypeEvent:
-    case SpvOpTypeDeviceEvent:
-    case SpvOpTypeReserveId:
-    case SpvOpTypeQueue:
-    case SpvOpTypePipeStorage:
-    case SpvOpTypeNamedBarrier:
-      return true;
-    case SpvOpTypeInt:
-      return inst1.GetSingleWordInOperand(0u) ==
-                 inst2.GetSingleWordInOperand(0u) &&
-             inst1.GetSingleWordInOperand(1u) ==
-                 inst2.GetSingleWordInOperand(1u);
-    case SpvOpTypeFloat:
-    case SpvOpTypePipe:
-    case SpvOpTypeForwardPointer:
-      return inst1.GetSingleWordInOperand(0u) ==
-             inst2.GetSingleWordInOperand(0u);
-    case SpvOpTypeVector:
-    case SpvOpTypeMatrix:
-      return AreTypesEqual(
-                 *defUseManager.GetDef(inst1.GetSingleWordInOperand(0u)),
-                 *defUseManager.GetDef(inst2.GetSingleWordInOperand(0u)),
-                 defUseManager, decManager) &&
-             inst1.GetSingleWordInOperand(1u) ==
-                 inst2.GetSingleWordInOperand(1u);
-    case SpvOpTypeImage:
-      return AreTypesEqual(
-                 *defUseManager.GetDef(inst1.GetSingleWordInOperand(0u)),
-                 *defUseManager.GetDef(inst2.GetSingleWordInOperand(0u)),
-                 defUseManager, decManager) &&
-             inst1.GetSingleWordInOperand(1u) ==
-                 inst2.GetSingleWordInOperand(1u) &&
-             inst1.GetSingleWordInOperand(2u) ==
-                 inst2.GetSingleWordInOperand(2u) &&
-             inst1.GetSingleWordInOperand(3u) ==
-                 inst2.GetSingleWordInOperand(3u) &&
-             inst1.GetSingleWordInOperand(4u) ==
-                 inst2.GetSingleWordInOperand(4u) &&
-             inst1.GetSingleWordInOperand(5u) ==
-                 inst2.GetSingleWordInOperand(5u) &&
-             inst1.GetSingleWordInOperand(6u) ==
-                 inst2.GetSingleWordInOperand(6u) &&
-             inst1.NumOperands() == inst2.NumOperands() &&
-             (inst1.NumInOperands() == 7u ||
-              inst1.GetSingleWordInOperand(7u) ==
-                  inst2.GetSingleWordInOperand(7u));
-    case SpvOpTypeSampledImage:
-    case SpvOpTypeRuntimeArray:
-      return AreTypesEqual(
-          *defUseManager.GetDef(inst1.GetSingleWordInOperand(0u)),
-          *defUseManager.GetDef(inst2.GetSingleWordInOperand(0u)),
-          defUseManager, decManager);
-    case SpvOpTypeArray:
-      return AreTypesEqual(
-                 *defUseManager.GetDef(inst1.GetSingleWordInOperand(0u)),
-                 *defUseManager.GetDef(inst2.GetSingleWordInOperand(0u)),
-                 defUseManager, decManager) &&
-             AreTypesEqual(
-                 *defUseManager.GetDef(inst1.GetSingleWordInOperand(1u)),
-                 *defUseManager.GetDef(inst2.GetSingleWordInOperand(1u)),
-                 defUseManager, decManager);
-    case SpvOpTypeStruct:
-    case SpvOpTypeFunction: {
-      bool res = inst1.NumInOperands() == inst2.NumInOperands();
-      for (uint32_t i = 0u; i < inst1.NumInOperands() && res; ++i)
-        res &= AreTypesEqual(
-            *defUseManager.GetDef(inst1.GetSingleWordInOperand(i)),
-            *defUseManager.GetDef(inst2.GetSingleWordInOperand(i)),
-            defUseManager, decManager);
-      return res;
-    }
-    case SpvOpTypeOpaque:
-      return std::strcmp(reinterpret_cast<const char*>(
-                             inst1.GetInOperand(0u).words.data()),
-                         reinterpret_cast<const char*>(
-                             inst2.GetInOperand(0u).words.data())) == 0;
-    case SpvOpTypePointer:
-      return inst1.GetSingleWordInOperand(0u) ==
-                 inst2.GetSingleWordInOperand(0u) &&
-             AreTypesEqual(
-                 *defUseManager.GetDef(inst1.GetSingleWordInOperand(1u)),
-                 *defUseManager.GetDef(inst2.GetSingleWordInOperand(1u)),
-                 defUseManager, decManager);
-    default:
-      return false;
-  }
+  if (!ir::IsTypeInst(inst1.opcode())) return false;
+
+  const analysis::Type* type1 =
+      context->get_type_mgr()->GetType(inst1.result_id());
+  const analysis::Type* type2 =
+      context->get_type_mgr()->GetType(inst2.result_id());
+  if (type1 && type2 && *type1 == *type2) return true;
+
+  return false;
 }
 
 }  // namespace opt
index 72c1924..a687937 100644 (file)
@@ -37,8 +37,7 @@ class RemoveDuplicatesPass : public Pass {
   // Returns whether two types are equal, and have the same decorations.
   static bool AreTypesEqual(const ir::Instruction& inst1,
                             const ir::Instruction& inst2,
-                            const analysis::DefUseManager& defUseManager,
-                            const analysis::DecorationManager& decoManager);
+                            ir::IRContext* context);
 
  private:
   bool RemoveDuplicateCapabilities(ir::IRContext* irContext) const;
index 02ffe2d..b0e6114 100644 (file)
@@ -40,7 +40,7 @@ TypeManager::TypeManager(const MessageConsumer& consumer,
 
 Type* TypeManager::GetType(uint32_t id) const {
   auto iter = id_to_type_.find(id);
-  if (iter != id_to_type_.end()) return (*iter).second.get();
+  if (iter != id_to_type_.end()) return (*iter).second;
   return nullptr;
 }
 
@@ -67,7 +67,6 @@ ForwardPointer* TypeManager::GetForwardPointer(uint32_t index) const {
 
 void TypeManager::AnalyzeTypes(const spvtools::ir::Module& module) {
   for (const auto* inst : module.GetTypes()) RecordIfTypeDefinition(*inst);
-  for (const auto& inst : module.annotations()) AttachIfTypeDecoration(inst);
 }
 
 void TypeManager::RemoveId(uint32_t id) {
@@ -76,22 +75,26 @@ void TypeManager::RemoveId(uint32_t id) {
 
   auto& type = iter->second;
   if (!type->IsUniqueType(true)) {
-    // Search for an equivalent type to re-map.
-    bool found = false;
-    for (auto& pair : id_to_type_) {
-      if (pair.first != id && *pair.second == *type) {
-        // Equivalent ambiguous type, re-map type.
-        type_to_id_.erase(type.get());
-        type_to_id_[pair.second.get()] = pair.first;
-        found = true;
-        break;
+    auto tIter = type_to_id_.find(type);
+    if (tIter != type_to_id_.end() && tIter->second == id) {
+      // |type| currently maps to |id|.
+      // Search for an equivalent type to re-map.
+      bool found = false;
+      for (auto& pair : id_to_type_) {
+        if (pair.first != id && *pair.second == *type) {
+          // Equivalent ambiguous type, re-map type.
+          type_to_id_.erase(type);
+          type_to_id_[pair.second] = pair.first;
+          found = true;
+          break;
+        }
       }
       // No equivalent ambiguous type, remove mapping.
-      if (!found) type_to_id_.erase(type.get());
+      if (!found) type_to_id_.erase(tIter);
     }
   } else {
     // Unique type, so just erase the entry.
-    type_to_id_.erase(type.get());
+    type_to_id_.erase(type);
   }
 
   // Erase the entry for |id|.
@@ -347,10 +350,15 @@ void TypeManager::CreateDecoration(uint32_t target,
 }
 
 void TypeManager::RegisterType(uint32_t id, const Type& type) {
-  auto& t = id_to_type_[id];
-  t.reset(type.Clone().release());
-  if (GetId(t.get()) == 0) {
-    type_to_id_[t.get()] = id;
+  // The comparison and hash on the type pool will avoid inserting the clone if
+  // an equivalent type already exists. The clone will be deleted when it goes
+  // out of scope at the end of the function in that case. Repeated insertions
+  // of the same Type will atmost keep one corresponding object in the type
+  // pool.
+  auto pair = type_pool_.insert(type.Clone());
+  id_to_type_[id] = pair.first->get();
+  if (GetId(pair.first->get()) == 0) {
+    type_to_id_[pair.first->get()] = id;
   }
 }
 
@@ -482,20 +490,23 @@ Type* TypeManager::RecordIfTypeDefinition(
   } else {
     SPIRV_ASSERT(consumer_, type != nullptr,
                  "type should not be nullptr at this point");
-    id_to_type_[id].reset(type);
-    type_to_id_[type] = id;
+    std::vector<ir::Instruction*> decorations =
+        context()->get_decoration_mgr()->GetDecorationsFor(id, true);
+    for (auto dec : decorations) {
+      AttachDecoration(*dec, type);
+    }
+    std::unique_ptr<Type> unique(type);
+    auto pair = type_pool_.insert(std::move(unique));
+    id_to_type_[id] = pair.first->get();
+    type_to_id_[pair.first->get()] = id;
   }
   return type;
 }
 
-void TypeManager::AttachIfTypeDecoration(const ir::Instruction& inst) {
+void TypeManager::AttachDecoration(const ir::Instruction& inst, Type* type) {
   const SpvOp opcode = inst.opcode();
   if (!ir::IsAnnotationInst(opcode)) return;
-  const uint32_t id = inst.GetSingleWordOperand(0);
-  // Do nothing if the id to be decorated is not for a known type.
-  if (!id_to_type_.count(id)) return;
 
-  Type* target_type = id_to_type_[id].get();
   switch (opcode) {
     case SpvOpDecorate: {
       const auto count = inst.NumOperands();
@@ -503,7 +514,7 @@ void TypeManager::AttachIfTypeDecoration(const ir::Instruction& inst) {
       for (uint32_t i = 1; i < count; ++i) {
         data.push_back(inst.GetSingleWordOperand(i));
       }
-      target_type->AddDecoration(std::move(data));
+      type->AddDecoration(std::move(data));
     } break;
     case SpvOpMemberDecorate: {
       const auto count = inst.NumOperands();
@@ -512,17 +523,12 @@ void TypeManager::AttachIfTypeDecoration(const ir::Instruction& inst) {
       for (uint32_t i = 2; i < count; ++i) {
         data.push_back(inst.GetSingleWordOperand(i));
       }
-      if (Struct* st = target_type->AsStruct()) {
+      if (Struct* st = type->AsStruct()) {
         st->AddMemberDecoration(index, std::move(data));
       } else {
         SPIRV_UNIMPLEMENTED(consumer_, "OpMemberDecorate non-struct type");
       }
     } break;
-    case SpvOpDecorationGroup:
-    case SpvOpGroupDecorate:
-    case SpvOpGroupMemberDecorate:
-      SPIRV_UNIMPLEMENTED(consumer_, "unhandled decoration");
-      break;
     default:
       SPIRV_UNREACHABLE(consumer_);
       break;
index dd0bca5..ded313e 100644 (file)
@@ -40,6 +40,12 @@ struct HashTypePointer {
     return type->HashValue();
   }
 };
+struct HashTypeUniquePointer {
+  size_t operator()(const std::unique_ptr<Type>& type) const {
+    assert(type);
+    return type->HashValue();
+  }
+};
 
 // Equality functor.
 //
@@ -52,11 +58,18 @@ struct CompareTypePointers {
     return lhs->IsSame(rhs);
   }
 };
+struct CompareTypeUniquePointers {
+  bool operator()(const std::unique_ptr<Type>& lhs,
+                  const std::unique_ptr<Type>& rhs) const {
+    assert(lhs && rhs);
+    return lhs->IsSame(rhs.get());
+  }
+};
 
 // A class for managing the SPIR-V type hierarchy.
 class TypeManager {
  public:
-  using IdToTypeMap = std::unordered_map<uint32_t, std::unique_ptr<Type>>;
+  using IdToTypeMap = std::unordered_map<uint32_t, Type*>;
 
   // Constructs a type manager from the given |module|. All internal messages
   // will be communicated to the outside via the given message |consumer|.
@@ -105,7 +118,7 @@ class TypeManager {
 
   // Registers |id| to |type|.
   //
-  // If GetId(|type|) already returns a non-zero id, the return value will be
+  // If GetId(|type|) already returns a non-zero id, that mapping will be
   // unchanged.
   void RegisterType(uint32_t id, const Type& type);
 
@@ -120,6 +133,9 @@ class TypeManager {
  private:
   using TypeToIdMap = std::unordered_map<const Type*, uint32_t, HashTypePointer,
                                          CompareTypePointers>;
+  using TypePool =
+      std::unordered_set<std::unique_ptr<Type>, HashTypeUniquePointer,
+                         CompareTypeUniquePointers>;
   using ForwardPointerVector = std::vector<std::unique_ptr<ForwardPointer>>;
 
   // Analyzes the types and decorations on types in the given |module|.
@@ -141,14 +157,16 @@ class TypeManager {
   // Creates and returns a type from the given SPIR-V |inst|. Returns nullptr if
   // the given instruction is not for defining a type.
   Type* RecordIfTypeDefinition(const spvtools::ir::Instruction& inst);
-  // Attaches the decoration encoded in |inst| to a type. Does nothing if the
-  // given instruction is not a decoration instruction or not decorating a type.
-  void AttachIfTypeDecoration(const spvtools::ir::Instruction& inst);
+  // Attaches the decoration encoded in |inst| to |type|. Does nothing if the
+  // given instruction is not a decoration instruction. Assumes the target is
+  // |type| (e.g. should be called in loop of |type|'s decorations).
+  void AttachDecoration(const spvtools::ir::Instruction& inst, Type* type);
 
   const MessageConsumer& consumer_;  // Message consumer.
   spvtools::ir::IRContext* context_;
   IdToTypeMap id_to_type_;  // Mapping from ids to their type representations.
   TypeToIdMap type_to_id_;  // Mapping from types to their defining ids.
+  TypePool type_pool_;      // Memory owner of type pointers.
   ForwardPointerVector forward_pointers_;  // All forward pointer declarations.
   // All unresolved forward pointer declarations.
   // Refers the contents in the above vector.
index eaf8993..bffb0bb 100644 (file)
@@ -20,9 +20,9 @@ add_spvtools_unittest(TARGET instruction
 )
 
 add_spvtools_unittest(TARGET instruction_list
-        SRCS instruction_list_test.cpp
-        LIBS SPIRV-Tools-opt
-        )
+  SRCS instruction_list_test.cpp
+  LIBS SPIRV-Tools-opt
+)
 
 add_spvtools_unittest(TARGET ir_loader
   SRCS ir_loader_test.cpp
@@ -106,9 +106,9 @@ add_spvtools_unittest(TARGET pass_dead_branch_elim
 )
 
 add_spvtools_unittest(TARGET pass_dead_variable_elim
-        SRCS dead_variable_elim_test.cpp pass_utils.cpp
-        LIBS SPIRV-Tools-opt
-        )
+  SRCS dead_variable_elim_test.cpp pass_utils.cpp
+  LIBS SPIRV-Tools-opt
+)
 
 add_spvtools_unittest(TARGET pass_aggressive_dce
   SRCS aggressive_dead_code_elim_test.cpp pass_utils.cpp
@@ -255,4 +255,3 @@ add_spvtools_unittest(TARGET pass_remove_duplicates
   SRCS pass_remove_duplicates_test.cpp
   LIBS SPIRV-Tools-opt
 )
-
index 544cc47..0bb11ba 100644 (file)
@@ -517,6 +517,33 @@ OpMemoryModel Logical GLSL450
   ASSERT_EQ(context->get_type_mgr()->GetId(&st), toStay);
 }
 
+TEST(TypeManager, RemoveIdDoesntUnmapOtherTypes) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+%1 = OpTypeInt 32 0
+%2 = OpTypeStruct %1
+%3 = OpTypeStruct %1
+  )";
+
+  std::unique_ptr<ir::IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  EXPECT_NE(context, nullptr);
+
+  Integer u32(32, false);
+  Struct st({&u32});
+
+  EXPECT_EQ(1u, context->get_type_mgr()->GetId(&u32));
+  uint32_t id = context->get_type_mgr()->GetId(&st);
+  ASSERT_NE(id, 0u);
+  uint32_t toRemove = id == 2u ? 3u : 2u;
+  uint32_t toStay = id == 2u ? 2u : 3u;
+  context->get_type_mgr()->RemoveId(toRemove);
+  ASSERT_EQ(context->get_type_mgr()->GetType(toRemove), nullptr);
+  ASSERT_EQ(context->get_type_mgr()->GetId(&st), toStay);
+}
+
 TEST(TypeManager, GetTypeAndPointerType) {
   const std::string text = R"(
 OpCapability Shader
@@ -552,6 +579,69 @@ OpMemoryModel Logical GLSL450
   ASSERT_TRUE(pair.second->IsSame(&stPtr));
 }
 
+TEST(TypeManager, DuplicateType) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+%1 = OpTypeInt 32 0
+%2 = OpTypeInt 32 0
+  )";
+
+  std::unique_ptr<ir::IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  EXPECT_NE(context, nullptr);
+
+  const Type* type1 = context->get_type_mgr()->GetType(1u);
+  const Type* type2 = context->get_type_mgr()->GetType(2u);
+  EXPECT_NE(type1, nullptr);
+  EXPECT_NE(type2, nullptr);
+  EXPECT_EQ(*type1, *type2);
+}
+
+TEST(TypeManager, MultipleStructs) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpDecorate %3 Constant
+%1 = OpTypeInt 32 0
+%2 = OpTypeStruct %1
+%3 = OpTypeStruct %1
+  )";
+
+  std::unique_ptr<ir::IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  EXPECT_NE(context, nullptr);
+
+  const Type* type1 = context->get_type_mgr()->GetType(2u);
+  const Type* type2 = context->get_type_mgr()->GetType(3u);
+  EXPECT_NE(type1, nullptr);
+  EXPECT_NE(type2, nullptr);
+  EXPECT_FALSE(type1->IsSame(type2));
+}
+
+TEST(TypeManager, RemovingIdAvoidsUseAfterFree) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+%1 = OpTypeInt 32 0
+%2 = OpTypeStruct %1
+  )";
+
+  std::unique_ptr<ir::IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  EXPECT_NE(context, nullptr);
+
+  Integer u32(32, false);
+  Struct st({&u32});
+  const Type* type = context->get_type_mgr()->GetType(2u);
+  EXPECT_NE(type, nullptr);
+  context->get_type_mgr()->RemoveId(1u);
+  EXPECT_TRUE(type->IsSame(&st));
+}
+
 #ifdef SPIRV_EFFCEE
 TEST(TypeManager, GetTypeInstructionInt) {
   const std::string text = R"(
index 1eab922..6b94a3c 100644 (file)
@@ -215,6 +215,9 @@ Options (in lexicographical order):
   --private-to-local
                Change the scope of private variables that are used in a single
                function to that function.
+  --remove-duplicates
+               Removes duplicate types, decorations, capabilities and extension
+               instructions.
   --redundancy-elimination
                Looks for instructions in the same function that compute the
                same value, and deletes the redundant ones.
@@ -427,6 +430,8 @@ OptStatus ParseFlags(int argc, const char** argv, Optimizer* optimizer,
         optimizer->RegisterPass(CreateRedundancyEliminationPass());
       } else if (0 == strcmp(cur_arg, "--private-to-local")) {
         optimizer->RegisterPass(CreatePrivateToLocalPass());
+      } else if (0 == strcmp(cur_arg, "--remove-duplicates")) {
+        optimizer->RegisterPass(CreateRemoveDuplicatesPass());
       } else if (0 == strcmp(cur_arg, "--relax-struct-store")) {
         options->relax_struct_store = true;
       } else if (0 == strcmp(cur_arg, "--skip-validation")) {