Merge pull request #2163 from KhronosGroup/fix-2162

GLSL: Fix bug with mixed precision on PHI variables.

reference/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag.vk

@@ -0,0 +1,29 @@
+#version 450
+
+layout(location = 0) in mediump float b;
+layout(location = 0) out float FragColor;
+
+void main()
+{
+    float hp_copy_7;
+    mediump float _7;
+    int _22;
+    _22 = 0;
+    _7 = 0.0;
+    for (;;)
+    {
+        hp_copy_7 = _7;
+        int _23 = _22 + 1;
+        if (_23 < 4)
+        {
+            _22 = _23;
+            _7 = fma(_7, b, b);
+        }
+        else
+        {
+            break;
+        }
+    }
+    FragColor = hp_copy_7 * 4.0;
+}
+

reference/shaders-no-opt/asm/frag/relaxed-precision-inheritance-rules.asm.frag

@@ -16,6 +16,8 @@ layout(location = 0) in vec4 V4;
 
 void main()
 {
+    highp float hp_copy_phi_mp;
+    float mp_copy_phi_hp;
     vec4 V4_value0 = V4;
     highp vec4 hp_copy_V4_value0 = V4_value0;
     float V1_value0 = V4.x;
@@ -46,8 +48,8 @@ void main()
     highp float phi_hp;
     phi_mp = _21;
     phi_hp = _49;
-    highp float hp_copy_phi_mp = phi_mp;
-    float mp_copy_phi_hp = phi_hp;
+    hp_copy_phi_mp = phi_mp;
+    mp_copy_phi_hp = phi_hp;
     FragColor2 = vec4(phi_mp + phi_mp, hp_copy_phi_mp + hp_copy_phi_mp, mp_copy_phi_hp + mp_copy_phi_hp, phi_hp + phi_hp);
 }
 

shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag

@@ -0,0 +1,51 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 11
+; Bound: 41
+; Schema: 0
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %main "main" %b %FragColor
+               OpExecutionMode %main OriginUpperLeft
+               OpSource GLSL 450
+               OpName %main "main"
+               OpName %b "b"
+               OpName %FragColor "FragColor"
+               OpDecorate %b RelaxedPrecision
+               OpDecorate %b Location 0
+               OpDecorate %21 RelaxedPrecision
+               OpDecorate %24 RelaxedPrecision
+               OpDecorate %FragColor Location 0
+               OpDecorate %39 RelaxedPrecision
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+    %float_0 = OpConstant %float 0
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+%_ptr_Input_float = OpTypePointer Input %float
+          %b = OpVariable %_ptr_Input_float Input
+      %int_1 = OpConstant %int 1
+      %int_4 = OpConstant %int 4
+       %bool = OpTypeBool
+    %float_4 = OpConstant %float 4
+%_ptr_Output_float = OpTypePointer Output %float
+  %FragColor = OpVariable %_ptr_Output_float Output
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+               OpBranch %14
+         %14 = OpLabel
+         %40 = OpPhi %int %int_0 %5 %27 %14
+         %39 = OpPhi %float %float_0 %5 %24 %14
+         %21 = OpLoad %float %b
+         %24 = OpExtInst %float %1 Fma %39 %21 %21
+         %27 = OpIAdd %int %40 %int_1
+         %31 = OpSLessThan %bool %27 %int_4
+               OpLoopMerge %16 %14 None
+               OpBranchConditional %31 %14 %16
+         %16 = OpLabel
+         %38 = OpFMul %float %39 %float_4
+               OpStore %FragColor %38
+               OpReturn
+               OpFunctionEnd

spirv_glsl.cpp

@@ -11282,8 +11282,15 @@ void CompilerGLSL::emit_block_instructions(SPIRBlock &block)
 	if (backend.requires_relaxed_precision_analysis)
 	{
 		// If PHI variables are consumed in unexpected precision contexts, copy them here.
-		for (auto &phi : block.phi_variables)
+		for (size_t i = 0, n = block.phi_variables.size(); i < n; i++)
 		{
+			auto &phi = block.phi_variables[i];
+
+			// Ensure we only copy once. We know a-priori that this array will lay out
+			// the same function variables together.
+			if (i && block.phi_variables[i - 1].function_variable == phi.function_variable)
+				continue;
+
 			auto itr = temporary_to_mirror_precision_alias.find(phi.function_variable);
 			if (itr != temporary_to_mirror_precision_alias.end())
 			{
@@ -16647,6 +16654,24 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
 	if (block.merge == SPIRBlock::MergeLoop)
 		add_loop_level();
 
+	// If we're emitting PHI variables with precision aliases, we have to emit them as hoisted temporaries.
+	for (auto var_id : block.dominated_variables)
+	{
+		auto &var = get<SPIRVariable>(var_id);
+		if (var.phi_variable)
+		{
+			auto mirrored_precision_itr = temporary_to_mirror_precision_alias.find(var_id);
+			if (mirrored_precision_itr != temporary_to_mirror_precision_alias.end() &&
+			    find_if(block.declare_temporary.begin(), block.declare_temporary.end(),
+			            [mirrored_precision_itr](const std::pair<TypeID, VariableID> &p) {
+			              return p.second == mirrored_precision_itr->second;
+			            }) == block.declare_temporary.end())
+			{
+				block.declare_temporary.push_back({ var.basetype, mirrored_precision_itr->second });
+			}
+		}
+	}
+
 	emit_hoisted_temporaries(block.declare_temporary);
 
 	SPIRBlock::ContinueBlockType continue_type = SPIRBlock::ContinueNone;