Opt: Fix implementation and comment of AreDecorationsTheSame
authorPierre Moreau <dev@pmoreau.org>
Wed, 29 Nov 2017 23:29:34 +0000 (00:29 +0100)
committerAlan Baker <alanbaker@google.com>
Tue, 19 Dec 2017 20:36:47 +0000 (15:36 -0500)
Target should not be ignored when comparing decorations in RemoveDuplicates
Opt: Remove unused code in RemoveDuplicateDecorations

source/opt/decoration_manager.cpp
source/opt/decoration_manager.h
source/opt/remove_duplicates_pass.cpp
test/opt/CMakeLists.txt
test/opt/decoration_manager_test.cpp [new file with mode: 0644]
test/opt/pass_remove_duplicates_test.cpp [new file with mode: 0644]

index 072c1ea..761cdbc 100644 (file)
@@ -50,7 +50,7 @@ bool DecorationManager::HaveTheSameDecorations(uint32_t id1,
   for (const ir::Instruction* inst1 : decorationsFor1) {
     bool didFindAMatch = false;
     for (const ir::Instruction* inst2 : decorationsFor2) {
-      if (AreDecorationsTheSame(inst1, inst2)) {
+      if (AreDecorationsTheSame(inst1, inst2, true)) {
         didFindAMatch = true;
         break;
       }
@@ -60,39 +60,27 @@ bool DecorationManager::HaveTheSameDecorations(uint32_t id1,
   return true;
 }
 
-// TODO(pierremoreau): Handle SpvOpDecorateId by converting them to a regular
-//                     SpvOpDecorate.
-bool DecorationManager::AreDecorationsTheSame(
-    const ir::Instruction* inst1, const ir::Instruction* inst2) const {
-  //  const auto decorateIdToDecorate = [&constants](const Instruction& inst) {
-  //    std::vector<Operand> operands;
-  //    operands.reserve(inst.NumInOperands());
-  //    for (uint32_t i = 2u; i < inst.NumInOperands(); ++i) {
-  //      const auto& j = constants.find(inst.GetSingleWordInOperand(i));
-  //      if (j == constants.end())
-  //        return Instruction(inst.context());
-  //      const auto operand = j->second->GetOperand(0u);
-  //      operands.emplace_back(operand.type, operand.words);
-  //    }
-  //    return Instruction(inst.context(), SpvOpDecorate, 0u, 0u, operands);
-  //  };
-  //  Instruction tmpA = (deco1.opcode() == SpvOpDecorateId) ?
-  //  decorateIdToDecorate(deco1) : deco1;
-  //  Instruction tmpB = (deco2.opcode() == SpvOpDecorateId) ?
-  //  decorateIdToDecorate(deco2) : deco2;
-  //
-  if (inst1->opcode() == SpvOpDecorateId || inst2->opcode() == SpvOpDecorateId)
-    return false;
+// TODO(pierremoreau): If OpDecorateId is referencing an OpConstant, one could
+//                     check that the constants are the same rather than just
+//                     looking at the constant ID.
+bool DecorationManager::AreDecorationsTheSame(const ir::Instruction* inst1,
+                                              const ir::Instruction* inst2,
+                                              bool ignore_target) const {
+  switch (inst1->opcode()) {
+    case SpvOpDecorate:
+    case SpvOpMemberDecorate:
+    case SpvOpDecorateId:
+      break;
+    default:
+      return false;
+  }
 
-  ir::Instruction tmpA = *inst1, tmpB = *inst2;
-  if (tmpA.opcode() != tmpB.opcode() ||
-      tmpA.NumInOperands() != tmpB.NumInOperands() ||
-      tmpA.opcode() == SpvOpNop || tmpB.opcode() == SpvOpNop)
+  if (inst1->opcode() != inst2->opcode() ||
+      inst1->NumInOperands() != inst2->NumInOperands())
     return false;
 
-  for (uint32_t i = (tmpA.opcode() == SpvOpDecorate) ? 1u : 2u;
-       i < tmpA.NumInOperands(); ++i)
-    if (tmpA.GetInOperand(i) != tmpB.GetInOperand(i)) return false;
+  for (uint32_t i = ignore_target ? 1u : 0u; i < inst1->NumInOperands(); ++i)
+    if (inst1->GetInOperand(i) != inst2->GetInOperand(i)) return false;
 
   return true;
 }
index fca78d3..d0c3da8 100644 (file)
@@ -53,10 +53,15 @@ class DecorationManager {
   // instructions that apply the same decorations but to different IDs, still
   // count as being the same.
   bool HaveTheSameDecorations(uint32_t id1, uint32_t id2) const;
-  // Returns whether two decorations are the same. SpvOpDecorateId is currently
-  // not handled and will return false no matter what.
+  // Returns whether the two decorations instructions are the same and are
+  // applying the same decorations; unless |ignore_target| is false, the targets
+  // to which they are applied to does not matter, except for the member part.
+  //
+  // This is only valid for OpDecorate, OpMemberDecorate and OpDecorateId; it
+  // will return false for other opcodes.
   bool AreDecorationsTheSame(const ir::Instruction* inst1,
-                             const ir::Instruction* inst2) const;
+                             const ir::Instruction* inst2,
+                             bool ignore_target) const;
 
   // |f| is run on each decoration instruction for |id| with decoration
   // |decoration|. Processed are all decorations which target |id| either
index 6dc84ea..24f0e37 100644 (file)
@@ -138,16 +138,19 @@ bool RemoveDuplicatesPass::RemoveDuplicateTypes(
   return modified;
 }
 
+// TODO(pierremoreau): Duplicate decoration groups should be removed. For
+// example, in
+//     OpDecorate %1 Constant
+//     %1 = OpDecorationGroup
+//     OpDecorate %2 Constant
+//     %2 = OpDecorationGroup
+//     OpGroupDecorate %1 %3
+//     OpGroupDecorate %2 %4
+// group %2 could be removed.
 bool RemoveDuplicatesPass::RemoveDuplicateDecorations(
     ir::IRContext* irContext) const {
   bool modified = false;
 
-  std::unordered_map<SpvId, const Instruction*> constants;
-  for (const auto& i : irContext->types_values())
-    if (i.opcode() == SpvOpConstant) constants[i.result_id()] = &i;
-  for (const auto& i : irContext->types_values())
-    if (i.opcode() == SpvOpConstant) constants[i.result_id()] = &i;
-
   std::vector<const Instruction*> visitedDecorations;
 
   opt::analysis::DecorationManager decorationManager(irContext->module());
@@ -156,7 +159,7 @@ bool RemoveDuplicatesPass::RemoveDuplicateDecorations(
     // visited?
     bool alreadyVisited = false;
     for (const Instruction* j : visitedDecorations) {
-      if (decorationManager.AreDecorationsTheSame(&*i, j)) {
+      if (decorationManager.AreDecorationsTheSame(&*i, j, false)) {
         alreadyVisited = true;
         break;
       }
index 73a6374..eaf8993 100644 (file)
@@ -246,3 +246,13 @@ add_spvtools_unittest(TARGET private_to_local
   LIBS SPIRV-Tools-opt
 )
 
+add_spvtools_unittest(TARGET decoration_manager
+  SRCS decoration_manager_test.cpp
+  LIBS SPIRV-Tools-opt
+)
+
+add_spvtools_unittest(TARGET pass_remove_duplicates
+  SRCS pass_remove_duplicates_test.cpp
+  LIBS SPIRV-Tools-opt
+)
+
diff --git a/test/opt/decoration_manager_test.cpp b/test/opt/decoration_manager_test.cpp
new file mode 100644 (file)
index 0000000..7e5e4f2
--- /dev/null
@@ -0,0 +1,304 @@
+// Copyright (c) 2017 Pierre Moreau
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <iostream>
+
+#include <gmock/gmock.h>
+
+#include "source/opt/build_module.h"
+#include "source/opt/decoration_manager.h"
+#include "source/opt/ir_context.h"
+#include "source/spirv_constant.h"
+#include "unit_spirv.h"
+
+namespace {
+
+using spvtools::ir::IRContext;
+using spvtools::ir::Instruction;
+using spvtools::opt::analysis::DecorationManager;
+
+class DecorationManagerTest : public ::testing::Test {
+ public:
+  DecorationManagerTest()
+      : tools_(SPV_ENV_UNIVERSAL_1_2),
+        context_(),
+        consumer_([this](spv_message_level_t level, const char*,
+                         const spv_position_t& position, const char* message) {
+          if (!error_message_.empty()) error_message_ += "\n";
+          switch (level) {
+            case SPV_MSG_FATAL:
+            case SPV_MSG_INTERNAL_ERROR:
+            case SPV_MSG_ERROR:
+              error_message_ += "ERROR";
+              break;
+            case SPV_MSG_WARNING:
+              error_message_ += "WARNING";
+              break;
+            case SPV_MSG_INFO:
+              error_message_ += "INFO";
+              break;
+            case SPV_MSG_DEBUG:
+              error_message_ += "DEBUG";
+              break;
+          }
+          error_message_ +=
+              ": " + std::to_string(position.index) + ": " + message;
+        }),
+        disassemble_options_(spvtools::SpirvTools::kDefaultDisassembleOption),
+        error_message_() {
+    tools_.SetMessageConsumer(consumer_);
+  }
+
+  virtual void TearDown() override { error_message_.clear(); }
+
+  DecorationManager* GetDecorationManager(const std::string& text) {
+    context_ = spvtools::BuildModule(SPV_ENV_UNIVERSAL_1_2, consumer_, text);
+    if (context_.get())
+      return context_->get_decoration_mgr();
+    else
+      return nullptr;
+  }
+
+  // Disassembles |binary| and outputs the result in |text|. If |text| is a
+  // null pointer, SPV_ERROR_INVALID_POINTER is returned.
+  spv_result_t Disassemble(const std::vector<uint32_t>& binary,
+                           std::string* text) {
+    if (!text) return SPV_ERROR_INVALID_POINTER;
+    return tools_.Disassemble(binary, text, disassemble_options_)
+               ? SPV_SUCCESS
+               : SPV_ERROR_INVALID_BINARY;
+  }
+
+  // Returns the accumulated error messages for the test.
+  std::string GetErrorMessage() const { return error_message_; }
+
+  std::string ToText(const std::vector<Instruction*>& inst) {
+    std::vector<uint32_t> binary = {SpvMagicNumber, 0x10200, 0u, 2u, 0u};
+    for (const Instruction* i : inst)
+      i->ToBinaryWithoutAttachedDebugInsts(&binary);
+    std::string text;
+    Disassemble(binary, &text);
+    return text;
+  }
+
+  std::string ModuleToText() {
+    std::vector<uint32_t> binary;
+    context_->module()->ToBinary(&binary, false);
+    std::string text;
+    Disassemble(binary, &text);
+    return text;
+  }
+
+  spvtools::MessageConsumer GetConsumer() { return consumer_; }
+
+ private:
+  spvtools::SpirvTools
+      tools_;  // An instance for calling SPIRV-Tools functionalities.
+  std::unique_ptr<IRContext> context_;
+  spvtools::MessageConsumer consumer_;
+  uint32_t disassemble_options_;
+  std::string error_message_;
+};
+
+TEST_F(DecorationManagerTest, ComparingDecorationsWithDiffOpcodes) {
+  spvtools::ir::IRContext ir_context(GetConsumer());
+  // OpDecorate %1 Constant
+  Instruction inst1(&ir_context, SpvOpDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  // OpDecorateId %1 %2
+  Instruction inst2(&ir_context, SpvOpDecorateId, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}}, {SPV_OPERAND_TYPE_ID, {2u}}});
+  DecorationManager* decoManager = ir_context.get_decoration_mgr();
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_FALSE(decoManager->AreDecorationsTheSame(&inst1, &inst2, true));
+}
+
+TEST_F(DecorationManagerTest, ComparingDecorationsWithDiffDeco) {
+  spvtools::ir::IRContext ir_context(GetConsumer());
+  // OpDecorate %1 Constant
+  Instruction inst1(&ir_context, SpvOpDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  // OpDecorate %1 Restrict
+  Instruction inst2(&ir_context, SpvOpDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationRestrict}}});
+  DecorationManager* decoManager = ir_context.get_decoration_mgr();
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_FALSE(decoManager->AreDecorationsTheSame(&inst1, &inst2, true));
+}
+
+TEST_F(DecorationManagerTest, ComparingSameDecorationsOnDiffTargetAllowed) {
+  spvtools::ir::IRContext ir_context(GetConsumer());
+  // OpDecorate %1 Constant
+  Instruction inst1(&ir_context, SpvOpDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  // OpDecorate %2 Constant
+  Instruction inst2(&ir_context, SpvOpDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {2u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  DecorationManager* decoManager = ir_context.get_decoration_mgr();
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_TRUE(decoManager->AreDecorationsTheSame(&inst1, &inst2, true));
+}
+
+TEST_F(DecorationManagerTest, ComparingSameDecorationsOnDiffTargetDisallowed) {
+  spvtools::ir::IRContext ir_context(GetConsumer());
+  // OpDecorate %1 Constant
+  Instruction inst1(&ir_context, SpvOpDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  // OpDecorate %2 Constant
+  Instruction inst2(&ir_context, SpvOpDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {2u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  DecorationManager* decoManager = ir_context.get_decoration_mgr();
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_FALSE(decoManager->AreDecorationsTheSame(&inst1, &inst2, false));
+}
+
+TEST_F(DecorationManagerTest, ComparingMemberDecorationsOnSameTypeDiffMember) {
+  spvtools::ir::IRContext ir_context(GetConsumer());
+  // OpMemberDecorate %1 0 Constant
+  Instruction inst1(&ir_context, SpvOpMemberDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_LITERAL_INTEGER, {0u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  // OpMemberDecorate %1 1 Constant
+  Instruction inst2(&ir_context, SpvOpMemberDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_LITERAL_INTEGER, {1u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  DecorationManager* decoManager = ir_context.get_decoration_mgr();
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_FALSE(decoManager->AreDecorationsTheSame(&inst1, &inst2, true));
+}
+
+TEST_F(DecorationManagerTest,
+       ComparingSameMemberDecorationsOnDiffTargetAllowed) {
+  spvtools::ir::IRContext ir_context(GetConsumer());
+  // OpMemberDecorate %1 0 Constant
+  Instruction inst1(&ir_context, SpvOpMemberDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_LITERAL_INTEGER, {0u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  // OpMemberDecorate %2 0 Constant
+  Instruction inst2(&ir_context, SpvOpMemberDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {2u}},
+                     {SPV_OPERAND_TYPE_LITERAL_INTEGER, {0u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  DecorationManager* decoManager = ir_context.get_decoration_mgr();
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_TRUE(decoManager->AreDecorationsTheSame(&inst1, &inst2, true));
+}
+
+TEST_F(DecorationManagerTest,
+       ComparingSameMemberDecorationsOnDiffTargetDisallowed) {
+  spvtools::ir::IRContext ir_context(GetConsumer());
+  // OpMemberDecorate %1 0 Constant
+  Instruction inst1(&ir_context, SpvOpMemberDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {1u}},
+                     {SPV_OPERAND_TYPE_LITERAL_INTEGER, {0u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  // OpMemberDecorate %2 0 Constant
+  Instruction inst2(&ir_context, SpvOpMemberDecorate, 0u, 0u,
+                    {{SPV_OPERAND_TYPE_ID, {2u}},
+                     {SPV_OPERAND_TYPE_LITERAL_INTEGER, {0u}},
+                     {SPV_OPERAND_TYPE_DECORATION, {SpvDecorationConstant}}});
+  DecorationManager* decoManager = ir_context.get_decoration_mgr();
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_FALSE(decoManager->AreDecorationsTheSame(&inst1, &inst2, false));
+}
+
+TEST_F(DecorationManagerTest, HaveTheSameDecorationsWithoutGroupsTrue) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Restrict
+OpDecorate %2 Constant
+OpDecorate %2 Restrict
+OpDecorate %1 Constant
+%u32    = OpTypeInt 32 0
+%1      = OpVariable %u32 Uniform
+%2      = OpVariable %u32 Uniform
+)";
+  DecorationManager* decoManager = GetDecorationManager(spirv);
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_TRUE(decoManager->HaveTheSameDecorations(1u, 2u));
+}
+
+TEST_F(DecorationManagerTest, HaveTheSameDecorationsWithoutGroupsFalse) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Restrict
+OpDecorate %2 Constant
+OpDecorate %2 Restrict
+%u32    = OpTypeInt 32 0
+%1      = OpVariable %u32 Uniform
+%2      = OpVariable %u32 Uniform
+)";
+  DecorationManager* decoManager = GetDecorationManager(spirv);
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_FALSE(decoManager->HaveTheSameDecorations(1u, 2u));
+}
+
+TEST_F(DecorationManagerTest, HaveTheSameDecorationsWithGroupsTrue) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Restrict
+OpDecorate %2 Constant
+OpDecorate %1 Constant
+OpDecorate %3 Restrict
+%3 = OpDecorationGroup
+OpGroupDecorate %3 %2
+OpDecorate %4 Invariant
+%4 = OpDecorationGroup
+OpGroupDecorate %4 %1 %2
+%u32    = OpTypeInt 32 0
+%1      = OpVariable %u32 Uniform
+%2      = OpVariable %u32 Uniform
+)";
+  DecorationManager* decoManager = GetDecorationManager(spirv);
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_TRUE(decoManager->HaveTheSameDecorations(1u, 2u));
+}
+
+TEST_F(DecorationManagerTest, HaveTheSameDecorationsWithGroupsFalse) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Restrict
+OpDecorate %2 Constant
+OpDecorate %1 Constant
+OpDecorate %4 Invariant
+%4 = OpDecorationGroup
+OpGroupDecorate %4 %1 %2
+%u32    = OpTypeInt 32 0
+%1      = OpVariable %u32 Uniform
+%2      = OpVariable %u32 Uniform
+)";
+  DecorationManager* decoManager = GetDecorationManager(spirv);
+  EXPECT_THAT(GetErrorMessage(), "");
+  EXPECT_FALSE(decoManager->HaveTheSameDecorations(1u, 2u));
+}
+
+}  // namespace
diff --git a/test/opt/pass_remove_duplicates_test.cpp b/test/opt/pass_remove_duplicates_test.cpp
new file mode 100644 (file)
index 0000000..4434dd0
--- /dev/null
@@ -0,0 +1,287 @@
+// Copyright (c) 2017 Pierre Moreau
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <iostream>
+
+#include <gmock/gmock.h>
+
+#include "source/opt/build_module.h"
+#include "source/opt/ir_context.h"
+#include "source/opt/pass_manager.h"
+#include "source/opt/remove_duplicates_pass.h"
+#include "source/spirv_constant.h"
+#include "unit_spirv.h"
+
+namespace {
+
+using spvtools::ir::IRContext;
+using spvtools::ir::Instruction;
+using spvtools::opt::PassManager;
+using spvtools::opt::RemoveDuplicatesPass;
+
+class RemoveDuplicatesTest : public ::testing::Test {
+ public:
+  RemoveDuplicatesTest()
+      : tools_(SPV_ENV_UNIVERSAL_1_2),
+        context_(),
+        consumer_([this](spv_message_level_t level, const char*,
+                         const spv_position_t& position, const char* message) {
+          if (!error_message_.empty()) error_message_ += "\n";
+          switch (level) {
+            case SPV_MSG_FATAL:
+            case SPV_MSG_INTERNAL_ERROR:
+            case SPV_MSG_ERROR:
+              error_message_ += "ERROR";
+              break;
+            case SPV_MSG_WARNING:
+              error_message_ += "WARNING";
+              break;
+            case SPV_MSG_INFO:
+              error_message_ += "INFO";
+              break;
+            case SPV_MSG_DEBUG:
+              error_message_ += "DEBUG";
+              break;
+          }
+          error_message_ +=
+              ": " + std::to_string(position.index) + ": " + message;
+        }),
+        disassemble_options_(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER),
+        error_message_() {
+    tools_.SetMessageConsumer(consumer_);
+  }
+
+  virtual void TearDown() override { error_message_.clear(); }
+
+  std::string RunPass(const std::string& text) {
+    context_ = spvtools::BuildModule(SPV_ENV_UNIVERSAL_1_2, consumer_, text);
+    if (!context_.get()) return std::string();
+
+    PassManager manager;
+    manager.SetMessageConsumer(consumer_);
+    manager.AddPass<RemoveDuplicatesPass>();
+
+    spvtools::opt::Pass::Status pass_res = manager.Run(context_.get());
+    if (pass_res == spvtools::opt::Pass::Status::Failure) return std::string();
+
+    return ModuleToText();
+  }
+
+  // Disassembles |binary| and outputs the result in |text|. If |text| is a
+  // null pointer, SPV_ERROR_INVALID_POINTER is returned.
+  spv_result_t Disassemble(const std::vector<uint32_t>& binary,
+                           std::string* text) {
+    if (!text) return SPV_ERROR_INVALID_POINTER;
+    return tools_.Disassemble(binary, text, disassemble_options_)
+               ? SPV_SUCCESS
+               : SPV_ERROR_INVALID_BINARY;
+  }
+
+  // Returns the accumulated error messages for the test.
+  std::string GetErrorMessage() const { return error_message_; }
+
+  std::string ToText(const std::vector<Instruction*>& inst) {
+    std::vector<uint32_t> binary = {SpvMagicNumber, 0x10200, 0u, 2u, 0u};
+    for (const Instruction* i : inst)
+      i->ToBinaryWithoutAttachedDebugInsts(&binary);
+    std::string text;
+    Disassemble(binary, &text);
+    return text;
+  }
+
+  std::string ModuleToText() {
+    std::vector<uint32_t> binary;
+    context_->module()->ToBinary(&binary, false);
+    std::string text;
+    Disassemble(binary, &text);
+    return text;
+  }
+
+ private:
+  spvtools::SpirvTools
+      tools_;  // An instance for calling SPIRV-Tools functionalities.
+  std::unique_ptr<IRContext> context_;
+  spvtools::MessageConsumer consumer_;
+  uint32_t disassemble_options_;
+  std::string error_message_;
+};
+
+TEST_F(RemoveDuplicatesTest, DuplicateCapabilities) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+)";
+
+  EXPECT_THAT(RunPass(spirv), after);
+  EXPECT_THAT(GetErrorMessage(), "");
+}
+
+TEST_F(RemoveDuplicatesTest, DuplicateExtInstImports) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+%1 = OpExtInstImport "OpenCL.std"
+%2 = OpExtInstImport "OpenCL.std"
+%3 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+%1 = OpExtInstImport "OpenCL.std"
+%3 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+)";
+
+  EXPECT_THAT(RunPass(spirv), after);
+  EXPECT_THAT(GetErrorMessage(), "");
+}
+
+TEST_F(RemoveDuplicatesTest, DuplicateTypes) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpTypeInt 32 0
+%2 = OpTypeInt 32 0
+%3 = OpTypeStruct %1 %2
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpTypeInt 32 0
+%3 = OpTypeStruct %1 %1
+)";
+
+  EXPECT_THAT(RunPass(spirv), after);
+  EXPECT_THAT(GetErrorMessage(), "");
+}
+
+TEST_F(RemoveDuplicatesTest, SameTypeDifferentMemberDecoration) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 GLSLPacked
+%2 = OpTypeInt 32 0
+%1 = OpTypeStruct %2 %2
+%3 = OpTypeStruct %2 %2
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 GLSLPacked
+%2 = OpTypeInt 32 0
+%1 = OpTypeStruct %2 %2
+%3 = OpTypeStruct %2 %2
+)";
+
+  EXPECT_THAT(RunPass(spirv), after);
+  EXPECT_THAT(GetErrorMessage(), "");
+}
+
+TEST_F(RemoveDuplicatesTest, SameTypeAndMemberDecoration) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 GLSLPacked
+OpDecorate %2 GLSLPacked
+%3 = OpTypeInt 32 0
+%1 = OpTypeStruct %3 %3
+%2 = OpTypeStruct %3 %3
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 GLSLPacked
+%3 = OpTypeInt 32 0
+%1 = OpTypeStruct %3 %3
+)";
+
+  EXPECT_THAT(RunPass(spirv), after);
+  EXPECT_THAT(GetErrorMessage(), "");
+}
+
+// Check that #1033 has been fixed.
+TEST_F(RemoveDuplicatesTest, DoNotRemoveDifferentOpDecorationGroup) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Constant
+%1 = OpDecorationGroup
+OpDecorate %2 Restrict
+%2 = OpDecorationGroup
+OpGroupDecorate %3 %1 %2
+%4 = OpTypeInt 32 0
+%3 = OpVariable %4 Uniform
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Constant
+%1 = OpDecorationGroup
+OpDecorate %2 Restrict
+%2 = OpDecorationGroup
+OpGroupDecorate %3 %1 %2
+%4 = OpTypeInt 32 0
+%3 = OpVariable %4 Uniform
+)";
+
+  EXPECT_THAT(RunPass(spirv), after);
+  EXPECT_THAT(GetErrorMessage(), "");
+}
+
+TEST_F(RemoveDuplicatesTest, DifferentDecorationGroup) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Constant
+OpDecorate %1 Restrict
+%1 = OpDecorationGroup
+OpDecorate %2 Constant
+%2 = OpDecorationGroup
+OpGroupDecorate %1 %3
+OpGroupDecorate %2 %4
+%5 = OpTypeInt 32 0
+%3 = OpVariable %5 Uniform
+%4 = OpVariable %5 Uniform
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpDecorate %1 Constant
+OpDecorate %1 Restrict
+%1 = OpDecorationGroup
+OpDecorate %2 Constant
+%2 = OpDecorationGroup
+OpGroupDecorate %1 %3
+OpGroupDecorate %2 %4
+%5 = OpTypeInt 32 0
+%3 = OpVariable %5 Uniform
+%4 = OpVariable %5 Uniform
+)";
+
+  EXPECT_THAT(RunPass(spirv), after);
+  EXPECT_THAT(GetErrorMessage(), "");
+}
+
+}  // namespace