From e803fe67177cebbce014b69707243361a5d8aefe Mon Sep 17 00:00:00 2001
From: Chris Oattes <cjo20@users.noreply.github.com>
Date: Mon, 8 May 2023 17:14:42 +0100
Subject: [PATCH] Don't convert struct members to half (#5201)

---
 source/opt/convert_to_half_pass.cpp       |  8 +++++
 source/opt/convert_to_half_pass.h         |  1 +
 test/opt/convert_relaxed_to_half_test.cpp | 43 +++++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/source/opt/convert_to_half_pass.cpp b/source/opt/convert_to_half_pass.cpp
index 7a4c1f409..2c4a631e1 100644
--- a/source/opt/convert_to_half_pass.cpp
+++ b/source/opt/convert_to_half_pass.cpp
@@ -39,6 +39,13 @@ bool ConvertToHalfPass::IsFloat(Instruction* inst, uint32_t width) {
   return Pass::IsFloat(ty_id, width);
 }
 
+bool ConvertToHalfPass::IsStruct(Instruction* inst) {
+  uint32_t ty_id = inst->type_id();
+  if (ty_id == 0) return false;
+  Instruction* ty_inst = Pass::GetBaseType(ty_id);
+  return (ty_inst->opcode() == spv::Op::OpTypeStruct);
+}
+
 bool ConvertToHalfPass::IsDecoratedRelaxed(Instruction* inst) {
   uint32_t r_id = inst->result_id();
   for (auto r_inst : get_decoration_mgr()->GetDecorationsFor(r_id, false))
@@ -294,6 +301,7 @@ bool ConvertToHalfPass::CloseRelaxInst(Instruction* inst) {
   bool relax = true;
   inst->ForEachInId([&relax, this](uint32_t* idp) {
     Instruction* op_inst = get_def_use_mgr()->GetDef(*idp);
+    if (IsStruct(op_inst)) relax = false;
     if (!IsFloat(op_inst, 32)) return;
     if (!IsRelaxed(*idp)) relax = false;
   });
diff --git a/source/opt/convert_to_half_pass.h b/source/opt/convert_to_half_pass.h
index feabfba3e..24a478ffc 100644
--- a/source/opt/convert_to_half_pass.h
+++ b/source/opt/convert_to_half_pass.h
@@ -45,6 +45,7 @@ class ConvertToHalfPass : public Pass {
   // Return true if |inst| returns scalar, vector or matrix type with base
   // float and |width|
   bool IsFloat(Instruction* inst, uint32_t width);
+  bool IsStruct(Instruction* inst);
 
   // Return true if |inst| is decorated with RelaxedPrecision
   bool IsDecoratedRelaxed(Instruction* inst);
diff --git a/test/opt/convert_relaxed_to_half_test.cpp b/test/opt/convert_relaxed_to_half_test.cpp
index 6a06de84f..27330e109 100644
--- a/test/opt/convert_relaxed_to_half_test.cpp
+++ b/test/opt/convert_relaxed_to_half_test.cpp
@@ -1570,6 +1570,49 @@ TEST_F(ConvertToHalfTest, HandleNonRelaxedPhi) {
   EXPECT_EQ(Pass::Status::SuccessWithChange, std::get<1>(result));
 }
 
+TEST_F(ConvertToHalfTest, DoNotReplaceStructMember) {
+  // See https://github.com/KhronosGroup/SPIRV-Tools/issues/4814
+
+  // This test is a case with a non-relaxed phi with a relaxed operand.
+  // A convert must be inserted at the end of the block associated with
+  // the operand.
+  const std::string test =
+      R"(OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %PSMain "PSMain" %out_var_SV_TARGET %MyConstants
+OpExecutionMode %PSMain OriginUpperLeft
+OpSource HLSL 600
+OpName %type_ConstantBuffer_myStruct "type.ConstantBuffer.myStruct"
+OpMemberName %type_ConstantBuffer_myStruct 0 "f"
+OpName %MyConstants "MyConstants"
+OpName %out_var_SV_TARGET "out.var.SV_TARGET"
+OpName %PSMain "PSMain"
+OpDecorate %out_var_SV_TARGET Location 0
+OpDecorate %MyConstants DescriptorSet 1
+OpDecorate %MyConstants Binding 2
+OpMemberDecorate %type_ConstantBuffer_myStruct 0 Offset 0
+OpDecorate %type_ConstantBuffer_myStruct Block
+%float = OpTypeFloat 32
+%type_ConstantBuffer_myStruct = OpTypeStruct %float
+%_ptr_Uniform_type_ConstantBuffer_myStruct = OpTypePointer Uniform %type_ConstantBuffer_myStruct
+%_ptr_Output_float = OpTypePointer Output %float
+%void = OpTypeVoid
+%9 = OpTypeFunction %void
+%MyConstants = OpVariable %_ptr_Uniform_type_ConstantBuffer_myStruct Uniform
+%out_var_SV_TARGET = OpVariable %_ptr_Output_float Output
+%PSMain = OpFunction %void None %9
+%10 = OpLabel
+%11 = OpLoad %type_ConstantBuffer_myStruct %MyConstants
+%12 = OpCompositeExtract %float %11 0
+OpStore %out_var_SV_TARGET %12
+OpReturn
+OpFunctionEnd
+)";
+
+  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  SinglePassRunAndCheck<ConvertToHalfPass>(test, test, true);
+}
+
 }  // namespace
 }  // namespace opt
 }  // namespace spvtools