Merge pull request #1466 from KhronosGroup/fix-14652020-09-17

Clean up and fix awkward conditional branch codegen.

4 files changed, 215 insertions, 37 deletions

diff --git a/reference/opt/shaders/asm/frag/loop-body-dominator-continue-access.asm.frag b/reference/opt/shaders/asm/frag/loop-body-dominator-continue-access.asm.frag
index 672fd9a8..df32f1a4 100644
--- a/reference/opt/shaders/asm/frag/loop-body-dominator-continue-access.asm.frag
+++ b/reference/opt/shaders/asm/frag/loop-body-dominator-continue-access.asm.frag
@@ -55,6 +55,11 @@ void main()
                 break;
             }
         }
+        if (_225)
+        {
+            _228 = _229;
+            break;
+        }
         _228 = -1;
         break;
     } while(false);
diff --git a/reference/shaders-no-opt/asm/frag/early-conditional-return-switch.asm.frag b/reference/shaders-no-opt/asm/frag/early-conditional-return-switch.asm.frag
new file mode 100644
index 00000000..34fffb85
--- /dev/null
+++ b/reference/shaders-no-opt/asm/frag/early-conditional-return-switch.asm.frag
@@ -0,0 +1,67 @@
+#version 450
+
+layout(binding = 0, std140) uniform type_gCBuffarrayIndex
+{
+    uint gArrayIndex;
+} gCBuffarrayIndex;
+
+uniform sampler2D SPIRV_Cross_Combinedg_textureArray0SPIRV_Cross_DummySampler;
+uniform sampler2D SPIRV_Cross_Combinedg_textureArray1SPIRV_Cross_DummySampler;
+uniform sampler2D SPIRV_Cross_Combinedg_textureArray2SPIRV_Cross_DummySampler;
+uniform sampler2D SPIRV_Cross_Combinedg_textureArray3SPIRV_Cross_DummySampler;
+
+layout(location = 0) out vec4 out_var_SV_TARGET;
+
+vec4 _32;
+
+void main()
+{
+    vec4 _80;
+    do
+    {
+        vec4 _77;
+        bool _78;
+        switch (gCBuffarrayIndex.gArrayIndex)
+        {
+            case 0u:
+            {
+                _77 = texelFetch(SPIRV_Cross_Combinedg_textureArray0SPIRV_Cross_DummySampler, ivec3(int(gl_FragCoord.x), int(gl_FragCoord.y), 0).xy, 0);
+                _78 = true;
+                break;
+            }
+            case 1u:
+            {
+                _77 = texelFetch(SPIRV_Cross_Combinedg_textureArray1SPIRV_Cross_DummySampler, ivec3(int(gl_FragCoord.x), int(gl_FragCoord.y), 0).xy, 0);
+                _78 = true;
+                break;
+            }
+            case 2u:
+            {
+                _77 = texelFetch(SPIRV_Cross_Combinedg_textureArray2SPIRV_Cross_DummySampler, ivec3(int(gl_FragCoord.x), int(gl_FragCoord.y), 0).xy, 0);
+                _78 = true;
+                break;
+            }
+            case 3u:
+            {
+                _77 = texelFetch(SPIRV_Cross_Combinedg_textureArray3SPIRV_Cross_DummySampler, ivec3(int(gl_FragCoord.x), int(gl_FragCoord.y), 0).xy, 0);
+                _78 = true;
+                break;
+            }
+            default:
+            {
+                _77 = _32;
+                _78 = false;
+                break;
+            }
+        }
+        if (_78)
+        {
+            _80 = _77;
+            break;
+        }
+        _80 = vec4(0.0, 1.0, 0.0, 1.0);
+        break;
+    } while(false);
+    out_var_SV_TARGET = _80;
+}
+
diff --git a/shaders-no-opt/asm/frag/early-conditional-return-switch.asm.frag b/shaders-no-opt/asm/frag/early-conditional-return-switch.asm.frag
new file mode 100644
index 00000000..d789ce36
--- /dev/null
+++ b/shaders-no-opt/asm/frag/early-conditional-return-switch.asm.frag
@@ -0,0 +1,133 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google spiregg; 0
+; Bound: 81
+; Schema: 0
+               OpCapability Shader
+               OpCapability Sampled1D
+               OpCapability Image1D
+               OpCapability SampledBuffer
+               OpCapability ImageBuffer
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %PsTextureLoadArray "main" %gl_FragCoord %out_var_SV_TARGET
+               OpExecutionMode %PsTextureLoadArray OriginUpperLeft
+               OpSource HLSL 500
+               OpName %type_2d_image "type.2d.image"
+               OpName %type_gCBuffarrayIndex "type.gCBuffarrayIndex"
+               OpMemberName %type_gCBuffarrayIndex 0 "gArrayIndex"
+               OpName %gCBuffarrayIndex "gCBuffarrayIndex"
+               OpName %g_textureArray0 "g_textureArray0"
+               OpName %g_textureArray1 "g_textureArray1"
+               OpName %g_textureArray2 "g_textureArray2"
+               OpName %g_textureArray3 "g_textureArray3"
+               OpName %out_var_SV_TARGET "out.var.SV_TARGET"
+               OpName %PsTextureLoadArray "PsTextureLoadArray"
+               OpDecorate %gl_FragCoord BuiltIn FragCoord
+               OpDecorate %out_var_SV_TARGET Location 0
+               OpDecorate %gCBuffarrayIndex DescriptorSet 0
+               OpDecorate %gCBuffarrayIndex Binding 0
+               OpDecorate %g_textureArray0 DescriptorSet 0
+               OpDecorate %g_textureArray0 Binding 0
+               OpDecorate %g_textureArray1 DescriptorSet 0
+               OpDecorate %g_textureArray1 Binding 1
+               OpDecorate %g_textureArray2 DescriptorSet 0
+               OpDecorate %g_textureArray2 Binding 2
+               OpDecorate %g_textureArray3 DescriptorSet 0
+               OpDecorate %g_textureArray3 Binding 3
+               OpMemberDecorate %type_gCBuffarrayIndex 0 Offset 0
+               OpDecorate %type_gCBuffarrayIndex Block
+       %uint = OpTypeInt 32 0
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+      %float = OpTypeFloat 32
+    %float_0 = OpConstant %float 0
+    %float_1 = OpConstant %float 1
+    %v4float = OpTypeVector %float 4
+         %18 = OpConstantComposite %v4float %float_0 %float_1 %float_0 %float_1
+%type_2d_image = OpTypeImage %float 2D 2 0 0 1 Unknown
+%_ptr_UniformConstant_type_2d_image = OpTypePointer UniformConstant %type_2d_image
+%type_gCBuffarrayIndex = OpTypeStruct %uint
+%_ptr_Uniform_type_gCBuffarrayIndex = OpTypePointer Uniform %type_gCBuffarrayIndex
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+       %void = OpTypeVoid
+         %24 = OpTypeFunction %void
+%_ptr_Uniform_uint = OpTypePointer Uniform %uint
+      %v3int = OpTypeVector %int 3
+      %v2int = OpTypeVector %int 2
+%gCBuffarrayIndex = OpVariable %_ptr_Uniform_type_gCBuffarrayIndex Uniform
+%g_textureArray0 = OpVariable %_ptr_UniformConstant_type_2d_image UniformConstant
+%g_textureArray1 = OpVariable %_ptr_UniformConstant_type_2d_image UniformConstant
+%g_textureArray2 = OpVariable %_ptr_UniformConstant_type_2d_image UniformConstant
+%g_textureArray3 = OpVariable %_ptr_UniformConstant_type_2d_image UniformConstant
+%gl_FragCoord = OpVariable %_ptr_Input_v4float Input
+%out_var_SV_TARGET = OpVariable %_ptr_Output_v4float Output
+     %uint_0 = OpConstant %uint 0
+       %bool = OpTypeBool
+      %false = OpConstantFalse %bool
+       %true = OpConstantTrue %bool
+         %32 = OpUndef %v4float
+%PsTextureLoadArray = OpFunction %void None %24
+         %33 = OpLabel
+         %34 = OpLoad %v4float %gl_FragCoord
+               OpSelectionMerge %35 None
+               OpSwitch %uint_0 %36
+         %36 = OpLabel
+         %37 = OpAccessChain %_ptr_Uniform_uint %gCBuffarrayIndex %int_0
+         %38 = OpLoad %uint %37
+               OpSelectionMerge %39 None
+               OpSwitch %38 %40 0 %41 1 %42 2 %43 3 %44
+         %41 = OpLabel
+         %45 = OpCompositeExtract %float %34 0
+         %46 = OpCompositeExtract %float %34 1
+         %47 = OpConvertFToS %int %45
+         %48 = OpConvertFToS %int %46
+         %49 = OpCompositeConstruct %v3int %47 %48 %int_0
+         %50 = OpVectorShuffle %v2int %49 %49 0 1
+         %51 = OpLoad %type_2d_image %g_textureArray0
+         %52 = OpImageFetch %v4float %51 %50 Lod %int_0
+               OpBranch %39
+         %42 = OpLabel
+         %53 = OpCompositeExtract %float %34 0
+         %54 = OpCompositeExtract %float %34 1
+         %55 = OpConvertFToS %int %53
+         %56 = OpConvertFToS %int %54
+         %57 = OpCompositeConstruct %v3int %55 %56 %int_0
+         %58 = OpVectorShuffle %v2int %57 %57 0 1
+         %59 = OpLoad %type_2d_image %g_textureArray1
+         %60 = OpImageFetch %v4float %59 %58 Lod %int_0
+               OpBranch %39
+         %43 = OpLabel
+         %61 = OpCompositeExtract %float %34 0
+         %62 = OpCompositeExtract %float %34 1
+         %63 = OpConvertFToS %int %61
+         %64 = OpConvertFToS %int %62
+         %65 = OpCompositeConstruct %v3int %63 %64 %int_0
+         %66 = OpVectorShuffle %v2int %65 %65 0 1
+         %67 = OpLoad %type_2d_image %g_textureArray2
+         %68 = OpImageFetch %v4float %67 %66 Lod %int_0
+               OpBranch %39
+         %44 = OpLabel
+         %69 = OpCompositeExtract %float %34 0
+         %70 = OpCompositeExtract %float %34 1
+         %71 = OpConvertFToS %int %69
+         %72 = OpConvertFToS %int %70
+         %73 = OpCompositeConstruct %v3int %71 %72 %int_0
+         %74 = OpVectorShuffle %v2int %73 %73 0 1
+         %75 = OpLoad %type_2d_image %g_textureArray3
+         %76 = OpImageFetch %v4float %75 %74 Lod %int_0
+               OpBranch %39
+         %40 = OpLabel
+               OpBranch %39
+         %39 = OpLabel
+         %77 = OpPhi %v4float %52 %41 %60 %42 %68 %43 %76 %44 %32 %40
+         %78 = OpPhi %bool %true %41 %true %42 %true %43 %true %44 %false %40
+               OpSelectionMerge %79 None
+               OpBranchConditional %78 %35 %79
+         %79 = OpLabel
+               OpBranch %35
+         %35 = OpLabel
+         %80 = OpPhi %v4float %77 %39 %18 %79
+               OpStore %out_var_SV_TARGET %80
+               OpReturn
+               OpFunctionEnd
diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp
index 23f4f30f..b17c0e9f 100644
--- a/spirv_glsl.cpp
+++ b/spirv_glsl.cpp
@@ -12732,64 +12732,37 @@ void CompilerGLSL::branch(BlockID from, uint32_t cond, BlockID true_block, Block
 	auto &from_block = get<SPIRBlock>(from);
 	BlockID merge_block = from_block.merge == SPIRBlock::MergeSelection ? from_block.next_block : BlockID(0);
 
-	// If we branch directly to a selection merge target, we don't need a code path.
-	// This covers both merge out of if () / else () as well as a break for switch blocks.
-	bool true_sub = !is_conditional(true_block);
-	bool false_sub = !is_conditional(false_block);
+	// If we branch directly to our selection merge target, we don't need a code path.
+	bool true_block_needs_code = true_block != merge_block || flush_phi_required(from, true_block);
+	bool false_block_needs_code = false_block != merge_block || flush_phi_required(from, false_block);
 
-	bool true_block_is_selection_merge = true_block == merge_block;
-	bool false_block_is_selection_merge = false_block == merge_block;
+	if (!true_block_needs_code && !false_block_needs_code)
+		return;
+
+	emit_block_hints(get<SPIRBlock>(from));
 
-	if (true_sub)
+	if (true_block_needs_code)
 	{
-		emit_block_hints(get<SPIRBlock>(from));
 		statement("if (", to_expression(cond), ")");
 		begin_scope();
 		branch(from, true_block);
 		end_scope();
 
-		// If we merge to continue, we handle that explicitly in emit_block_chain(),
-		// so there is no need to branch to it directly here.
-		// break; is required to handle ladder fallthrough cases, so keep that in for now, even
-		// if we could potentially handle it in emit_block_chain().
-		if (false_sub || (!false_block_is_selection_merge && is_continue(false_block)) || is_break(false_block))
+		if (false_block_needs_code)
 		{
 			statement("else");
 			begin_scope();
 			branch(from, false_block);
 			end_scope();
 		}
-		else if (flush_phi_required(from, false_block))
-		{
-			statement("else");
-			begin_scope();
-			flush_phi(from, false_block);
-			end_scope();
-		}
 	}
-	else if (false_sub)
+	else if (false_block_needs_code)
 	{
 		// Only need false path, use negative conditional.
-		emit_block_hints(get<SPIRBlock>(from));
 		statement("if (!", to_enclosed_expression(cond), ")");
 		begin_scope();
 		branch(from, false_block);
 		end_scope();
-
-		if ((!true_block_is_selection_merge && is_continue(true_block)) || is_break(true_block))
-		{
-			statement("else");
-			begin_scope();
-			branch(from, true_block);
-			end_scope();
-		}
-		else if (flush_phi_required(from, true_block))
-		{
-			statement("else");
-			begin_scope();
-			flush_phi(from, true_block);
-			end_scope();
-		}
 	}
 }