Merge pull request #1129 from KhronosGroup/fix-1110

Fix variable scope when switch block exits multiple times.

reference/shaders-no-opt/asm/frag/switch-single-case-multiple-exit-cfg.asm.frag

@@ -0,0 +1,29 @@
+#version 310 es
+precision mediump float;
+precision highp int;
+
+layout(location = 0) out highp vec4 _GLF_color;
+
+vec2 _19;
+
+void main()
+{
+    highp vec2 _30;
+    switch (0)
+    {
+        default:
+        {
+            if (gl_FragCoord.x != gl_FragCoord.x)
+            {
+                _30 = _19;
+                break;
+            }
+            highp vec2 _29 = _19;
+            _29.y = _19.y;
+            _30 = _29;
+            break;
+        }
+    }
+    _GLF_color = vec4(_30, 1.0, 1.0);
+}
+

shaders-no-opt/asm/frag/switch-single-case-multiple-exit-cfg.asm.frag

@@ -0,0 +1,57 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 7
+; Bound: 54
+; Schema: 0
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %main "main" %gl_FragCoord %_GLF_color
+               OpExecutionMode %main OriginUpperLeft
+               OpSource ESSL 310
+               OpName %main "main"
+               OpName %gl_FragCoord "gl_FragCoord"
+               OpName %_GLF_color "_GLF_color"
+               OpDecorate %gl_FragCoord BuiltIn FragCoord
+               OpDecorate %_GLF_color Location 0
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+      %float = OpTypeFloat 32
+    %v4float = OpTypeVector %float 4
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%gl_FragCoord = OpVariable %_ptr_Input_v4float Input
+       %uint = OpTypeInt 32 0
+     %uint_0 = OpConstant %uint 0
+%_ptr_Input_float = OpTypePointer Input %float
+       %bool = OpTypeBool
+    %v2float = OpTypeVector %float 2
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+ %_GLF_color = OpVariable %_ptr_Output_v4float Output
+    %float_1 = OpConstant %float 1
+         %52 = OpUndef %v2float
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+               OpSelectionMerge %9 None
+               OpSwitch %int_0 %8
+          %8 = OpLabel
+         %17 = OpAccessChain %_ptr_Input_float %gl_FragCoord %uint_0
+         %18 = OpLoad %float %17
+         %22 = OpFOrdNotEqual %bool %18 %18
+               OpSelectionMerge %24 None
+               OpBranchConditional %22 %23 %24
+         %23 = OpLabel
+               OpBranch %9
+         %24 = OpLabel
+         %33 = OpCompositeExtract %float %52 1
+         %51 = OpCompositeInsert %v2float %33 %52 1
+               OpBranch %9
+          %9 = OpLabel
+         %53 = OpPhi %v2float %52 %23 %51 %24
+         %42 = OpCompositeExtract %float %53 0
+         %43 = OpCompositeExtract %float %53 1
+         %48 = OpCompositeConstruct %v4float %42 %43 %float_1 %float_1
+               OpStore %_GLF_color %48
+               OpReturn
+               OpFunctionEnd

spirv_cfg.cpp

@@ -148,15 +148,36 @@ bool CFG::post_order_visit(uint32_t block_id)
 		// Add a fake branch so any dominator in either the if (), or else () block, or a lone case statement
 		// will be hoisted out to outside the selection merge.
 		// If size > 1, the variable will be automatically hoisted, so we should not mess with it.
+		// The exception here is switch blocks, where we can have multiple edges to merge block,
+		// all coming from same scope, so be more conservative in this case.
 		// Adding fake branches unconditionally breaks parameter preservation analysis,
 		// which looks at how variables are accessed through the CFG.
 		auto pred_itr = preceding_edges.find(block.next_block);
 		if (pred_itr != end(preceding_edges))
 		{
 			auto &pred = pred_itr->second;
+			auto succ_itr = succeeding_edges.find(block_id);
+			uint32_t num_succeeding_edges = 0;
+			if (succ_itr != end(succeeding_edges))
+				num_succeeding_edges = succ_itr->second.size();
+
+			if (block.terminator == SPIRBlock::MultiSelect && num_succeeding_edges == 1)
+			{
+				// Multiple branches can come from the same scope due to "break;", so we need to assume that all branches
+				// come from same case scope in worst case, even if there are multiple preceding edges.
+				// If we have more than one succeeding edge from the block header, it should be impossible
+				// to have a dominator be inside the block.
+				// Only case this can go wrong is if we have 2 or more edges from block header and
+				// 2 or more edges to merge block, and still have dominator be inside a case label.
+				if (!pred.empty())
+					add_branch(block_id, block.next_block);
+			}
+			else
+			{
 				if (pred.size() == 1 && *pred.begin() != block_id)
 					add_branch(block_id, block.next_block);
 			}
+		}
 		else
 		{
 			// If the merge block does not have any preceding edges, i.e. unreachable, hallucinate it.