Don't change type of input and output var in dead member elim (#2412)
The types of input and output variables must match for the pipeline. We
cannot see the uses in all of the shader, so dead member
elimination cannot safely change the type of input and output variables.
diff --git a/source/opt/eliminate_dead_members_pass.cpp b/source/opt/eliminate_dead_members_pass.cpp
index d9cfb5c..0b73b2d 100644
--- a/source/opt/eliminate_dead_members_pass.cpp
+++ b/source/opt/eliminate_dead_members_pass.cpp
@@ -39,10 +39,18 @@
// Until we have implemented the rewritting of OpSpecConsantOp instructions,
// we have to mark them as fully used just to be safe.
for (auto& inst : get_module()->types_values()) {
- if (inst.opcode() != SpvOpSpecConstantOp) {
- continue;
+ if (inst.opcode() == SpvOpSpecConstantOp) {
+ MarkTypeAsFullyUsed(inst.type_id());
+ } else if (inst.opcode() == SpvOpVariable) {
+ switch (inst.GetSingleWordInOperand(0)) {
+ case SpvStorageClassInput:
+ case SpvStorageClassOutput:
+ MarkPointeeTypeAsFullUsed(inst.type_id());
+ break;
+ default:
+ break;
+ }
}
- MarkTypeAsFullyUsed(inst.type_id());
}
for (const Function& func : *get_module()) {
@@ -127,6 +135,12 @@
}
}
+void EliminateDeadMembersPass::MarkPointeeTypeAsFullUsed(uint32_t ptr_type_id) {
+ Instruction* ptr_type_inst = get_def_use_mgr()->GetDef(ptr_type_id);
+ assert(ptr_type_inst->opcode() == SpvOpTypePointer);
+ MarkTypeAsFullyUsed(ptr_type_inst->GetSingleWordInOperand(1));
+}
+
void EliminateDeadMembersPass::MarkMembersAsLiveForCopyMemory(
const Instruction* inst) {
uint32_t target_id = inst->GetSingleWordInOperand(0);
@@ -619,6 +633,5 @@
}
});
}
-
} // namespace opt
} // namespace spvtools
diff --git a/source/opt/eliminate_dead_members_pass.h b/source/opt/eliminate_dead_members_pass.h
index 8729f51..4feaa55 100644
--- a/source/opt/eliminate_dead_members_pass.h
+++ b/source/opt/eliminate_dead_members_pass.h
@@ -137,6 +137,7 @@
// type that are used, and must be kept.
std::unordered_map<uint32_t, std::set<uint32_t>> used_members_;
void MarkStructOperandsAsFullyUsed(const Instruction* inst);
+ void MarkPointeeTypeAsFullUsed(uint32_t ptr_type_id);
};
} // namespace opt
diff --git a/test/opt/eliminate_dead_member_test.cpp b/test/opt/eliminate_dead_member_test.cpp
index 87c79d7..7d5db7d 100644
--- a/test/opt/eliminate_dead_member_test.cpp
+++ b/test/opt/eliminate_dead_member_test.cpp
@@ -1007,14 +1007,14 @@
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
- OpEntryPoint Fragment %PS_TerrainElevation "PS_TerrainElevation"
- OpExecutionMode %PS_TerrainElevation OriginUpperLeft
+ OpEntryPoint Fragment %main "main"
+ OpExecutionMode %main OriginUpperLeft
OpSource HLSL 600
%float = OpTypeFloat 32
%void = OpTypeVoid
%21 = OpTypeFunction %void
%ModfStructType = OpTypeStruct %float %float
-%PS_TerrainElevation = OpFunction %void None %21
+%main = OpFunction %void None %21
%22 = OpLabel
%23 = OpUndef %float
%24 = OpExtInst %ModfStructType %1 ModfStruct %23
@@ -1028,4 +1028,60 @@
EXPECT_EQ(opt::Pass::Status::SuccessWithoutChange, std::get<1>(result));
}
+TEST_F(EliminateDeadMemberTest, DontChangeInputStructs) {
+ // The input for a shader has to match the type of the output from the
+ // previous shader in the pipeline. Because of that, we cannot change the
+ // types of input variables.
+ const std::string text = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %main "main" %input_var
+ OpExecutionMode %main OriginUpperLeft
+ OpSource HLSL 600
+ %float = OpTypeFloat 32
+ %void = OpTypeVoid
+ %21 = OpTypeFunction %void
+%in_var_type = OpTypeStruct %float %float
+%in_ptr_type = OpTypePointer Input %in_var_type
+%input_var = OpVariable %in_ptr_type Input
+%main = OpFunction %void None %21
+ %22 = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ auto result = SinglePassRunAndDisassemble<opt::EliminateDeadMembersPass>(
+ text, /* skip_nop = */ true, /* do_validation = */ true);
+ EXPECT_EQ(opt::Pass::Status::SuccessWithoutChange, std::get<1>(result));
+}
+
+TEST_F(EliminateDeadMemberTest, DontChangeOutputStructs) {
+ // The output for a shader has to match the type of the output from the
+ // previous shader in the pipeline. Because of that, we cannot change the
+ // types of output variables.
+ const std::string text = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %main "main" %output_var
+ OpExecutionMode %main OriginUpperLeft
+ OpSource HLSL 600
+ %float = OpTypeFloat 32
+ %void = OpTypeVoid
+ %21 = OpTypeFunction %void
+%out_var_type = OpTypeStruct %float %float
+%out_ptr_type = OpTypePointer Output %out_var_type
+%output_var = OpVariable %out_ptr_type Output
+%main = OpFunction %void None %21
+ %22 = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ auto result = SinglePassRunAndDisassemble<opt::EliminateDeadMembersPass>(
+ text, /* skip_nop = */ true, /* do_validation = */ true);
+ EXPECT_EQ(opt::Pass::Status::SuccessWithoutChange, std::get<1>(result));
+}
+
} // namespace
