Avoid generating duplicate names when merging types
authorSteven Perron <stevenperron@google.com>
Mon, 5 Mar 2018 14:57:41 +0000 (09:57 -0500)
committerSteven Perron <31666470+s-perron@users.noreply.github.com>
Mon, 5 Mar 2018 17:02:50 +0000 (12:02 -0500)
The merging types we do not remove other information related to the
types.  We simply leave it duplicated, and hope it is removed later.
This is what happens with decorations.  They are removed in the next
phase of remove duplicates.  However, for OpNames that is not the case.
We end up with two different names for the same id, which does not make
sense.

The solution is to remove the names and decorations for the type being
removed instead of rewriting them to refer to the other type.

Note that it is possible that if the first type does not have a name,
then the types will end up with no name.  That is fine because the names
should not have any semantic significance anyway.

The was identified in issue #1372, but this does not fix that issue.

source/opt/remove_duplicates_pass.cpp
test/opt/pass_remove_duplicates_test.cpp

index 262b6bb..0a54d76 100644 (file)
@@ -130,6 +130,7 @@ bool RemoveDuplicatesPass::RemoveDuplicateTypes(
       visited_types.emplace_back(i);
     } else {
       // The same type has already been seen before, remove this one.
+      ir_context->KillNamesAndDecorates(i->result_id());
       ir_context->ReplaceAllUsesWith(i->result_id(), id_to_keep);
       modified = true;
       to_delete.emplace_back(i);
index 4434dd0..d269daa 100644 (file)
@@ -219,6 +219,29 @@ OpDecorate %1 GLSLPacked
   EXPECT_THAT(GetErrorMessage(), "");
 }
 
+TEST_F(RemoveDuplicatesTest, SameTypeAndDifferentName) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpName %1 "Type1"
+OpName %2 "Type2"
+%3 = OpTypeInt 32 0
+%1 = OpTypeStruct %3 %3
+%2 = OpTypeStruct %3 %3
+)";
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpName %1 "Type1"
+%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"(