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.

source/opt/eliminate_dead_members_pass.cpp

@@ -39,10 +39,18 @@ void EliminateDeadMembersPass::FindLiveMembers() {
   // 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::MarkTypeAsFullyUsed(uint32_t type_id) {
   }
 }
 
+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 @@ void EliminateDeadMembersPass::MarkStructOperandsAsFullyUsed(
     }
   });
 }
-
 }  // namespace opt
 }  // namespace spvtools

source/opt/eliminate_dead_members_pass.h

@@ -137,6 +137,7 @@ class EliminateDeadMembersPass : public MemPass {
   // 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

test/opt/eliminate_dead_member_test.cpp

@@ -1007,14 +1007,14 @@ TEST_F(EliminateDeadMemberTest, DontRemoveModfStructResultTypeMembers) {
                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 @@ TEST_F(EliminateDeadMemberTest, DontRemoveModfStructResultTypeMembers) {
   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