From 9ba50e34f26cac5e6baf19cef2b646c66f4db025 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 5 Mar 2018 09:57:41 -0500 Subject: [PATCH] Avoid generating duplicate names when merging types 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 | 1 + test/opt/pass_remove_duplicates_test.cpp | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/source/opt/remove_duplicates_pass.cpp b/source/opt/remove_duplicates_pass.cpp index 262b6bb61d..0a54d76ea9 100644 --- a/source/opt/remove_duplicates_pass.cpp +++ b/source/opt/remove_duplicates_pass.cpp @@ -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); diff --git a/test/opt/pass_remove_duplicates_test.cpp b/test/opt/pass_remove_duplicates_test.cpp index 4434dd0e36..d269daa41f 100644 --- a/test/opt/pass_remove_duplicates_test.cpp +++ b/test/opt/pass_remove_duplicates_test.cpp @@ -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"(