Merge pull request #1732 from KhronosGroup/fix-1731
Fix switch fallthrough case in some cases.
diff --git a/reference/shaders-no-opt/asm/frag/switch-non-default-fallthrough-no-phi.asm.frag b/reference/shaders-no-opt/asm/frag/switch-non-default-fallthrough-no-phi.asm.frag
new file mode 100644
index 0000000..34fce56
--- /dev/null
+++ b/reference/shaders-no-opt/asm/frag/switch-non-default-fallthrough-no-phi.asm.frag
@@ -0,0 +1,88 @@
+#version 450
+
+struct _4
+{
+ uint _m0;
+ int _m1;
+};
+
+struct _5
+{
+ int _m0;
+ int _m1;
+};
+
+layout(location = 0) flat in int _2;
+layout(location = 0) out int _3;
+
+_4 _16;
+int _21;
+
+void main()
+{
+ bool _25 = false;
+ do
+ {
+ _5 _26;
+ _26._m0 = 0;
+ _26._m1 = 10;
+ _4 _35;
+ _35 = _16;
+ int _39;
+ _4 _36;
+ bool _59;
+ int _38 = 0;
+ for (;;)
+ {
+ if (_26._m0 < _26._m1)
+ {
+ int _27 = _26._m0;
+ int _28 = _26._m0 + int(1u);
+ _26._m0 = _28;
+ _36 = _4(1u, _27);
+ }
+ else
+ {
+ _4 _48 = _35;
+ _48._m0 = 0u;
+ _36 = _48;
+ }
+ bool _45_ladder_break = false;
+ switch (int(_36._m0))
+ {
+ case 0:
+ {
+ _3 = _38;
+ _25 = true;
+ _59 = true;
+ _45_ladder_break = true;
+ break;
+ }
+ default:
+ {
+ _59 = false;
+ _45_ladder_break = true;
+ break;
+ }
+ case 1:
+ {
+ break;
+ }
+ }
+ if (_45_ladder_break)
+ {
+ break;
+ }
+ _39 = _38 + _2;
+ _35 = _36;
+ _38 = _39;
+ continue;
+ }
+ if (_59)
+ {
+ break;
+ }
+ break;
+ } while(false);
+}
+
diff --git a/shaders-no-opt/asm/frag/switch-non-default-fallthrough-no-phi.asm.frag b/shaders-no-opt/asm/frag/switch-non-default-fallthrough-no-phi.asm.frag
new file mode 100644
index 0000000..dd9a5a9
--- /dev/null
+++ b/shaders-no-opt/asm/frag/switch-non-default-fallthrough-no-phi.asm.frag
@@ -0,0 +1,105 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google rspirv; 0
+; Bound: 80
+; Schema: 0
+ OpCapability Shader
+ OpCapability VulkanMemoryModel
+ OpExtension "SPV_KHR_vulkan_memory_model"
+ OpMemoryModel Logical Vulkan
+ OpEntryPoint Fragment %1 "main" %2 %3
+ OpExecutionMode %1 OriginUpperLeft
+ OpMemberDecorate %_struct_14 0 Offset 0
+ OpMemberDecorate %_struct_14 1 Offset 4
+ OpMemberDecorate %_struct_15 0 Offset 0
+ OpMemberDecorate %_struct_15 1 Offset 4
+ OpDecorate %2 Location 0
+ OpDecorate %3 Location 0
+ OpDecorate %2 Flat
+ %int = OpTypeInt 32 1
+ %uint = OpTypeInt 32 0
+ %bool = OpTypeBool
+%_ptr_Input_int = OpTypePointer Input %int
+%_ptr_Output_int = OpTypePointer Output %int
+%_ptr_Function_int = OpTypePointer Function %int
+ %void = OpTypeVoid
+ %_struct_14 = OpTypeStruct %uint %int
+ %_struct_15 = OpTypeStruct %int %int
+%_ptr_Function__struct_15 = OpTypePointer Function %_struct_15
+ %24 = OpTypeFunction %void
+ %2 = OpVariable %_ptr_Input_int Input
+ %3 = OpVariable %_ptr_Output_int Output
+ %uint_1 = OpConstant %uint 1
+ %26 = OpUndef %_struct_14
+ %uint_0 = OpConstant %uint 0
+ %int_0 = OpConstant %int 0
+ %int_10 = OpConstant %int 10
+ %true = OpConstantTrue %bool
+ %31 = OpUndef %int
+ %false = OpConstantFalse %bool
+%_ptr_Function_bool = OpTypePointer Function %bool
+ %1 = OpFunction %void None %24
+ %32 = OpLabel
+ %76 = OpVariable %_ptr_Function_bool Function %false
+ %33 = OpVariable %_ptr_Function__struct_15 Function
+ %34 = OpVariable %_ptr_Function_int Function
+ %35 = OpVariable %_ptr_Function_int Function
+ OpSelectionMerge %72 None
+ OpSwitch %uint_0 %73
+ %73 = OpLabel
+ %36 = OpLoad %int %2
+ %37 = OpAccessChain %_ptr_Function_int %33 %uint_0
+ OpStore %37 %int_0
+ %38 = OpAccessChain %_ptr_Function_int %33 %uint_1
+ OpStore %38 %int_10
+ OpBranch %40
+ %40 = OpLabel
+ %41 = OpPhi %_struct_14 %26 %73 %42 %43
+ %44 = OpPhi %int %int_0 %73 %45 %43
+ OpLoopMerge %48 %43 None
+ OpBranch %49
+ %49 = OpLabel
+ %52 = OpLoad %int %37
+ %53 = OpLoad %int %38
+ %54 = OpSLessThan %bool %52 %53
+ OpSelectionMerge %55 None
+ OpBranchConditional %54 %56 %57
+ %57 = OpLabel
+ %65 = OpCompositeInsert %_struct_14 %uint_0 %41 0
+ OpBranch %55
+ %56 = OpLabel
+ %59 = OpLoad %int %37
+ %60 = OpBitcast %int %uint_1
+ %61 = OpIAdd %int %59 %60
+ OpCopyMemory %34 %37
+ %63 = OpLoad %int %34
+ OpStore %35 %61
+ OpCopyMemory %37 %35
+ %64 = OpCompositeConstruct %_struct_14 %uint_1 %63
+ OpBranch %55
+ %55 = OpLabel
+ %42 = OpPhi %_struct_14 %64 %56 %65 %57
+ %66 = OpCompositeExtract %uint %42 0
+ %67 = OpBitcast %int %66
+ OpSelectionMerge %71 None
+ OpSwitch %67 %69 0 %70 1 %71
+ %71 = OpLabel
+ %45 = OpIAdd %int %44 %36
+ OpBranch %43
+ %70 = OpLabel
+ OpStore %3 %44
+ OpStore %76 %true
+ OpBranch %48
+ %69 = OpLabel
+ OpBranch %48
+ %43 = OpLabel
+ OpBranch %40
+ %48 = OpLabel
+ %79 = OpPhi %bool %false %69 %true %70
+ OpSelectionMerge %77 None
+ OpBranchConditional %79 %72 %77
+ %77 = OpLabel
+ OpBranch %72
+ %72 = OpLabel
+ OpReturn
+ OpFunctionEnd
diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp
index c5550ee..472dd88 100644
--- a/spirv_glsl.cpp
+++ b/spirv_glsl.cpp
@@ -14974,26 +14974,30 @@
}
// Might still have to flush phi variables if we branch from loop header directly to merge target.
- if (flush_phi_required(block.self, block.next_block))
+ // This is supposed to emit all cases where we branch from header to merge block directly.
+ // There are two main scenarios where cannot rely on default fallthrough.
+ // - There is an explicit default: label already.
+ // In this case, literals_to_merge need to form their own "default" case, so that we avoid executing that block.
+ // - Header -> Merge requires flushing PHI. In this case, we need to collect all cases and flush PHI there.
+ bool header_merge_requires_phi = flush_phi_required(block.self, block.next_block);
+ bool need_fallthrough_block = block.default_block == block.next_block || !literals_to_merge.empty();
+ if ((header_merge_requires_phi && need_fallthrough_block) || !literals_to_merge.empty())
{
- if (block.default_block == block.next_block || !literals_to_merge.empty())
+ for (auto &case_literal : literals_to_merge)
+ statement("case ", to_case_label(case_literal, unsigned_case), label_suffix, ":");
+
+ if (block.default_block == block.next_block)
{
- for (auto &case_literal : literals_to_merge)
- statement("case ", to_case_label(case_literal, unsigned_case), label_suffix, ":");
-
- if (block.default_block == block.next_block)
- {
- if (is_legacy_es())
- statement("else");
- else
- statement("default:");
- }
-
- begin_scope();
- flush_phi(block.self, block.next_block);
- statement("break;");
- end_scope();
+ if (is_legacy_es())
+ statement("else");
+ else
+ statement("default:");
}
+
+ begin_scope();
+ flush_phi(block.self, block.next_block);
+ statement("break;");
+ end_scope();
}
if (degenerate_switch && !is_legacy_es())
